From 48e4fa7dd8c4b777989c4a731d6ac54cfe6c24eb Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Thu, 16 Aug 2018 14:31:28 -0500 Subject: [PATCH] glsl/link,i965: Make ImageAccess four-state 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. Reviewed-by: Kenneth Graunke --- 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 434ecefb289..63e688b19a7 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 5ce0b4b5847..718293be116 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2014,7 +2014,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 both read-only and write-only) + */ GLenum16 access; /** Pointer to the base of the data. */ @@ -2131,8 +2133,9 @@ struct gl_program /** * Access qualifier specified in the shader for each image uniform - * index. Either \c GL_READ_ONLY, \c GL_WRITE_ONLY or \c - * GL_READ_WRITE. + * index. Either \c GL_READ_ONLY, \c GL_WRITE_ONLY, \c + * GL_READ_WRITE, or \c GL_NONE to indicate both read-only and + * write-only. * * It may be different, though only more strict than the value of * \c gl_image_unit::Access for the corresponding image unit. -- 2.30.2