[Mesa-dev] [PATCHv2 07/14] i965: Implement surface state set-up for shader images.
Francisco Jerez
currojerez at riseup.net
Sat Aug 8 04:04:32 PDT 2015
Jason Ekstrand <jason at jlekstrand.net> writes:
> On Wed, May 13, 2015 at 9:43 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>> v2: Add SKL support.
>> ---
>> src/mesa/drivers/dri/i965/brw_context.h | 2 +
>> src/mesa/drivers/dri/i965/brw_surface_formats.c | 109 +++++++++++++++++++++++
>> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 77 ++++++++++++++++
>> 3 files changed, 188 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
>> index 2dcc23c..c55dcec 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.h
>> +++ b/src/mesa/drivers/dri/i965/brw_context.h
>> @@ -1741,6 +1741,8 @@ void brw_upload_abo_surfaces(struct brw_context *brw,
>> bool brw_render_target_supported(struct brw_context *brw,
>> struct gl_renderbuffer *rb);
>> uint32_t brw_depth_format(struct brw_context *brw, mesa_format format);
>> +mesa_format brw_lower_mesa_image_format(const struct brw_device_info *devinfo,
>> + mesa_format format);
>>
>> /* brw_performance_monitor.c */
>> void brw_init_performance_monitors(struct brw_context *brw);
>> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c b/src/mesa/drivers/dri/i965/brw_surface_formats.c
>> index 016f87a..e0e7fb6 100644
>> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
>> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
>> @@ -805,3 +805,112 @@ brw_depth_format(struct brw_context *brw, mesa_format format)
>> unreachable("Unexpected depth format.");
>> }
>> }
>> +
>> +mesa_format
>> +brw_lower_mesa_image_format(const struct brw_device_info *devinfo,
>> + mesa_format format)
>> +{
>> + switch (format) {
>> + /* These are never lowered. Up to BDW we'll have to fall back to untyped
>> + * surface access for 128bpp formats.
>> + */
>> + case MESA_FORMAT_RGBA_UINT32:
>> + case MESA_FORMAT_RGBA_SINT32:
>> + case MESA_FORMAT_RGBA_FLOAT32:
>> + case MESA_FORMAT_R_UINT32:
>> + case MESA_FORMAT_R_SINT32:
>> + case MESA_FORMAT_R_FLOAT32:
>> + return format;
>
> If it's an unsupported format, why not use MESA_FORMAT_NONE? That
> would make it easier for functions that call this function to know
> that it's just not supported and they shouldn't bother trying.
>
Because they are all supported. As the comment says these formats are
always accessed unlowered, the catch is that the shader may have to use
a different (much more painful) mechanism to access them depending on
the hardware generation -- But this function simply implements the
mapping between API image formats and what the 3D pipeline will see,
which is R(GBA)_*32 in this case regardless of whether untyped or typed
messages can be used.
>> +
>> + /* From HSW to BDW the only 64bpp format supported for typed access is
>> + * RGBA_UINT16. IVB falls back to untyped.
>> + */
>> + case MESA_FORMAT_RGBA_UINT16:
>> + case MESA_FORMAT_RGBA_SINT16:
>> + case MESA_FORMAT_RGBA_FLOAT16:
>> + case MESA_FORMAT_RG_UINT32:
>> + case MESA_FORMAT_RG_SINT32:
>> + case MESA_FORMAT_RG_FLOAT32:
>> + return (devinfo->gen >= 9 ? format :
>> + devinfo->gen >= 8 || devinfo->is_haswell ?
>> + MESA_FORMAT_RGBA_UINT16 : MESA_FORMAT_RG_UINT32);
>> +
>> + /* Up to BDW no SINT or FLOAT formats of less than 32 bits per component
>> + * are supported. IVB doesn't support formats with more than one component
>> + * for typed access. For 8 and 16 bpp formats IVB relies on the
>> + * undocumented behavior that typed reads from R_UINT8 and R_UINT16
>> + * surfaces actually do a 32-bit misaligned read. The alternative would be
>> + * to use two surface state entries with different formats for each image,
>> + * one for reading (using R_UINT32) and another one for writing (using
>> + * R_UINT8 or R_UINT16), but that would complicate the shaders we generate
>> + * even more.
>> + */
>
> Let's make sure I'm understanding this correctly. On BDW and HSW, we
> can just use the UINT format for SINT and FLOAT. On IVB, we set the
> surface state to UINT32 and unpack the components in the shader? What
> happens with writes on IVB to 16-bpp images?
>
Heh, not exactly. On IVB we disobey the hardware spec and bind 16-bpp
formats as R16_UINT and 8-bpp formats as R8_UINT, which aren't
documented to work for typed reads. Writes work just fine and update
the right 8- or 16-bit word on the image. Reads *almost* do what one
would expect: The LSBs of the values returned from the data port will
contain the texel we asked for, but the MSBs will be filled with garbage
which we simply mask out
(c.f. image_format_info::has_undefined_high_bits).
>> + case MESA_FORMAT_RGBA_UINT8:
>> + case MESA_FORMAT_RGBA_SINT8:
>> + return (devinfo->gen >= 9 ? format :
>> + devinfo->gen >= 8 || devinfo->is_haswell ?
>> + MESA_FORMAT_RGBA_UINT8 : MESA_FORMAT_R_UINT32);
>> +
>> + case MESA_FORMAT_RG_UINT16:
>> + case MESA_FORMAT_RG_SINT16:
>> + case MESA_FORMAT_RG_FLOAT16:
>> + return (devinfo->gen >= 9 ? format :
>> + devinfo->gen >= 8 || devinfo->is_haswell ?
>> + MESA_FORMAT_RG_UINT16 : MESA_FORMAT_R_UINT32);
>> +
>> + case MESA_FORMAT_RG_UINT8:
>> + case MESA_FORMAT_RG_SINT8:
>> + return (devinfo->gen >= 9 ? format :
>> + devinfo->gen >= 8 || devinfo->is_haswell ?
>> + MESA_FORMAT_RG_UINT8 : MESA_FORMAT_R_UINT16);
>> +
>> + case MESA_FORMAT_R_UINT16:
>> + case MESA_FORMAT_R_FLOAT16:
>> + case MESA_FORMAT_R_SINT16:
>> + return (devinfo->gen >= 9 ? format : MESA_FORMAT_R_UINT16);
>> +
>> + case MESA_FORMAT_R_UINT8:
>> + case MESA_FORMAT_R_SINT8:
>> + return (devinfo->gen >= 9 ? format : MESA_FORMAT_R_UINT8);
>> +
>> + /* Neither the 2/10/10/10 nor the 11/11/10 packed formats are supported
>> + * by the hardware.
>> + */
>> + case MESA_FORMAT_R10G10B10A2_UINT:
>> + case MESA_FORMAT_R10G10B10A2_UNORM:
>> + case MESA_FORMAT_R11G11B10_FLOAT:
>> + return MESA_FORMAT_R_UINT32;
>> +
>> + /* No normalized fixed-point formats are supported by the hardware. */
>> + case MESA_FORMAT_RGBA_UNORM16:
>> + case MESA_FORMAT_RGBA_SNORM16:
>> + return (devinfo->gen >= 8 || devinfo->is_haswell ?
>> + MESA_FORMAT_RGBA_UINT16 : MESA_FORMAT_RG_UINT32);
>> +
>> + case MESA_FORMAT_R8G8B8A8_UNORM:
>> + case MESA_FORMAT_R8G8B8A8_SNORM:
>> + return (devinfo->gen >= 8 || devinfo->is_haswell ?
>> + MESA_FORMAT_RGBA_UINT8 : MESA_FORMAT_R_UINT32);
>> +
>> + case MESA_FORMAT_R16G16_UNORM:
>> + case MESA_FORMAT_R16G16_SNORM:
>> + return (devinfo->gen >= 8 || devinfo->is_haswell ?
>> + MESA_FORMAT_RG_UINT16 : MESA_FORMAT_R_UINT32);
>> +
>> + case MESA_FORMAT_R8G8_UNORM:
>> + case MESA_FORMAT_R8G8_SNORM:
>> + return (devinfo->gen >= 8 || devinfo->is_haswell ?
>> + MESA_FORMAT_RG_UINT8 : MESA_FORMAT_R_UINT16);
>> +
>> + case MESA_FORMAT_R_UNORM16:
>> + case MESA_FORMAT_R_SNORM16:
>> + return MESA_FORMAT_R_UINT16;
>> +
>> + case MESA_FORMAT_R_UNORM8:
>> + case MESA_FORMAT_R_SNORM8:
>> + return MESA_FORMAT_R_UINT8;
>> +
>> + default:
>> + unreachable("Not reached");
>
> Please be more descriptive.
>
Changed to "Unknown image format" locally.
>> + }
>> +}
>> 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 160dd2f..f8cb143 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> @@ -1022,6 +1022,83 @@ const struct brw_tracked_state brw_cs_abo_surfaces = {
>> .emit = brw_upload_cs_abo_surfaces,
>> };
>>
>> +static uint32_t
>> +get_image_format(struct brw_context *brw, mesa_format format, GLenum access)
>> +{
>> + if (access == GL_WRITE_ONLY) {
>> + return brw_format_for_mesa_format(format);
>> + } else {
>> + /* Typed surface reads support a very limited subset of the shader
>> + * image formats. Translate it into the closest format the
>> + * hardware supports.
>> + */
>> + if ((_mesa_get_format_bytes(format) >= 16 && brw->gen <= 8) ||
>> + (_mesa_get_format_bytes(format) >= 8 &&
>> + (brw->gen == 7 && !brw->is_haswell)))
>
> It's tricky to verify, but this list of conditions doesn't look much
> like the switch above. Maybe I'm missing something.
>
This check is equivalent to the one in
image_format_info::has_matching_typed_format (I considered re-using the
same function but then noticed that this file is C so I'd have to change
a number of things of the image_format_info helpers in order to be able
to include them here, what I didn't quite feel like doing and didn't
seem worth the effort in order to save 2 LOC). The check is orthogonal
to format lowering -- In cases where the lowered format can be accessed
using typed messages we just go ahead and bind it, otherwise I set the
format in the surface state structure to RAW in the expectation that the
shader will want access them using untyped messages, because it's a
requirement for them to work.
>> + return BRW_SURFACEFORMAT_RAW;
>> + else
>> + return brw_format_for_mesa_format(
>> + brw_lower_mesa_image_format(brw->intelScreen->devinfo, format));
>> + }
>> +}
>
> Why are get_image_format and lower_mesa_image_format separate
> functions? Again, it seems like we have decisions being made two
> different places. brw_lower_mesa_image_format lowers it to the format
> as if it were write_only and then we do more switching here based on
> gen. Maybe this is truly easier, but it seems odd.
>
brw_lower_mesa_image_format() implements the mapping between API formats
and the format the shader will see. This function translates the
lowered shader-format into the format that will be set in the surface
state structure. None of these conditions directly apply to the format
translation that will be done in the compiler, which is why it's kept
separate from brw_lower_mesa_image_format() (which is also called from
the compiler back-end). One condition relates to setting the surface
state format to RAW (which forgets useful format information so we don't
want to return that to the compiler back-end), and the other is about
the additional optimization done in cases where we know that the image
is only ever written to by the shader -- In those cases we just bind the
original unlowered format and expect the shader to skip format
conversion completely. You could argue that this could be handled in
brw_lower_mesa_image_format() by adding a "GLenum access" parameter,
however almost none of the callers of brw_lower_mesa_image_format() have
the image access mode readily available, and passing it down from
brw_fs_nir.cpp would uglify a number of interfaces, so it seemed easier
to keep the access mode-dependent translation orthogonal from the actual
format lowering.
>> +
>> +static void
>> +update_image_surface(struct brw_context *brw,
>> + struct gl_image_unit *u,
>> + GLenum access,
>> + unsigned surface_idx,
>> + uint32_t *surf_offset,
>> + struct brw_image_param *param)
>> +{
>> + if (u->_Valid) {
>> + struct gl_texture_object *obj = u->TexObj;
>> + const unsigned format = get_image_format(brw, u->_ActualFormat, access);
>> +
>> + if (obj->Target == GL_TEXTURE_BUFFER) {
>> + struct intel_buffer_object *intel_obj =
>> + intel_buffer_object(obj->BufferObject);
>> + const unsigned texel_size = (format == BRW_SURFACEFORMAT_RAW ? 1 :
>> + _mesa_get_format_bytes(u->_ActualFormat));
>> +
>> + brw->vtbl.emit_buffer_surface_state(
>> + brw, surf_offset, intel_obj->buffer, obj->BufferOffset,
>> + format, intel_obj->Base.Size / texel_size, texel_size,
>> + access != GL_READ_ONLY);
>> +
>> + } else {
>> + struct intel_texture_object *intel_obj = intel_texture_object(obj);
>> + struct intel_mipmap_tree *mt = intel_obj->mt;
>> +
>> + if (format == BRW_SURFACEFORMAT_RAW) {
>> + brw->vtbl.emit_buffer_surface_state(
>> + brw, surf_offset, mt->bo, mt->offset,
>> + format, mt->bo->size - mt->offset, 1 /* pitch */,
>> + access != GL_READ_ONLY);
>> +
>> + } else {
>> + const unsigned min_layer = obj->MinLayer + u->Layer;
>> + const unsigned min_level = obj->MinLevel + u->Level;
>> + const unsigned num_layers = (!u->Layered ? 1 :
>> + obj->Target == GL_TEXTURE_CUBE_MAP ? 6 :
>> + mt->logical_depth0);
>> + const GLenum target = (obj->Target == GL_TEXTURE_CUBE_MAP ||
>> + obj->Target == GL_TEXTURE_CUBE_MAP_ARRAY ?
>> + GL_TEXTURE_2D_ARRAY : obj->Target);
>> +
>> + brw->vtbl.emit_texture_surface_state(
>> + brw, mt, target,
>> + min_layer, min_layer + num_layers,
>> + min_level, min_level + 1,
>> + format, SWIZZLE_XYZW,
>> + surf_offset, access != GL_READ_ONLY, false);
>> + }
>> + }
>> +
>> + } else {
>> + brw->vtbl.emit_null_surface_state(brw, 1, 1, 1, surf_offset);
>> + }
>> +}
>> +
>> void
>> gen4_init_vtable_surface_functions(struct brw_context *brw)
>> {
>> --
>> 2.3.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150808/be553cb0/attachment.sig>
More information about the mesa-dev
mailing list