[Mesa-dev] [PATCHv2 08/14] i965: Define and initialize image parameter structure.

[Pohjolainen, Topi]

On Wed, Aug 05, 2015 at 12:51:19PM +0300, Pohjolainen, Topi wrote:
> On Mon, Jul 20, 2015 at 07:17:48PM +0300, Francisco Jerez wrote:
> > This will be used to pass image meta-data to the shader when we cannot
> > use typed surface reads and writes.  All entries except surface_idx
> > and size are otherwise unused and will get eliminated by the uniform
> > packing pass.  size will be used for bounds checking with some image
> > formats and will be useful for ARB_shader_image_size too.  surface_idx
> > is always used.
> > 
> > v2: Add CS support.  Move the image_params array back to
> >     brw_stage_prog_data.
> > ---
> > I'm resending this (and also patches 9 and 10) because I had to make
> > some rather intrusive changes during one of my last rebases -- The
> > image_param array is now part of brw_stage_prog_data again instead of
> > brw_stage_state (ironically as it was in my very first submission of
> > these patches) because the compiler no longer has access to
> > brw_stage_state since the brw_context pointer was removed from the
> > visitors.
> > 
> >  src/mesa/drivers/dri/i965/brw_context.h          | 54 ++++++++++++++++
> >  src/mesa/drivers/dri/i965/brw_cs.cpp             |  3 +
> >  src/mesa/drivers/dri/i965/brw_gs.c               |  3 +
> >  src/mesa/drivers/dri/i965/brw_vs.c               |  5 +-
> >  src/mesa/drivers/dri/i965/brw_wm.c               |  4 ++
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 82 ++++++++++++++++++++++++
> >  6 files changed, 150 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> > index e16ad10..9ebad5b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > @@ -361,6 +361,7 @@ struct brw_stage_prog_data {
> >  
> >     GLuint nr_params;       /**< number of float params/constants */
> >     GLuint nr_pull_params;
> > +   unsigned nr_image_params;
> >  
> >     unsigned curb_read_length;
> >     unsigned total_scratch;
> > @@ -381,6 +382,59 @@ struct brw_stage_prog_data {
> >      */
> >     const gl_constant_value **param;
> >     const gl_constant_value **pull_param;
> > +
> > +   /**
> > +    * Image metadata passed to the shader as uniforms.  This is deliberately
> > +    * ignored by brw_stage_prog_data_compare() because its contents don't have
> > +    * any influence on program compilation.
> > +    */
> > +   struct brw_image_param *image_param;
> > +};
> > +
> > +/*
> > + * Image metadata structure as laid out in the shader parameter
> > + * buffer.  Entries have to be 16B-aligned for the vec4 back-end to be
> > + * able to use them.  That's okay because the padding and any unused
> > + * entries [most of them except when we're doing untyped surface
> > + * access] will be removed by the uniform packing pass.
> > + */
> > +#define BRW_IMAGE_PARAM_SURFACE_IDX_OFFSET      0
> > +#define BRW_IMAGE_PARAM_OFFSET_OFFSET           4
> > +#define BRW_IMAGE_PARAM_SIZE_OFFSET             8
> > +#define BRW_IMAGE_PARAM_STRIDE_OFFSET           12
> > +#define BRW_IMAGE_PARAM_TILING_OFFSET           16
> > +#define BRW_IMAGE_PARAM_SWIZZLING_OFFSET        20
> > +#define BRW_IMAGE_PARAM_SIZE                    24
> > +
> > +struct brw_image_param {
> > +   /** Surface binding table index. */
> > +   uint32_t surface_idx;
> > +
> > +   /** Surface X, Y and Z dimensions. */
> > +   uint32_t size[3];
> > +
> > +   /** Offset applied to the X and Y surface coordinates. */
> > +   uint32_t offset[2];
> > +
> > +   /** X-stride in bytes, Y-stride in bytes, horizontal slice stride in
> > +    * pixels, vertical slice stride in pixels.
> > +    */
> > +   uint32_t stride[4];
> > +
> > +   /** Log2 of the tiling modulus in the X, Y and Z dimension. */
> > +   uint32_t tiling[3];
> > +
> > +   /**
> > +    * Right shift to apply for bit 6 address swizzling.  Two different
> > +    * swizzles can be specified and will be applied one after the other.  The
> > +    * resulting address will be:
> > +    *
> > +    *  addr' = addr ^ ((1 << 6) & ((addr >> swizzling[0]) ^
> > +    *                              (addr >> swizzling[1])))
> > +    *
> > +    * Use \c 0xff if any of the swizzles is not required.
> > +    */
> > +   uint32_t swizzling[2];
> 
> I couldn't find any patches actually reading the swizzle values (only found
> writes to it). Hence I checked your branch "image-load-store-lower" and still
> couldn't find any users for it. Could you help me?

Okay, we discussed this offline and it was my mistake. There is a consumer
for this in brw_fs_surface_builder.cpp::emit_address_calculation().