[Mesa-dev] [PATCH 17/22] glsl/link, i965: Make ImageAccess four-state

Kenneth Graunke kenneth at whitecape.org
Wed Aug 29 06:02:19 UTC 2018


On Friday, August 17, 2018 1:06:23 PM PDT Jason Ekstrand wrote:
> The GLSL spec allows you to set both the "readonly" and "writeonly"
> qualifiers on images to indicate that it can only be used with
> imageSize.  However, we had no way of representing this int he linked
> shader and flagged it as GL_READ_ONLY.  This is good from a "does it use
> this buffer?" perspective but not from a format and access lowering
> perspective.  By using GL_NONE for if "readonly" and "writeonly" are
> both set, we can detect this case in the driver and handle it correctly.
> 
> Nothing currently relies on the type of surface in the "readonly" +
> "writeonly" case but that's about to change.  i965 is the only drier
> which uses the ImageAccess field and gl_bindless_image::access is
> currently unused.
> ---
>  src/compiler/glsl/gl_nir_link_uniforms.c         |  8 +++++---
>  src/compiler/glsl/link_uniforms.cpp              |  8 +++++---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 ++++++------
>  src/mesa/main/mtypes.h                           |  9 ++++++---
>  4 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/src/compiler/glsl/gl_nir_link_uniforms.c b/src/compiler/glsl/gl_nir_link_uniforms.c
> index 1573e30c41e..159dfeca59f 100644
> --- a/src/compiler/glsl/gl_nir_link_uniforms.c
> +++ b/src/compiler/glsl/gl_nir_link_uniforms.c
> @@ -425,9 +425,11 @@ nir_link_uniform(struct gl_context *ctx,
>  
>           /* Set image access qualifiers */
>           const GLenum access =
> -            (state->current_var->data.image.read_only ? GL_READ_ONLY :
> -             state->current_var->data.image.write_only ? GL_WRITE_ONLY :
> -             GL_READ_WRITE);
> +            state->current_var->data.image.read_only ?
> +            (state->current_var->data.image.write_only ? GL_NONE :
> +                                                         GL_READ_ONLY) :
> +            (state->current_var->data.image.write_only ? GL_WRITE_ONLY :
> +                                                         GL_READ_WRITE);
>           for (unsigned i = image_index;
>                i < MIN2(state->next_image_index, MAX_IMAGE_UNIFORMS);
>                i++) {
> diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp
> index 8d3f95fe114..f86d354c437 100644
> --- a/src/compiler/glsl/link_uniforms.cpp
> +++ b/src/compiler/glsl/link_uniforms.cpp
> @@ -690,9 +690,11 @@ private:
>  
>           /* Set image access qualifiers */
>           const GLenum access =
> -            (current_var->data.memory_read_only ? GL_READ_ONLY :
> -             current_var->data.memory_write_only ? GL_WRITE_ONLY :
> -                GL_READ_WRITE);
> +            current_var->data.memory_read_only ?
> +            (current_var->data.memory_write_only ? GL_NONE :
> +                                                   GL_READ_ONLY) :
> +            (current_var->data.memory_write_only ? GL_WRITE_ONLY :
> +                                                   GL_READ_WRITE);
>  
>           if (current_var->data.bindless) {
>              if (!set_opaque_indices(base_type, uniform, name,
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 2aef0ef59f7..35bb49f30f2 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -1455,7 +1455,7 @@ get_image_format(struct brw_context *brw, mesa_format format, GLenum access)
>  {
>     const struct gen_device_info *devinfo = &brw->screen->devinfo;
>     enum isl_format hw_format = brw_isl_format_for_mesa_format(format);
> -   if (access == GL_WRITE_ONLY) {
> +   if (access == GL_WRITE_ONLY || access == GL_NONE) {
>        return hw_format;
>     } else if (isl_has_matching_typed_storage_image_format(devinfo, hw_format)) {
>        /* Typed surface reads support a very limited subset of the shader
> @@ -1523,6 +1523,7 @@ update_image_surface(struct brw_context *brw,
>     if (_mesa_is_image_unit_valid(&brw->ctx, u)) {
>        struct gl_texture_object *obj = u->TexObj;
>        const unsigned format = get_image_format(brw, u->_ActualFormat, access);
> +      const bool written = (access != GL_READ_ONLY && access != GL_NONE);
>  
>        if (obj->Target == GL_TEXTURE_BUFFER) {
>           const unsigned texel_size = (format == ISL_FORMAT_RAW ? 1 :
> @@ -1530,13 +1531,12 @@ update_image_surface(struct brw_context *brw,
>           const unsigned buffer_size = buffer_texture_range_size(brw, obj);
>           struct brw_bo *const bo = !obj->BufferObject ? NULL :
>              intel_bufferobj_buffer(brw, intel_buffer_object(obj->BufferObject),
> -                                   obj->BufferOffset, buffer_size,
> -                                   access != GL_READ_ONLY);
> +                                   obj->BufferOffset, buffer_size, written);
>  
>           brw_emit_buffer_surface_state(
>              brw, surf_offset, bo, obj->BufferOffset,
>              format, buffer_size, texel_size,
> -            access != GL_READ_ONLY ? RELOC_WRITE : 0);
> +            written ? RELOC_WRITE : 0);
>  
>           update_buffer_image_param(brw, u, surface_idx, param);
>  
> @@ -1560,7 +1560,7 @@ update_image_surface(struct brw_context *brw,
>              brw_emit_buffer_surface_state(
>                 brw, surf_offset, mt->bo, mt->offset,
>                 format, mt->bo->size - mt->offset, 1 /* pitch */,
> -               access != GL_READ_ONLY ? RELOC_WRITE : 0);
> +               written ? RELOC_WRITE : 0);
>  
>           } else {
>              const int surf_index = surf_offset - &brw->wm.base.surf_offset[0];
> @@ -1571,7 +1571,7 @@ update_image_surface(struct brw_context *brw,
>              brw_emit_surface_state(brw, mt, mt->target, view,
>                                     ISL_AUX_USAGE_NONE,
>                                     surf_offset, surf_index,
> -                                   access == GL_READ_ONLY ? 0 : RELOC_WRITE);
> +                                   written ? RELOC_WRITE : 0);
>           }
>  
>           isl_surf_fill_image_param(&brw->isl_dev, param, &mt->surf, &view);
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 50ef898fc4c..4fce0418a6f 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2013,7 +2013,9 @@ struct gl_bindless_image
>     /** Whether this bindless image is bound to a unit. */
>     GLboolean bound;
>  
> -   /** Access qualifier (GL_READ_WRITE, GL_READ_ONLY, GL_WRITE_ONLY) */
> +   /** Access qualifier (GL_READ_WRITE, GL_READ_ONLY, GL_WRITE_ONLY, or
> +    * GL_NONE to indicate bot read-only and write-only)

typo - "bot" -> "both"

Patches 15-17 are:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180828/7426d899/attachment.sig>


More information about the mesa-dev mailing list