On Thu, Jul 28, 2016 at 07:07:34AM -0700, Jason Ekstrand wrote:
> On Jul 28, 2016 9:00 AM, "Pohjolainen, Topi"
> <[1]topi.pohjolai...@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.
Thanks.
Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
>
> > > 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
> > > [2]mesa-dev@lists.freedesktop.org
> > > [3]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> References
>
> 1. mailto:topi.pohjolai...@intel.com
> 2. mailto:mesa-dev@lists.freedesktop.org
> 3. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
