[Mesa-dev] [PATCH 14/20] i965: Implement logic to set up and upload an image uniform.

[Pohjolainen, Topi]

On Wed, Aug 05, 2015 at 10:36:09AM +0300, Pohjolainen, Topi wrote:
> On Tue, Jul 21, 2015 at 07:38:49PM +0300, Francisco Jerez wrote:
> > v2: Move the image_params array back to brw_stage_prog_data.
> > ---
> >  src/mesa/drivers/dri/i965/brw_shader.cpp | 31 +++++++++++++++++++++++++++++++
> >  src/mesa/drivers/dri/i965/brw_shader.h   |  1 +
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > index 24bf42d..f7186a4 100644
> > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > @@ -1402,3 +1402,34 @@ backend_shader::assign_common_binding_table_offsets(uint32_t next_binding_table_
> >  
> >     /* prog_data->base.binding_table.size will be set by brw_mark_surface_used. */
> >  }
> > +
> > +void
> > +backend_shader::setup_image_uniform_values(const gl_uniform_storage *storage)
> > +{
> > +   const unsigned stage = _mesa_program_enum_to_shader_stage(prog->Target);
> > +
> > +   for (unsigned i = 0; i < MAX2(storage->array_elements, 1); i++) {
> > +      const unsigned image_idx = storage->image[stage].index + i;
> > +      const brw_image_param *param = &stage_prog_data->image_param[image_idx];
> > +
> > +      /* Upload the brw_image_param structure.  The order is expected to match
> > +       * the BRW_IMAGE_PARAM_*_OFFSET defines.
> > +       */
> > +      setup_vector_uniform_values(
> > +         (const gl_constant_value *)&param->surface_idx, 1);
> > +      setup_vector_uniform_values(
> > +         (const gl_constant_value *)param->offset, 2);
> > +      setup_vector_uniform_values(
> > +         (const gl_constant_value *)param->size, 3);
> > +      setup_vector_uniform_values(
> > +         (const gl_constant_value *)param->stride, 4);
> > +      setup_vector_uniform_values(
> > +         (const gl_constant_value *)param->tiling, 3);
> > +      setup_vector_uniform_values(
> > +         (const gl_constant_value *)param->swizzling, 2);
> > +
> 
> I need to understand the concept of image index before I can tell how this
> works.

The mechanism looks fine to me:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

> 
> But I checked that the order and dimensions of the individual fields match the
> BRW_IMAGE_PARAM_* defines and the members of "struct brw_image_param". I
> noticed that in the structure the member "size" is the second while we give
> it as the third for the hw. Might be worth keep the order consistent between
> the two just for clarity. (Of course it works as is as the compiled program
> and the ubo-layout match).
> 
> > +      brw_mark_surface_used(
> > +         stage_prog_data,
> > +         stage_prog_data->binding_table.image_start + image_idx);
> > +   }
> > +}
> > diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h
> > index 925072f..2cc97f2 100644
> > --- a/src/mesa/drivers/dri/i965/brw_shader.h
> > +++ b/src/mesa/drivers/dri/i965/brw_shader.h
> > @@ -272,6 +272,7 @@ public:
> >  
> >     virtual void setup_vector_uniform_values(const gl_constant_value *values,
> >                                              unsigned n) = 0;
> > +   void setup_image_uniform_values(const gl_uniform_storage *storage);
> >  };
> >  
> >  uint32_t brw_texture_offset(int *offsets, unsigned num_components);
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev