[Mesa-dev] [PATCH v4 21/34] i965/state: Use ISL for emitting image surfaces

[Jason Ekstrand]

On Thu, Jul 14, 2016 at 3:10 PM, Chad Versace <chad.versace at intel.com>
wrote:

> On Wed 13 Jul 2016, Jason Ekstrand wrote:
> > ---
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 32
> ++++++++++++++++--------
> >  1 file changed, 21 insertions(+), 11 deletions(-)
> >
> > 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 65a1f3c..5873ea5 100644
> > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > @@ -1400,22 +1400,32 @@ update_image_surface(struct brw_context *brw,
> >                 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);
> > +
> > +            struct isl_view view = {
> > +               .format = format,
> > +               .base_level = obj->MinLevel + u->Level,
> > +               .levels = 1,
> > +               .base_array_layer = obj->MinLayer + u->_Layer,
> > +               .array_len = num_layers,
>
> FYI, I carefully checked that replacing
>    brw->vtbl.emit_texture_surface_state(..., min_layer + num_layers, ...)
> with
>    .base_array_layer = obj->MinLayer + u->_Layer,
>    .array_len = num_layers,
> does the correct thing.
>
> > +               .channel_select = {
> > +                  ISL_CHANNEL_SELECT_RED,
> > +                  ISL_CHANNEL_SELECT_GREEN,
> > +                  ISL_CHANNEL_SELECT_BLUE,
> > +                  ISL_CHANNEL_SELECT_ALPHA,
> > +               },
> > +               .usage = ISL_SURF_USAGE_STORAGE_BIT,
> > +            };
> > +
> >              const int surf_index = surf_offset -
> &brw->wm.base.surf_offset[0];
> >
> > -            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, surf_index, access != GL_READ_ONLY, false);
> > +            brw_emit_surface_state(brw, mt, &view,
> > +
>  surface_state_infos[brw->gen].rb_mocs, false,
> > +                                   surf_offset, surf_index,
> > +                                   I915_GEM_DOMAIN_SAMPLER,
> > +                                   I915_GEM_DOMAIN_SAMPLER);
> >           }
>
> I have concerns about the gem write domain. If I read
> gen8_update_texture_surface() correctly, then pre-patch the last
> argument to brw_emit_surface_state() was effectively
>     access == GL_READ_ONLY ? 0 : I915_GEM_DOMAIN_SAMPLER
> I don't want to change the gem write domain without justification.
>

I talked to Curro about this briefly.  Doing what you say above is probably
best.


>
> >           update_texture_image_param(brw, u, surface_idx, param);
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160714/be016d02/attachment.html>