[Mesa-dev] [RFC PATCH 52/65] radeonsi: implement ARB_bindless_texture
Marek Olšák
maraeo at gmail.com
Wed May 24 11:25:01 UTC 2017
On Fri, May 19, 2017 at 6:52 PM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
> This implements the Gallium interface. Decompression of resident
> textures/images will follow in the next patches.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
> src/gallium/drivers/radeonsi/si_descriptors.c | 340 ++++++++++++++++++++++++++
> src/gallium/drivers/radeonsi/si_pipe.c | 12 +
> src/gallium/drivers/radeonsi/si_pipe.h | 26 ++
> 3 files changed, 378 insertions(+)
>
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
> index abe39de583..a687506f7f 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -60,6 +60,7 @@
> #include "sid.h"
> #include "gfx9d.h"
>
> +#include "util/hash_table.h"
> #include "util/u_format.h"
> #include "util/u_memory.h"
> #include "util/u_upload_mgr.h"
> @@ -2193,6 +2194,339 @@ void si_resident_descriptor_slab_free(void *priv, struct pb_slab *pslab)
> FREE(slab);
> }
>
> +static int si_add_resident_tex_handle(struct si_context *sctx,
> + struct si_texture_handle *tex_handle)
> +{
> + int idx;
> +
> + /* New resident handle, check if the backing array is large enough. */
> + if (sctx->num_resident_tex_handles >= sctx->max_resident_tex_handles) {
> + unsigned new_max_handles =
> + MAX2(1, sctx->max_resident_tex_handles * 2);
> + struct si_texture_handle **new_handles =
> + REALLOC(sctx->resident_tex_handles,
> + sctx->num_resident_tex_handles * (sizeof(*new_handles)),
> + new_max_handles * sizeof(*new_handles));
> +
> + if (new_handles) {
> + sctx->resident_tex_handles = new_handles;
> + sctx->max_resident_tex_handles = new_max_handles;
> + } else {
> + fprintf(stderr, "si_add_resident_tex_handle: "
> + "allocation failed\n");
> + return -1;
> + }
> + }
> +
> + idx = sctx->num_resident_tex_handles;
> + sctx->resident_tex_handles[idx] = tex_handle;
> + sctx->num_resident_tex_handles++;
> +
> + return 0;
> +}
> +
> +static void si_del_resident_tex_handle(struct si_context *sctx,
> + struct si_texture_handle *tex_handle)
> +{
> + unsigned i;
> + int size;
> +
> + for (i = 0; i < sctx->num_resident_tex_handles; i++) {
> + if (sctx->resident_tex_handles[i] != tex_handle)
> + continue;
> +
> + if (i < sctx->num_resident_tex_handles - 1) {
> + size = sizeof(*sctx->resident_tex_handles) *
> + (sctx->num_resident_tex_handles - 1 - i);
> +
> + memmove(&sctx->resident_tex_handles[i],
> + &sctx->resident_tex_handles[i + 1], size);
> + }
> +
> + sctx->num_resident_tex_handles--;
> + return;
> + }
> +}
> +
> +static int si_add_resident_img_handle(struct si_context *sctx,
> + struct si_image_handle *img_handle)
> +{
> + int idx;
> +
> + /* New resident handle, check if the backing array is large enough. */
> + if (sctx->num_resident_img_handles >= sctx->max_resident_img_handles) {
> + unsigned new_max_handles =
> + MAX2(1, sctx->max_resident_img_handles * 2);
> + struct si_image_handle **new_handles =
> + REALLOC(sctx->resident_img_handles,
> + sctx->num_resident_img_handles * (sizeof(*new_handles)),
> + new_max_handles * sizeof(*new_handles));
> +
> + if (new_handles) {
> + sctx->resident_img_handles = new_handles;
> + sctx->max_resident_img_handles = new_max_handles;
> + } else {
> + fprintf(stderr, "si_add_resident_img_handle: "
> + "allocation failed\n");
> + return -1;
> + }
> + }
> +
> + idx = sctx->num_resident_img_handles;
> + sctx->resident_img_handles[idx] = img_handle;
> + sctx->num_resident_img_handles++;
> +
> + return 0;
> +}
> +
> +static void si_del_resident_img_handle(struct si_context *sctx,
> + struct si_image_handle *img_handle)
> +{
> + unsigned i;
> + int size;
> +
> + for (i = 0; i < sctx->num_resident_img_handles; i++) {
> + if (sctx->resident_img_handles[i] != img_handle)
> + continue;
> +
> + if (i < sctx->num_resident_img_handles - 1) {
> + size = sizeof(*sctx->resident_img_handles) *
> + (sctx->num_resident_img_handles - 1 - i);
> +
> + memmove(&sctx->resident_img_handles[i],
> + &sctx->resident_img_handles[i + 1], size);
> + }
> +
> + sctx->num_resident_img_handles--;
> + return;
> + }
> +}
> +
> +static struct si_resident_descriptor *
> +si_create_resident_descriptor(struct si_context *sctx, uint32_t *desc_list,
> + unsigned size)
> +{
> + struct si_screen *sscreen = sctx->screen;
> + struct si_resident_descriptor *desc;
> + struct pb_slab_entry *entry;
> + void *ptr;
> +
> + /* Sub-allocate the resident descriptor from slabs. */
> + entry = pb_slab_alloc(&sctx->resident_descriptor_slabs, 64, 0);
> + if (!entry)
> + return NULL;
> +
> + desc = NULL;
> + desc = container_of(entry, desc, entry);
> +
> + /* Upload the descriptor. */
> + ptr = sscreen->b.ws->buffer_map(desc->buffer->buf, NULL,
> + PIPE_TRANSFER_WRITE |
> + PIPE_TRANSFER_UNSYNCHRONIZED);
> + util_memcpy_cpu_to_le32(ptr + desc->offset, desc_list, size);
> + sscreen->b.ws->buffer_unmap(desc->buffer->buf);
> +
> + return desc;
> +}
> +
> +static uint64_t si_create_texture_handle(struct pipe_context *ctx,
> + struct pipe_resource *texture,
> + struct pipe_sampler_view *view,
> + const struct pipe_sampler_state *state)
> +{
> + struct si_sampler_view *sview = (struct si_sampler_view *)view;
> + struct si_context *sctx = (struct si_context *)ctx;
> + struct si_texture_handle *tex_handle;
> + struct si_sampler_state *sstate;
> + uint32_t desc_list[16];
> + uint64_t handle;
> +
> + tex_handle = CALLOC_STRUCT(si_texture_handle);
> + if (!tex_handle)
> + return 0;
> +
> + memset(desc_list, 0, sizeof(desc_list));
> + si_init_descriptor_list(&desc_list[0], 16, 1, null_texture_descriptor);
> +
> + sstate = ctx->create_sampler_state(ctx, state);
> + if (!sstate) {
> + FREE(tex_handle);
> + return 0;
> + }
> +
> + si_set_sampler_view_desc(sctx, sview, sstate, &desc_list[0]);
> + ctx->delete_sampler_state(ctx, sstate);
> +
> + tex_handle->view = sview;
> + tex_handle->desc = si_create_resident_descriptor(sctx, desc_list,
> + sizeof(desc_list));
> + if (!tex_handle->desc) {
> + FREE(tex_handle);
> + return 0;
> + }
> +
> + handle = tex_handle->desc->buffer->gpu_address +
> + tex_handle->desc->offset;
> +
> + if (!_mesa_hash_table_insert(sctx->tex_handles, (void *)handle,
> + tex_handle)) {
> + pb_slab_free(&sctx->resident_descriptor_slabs,
> + &tex_handle->desc->entry);
> + FREE(tex_handle);
> + return 0;
> + }
> +
> + return handle;
Here's the reference counting issue. First of all, "texture" isn't
used, which is weird, because it's part of the interface. Then, the
"view" pointer is stored in tex_handle without incrementing the
reference counter, so st/mesa can release it.
It looks like the original idea was to use "texture" as the resource
and "view" as the view template, while this code ignores "texture" and
uses "view" as a full sampler view.
> +}
> +
> +static void si_delete_texture_handle(struct pipe_context *ctx, uint64_t handle)
> +{
> + struct si_context *sctx = (struct si_context *)ctx;
> + struct si_texture_handle *tex_handle;
> + struct hash_entry *entry;
> +
> + entry = _mesa_hash_table_search(sctx->tex_handles, (void *)handle);
> + if (!entry)
> + return;
> +
> + tex_handle = (struct si_texture_handle *)entry->data;
> +
> + _mesa_hash_table_remove(sctx->tex_handles, entry);
> + pb_slab_free(&sctx->resident_descriptor_slabs,
> + &tex_handle->desc->entry);
> + FREE(tex_handle);
> +}
> +
> +static void si_make_texture_handle_resident(struct pipe_context *ctx,
> + uint64_t handle, bool resident)
> +{
> + struct si_context *sctx = (struct si_context *)ctx;
> + struct si_texture_handle *tex_handle;
> + struct si_sampler_view *sview;
> + struct hash_entry *entry;
> +
> + entry = _mesa_hash_table_search(sctx->tex_handles, (void *)handle);
> + if (!entry)
> + return;
> +
> + tex_handle = (struct si_texture_handle *)entry->data;
> + sview = tex_handle->view;
> +
> + if (resident) {
> + si_add_resident_tex_handle(sctx, tex_handle);
> +
> + /* Add the buffers to the current CS in case si_begin_new_cs()
> + * is not going to be called.
> + */
> + radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx,
> + tex_handle->desc->buffer,
> + RADEON_USAGE_READ,
> + RADEON_PRIO_DESCRIPTORS);
> +
> + si_sampler_view_add_buffer(sctx, sview->base.texture,
> + RADEON_USAGE_READ,
> + sview->is_stencil_sampler, false);
> + } else {
> + si_del_resident_tex_handle(sctx, tex_handle);
> + }
> +}
> +
> +static uint64_t si_create_image_handle(struct pipe_context *ctx,
> + const struct pipe_image_view *view)
> +{
> + struct si_context *sctx = (struct si_context *)ctx;
> + struct si_image_handle *img_handle;
> + uint32_t desc_list[16];
> + uint64_t handle;
> +
> + if (!view || !view->resource)
> + return 0;
> +
> + img_handle = CALLOC_STRUCT(si_image_handle);
> + if (!img_handle)
> + return 0;
> +
> + memset(desc_list, 0, sizeof(desc_list));
> + si_init_descriptor_list(&desc_list[0], 8, 1, null_image_descriptor);
> +
> + si_set_shader_image_desc(sctx, view, false, &desc_list[0]);
> +
> + util_copy_image_view(&img_handle->view, view);
> + img_handle->desc = si_create_resident_descriptor(sctx, desc_list,
> + sizeof(desc_list));
> + if (!img_handle->desc) {
> + FREE(img_handle);
This is a memory leak. util_copy_image_view increments the reference
counter of view->resource, which isn't cleared before FREE.
> + return 0;
> + }
> +
> + handle = img_handle->desc->buffer->gpu_address +
> + img_handle->desc->offset;
> +
> + if (!_mesa_hash_table_insert(sctx->img_handles, (void *)handle,
> + img_handle)) {
> + pb_slab_free(&sctx->resident_descriptor_slabs,
> + &img_handle->desc->entry);
> + FREE(img_handle);
> + return 0;
> + }
> +
> + return handle;
> +}
> +
> +static void si_delete_image_handle(struct pipe_context *ctx, uint64_t handle)
> +{
> + struct si_context *sctx = (struct si_context *)ctx;
> + struct si_image_handle *img_handle;
> + struct hash_entry *entry;
> +
> + entry = _mesa_hash_table_search(sctx->img_handles, (void *)handle);
> + if (!entry)
> + return;
> +
> + img_handle = (struct si_image_handle *)entry->data;
> +
> + _mesa_hash_table_remove(sctx->img_handles, entry);
> + pb_slab_free(&sctx->resident_descriptor_slabs,
> + &img_handle->desc->entry);
> + FREE(img_handle);
Same memory leak here.
I've not yet looked at this patch thoroughly. It's just what I've noticed now.
Marek
More information about the mesa-dev
mailing list