[Mesa-dev] [PATCH] gallium: make image views non-persistent objects

Nicolai Hähnle nhaehnle at gmail.com
Tue Jan 19 07:42:02 PST 2016


On 18.01.2016 22:08, Ilia Mirkin wrote:
> Make them akin to shader buffers, with no refcounting/etc. Just used to
> pass data about the bound image in ->set_shader_images.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>
> I don't really see a reason why these were refcounted objects. It seems like
> it would be convenient to make these line up with shader buffers, so that's
> what I've done here.
>
> Please let me know if I'm missing something.

I haven't thought about this much, but at least Radeon does quite a bit 
of work in create_sampler_view.

Since everything boils down to the same hardware resource descriptors in 
the end, I'd expect the same to happen for a create_image_view. I 
believe we'll want a create_image_view which ends up calling code that 
is shared with create_sampler_view.

So make that a vote against this change from me.

Come to think of it, from a Radeon perspective I'm not sure why there is 
a separate pipe_image_view structure in the first place (other than 
perhaps reducing confusion about which combination of fields make sense).

Cheers,
Nicolai

>
>   src/gallium/auxiliary/util/u_inlines.h        | 11 -----------
>   src/gallium/drivers/ddebug/dd_context.c       | 28 +--------------------------
>   src/gallium/drivers/ddebug/dd_pipe.h          |  2 +-
>   src/gallium/drivers/ilo/ilo_state.c           |  2 +-
>   src/gallium/drivers/nouveau/nvc0/nvc0_state.c |  2 +-
>   src/gallium/include/pipe/p_context.h          | 14 ++------------
>   src/gallium/include/pipe/p_state.h            |  4 +---
>   7 files changed, 7 insertions(+), 56 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_inlines.h b/src/gallium/auxiliary/util/u_inlines.h
> index 57a3b0b..d081203 100644
> --- a/src/gallium/auxiliary/util/u_inlines.h
> +++ b/src/gallium/auxiliary/util/u_inlines.h
> @@ -174,17 +174,6 @@ pipe_sampler_view_release(struct pipe_context *ctx,
>   }
>
>   static inline void
> -pipe_image_view_reference(struct pipe_image_view **ptr, struct pipe_image_view *view)
> -{
> -   struct pipe_image_view *old_view = *ptr;
> -
> -   if (pipe_reference_described(&(*ptr)->reference, &view->reference,
> -                                (debug_reference_descriptor)debug_describe_image_view))
> -      old_view->context->image_view_destroy(old_view->context, old_view);
> -   *ptr = view;
> -}
> -
> -static inline void
>   pipe_so_target_reference(struct pipe_stream_output_target **ptr,
>                            struct pipe_stream_output_target *target)
>   {
> diff --git a/src/gallium/drivers/ddebug/dd_context.c b/src/gallium/drivers/ddebug/dd_context.c
> index 3ae7764..9dfaa0a 100644
> --- a/src/gallium/drivers/ddebug/dd_context.c
> +++ b/src/gallium/drivers/ddebug/dd_context.c
> @@ -415,30 +415,6 @@ dd_context_sampler_view_destroy(struct pipe_context *_pipe,
>      pipe->sampler_view_destroy(pipe, view);
>   }
>
> -static struct pipe_image_view *
> -dd_context_create_image_view(struct pipe_context *_pipe,
> -                             struct pipe_resource *resource,
> -                             const struct pipe_image_view *templ)
> -{
> -   struct pipe_context *pipe = dd_context(_pipe)->pipe;
> -   struct pipe_image_view *view =
> -      pipe->create_image_view(pipe, resource, templ);
> -
> -   if (!view)
> -      return NULL;
> -   view->context = _pipe;
> -   return view;
> -}
> -
> -static void
> -dd_context_image_view_destroy(struct pipe_context *_pipe,
> -                              struct pipe_image_view *view)
> -{
> -   struct pipe_context *pipe = dd_context(_pipe)->pipe;
> -
> -   pipe->image_view_destroy(pipe, view);
> -}
> -
>   static struct pipe_stream_output_target *
>   dd_context_create_stream_output_target(struct pipe_context *_pipe,
>                                          struct pipe_resource *res,
> @@ -486,7 +462,7 @@ dd_context_set_sampler_views(struct pipe_context *_pipe, unsigned shader,
>   static void
>   dd_context_set_shader_images(struct pipe_context *_pipe, unsigned shader,
>                                unsigned start, unsigned num,
> -                             struct pipe_image_view **views)
> +                             struct pipe_image_view *views)
>   {
>      struct dd_context *dctx = dd_context(_pipe);
>      struct pipe_context *pipe = dctx->pipe;
> @@ -744,8 +720,6 @@ dd_context_create(struct dd_screen *dscreen, struct pipe_context *pipe)
>      CTX_INIT(sampler_view_destroy);
>      CTX_INIT(create_surface);
>      CTX_INIT(surface_destroy);
> -   CTX_INIT(create_image_view);
> -   CTX_INIT(image_view_destroy);
>      CTX_INIT(transfer_map);
>      CTX_INIT(transfer_flush_region);
>      CTX_INIT(transfer_unmap);
> diff --git a/src/gallium/drivers/ddebug/dd_pipe.h b/src/gallium/drivers/ddebug/dd_pipe.h
> index a045518..6505cea 100644
> --- a/src/gallium/drivers/ddebug/dd_pipe.h
> +++ b/src/gallium/drivers/ddebug/dd_pipe.h
> @@ -93,7 +93,7 @@ struct dd_context
>      struct pipe_constant_buffer constant_buffers[PIPE_SHADER_TYPES][PIPE_MAX_CONSTANT_BUFFERS];
>      struct pipe_sampler_view *sampler_views[PIPE_SHADER_TYPES][PIPE_MAX_SAMPLERS];
>      struct dd_state *sampler_states[PIPE_SHADER_TYPES][PIPE_MAX_SAMPLERS];
> -   struct pipe_image_view *shader_images[PIPE_SHADER_TYPES][PIPE_MAX_SHADER_IMAGES];
> +   struct pipe_image_view shader_images[PIPE_SHADER_TYPES][PIPE_MAX_SHADER_IMAGES];
>      struct pipe_shader_buffer shader_buffers[PIPE_SHADER_TYPES][PIPE_MAX_SHADER_BUFFERS];
>
>      struct dd_state *velems;
> diff --git a/src/gallium/drivers/ilo/ilo_state.c b/src/gallium/drivers/ilo/ilo_state.c
> index 8dc2d38..f8d2637 100644
> --- a/src/gallium/drivers/ilo/ilo_state.c
> +++ b/src/gallium/drivers/ilo/ilo_state.c
> @@ -1851,7 +1851,7 @@ ilo_set_sampler_views(struct pipe_context *pipe, unsigned shader,
>   static void
>   ilo_set_shader_images(struct pipe_context *pipe, unsigned shader,
>                         unsigned start, unsigned count,
> -                      struct pipe_image_view **views)
> +                      struct pipe_image_view *views)
>   {
>   #if 0
>      struct ilo_state_vector *vec = &ilo_context(pipe)->state_vector;
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
> index cf3d349..23fc721 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
> @@ -1241,7 +1241,7 @@ nvc0_set_compute_resources(struct pipe_context *pipe,
>   static void
>   nvc0_set_shader_images(struct pipe_context *pipe, unsigned shader,
>                          unsigned start_slot, unsigned count,
> -                       struct pipe_image_view **views)
> +                       struct pipe_image_view *views)
>   {
>   }
>
> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
> index 4b551ed..11c00ff 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -290,14 +290,14 @@ struct pipe_context {
>       * \param shader     selects shader stage
>       * \param start_slot first image slot to bind.
>       * \param count      number of consecutive images to bind.
> -    * \param buffers    array of pointers to the images to bind, it
> +    * \param buffers    array of the images to bind, it
>       *                   should contain at least \a count elements
>       *                   unless it's NULL, in which case no images will
>       *                   be bound.
>       */
>      void (*set_shader_images)(struct pipe_context *, unsigned shader,
>                                unsigned start_slot, unsigned count,
> -                             struct pipe_image_view **images);
> +                             struct pipe_image_view *images);
>
>      void (*set_vertex_buffers)( struct pipe_context *,
>                                  unsigned start_slot,
> @@ -455,16 +455,6 @@ struct pipe_context {
>      void (*surface_destroy)(struct pipe_context *ctx,
>                              struct pipe_surface *);
>
> -   /**
> -    * Create an image view into a buffer or texture to be used with load,
> -    * store, and atomic instructions by a shader stage.
> -    */
> -   struct pipe_image_view * (*create_image_view)(struct pipe_context *ctx,
> -                                                 struct pipe_resource *texture,
> -                                                 const struct pipe_image_view *templat);
> -
> -   void (*image_view_destroy)(struct pipe_context *ctx,
> -                              struct pipe_image_view *view);
>
>      /**
>       * Map a resource.
> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
> index 2e4d283..ca7775d 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -393,14 +393,12 @@ struct pipe_sampler_view
>
>
>   /**
> - * A view into a writable buffer or texture that can be bound to a shader
> + * A description of a writable buffer or texture that can be bound to a shader
>    * stage.
>    */
>   struct pipe_image_view
>   {
> -   struct pipe_reference reference;
>      struct pipe_resource *resource; /**< resource into which this is a view  */
> -   struct pipe_context *context; /**< context this view belongs to */
>      enum pipe_format format;      /**< typed PIPE_FORMAT_x */
>
>      union {
>


More information about the mesa-dev mailing list