[Mesa-dev] [PATCHv2 07/14] i965: Implement surface state set-up for shader images.

Jason Ekstrand jason at jlekstrand.net
Fri Aug 7 16:34:44 PDT 2015


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.

> +
> +   /* 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?

> +   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.

> +   }
> +}
> 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.

> +         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.

> +
> +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


More information about the mesa-dev mailing list