[Intel-gfx] [PATCH 1/3] drm/i915/gt: Widen CSB pointer to u64 for the parsers

[Chris Wilson]

Quoting Mika Kuoppala (2020-08-14 19:29:03)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > A CSB entry is 64b, and it is simpler for us to treat it as an array of
> > 64b entries than as an array of pairs of 32b entries.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_engine_types.h |  2 +-
> >  drivers/gpu/drm/i915/gt/intel_lrc.c          | 33 ++++++++++----------
> >  2 files changed, 17 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > index c400aaa2287b..ee6312601c56 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > @@ -278,7 +278,7 @@ struct intel_engine_execlists {
> >        *
> >        * Note these register may be either mmio or HWSP shadow.
> >        */
> > -     u32 *csb_status;
> > +     u64 *csb_status;
> >  
> >       /**
> >        * @csb_size: context status buffer FIFO size
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 82742c6f423c..db982fc0f0bc 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -2464,7 +2464,7 @@ cancel_port_requests(struct intel_engine_execlists * const execlists)
> >  }
> >  
> >  static inline void
> > -invalidate_csb_entries(const u32 *first, const u32 *last)
> > +invalidate_csb_entries(const u64 *first, const u64 *last)
> >  {
> >       clflush((void *)first);
> >       clflush((void *)last);
> > @@ -2496,14 +2496,12 @@ invalidate_csb_entries(const u32 *first, const u32 *last)
> >   *     bits 47-57: sw context id of the lrc the GT switched away from
> >   *     bits 58-63: sw counter of the lrc the GT switched away from
> >   */
> > -static inline bool
> > -gen12_csb_parse(const struct intel_engine_execlists *execlists, const u32 *csb)
> > +static inline bool gen12_csb_parse(const u64 *csb)
> >  {
> > -     u32 lower_dw = csb[0];
> > -     u32 upper_dw = csb[1];
> > -     bool ctx_to_valid = GEN12_CSB_CTX_VALID(lower_dw);
> > -     bool ctx_away_valid = GEN12_CSB_CTX_VALID(upper_dw);
> > -     bool new_queue = lower_dw & GEN12_CTX_STATUS_SWITCHED_TO_NEW_QUEUE;
> > +     u64 entry = READ_ONCE(*csb);
> > +     bool ctx_away_valid = GEN12_CSB_CTX_VALID(upper_32_bits(entry));
> > +     bool new_queue =
> > +             lower_32_bits(entry) & GEN12_CTX_STATUS_SWITCHED_TO_NEW_QUEUE;
> 
> Opportunity to constify, tho stylistic.

Opportunity lost in the next patch, found again in the 3rd patch. If you
get really fancy, we only use them once. gcc is already smart enough to
reduce the pair down to a trivial set of bit ops rather than conditions.
So I left it alone.
-Chris