[Mesa-dev] [PATCH 6/9] intel/blorp: Make blorp_ccs_ambiguate just an internal helper

[Iago Toral]

On Wed, 2018-05-16 at 08:44 -0700, Jason Ekstrand wrote:
> On Wed, May 16, 2018 at 4:00 AM, Iago Toral <itoral at igalia.com>
> wrote:
> > On Tue, 2018-05-15 at 15:28 -0700, Jason Ekstrand wrote:
> > 
> > > Now that anv uses blorp_ccs_op for everything, we no longer need
> > to
> > 
> > > expose the ccs_ambiguate function directly.  It's much better
> > tucked
> > 
> > > away as an implementation detail.
> > 
> > > ---
> > 
> > >  src/intel/blorp/blorp.h       |  5 -----
> > 
> > >  src/intel/blorp/blorp_clear.c | 21 ++++++++++-----------
> > 
> > >  2 files changed, 10 insertions(+), 16 deletions(-)
> > 
> > > 
> > 
> > > diff --git a/src/intel/blorp/blorp.h b/src/intel/blorp/blorp.h
> > 
> > > index 8c775bf..e27ea7e 100644
> > 
> > > --- a/src/intel/blorp/blorp.h
> > 
> > > +++ b/src/intel/blorp/blorp.h
> > 
> > > @@ -208,11 +208,6 @@ blorp_ccs_op(struct blorp_batch *batch,
> > 
> > >               enum isl_aux_op ccs_op);
> > 
> > >  
> > 
> > >  void
> > 
> > > -blorp_ccs_ambiguate(struct blorp_batch *batch,
> > 
> > > -                    struct blorp_surf *surf,
> > 
> > > -                    uint32_t level, uint32_t layer);
> > 
> > > -
> > 
> > > -void
> > 
> > >  blorp_mcs_partial_resolve(struct blorp_batch *batch,
> > 
> > >                            struct blorp_surf *surf,
> > 
> > >                            enum isl_format format,
> > 
> > > diff --git a/src/intel/blorp/blorp_clear.c
> > 
> > > b/src/intel/blorp/blorp_clear.c
> > 
> > > index 6f5549f..39bc0c6 100644
> > 
> > > --- a/src/intel/blorp/blorp_clear.c
> > 
> > > +++ b/src/intel/blorp/blorp_clear.c
> > 
> > > @@ -814,6 +814,11 @@ blorp_clear_attachments(struct blorp_batch
> > 
> > > *batch,
> > 
> > >     batch->blorp->exec(batch, &params);
> > 
> > >  }
> > 
> > >  
> > 
> > > +static void
> > 
> > > +blorp_legacy_ccs_ambiguate(struct blorp_batch *batch,
> > 
> > > +                           struct blorp_surf *surf,
> > 
> > > +                           uint32_t level, uint32_t layer);
> > 
> > > +
> > 
> > >  void
> > 
> > >  blorp_ccs_op(struct blorp_batch *batch,
> > 
> > >               struct blorp_surf *surf, uint32_t level,
> > 
> > > @@ -835,7 +840,7 @@ blorp_ccs_op(struct blorp_batch *batch,
> > 
> > >         * mess to another function.
> > 
> > >         */
> > 
> > >        for (uint32_t a = 0; a < num_layers; a++)
> > 
> > > -         blorp_ccs_ambiguate(batch, surf, level, start_layer +
> > a);
> > 
> > > +         blorp_legacy_ccs_ambiguate(batch, surf, level,
> > start_layer
> > 
> > > + a);
> > 
> > >        return;
> > 
> > >     }
> > 
> > >  
> > 
> > > @@ -1022,17 +1027,11 @@ blorp_mcs_partial_resolve(struct
> > blorp_batch
> > 
> > > *batch,
> > 
> > >   * for a given layer/level of a surface to 0x0 which is the
> > 
> > > "uncompressed"
> > 
> > >   * state which tells the sampler to go look at the main surface.
> > 
> > >   */
> > 
> > > -void
> > 
> > > -blorp_ccs_ambiguate(struct blorp_batch *batch,
> > 
> > > -                    struct blorp_surf *surf,
> > 
> > > -                    uint32_t level, uint32_t layer)
> > 
> > > +static void
> > 
> > > +blorp_legacy_ccs_ambiguate(struct blorp_batch *batch,
> > 
> > > +                           struct blorp_surf *surf,
> > 
> > > +                           uint32_t level, uint32_t layer)
> > 
> > >  {
> > 
> > > -   if (ISL_DEV_GEN(batch->blorp->isl_dev) >= 10) {
> > 
> > > -      /* On gen10 and above, we have a hardware resolve op for
> > this
> > 
> > > */
> > 
> > > -      return blorp_ccs_op(batch, surf, level, layer, 1,
> > 
> > > -                          surf->surf->format,
> > ISL_AUX_OP_AMBIGUATE);
> > 
> > > -   }
> > 
> > > -
> > 
> > 
> > 
> > Since we don't want to call this in gen10+, would it make sense to
> > an
> > 
> > assert for gen < 10?
> 
> It does work on gen10 and 11 (and we used it on gen 10 for a while). 
> I'll make it gen < 12.
> 

My point was that even if it works, we don't want this to be called for
these generations... maybe an assert would be too much for this?

Iago
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180517/b2a4bd9d/attachment.html>