Messages in this thread |  | | Subject | 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, 6 Jan 2020 15:07:57 +0100 |
| |
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?)
|  |