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

[John Harrison <john.c.harrison@xxxxxxxxx>]

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>

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