[Mesa-dev] [PATCH 04/11] i965/miptree Remove layout_flags parameter form is_mcs_supported

[Jordan Justen]

On 2017-08-03 10:56:08, Jason Ekstrand wrote:
> On Thu, Aug 3, 2017 at 10:22 AM, Jordan Justen <jordan.l.justen at intel.com>
> wrote:
> 
> > subect: form => from
> >
> > Commit message? Why is this being removed?
> >
> 
> See below...
> 
> 
> > On 2017-08-02 13:35:29, Jason Ekstrand wrote:
> > > ---
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 11 ++---------
> > >  1 file changed, 2 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > index daa49f9..0e0aaa8 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > @@ -59,8 +59,7 @@ intel_miptree_alloc_aux(struct brw_context *brw,
> > >                          struct intel_mipmap_tree *mt);
> > >
> > >  static bool
> > > -is_mcs_supported(const struct brw_context *brw, mesa_format format,
> > > -                 uint32_t layout_flags)
> > > +is_mcs_supported(const struct brw_context *brw, mesa_format format)
> > >  {
> > >     /* Prior to Gen7, all MSAA surfaces used IMS layout. */
> > >     if (brw->gen < 7)
> > > @@ -86,11 +85,6 @@ is_mcs_supported(const struct brw_context *brw,
> > mesa_format format,
> > >         */
> > >        if (brw->gen == 7 && _mesa_get_format_datatype(format) ==
> > GL_INT) {
> > >           return false;
> > > -      } else if (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX) {
> > > -         /* We can't use the CMS layout because it uses an aux buffer,
> > the MCS
> > > -          * buffer. So fallback to UMS, which is identical to CMS
> > without the
> > > -          * MCS. */
> > > -         return false;
> >
> > I see that intel_miptree_create_for_dri_image will set this flag for
> > 'winsys' images, so why is this change safe to make? (Maybe it isn't
> > quite 'safe' on it's own?)
> >
> > I think the answer is in the next patch 'i965/miptree: Refactor
> > is_mcs_supported' where is_mcs_supported is refactored to
> > intel_miptree_supports_mcs. I think maybe it move from asking if the
> > format potentially supports MCS to whether the miptree actually has
> > MCS enabled. ?
> >
> > If so, maybe a small commit message comment would help? Or, maybe
> > sqaush it with the next patch?
> >
> > -Jordan
> >
> > >        } else {
> > >           return true;
> > >        }
> > > @@ -329,8 +323,7 @@ intel_miptree_choose_aux_usage(struct brw_context
> > *brw,
> > >  {
> > >     assert(mt->aux_usage == ISL_AUX_USAGE_NONE);
> > >
> > > -   const unsigned no_flags = 0;
> > > -   if (mt->surf.samples > 1 && is_mcs_supported(brw, mt->format,
> > no_flags)) {
> >
> 
> The one caller of this function pases 0 in as the flags parameter.  I can
> put that in the commit message.

Ok, you can add Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

> > > +   if (mt->surf.samples > 1 && is_mcs_supported(brw, mt->format)) {
> > >        assert(mt->surf.msaa_layout == ISL_MSAA_LAYOUT_ARRAY);
> > >        mt->aux_usage = ISL_AUX_USAGE_MCS;
> > >     } else if (intel_tiling_supports_ccs(brw, mt->surf.tiling) &&
> > > --
> > > 2.5.0.400.gff86faf
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >