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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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.

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