On 1/16/2025 03:52, Krzysztof Karas wrote:
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(>->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(>->rps),
+ intel_rps_get_requested_frequency(>->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?
Are you meaning to swap the actual and requested values around? Changing
it to "<before> -> <requested>, actual <actual>" would make it more
confusing, IMHO.
The print is '<before> -> <actual> vs <requested>', i.e. "it started
<here> and went <there> compared to what we asked for which was <this>".
That seems like the logical order and description to me. The line is
already fairly long so the idea was not to make it any longer than
necessary while still giving all the useful information in a sensible
manner.
John.
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(>->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(>->rps),
+ intel_rps_get_requested_frequency(>->rps), status, count, ret);
...
