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

["Ceraolo Spurio, Daniele" <daniele.ceraolospurio@xxxxxxxxx>]

On 5/6/2022 12:51 AM, Tvrtko Ursulin wrote:
> 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.

I didn't add the tag because DG2 still requires force_probe. If sending 
fixes for DG2 is ok, I'll make a reminder and I'll send it manually later.

Thanks,
Daniele

> 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;
> > >       /*