[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