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

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

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@intel.com>
>>>
>>> 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@intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
>
> 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;
>>>       /*
>>