[Mesa-dev] [PATCH 15/32] i965/blorp: Be more accurate about aux usage in blorp_copy

[Jason Ekstrand]

On Fri, Jul 21, 2017 at 8:37 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Wed, Jul 19, 2017 at 02:01:41PM -0700, Jason Ekstrand wrote:
> > The only real change here is that we now reject clear colors for MCS
> > with certain formats on gen < 9 because we can't trust that the
> > reinterpretation will work.  This may cause some MCS partial resolves.
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp.c | 57
> +++++++++++++++++++++++++++--------
> >  1 file changed, 44 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> > index 5b5b4bc..4c61afc 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > @@ -389,23 +389,51 @@ brw_blorp_copy_miptrees(struct brw_context *brw,
> >         dst_mt->num_samples, _mesa_get_format_name(dst_mt->format),
> dst_mt,
> >         dst_level, dst_layer, dst_x, dst_y);
> >
> > -   enum isl_aux_usage src_aux_usage =
> > -      blorp_get_aux_usage(brw, src_mt,
> > -                          (1 << ISL_AUX_USAGE_MCS) |
> > -                          (1 << ISL_AUX_USAGE_CCS_E));
> > +   enum isl_aux_usage src_aux_usage, dst_aux_usage;
> > +   bool src_clear_supported, dst_clear_supported;
> > +
> > +   switch (src_mt->aux_usage) {
> > +   case ISL_AUX_USAGE_MCS:
> > +   case ISL_AUX_USAGE_CCS_E:
> > +      src_aux_usage = src_mt->aux_usage;
> > +      /* Prior to gen9, fast-clear only supported 0/1 clear colors.
> Since
> > +       * we're going to re-interpret the format as an integer format, a
> 0/1 in
> > +       * a non-integer format would end up not being 0/1 so we can't
> handle it
> > +       * until gen9.
> > +       */
> > +      src_clear_supported =
> > +         brw->gen >= 9 && !_mesa_is_format_integer(src_mt->format);
>
> Shouldn't this be || ?
>

Actually, it should just be "brw->gen >= 9" and it needs a new comment:

Prior to gen9, fast-clear only supported 0/1 clear colors.  Since we're
going to re-interpret the format as an integer format possibly with a
different channel layout, we can't handle clear colors until gen9.


> > +      break;
> > +   default:
> > +      src_aux_usage = ISL_AUX_USAGE_NONE;
> > +      src_clear_supported = false;
> > +      break;
> > +   }
> > +
> > +   switch (dst_mt->aux_usage) {
> > +   case ISL_AUX_USAGE_MCS:
> > +   case ISL_AUX_USAGE_CCS_E:
> > +      dst_aux_usage = dst_mt->aux_usage;
> > +      /* Prior to gen9, fast-clear only supported 0/1 clear colors.
> Since
> > +       * we're going to re-interpret the format as an integer format, a
> 0/1 in
> > +       * a non-integer format would end up not being 0/1 so we can't
> handle it
> > +       * until gen9.
> > +       */
> > +      dst_clear_supported =
> > +         brw->gen >= 9 && !_mesa_is_format_integer(dst_mt->format);
>
> Same here.
>

same as above.


> > +      break;
> > +   default:
> > +      dst_aux_usage = ISL_AUX_USAGE_NONE;
> > +      dst_clear_supported = false;
> > +      break;
> > +   }
> > +
> >     intel_miptree_prepare_access(brw, src_mt, src_level, 1, src_layer,
> 1,
> >                                  src_aux_usage != ISL_AUX_USAGE_NONE,
> > -                                src_aux_usage != ISL_AUX_USAGE_NONE);
> > -
> > -   enum isl_aux_usage dst_aux_usage =
> > -      blorp_get_aux_usage(brw, dst_mt,
> > -                          (1 << ISL_AUX_USAGE_MCS) |
> > -                          (1 << ISL_AUX_USAGE_CCS_E));
> > +                                src_clear_supported);
> >     intel_miptree_prepare_access(brw, dst_mt, dst_level, 1, dst_layer,
> 1,
> >                                  dst_aux_usage != ISL_AUX_USAGE_NONE,
> > -                                dst_aux_usage != ISL_AUX_USAGE_NONE);
> > -   intel_miptree_finish_write(brw, dst_mt, dst_level, dst_layer, 1,
> > -                              dst_aux_usage != ISL_AUX_USAGE_NONE);
> > +                                dst_clear_supported);
> >
> >     struct isl_surf tmp_surfs[2];
> >     struct blorp_surf src_surf, dst_surf;
> > @@ -420,6 +448,9 @@ brw_blorp_copy_miptrees(struct brw_context *brw,
> >                &dst_surf, dst_level, dst_layer,
> >                src_x, src_y, dst_x, dst_y, src_width, src_height);
> >     blorp_batch_finish(&batch);
> > +
> > +   intel_miptree_finish_write(brw, dst_mt, dst_level, dst_layer, 1,
> > +                              dst_aux_usage != ISL_AUX_USAGE_NONE);
>
> You are also moving this after the copy(). It only sets the state so I
> think
> it works either way, before or after the blorp op. Or does this really make
> a difference?
>
> >  }
> >
> >  static struct intel_mipmap_tree *
> > --
> > 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/20170721/2dd0f78b/attachment.html>