st/mesa: Record shader access qualifiers for images

Submitted by Kenneth Graunke on Oct. 15, 2018, 10:34 p.m.

Details

Message ID 20181015223453.25824-1-kenneth@whitecape.org
State New
Headers show
Series "st/mesa: Record shader access qualifiers for images" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Kenneth Graunke Oct. 15, 2018, 10:34 p.m.
From: Jason Ekstrand <jason.ekstrand@intel.com>

They're not required to be the same as the access flag on the image
unit.  For hardware that does shader image lowering based on the
qualifier (Intel), it may be required for state setup.
---
 src/gallium/include/pipe/p_state.h     |  1 +
 src/mesa/state_tracker/st_atom_image.c | 27 ++++++++++++++++++++++----
 src/mesa/state_tracker/st_cb_texture.c |  2 +-
 src/mesa/state_tracker/st_texture.c    |  2 +-
 src/mesa/state_tracker/st_texture.h    |  5 +++--
 5 files changed, 29 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
index a58d91fb3dd..331417b1d7f 100644
--- a/src/gallium/include/pipe/p_state.h
+++ b/src/gallium/include/pipe/p_state.h
@@ -485,6 +485,7 @@  struct pipe_image_view
    struct pipe_resource *resource; /**< resource into which this is a view  */
    enum pipe_format format;      /**< typed PIPE_FORMAT_x */
    unsigned access;              /**< PIPE_IMAGE_ACCESS_x */
