Re: [PATCH] drm/i915/uc: Includ requested frequency in slow firmware load messages

["Belgaumkar, Vinay" <vinay.belgaumkar@xxxxxxxxx>]

On 8/30/2024 9:58 AM, John.C.Harrison@xxxxxxxxx wrote:
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> To aid debug of sporadic issues, include the requested frequency in
> the debug message as well as the actual frequency. That way we know
> for certain that the clamping is not because the driver forgot to ask.
>
> Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 11 ++++++-----
>   drivers/gpu/drm/i915/gt/uc/intel_huc.c    | 12 +++++++-----
>   2 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> index 23f54c84cbab2..e37d227dc199e 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -259,13 +259,14 @@ static int guc_wait_ucode(struct intel_guc *guc)
>   	} else if (delta_ms > 200) {
>   		guc_warn(guc, "excessive init time: %lldms! [status = 0x%08X, count = %d, ret = %d]\n",
>   			 delta_ms, status, count, ret);
> -		guc_warn(guc, "excessive init time: [freq = %dMHz, before = %dMHz, perf_limit_reasons = 0x%08X]\n",
> -			 intel_rps_read_actual_frequency(&gt->rps), before_freq,
> +		guc_warn(guc, "excessive init time: [freq = %dMHz -> %dMHz vs %dMHz, perf_limit_reasons = 0x%08X]\n",
> +			 before_freq, intel_rps_read_actual_frequency(&gt->rps),
> +			 intel_rps_get_requested_frequency(&gt->rps),
>   			 intel_uncore_read(uncore, intel_gt_perf_limit_reasons_reg(gt)));
>   	} else {
> -		guc_dbg(guc, "init took %lldms, freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d\n",
> -			delta_ms, intel_rps_read_actual_frequency(&gt->rps),
> -			before_freq, status, count, ret);
> +		guc_dbg(guc, "init took %lldms, freq = %dMHz -> %dMHz vs %dMHz, status = 0x%08X, count = %d, ret = %d\n",
> +			delta_ms, before_freq, intel_rps_read_actual_frequency(&gt->rps),
> +			intel_rps_get_requested_frequency(&gt->rps), status, count, ret);
>   	}
>   
>   	return ret;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 2d9152eb72825..4324ccda667b1 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -502,13 +502,15 @@ int intel_huc_wait_for_auth_complete(struct intel_huc *huc,
>   	if (delta_ms > 50) {
>   		huc_warn(huc, "excessive auth time: %lldms! [status = 0x%08X, count = %d, ret = %d]\n",
>   			 delta_ms, huc->status[type].reg.reg, count, ret);
> -		huc_warn(huc, "excessive auth time: [freq = %dMHz, before = %dMHz, perf_limit_reasons = 0x%08X]\n",
> -			 intel_rps_read_actual_frequency(&gt->rps), before_freq,
> +		huc_warn(huc, "excessive auth time: [freq = %dMHz -> %dMHz vs %dMHz, perf_limit_reasons = 0x%08X]\n",
> +			 before_freq, intel_rps_read_actual_frequency(&gt->rps),
> +			 intel_rps_get_requested_frequency(&gt->rps),
>   			 intel_uncore_read(uncore, intel_gt_perf_limit_reasons_reg(gt)));
>   	} else {
> -		huc_dbg(huc, "auth took %lldms, freq = %dMHz, before = %dMHz, status = 0x%08X, count = %d, ret = %d\n",
> -			delta_ms, intel_rps_read_actual_frequency(&gt->rps),
> -			before_freq, huc->status[type].reg.reg, count, ret);
> +		huc_dbg(huc, "auth took %lldms, freq = %dMHz -> %dMHz vs %dMHz, status = 0x%08X, count = %d, ret = %d\n",
> +			delta_ms, before_freq, intel_rps_read_actual_frequency(&gt->rps),
> +			intel_rps_get_requested_frequency(&gt->rps),
> +			huc->status[type].reg.reg, count, ret);

LGTM,

Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx>

>   	}
>   
>   	/* mark the load process as complete even if the wait failed */