Re: [Intel-gfx] [PATCH v2] drm/i915/slpc: Use platform limits for min/max frequency

["Tauro, Riana" <riana.tauro@xxxxxxxxx>]

On 10/13/2022 9:25 PM, Vinay Belgaumkar wrote:
> GuC will set the min/max frequencies to theoretical max on
> ATS-M. This will break kernel ABI, so limit min/max frequency
> to RP0(platform max) instead.
>
> Also modify the SLPC selftest to update the min frequency
> when we have a server part so that we can iterate between
> platform min and max.
>
> v2: Check softlimits instead of platform limits(Riana)
>
> Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/7030
>
> Acked-by: Nirmoy Das <nirmoy.das@xxxxxxxxx>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx>
Reviewed-by: Riana Tauro <riana.tauro@xxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/gt/selftest_slpc.c       | 40 +++++++++++++------
>   drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   | 29 ++++++++++++++
>   .../gpu/drm/i915/gt/uc/intel_guc_slpc_types.h |  3 ++
>   3 files changed, 60 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
> index 4c6e9257e593..e42bc215e54d 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
> @@ -234,6 +234,7 @@ static int run_test(struct intel_gt *gt, int test_type)
>   	enum intel_engine_id id;
>   	struct igt_spinner spin;
>   	u32 slpc_min_freq, slpc_max_freq;
> +	u32 saved_min_freq;
>   	int err = 0;
>   
>   	if (!intel_uc_uses_guc_slpc(&gt->uc))
> @@ -252,20 +253,35 @@ static int run_test(struct intel_gt *gt, int test_type)
>   		return -EIO;
>   	}
>   
> -	/*
> -	 * FIXME: With efficient frequency enabled, GuC can request
> -	 * frequencies higher than the SLPC max. While this is fixed
> -	 * in GuC, we level set these tests with RPn as min.
> -	 */
> -	err = slpc_set_min_freq(slpc, slpc->min_freq);
> -	if (err)
> -		return err;
> +	if (slpc_min_freq == slpc_max_freq) {
> +		/* Server parts will have min/max clamped to RP0 */
> +		if (slpc->min_is_rpmax) {
> +			err = slpc_set_min_freq(slpc, slpc->min_freq);
> +			if (err) {
> +				pr_err("Unable to update min freq on server part");
> +				return err;
> +			}
>   
> -	if (slpc->min_freq == slpc->rp0_freq) {
> -		pr_err("Min/Max are fused to the same value\n");
> -		return -EINVAL;
> +		} else {
> +			pr_err("Min/Max are fused to the same value\n");
> +			return -EINVAL;
> +		}
> +	} else {
> +		/*
> +		 * FIXME: With efficient frequency enabled, GuC can request
> +		 * frequencies higher than the SLPC max. While this is fixed
> +		 * in GuC, we level set these tests with RPn as min.
> +		 */
> +		err = slpc_set_min_freq(slpc, slpc->min_freq);
> +		if (err)
> +			return err;
>   	}
>   
> +	saved_min_freq = slpc_min_freq;
> +
> +	/* New temp min freq = RPn */
> +	slpc_min_freq = slpc->min_freq;
> +
>   	intel_gt_pm_wait_for_idle(gt);
>   	intel_gt_pm_get(gt);
>   	for_each_engine(engine, gt, id) {
> @@ -347,7 +363,7 @@ static int run_test(struct intel_gt *gt, int test_type)
>   
>   	/* Restore min/max frequencies */
>   	slpc_set_max_freq(slpc, slpc_max_freq);
> -	slpc_set_min_freq(slpc, slpc_min_freq);
> +	slpc_set_min_freq(slpc, saved_min_freq);
>   
>   	if (igt_flush_test(gt->i915))
>   		err = -EIO;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index fdd895f73f9f..11613d373a49 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -263,6 +263,7 @@ int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
>   
>   	slpc->max_freq_softlimit = 0;
>   	slpc->min_freq_softlimit = 0;
> +	slpc->min_is_rpmax = false;
>   
>   	slpc->boost_freq = 0;
>   	atomic_set(&slpc->num_waiters, 0);
> @@ -588,6 +589,31 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
>   	return 0;
>   }
>   
> +static bool is_slpc_min_freq_rpmax(struct intel_guc_slpc *slpc)
> +{
> +	int slpc_min_freq;
> +
> +	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq))
> +		return false;
> +
> +	if (slpc_min_freq > slpc->rp0_freq)
> +		return true;
> +	else
> +		return false;
> +}
> +
> +static void update_server_min_softlimit(struct intel_guc_slpc *slpc)
> +{
> +	/* For server parts, SLPC min will be at RPMax.
> +	 * Use min softlimit to clamp it to RP0 instead.
> +	 */
> +	if (is_slpc_min_freq_rpmax(slpc) &&
> +	    !slpc->min_freq_softlimit) {
> +		slpc->min_is_rpmax = true;
> +		slpc->min_freq_softlimit = slpc->rp0_freq;
> +	}
> +}
> +
>   static int slpc_use_fused_rp0(struct intel_guc_slpc *slpc)
>   {
>   	/* Force SLPC to used platform rp0 */
> @@ -647,6 +673,9 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>   
>   	slpc_get_rp_values(slpc);
>   
> +	/* Handle the case where min=max=RPmax */
> +	update_server_min_softlimit(slpc);
> +
>   	/* Set SLPC max limit to RP0 */
>   	ret = slpc_use_fused_rp0(slpc);
>   	if (unlikely(ret)) {
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h
> index 73d208123528..a6ef53b04e04 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_types.h
> @@ -19,6 +19,9 @@ struct intel_guc_slpc {
>   	bool supported;
>   	bool selected;
>   
> +	/* Indicates this is a server part */
> +	bool min_is_rpmax;
> +
>   	/* platform frequency limits */
>   	u32 min_freq;
>   	u32 rp0_freq;