+   unsigned shader_access;       /**< PIPE_IMAGE_ACCESS_x */
 
    union {
       struct {
diff --git a/src/mesa/state_tracker/st_atom_image.c b/src/mesa/state_tracker/st_atom_image.c
index 421c926cf04..db3539259ce 100644
--- a/src/mesa/state_tracker/st_atom_image.c
+++ b/src/mesa/state_tracker/st_atom_image.c
@@ -50,7 +50,7 @@ 
  */
 void
 st_convert_image(const struct st_context *st, const struct gl_image_unit *u,
-                 struct pipe_image_view *img)
+                 struct pipe_image_view *img, unsigned shader_access)
 {
    struct st_texture_object *stObj = st_texture_object(u->TexObj);
 
@@ -70,6 +70,23 @@  st_convert_image(const struct st_context *st, const struct gl_image_unit *u,
       unreachable("bad gl_image_unit::Access");
    }
 
+   switch (shader_access) {
+   case GL_NONE:
+      img->shader_access = 0;
+      break;
+   case GL_READ_ONLY:
+      img->shader_access = PIPE_IMAGE_ACCESS_READ;
+      break;
+   case GL_WRITE_ONLY:
+      img->shader_access = PIPE_IMAGE_ACCESS_WRITE;
+      break;
+   case GL_READ_WRITE:
+      img->shader_access = PIPE_IMAGE_ACCESS_READ_WRITE;
+      break;
+   default:
+      unreachable("bad gl_image_unit::Access");
+   }
+
    if (stObj->base.Target == GL_TEXTURE_BUFFER) {
       struct st_buffer_object *stbuf =
          st_buffer_object(stObj->base.BufferObject);
@@ -125,7 +142,8 @@  st_convert_image(const struct st_context *st, const struct gl_image_unit *u,
 void
 st_convert_image_from_unit(const struct st_context *st,
                            struct pipe_image_view *img,
-                           GLuint imgUnit)
+                           GLuint imgUnit,
+                           unsigned shader_access)
 {
    struct gl_image_unit *u = &st->ctx->ImageUnits[imgUnit];
 
@@ -134,7 +152,7 @@  st_convert_image_from_unit(const struct st_context *st,
       return;
    }
 
-   st_convert_image(st, u, img);
+   st_convert_image(st, u, img, shader_access);
 }
 
 static void
@@ -153,7 +171,8 @@  st_bind_images(struct st_context *st, struct gl_program *prog,
    for (i = 0; i < prog->info.num_images; i++) {
       struct pipe_image_view *img = &images[i];
 
-      st_convert_image_from_unit(st, img, prog->sh.ImageUnits[i]);
+      st_convert_image_from_unit(st, img, prog->sh.ImageUnits[i],
+                                 prog->sh.ImageAccess[i]);
    }
    cso_set_shader_images(st->cso_context, shader_type, 0,
                          prog->info.num_images, images);
diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index e6e27a852f5..b8cc616d8f2 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -3237,7 +3237,7 @@  st_NewImageHandle(struct gl_context *ctx, struct gl_image_unit *imgObj)
    struct pipe_context *pipe = st->pipe;
    struct pipe_image_view image;
 
-   st_convert_image(st, imgObj, &image);
+   st_convert_image(st, imgObj, &image, GL_READ_WRITE);
 
    return pipe->create_image_handle(pipe, &image);
 }
diff --git a/src/mesa/state_tracker/st_texture.c b/src/mesa/state_tracker/st_texture.c
index 9655eede5fe..56d01d39bf0 100644
--- a/src/mesa/state_tracker/st_texture.c
+++ b/src/mesa/state_tracker/st_texture.c
@@ -540,7 +540,7 @@  st_create_image_handle_from_unit(struct st_context *st,
    struct pipe_context *pipe = st->pipe;
    struct pipe_image_view img;
 
-   st_convert_image_from_unit(st, &img, imgUnit);
+   st_convert_image_from_unit(st, &img, imgUnit, GL_READ_WRITE);
 
    return pipe->create_image_handle(pipe, &img);
 }
diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h
index 726ab78dad4..7fb3f09a1c2 100644
--- a/src/mesa/state_tracker/st_texture.h
+++ b/src/mesa/state_tracker/st_texture.h
@@ -320,12 +320,13 @@  st_compressed_format_fallback(struct st_context *st, mesa_format format);
 
 void
 st_convert_image(const struct st_context *st, const struct gl_image_unit *u,
-                 struct pipe_image_view *img);
+                 struct pipe_image_view *img, unsigned shader_access);
 
 void
 st_convert_image_from_unit(const struct st_context *st,
                            struct pipe_image_view *img,
-                           GLuint imgUnit);
+                           GLuint imgUnit,
+                           unsigned shader_access);
 
 void
 st_convert_sampler(const struct st_context *st,

Comments

On Mon, Oct 15, 2018 at 6:35 PM Kenneth Graunke <kenneth@whitecape.org>
wrote:

> From: Jason Ekstrand <jason.ekstrand@intel.com>
>
> They're not required to be the same as the access flag on the image
> unit.  For hardware that does shader image lowering based on the
> qualifier (Intel), it may be required for state setup.
> ---
>  src/gallium/include/pipe/p_state.h     |  1 +
>  src/mesa/state_tracker/st_atom_image.c | 27 ++++++++++++++++++++++----
>  src/mesa/state_tracker/st_cb_texture.c |  2 +-
>  src/mesa/state_tracker/st_texture.c    |  2 +-
>  src/mesa/state_tracker/st_texture.h    |  5 +++--
>  5 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/src/gallium/include/pipe/p_state.h
> b/src/gallium/include/pipe/p_state.h
> index a58d91fb3dd..331417b1d7f 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -485,6 +485,7 @@ struct pipe_image_view
>     struct pipe_resource *resource; /**< resource into which this is a
> view  */
>     enum pipe_format format;      /**< typed PIPE_FORMAT_x */
>     unsigned access;              /**< PIPE_IMAGE_ACCESS_x */
> +   unsigned shader_access;       /**< PIPE_IMAGE_ACCESS_x */
>

Can you use uint16_t for both access masks? Other than that:

Reviewed-by: Marek Olšák <marek.olsak@amd.com>

Marek
On Friday, October 19, 2018 3:04:45 PM PDT Marek Olšák wrote:
> On Mon, Oct 15, 2018 at 6:35 PM Kenneth Graunke <kenneth@whitecape.org>
> wrote:
> 
> > From: Jason Ekstrand <jason.ekstrand@intel.com>
> >
> > They're not required to be the same as the access flag on the image
> > unit.  For hardware that does shader image lowering based on the
> > qualifier (Intel), it may be required for state setup.
> > ---
> >  src/gallium/include/pipe/p_state.h     |  1 +
> >  src/mesa/state_tracker/st_atom_image.c | 27 ++++++++++++++++++++++----
> >  src/mesa/state_tracker/st_cb_texture.c |  2 +-
> >  src/mesa/state_tracker/st_texture.c    |  2 +-
> >  src/mesa/state_tracker/st_texture.h    |  5 +++--
> >  5 files changed, 29 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/gallium/include/pipe/p_state.h
> > b/src/gallium/include/pipe/p_state.h
> > index a58d91fb3dd..331417b1d7f 100644
> > --- a/src/gallium/include/pipe/p_state.h
> > +++ b/src/gallium/include/pipe/p_state.h
> > @@ -485,6 +485,7 @@ struct pipe_image_view
> >     struct pipe_resource *resource; /**< resource into which this is a
> > view  */
> >     enum pipe_format format;      /**< typed PIPE_FORMAT_x */
> >     unsigned access;              /**< PIPE_IMAGE_ACCESS_x */
> > +   unsigned shader_access;       /**< PIPE_IMAGE_ACCESS_x */
> >
> 
> Can you use uint16_t for both access masks? Other than that:
> 
> Reviewed-by: Marek Olšák <marek.olsak@amd.com>
> 
> Marek

Good call, that way we don't make the struct any larger.  I made that
changed and pushed the patch.  Thanks for the review!