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

[Jason Ekstrand]

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.


> > +   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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170803/e48b73c0/attachment.html>