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

[Krzysztof Karas <krzysztof.karas@xxxxxxxxx>]

Hi John,

> 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.
> 
...
>  	} 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),
While, <value> -> <value> is clear to me if I were to encounter
this type of log, I wonder if it would be apparent what "vs"
means here without having to look into the code. Maybe it would
be worth to add "vs actual:" or ", actual:" instead?

Krzysztof

>  			 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);
...