[Mesa-dev] [PATCH v2 05/27] i965/miptree: Use the isl helpers for creating aux surfaces

[Jason Ekstrand]

On Jul 28, 2016 9:00 AM, "Pohjolainen, Topi" <topi.pohjolainen at intel.com>
wrote:
>
> On Tue, Jul 26, 2016 at 03:11:09PM -0700, Jason Ekstrand wrote:
> > In order for the calculations of things such as fast clear rectangles to
> > work, we need more details of the auxiliary surface to be correct.  In
> > particular, we need to be able to trust the width and height fields.
> > (These are not necessarily what you want coming out of the miptree.)
The
> > only values state setup really cares about are the row and array pitch
and
> > those we can safely stomp from the miptree.
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 52
++++-----------------------
> >  1 file changed, 6 insertions(+), 46 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index 330291c..f762106 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -3189,9 +3189,6 @@ intel_miptree_get_aux_isl_surf(struct brw_context
*brw,
> >                                 struct isl_surf *surf,
> >                                 enum isl_aux_usage *usage)
> >  {
> > -   /* Much is the same as the regular surface */
> > -   intel_miptree_get_isl_surf(brw, mt->mcs_mt, surf);
> > -
> >     /* Figure out the layout */
> >     if (_mesa_get_format_base_format(mt->format) == GL_DEPTH_COMPONENT)
{
> >        *usage = ISL_AUX_USAGE_HIZ;
> > @@ -3217,9 +3214,7 @@ intel_miptree_get_aux_isl_surf(struct brw_context
*brw,
> >        unreachable("Invalid MCS miptree");
> >
> >     case ISL_AUX_USAGE_HIZ:
> > -      surf->format = ISL_FORMAT_HIZ;
> > -      surf->tiling = ISL_TILING_HIZ;
> > -      surf->usage = ISL_SURF_USAGE_HIZ_BIT;
> > +      isl_surf_get_hiz_surf(&brw->isl_dev, surf, surf);
>
> Using the same instance both as input and output is dangerous in my
opinion.
> Could we introduce a temporary for the input (main surface) instead?

It is safe but I agree adding a temp is a lot cleaner.  Eventually we
should probably make the function take the main surface as a parameter but
it was easier in the interim to just recreate it.

> >        break;
> >
> >     case ISL_AUX_USAGE_MCS:
> > @@ -3231,16 +3226,7 @@ intel_miptree_get_aux_isl_surf(struct
brw_context *brw,
> >        if (brw->gen >= 9)
> >           assert(mt->halign == 16);
> >
> > -      surf->usage = ISL_SURF_USAGE_MCS_BIT;
> > -
> > -      switch (mt->num_samples) {
> > -      case 2:  surf->format = ISL_FORMAT_MCS_2X;   break;
> > -      case 4:  surf->format = ISL_FORMAT_MCS_4X;   break;
> > -      case 8:  surf->format = ISL_FORMAT_MCS_8X;   break;
> > -      case 16: surf->format = ISL_FORMAT_MCS_16X;  break;
> > -      default:
> > -         unreachable("Invalid number of samples");
> > -      }
> > +      isl_surf_get_mcs_surf(&brw->isl_dev, surf, surf);
> >        break;
> >
> >     case ISL_AUX_USAGE_CCS_D:
> > @@ -3259,39 +3245,13 @@ intel_miptree_get_aux_isl_surf(struct
brw_context *brw,
> >        if (brw->gen >= 8)
> >           assert(mt->halign == 16);
> >
> > -      surf->tiling = ISL_TILING_CCS;
> > -      surf->usage = ISL_SURF_USAGE_CCS_BIT;
> > -
> > -      if (brw->gen >= 9) {
> > -         assert(mt->tiling == I915_TILING_Y);
> > -         switch (_mesa_get_format_bytes(mt->format)) {
> > -         case 4:  surf->format = ISL_FORMAT_GEN9_CCS_32BPP;   break;
> > -         case 8:  surf->format = ISL_FORMAT_GEN9_CCS_64BPP;   break;
> > -         case 16: surf->format = ISL_FORMAT_GEN9_CCS_128BPP;  break;
> > -         default:
> > -            unreachable("Invalid format size for color compression");
> > -         }
> > -      } else if (mt->tiling == I915_TILING_Y) {
> > -         switch (_mesa_get_format_bytes(mt->format)) {
> > -         case 4:  surf->format = ISL_FORMAT_GEN7_CCS_32BPP_Y;    break;
> > -         case 8:  surf->format = ISL_FORMAT_GEN7_CCS_64BPP_Y;    break;
> > -         case 16: surf->format = ISL_FORMAT_GEN7_CCS_128BPP_Y;   break;
> > -         default:
> > -            unreachable("Invalid format size for color compression");
> > -         }
> > -      } else {
> > -         assert(mt->tiling == I915_TILING_X);
> > -         switch (_mesa_get_format_bytes(mt->format)) {
> > -         case 4:  surf->format = ISL_FORMAT_GEN7_CCS_32BPP_X;    break;
> > -         case 8:  surf->format = ISL_FORMAT_GEN7_CCS_64BPP_X;    break;
> > -         case 16: surf->format = ISL_FORMAT_GEN7_CCS_128BPP_X;   break;
> > -         default:
> > -            unreachable("Invalid format size for color compression");
> > -         }
> > -      }
> > +      isl_surf_get_ccs_surf(&brw->isl_dev, surf, surf);
> >        break;
> >     }
> >
> > +   /* We want the pitch of the actual aux buffer. */
> > +   surf->row_pitch = mt->mcs_mt->pitch;
> > +
> >     /* Auxiliary surfaces in ISL have compressed formats and
array_pitch_el_rows
> >      * is in elements.  This doesn't match intel_mipmap_tree::qpitch
which is
> >      * in elements of the primary color surface so we have to divide by
the
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160728/3daac55f/attachment.html>