Re: Warning: check cp_fw_version and update it to realize GRBM requires 1-cycle delay in cp firmware
From: Christian KÃnig
Date: Mon Jan 06 2020 - 09:08:10 EST
Am 06.01.20 um 12:29 schrieb Michel DÃnzer:
On 2019-12-26 5:53 p.m., Alex Deucher wrote:
On Thu, Dec 26, 2019 at 5:11 AM Paul Menzel
[ 13.446975] [drm] Warning: check cp_fw_version and update it to realize GRBM requires 1-cycle delay in cp firmware
Chang, it looks like you added that warning in commit 11c6108934.
drm/amdgpu: add warning for GRBM 1-cycle delay issue in gfx9
It needs to add warning to update firmware in gfx9
in case that firmware is too old to have function to
realize dummy read in cp firmware.
Unfortunately, it looks like you did not even check how the warning is
formatted (needless spaces), so I guess this was totally untested. Also,
what is that warning about, and what is the user supposed to do? I am
unable to find `cp_fw_version` in the source code at all.
The code looks fine. Not sure why it's rendering funny in your log.
DRM_WARN_ONCE("Warning: check cp_fw_version and update
it to realize \
GRBM requires 1-cycle delay in cp firmware\n");
Looks like the leading spaces after the backslash are included in the
string. Something like this should be better:
DRM_WARN_ONCE("Warning: check cp_fw_version and update "
"GRBM requires 1-cycle delay in cp firmware\n");
Warning and error messages in the kernel should be on a single line if
possible to allow for searching the kernel code for the string of the
message.
I suggest to just shorten the message. Especially it is irrelevant why
the CP firmware is to old, that is debugging info at best. Additional to
that the "Warning:" is superfluous.
Something like DRM_WARN_ONCE("CP firmware version to old, please
update!") should perfectly do it.
Regards,
Christian.
(or maybe the intention was to put the second sentence on a new line?)