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 <kenn...@whitecape.org>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
