[Mesa-dev] [PATCH v2 09/27] i964/blorp: Set up most aux surfaces up-front

Pohjolainen, Topi topi.pohjolainen at intel.com
Fri Jul 29 07:18:14 UTC 2016


On Fri, Jul 29, 2016 at 09:58:36AM +0300, Pohjolainen, Topi wrote:
> On Tue, Jul 26, 2016 at 03:11:13PM -0700, Jason Ekstrand wrote:
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp.c | 43 ++++++++++++++++++++---------------
> >  src/mesa/drivers/dri/i965/brw_blorp.h |  4 ++++
> >  2 files changed, 29 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
> > index cf1615f..97eddf9 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > @@ -136,8 +136,17 @@ brw_blorp_surface_info_init(struct brw_context *brw,
> >     if (mt->mcs_mt) {
> >        intel_miptree_get_aux_isl_surf(brw, mt, &info->aux_surf,
> >                                       &info->aux_usage);
> > +      info->aux_bo = mt->mcs_mt->bo;
> > +      info->aux_offset = mt->mcs_mt->offset;
> > +
> > +      /* We only really need a clear color if we also have an auxiliary
> > +       * surface.  Without one, it does nothing.
> > +       */
> > +      info->clear_color = intel_miptree_get_isl_clear_color(brw, mt);
> >     } else {
> >        info->aux_usage = ISL_AUX_USAGE_NONE;
> > +      info->aux_bo = NULL;
> > +      info->aux_offset = 0;
> >     }
> >  
> >     info->view = (struct isl_view) {
> > @@ -341,20 +350,17 @@ brw_blorp_emit_surface_state(struct brw_context *brw,
> >        surf.dim = ISL_SURF_DIM_2D;
> >     }
> >  
> > -   union isl_color_value clear_color = { .u32 = { 0, 0, 0, 0 } };
> > -
> > -   const struct isl_surf *aux_surf = NULL;
> > -   uint64_t aux_offset = 0;
> > -   if (surface->mt->mcs_mt) {
> > -      aux_surf = &surface->aux_surf;
> > -      assert(surface->mt->mcs_mt->offset == 0);
> 
> We are dropping this assert. Follow-up question further down.
> 
> > -      aux_offset = surface->mt->mcs_mt->bo->offset64;
> > +   /* Blorp doesn't support HiZ in any of the blit or slow-clear paths */
> > +   enum isl_aux_usage aux_usage = surface->aux_usage;
> > +   if (aux_usage == ISL_AUX_USAGE_HIZ)
> > +      aux_usage = ISL_AUX_USAGE_NONE;
> >  
> > -      /* We only really need a clear color if we also have an auxiliary
> > -       * surface.  Without one, it does nothing.
> > -       */
> > -      clear_color = intel_miptree_get_isl_clear_color(brw, surface->mt);
> > -   }
> > +   /* If we don't have an aux surface, the clear color is meaningless.  Don't
> > +    * bother to set it up in the surface state.
> > +    */
> > +   union isl_color_value clear_color = surface->clear_color;
> > +   if (aux_usage == ISL_AUX_USAGE_NONE)
> > +      memset(&clear_color, 0, sizeof(clear_color));
> >  
> >     uint32_t surf_offset;
> >     uint32_t *dw = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> > @@ -362,11 +368,12 @@ brw_blorp_emit_surface_state(struct brw_context *brw,
> >                                    &surf_offset);
> >  
> >     const uint32_t mocs = is_render_target ? ss_info.rb_mocs : ss_info.tex_mocs;
> > +   uint64_t aux_bo_offset = surface->aux_bo ? surface->aux_bo->offset64 : 0;
> >  
> >     isl_surf_fill_state(&brw->isl_dev, dw, .surf = &surf, .view = &surface->view,
> >                         .address = surface->bo->offset64 + surface->offset,
> > -                       .aux_surf = aux_surf, .aux_usage = surface->aux_usage,
> > -                       .aux_address = aux_offset,
> > +                       .aux_surf = &surface->aux_surf, .aux_usage = aux_usage,
> > +                       .aux_address = aux_bo_offset + surface->aux_offset,
> 
> Am I also reading this correctly but here we actually start to take
> 'mt->mcs_mt->offset' into account?
> 
> I would like to keep the assert one way or the other. I don't think we are
> ready for offsetting aux buffers yet. Or what do you think?

Reading next patch I get the feeling you are thinking of using this for hiz?
If that is the case could we assert, for example:

        assert(surface->aux_offset == 0 || aux_usage == ISL_AUX_USAGE_HIZ);

> 
> >                         .mocs = mocs, .clear_color = clear_color,
> >                         .x_offset_sa = surface->tile_x_sa,
> >                         .y_offset_sa = surface->tile_y_sa);
> > @@ -378,15 +385,15 @@ brw_blorp_emit_surface_state(struct brw_context *brw,
> >                             dw[ss_info.reloc_dw] - surface->bo->offset64,
> >                             read_domains, write_domain);
> >  
> > -   if (aux_surf) {
> > +   if (aux_usage != ISL_AUX_USAGE_NONE) {
> >        /* On gen7 and prior, the bottom 12 bits of the MCS base address are
> >         * used to store other information.  This should be ok, however, because
> >         * surface buffer addresses are always 4K page alinged.
> >         */
> > -      assert((aux_offset & 0xfff) == 0);
> > +      assert((surface->aux_offset & 0xfff) == 0);
> >        drm_intel_bo_emit_reloc(brw->batch.bo,
> >                                surf_offset + ss_info.aux_reloc_dw * 4,
> > -                              surface->mt->mcs_mt->bo,
> > +                              surface->aux_bo,
> >                                dw[ss_info.aux_reloc_dw] & 0xfff,
> >                                read_domains, write_domain);
> >     }
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h b/src/mesa/drivers/dri/i965/brw_blorp.h
> > index 98a9436..d747880 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> > @@ -76,8 +76,12 @@ struct brw_blorp_surface_info
> >     uint32_t offset;
> >  
> >     struct isl_surf aux_surf;
> > +   drm_intel_bo *aux_bo;
> > +   uint32_t aux_offset;
> >     enum isl_aux_usage aux_usage;
> >  
> > +   union isl_color_value clear_color;
> > +
> >     struct isl_view view;
> >  
> >     /* Z offset into a 3-D texture or slice of a 2-D array texture. */
> > -- 
> > 2.5.0.400.gff86faf
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list