Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: Warning: check cp_fw_version and update it to realize GRBM
 requires 1-cycle delay in cp firmware
To:     =?UTF-8?Q?Michel_D=c3=a4nzer?= <michel@daenzer.net>,
        Alex Deucher <alexdeucher@gmail.com>,
        Paul Menzel <pmenzel+amd-gfx@molgen.mpg.de>
Cc:     David Airlie <airlied@linux.ie>,
        LKML <linux-kernel@vger.kernel.org>,
        amd-gfx list <amd-gfx@lists.freedesktop.org>,
        Chang Zhu <Changfeng.Zhu@amd.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
References: <745c5951-5304-9651-34f1-6b3f6a713ece@molgen.mpg.de>
 <CADnq5_PH=ww4nNzRmC6PkyfPNomH_1FXWeMTJpS2pt6CpuRZMA@mail.gmail.com>
 <3553af46-c9c5-cd33-e7f9-bf7a1a5f49a7@daenzer.net>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
Message-ID: <9e4d867b-cd30-eb2d-406b-d6a462bdb04c@amd.com>
Date:   Mon, 6 Jan 2020 15:07:57 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
In-Reply-To: <3553af46-c9c5-cd33-e7f9-bf7a1a5f49a7@daenzer.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
MIME-Version: 1.0
Received-SPF: None (protection.outlook.com: amd.com does not designate
 permitted sender hosts)
X-MS-Exchange-CrossTenant-Network-Message-Id: 5d8f66ea-4912-4271-5e3d-08d792b1d8d3
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jan 2020 14:08:04.8628
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: PGCcmF3ZLPNKwNh3JJUv38o6LIo4cPn0j3lF4+bjaWmzp3HBXuDGg8qYA80OWfg/
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB2455
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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?)