Re: [Intel-gfx] [PATCH v2] drm/i915/guc: Support programming the EU priority in the GuC descriptor

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 05/05/2022 19:56, John Harrison wrote:
> On 5/4/2022 16:46, Daniele Ceraolo Spurio wrote:
> > From: Matthew Brost <matthew.brost@xxxxxxxxx>
> >
> > In GuC submission mode the EU priority must be updated by the GuC rather
> > than the driver as the GuC owns the programming of the context 
> > descriptor.
> >
> > Given that the GuC code uses the GuC priorities, we can't use a generic
> > function using i915 priorities for both execlists and GuC submission.
> > The existing function has therefore been pushed to the execlists
> > back-end while a new one has been added for GuC.
> >
> > v2: correctly use the GuC prio.
> >
> > Cc: John Harrison <john.c.harrison@xxxxxxxxx>
> > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> > Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> > Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxx>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Reviewed-by: John Harrison <John.C.Harrison@xxxxxxxxx>

I've asked for this already - since this seems a fix relevant for DG2, 
but now that it has been merged without a Fixes: tag, it will not get 
picked up for 5.19 by the scripts.

Maybe I can cherry pick it manually in a few weeks, or maybe you guys 
can send it to stable manually once 5.19 is released, please make a 
reminder item if you think 5.19 should have it.

Regards,

Tvrtko

> > ---
> >   .../drm/i915/gt/intel_execlists_submission.c  | 12 +++++++++-
> >   drivers/gpu/drm/i915/gt/intel_lrc.h           | 10 ---------
> >   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 22 +++++++++++++++++++
> >   3 files changed, 33 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c 
> > b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > index 86f7a9ac1c394..2b0266cab66b9 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > @@ -661,6 +661,16 @@ static inline void execlists_schedule_out(struct 
> > i915_request *rq)
> >       i915_request_put(rq);
> >   }
> > +static u32 map_i915_prio_to_lrc_desc_prio(int prio)
> > +{
> > +    if (prio > I915_PRIORITY_NORMAL)
> > +        return GEN12_CTX_PRIORITY_HIGH;
> > +    else if (prio < I915_PRIORITY_NORMAL)
> > +        return GEN12_CTX_PRIORITY_LOW;
> > +    else
> > +        return GEN12_CTX_PRIORITY_NORMAL;
> > +}
> > +
> >   static u64 execlists_update_context(struct i915_request *rq)
> >   {
> >       struct intel_context *ce = rq->context;
> > @@ -669,7 +679,7 @@ static u64 execlists_update_context(struct 
> > i915_request *rq)
> >       desc = ce->lrc.desc;
> >       if (rq->engine->flags & I915_ENGINE_HAS_EU_PRIORITY)
> > -        desc |= lrc_desc_priority(rq_prio(rq));
> > +        desc |= map_i915_prio_to_lrc_desc_prio(rq_prio(rq));
> >       /*
> >        * WaIdleLiteRestore:bdw,skl
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h 
> > b/drivers/gpu/drm/i915/gt/intel_lrc.h
> > index 31be734010db3..a390f0813c8b6 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
> > @@ -111,16 +111,6 @@ enum {
> >   #define XEHP_SW_COUNTER_SHIFT            58
> >   #define XEHP_SW_COUNTER_WIDTH            6
> > -static inline u32 lrc_desc_priority(int prio)
> > -{
> > -    if (prio > I915_PRIORITY_NORMAL)
> > -        return GEN12_CTX_PRIORITY_HIGH;
> > -    else if (prio < I915_PRIORITY_NORMAL)
> > -        return GEN12_CTX_PRIORITY_LOW;
> > -    else
> > -        return GEN12_CTX_PRIORITY_NORMAL;
> > -}
> > -
> >   static inline void lrc_runtime_start(struct intel_context *ce)
> >   {
> >       struct intel_context_stats *stats = &ce->stats;
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
> > b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > index 75291e9846c50..8bf8b6d588d43 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > @@ -2394,6 +2394,26 @@ static int guc_context_policy_init(struct 
> > intel_context *ce, bool loop)
> >       return ret;
> >   }
> > +static u32 map_guc_prio_to_lrc_desc_prio(u8 prio)
> > +{
> > +    /*
> > +     * this matches the mapping we do in map_i915_prio_to_guc_prio()
> > +     * (e.g. prio < I915_PRIORITY_NORMAL maps to 
> > GUC_CLIENT_PRIORITY_NORMAL)
> > +     */
> > +    switch (prio) {
> > +    default:
> > +        MISSING_CASE(prio);
> > +        fallthrough;
> > +    case GUC_CLIENT_PRIORITY_KMD_NORMAL:
> > +        return GEN12_CTX_PRIORITY_NORMAL;
> > +    case GUC_CLIENT_PRIORITY_NORMAL:
> > +        return GEN12_CTX_PRIORITY_LOW;
> > +    case GUC_CLIENT_PRIORITY_HIGH:
> > +    case GUC_CLIENT_PRIORITY_KMD_HIGH:
> > +        return GEN12_CTX_PRIORITY_HIGH;
> > +    }
> > +}
> > +
> >   static void prepare_context_registration_info(struct intel_context *ce,
> >                             struct guc_ctxt_registration_info *info)
> >   {
> > @@ -2420,6 +2440,8 @@ static void 
> > prepare_context_registration_info(struct intel_context *ce,
> >        */
> >       info->hwlrca_lo = lower_32_bits(ce->lrc.lrca);
> >       info->hwlrca_hi = upper_32_bits(ce->lrc.lrca);
> > +    if (engine->flags & I915_ENGINE_HAS_EU_PRIORITY)
> > +        info->hwlrca_lo |= 
> > map_guc_prio_to_lrc_desc_prio(ce->guc_state.prio);
> >       info->flags = CONTEXT_REGISTRATION_FLAG_KMD;
> >       /*