[PATCH] drm/amdgpu/jpeg: keep drm kernel message consistent

[David Wu]

Hi Alex,

Well - the dev_info has 2 "amdgpu"s.

    [   67.227121] amdgpu 0000:43:00.0: amdgpu: JPEG decode is enabled
    in VM mode

In practice we use "drm" to grep the GPU info for VCN and JPEG support.
So I believe the DRM_INFO is the best and it is used by most of the 
amdgpu code with exception only a few jpeg code and one vcn code.
in case of a need to convert all of them to something like:

    [   67.227121] amdgpu 0000:43:00.0: JPEG decode is enabled in VM mode

We can just change the macro of DRM_INFO.
Do we agree on using DRM_INFO?

Regards,

David

On 2024-05-09 17:36, Alex Deucher wrote:
> On Thu, May 9, 2024 at 5:31 PM David Wu<davidwu2 at amd.com>  wrote:
>> Hi Alex,
>>
>> Thanks for the suggestion!
>> What I am thinking is "DRM_DEV_INFO" should not be the one we want - as it is more like a debug message.
>>
>> [drm:jpeg_v5_0_0_hw_init [amdgpu]] JPEG decode initialized successfully.
>>
>> instead I prefer to use this format:
>> "amdgpu 0000:43:00.0: amdgpu: JPEG decode initialized successfully."
>>
>> but again I dislike it as well as there are 2 "amdgpu"s in the same message.
> You can use just plain dev_info().
>
> Alex
>
>> To make it consistent the "DRM_INFO" is used everywhere in the amdgpu code.
>> only the following jpeg code uses DRM_DEV_INFO and one file for vcn. All other jpeg versions have already changed to DRM_INFO.
>>
>> grep -r DRM_DEV_INFO *
>> amdgpu/jpeg_v4_0_3.c: DRM_DEV_INFO(adev->dev, "JPEG decode initialized successfully.\n");
>> amdgpu/jpeg_v4_0_3.c: DRM_DEV_INFO(adev->dev, "JPEG decode is enabled in VM mode\n");
>> amdgpu/vcn_v4_0_3.c: DRM_DEV_INFO(adev->dev, "VCN decode initialized successfully(under %s).\n",
>> amdgpu/vcn_v4_0_3.c: DRM_DEV_INFO(adev->dev, "VCN decode is enabled in VM mode\n");
>> amdgpu/jpeg_v4_0_5.c: DRM_DEV_INFO(adev->dev, "JPEG decode initialized successfully under DPG Mode");
>> amdgpu/jpeg_v4_0_5.c: DRM_DEV_INFO(adev->dev, "JPEG%d decode is enabled in VM mode\n", i);
>> amdgpu/jpeg_v5_0_0.c: DRM_DEV_INFO(adev->dev, "JPEG decode initialized successfully under DPG Mode");
>> amdgpu/jpeg_v5_0_0.c: DRM_DEV_INFO(adev->dev, "JPEG%d decode is enabled in VM mode\n", i);
>> amdgpu/jpeg_v4_0.c: DRM_DEV_INFO(adev->dev, "JPEG decode initialized successfully.\n");
>> amdgpu/jpeg_v4_0.c: DRM_DEV_INFO(adev->dev, "JPEG decode is enabled in VM mode\n");
>>
>> If the rest of code in amdgpu uses DRM_INFO why should we make VCN and JPEG special?
>> To address the identification of which GPUs - we need to check the kernel message after each IP DISCOVERY.
>> I do not see a reason to mess them up.
>> Regards,
>> David
>> On 2024-05-09 16:59, Alex Deucher wrote:
>>
>> On Thu, May 9, 2024 at 4:57 PM David (Ming Qiang) Wu<David.Wu3 at amd.com>  wrote:
>>
>> amdgpu jpeg kernel message is different than others such as vcn:
>>    [drm:jpeg_v5_0_0_hw_init [amdgpu]] JPEG decode initialized successfully.
>>
>> This patch is to make them consistent.
>>
>> The message after the change is:
>>    [drm] JPEG decode initialized successfully.
>>
>> Please convert the others to DRM_DEV_INFO instead.  Otherwise we can't
>> tell which GPUs these messages refer to on multi-GPU systems.
>>
>> Alex
>>
>> Signed-off-by: David (Ming Qiang) Wu<David.Wu3 at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
>> index 64c856bfe0cb..4be0668ab97d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
>> @@ -145,7 +145,7 @@ static int jpeg_v5_0_0_hw_init(void *handle)
>>          if (r)
>>                  return r;
>>
>> -       DRM_DEV_INFO(adev->dev, "JPEG decode initialized successfully.\n");
>> +       DRM_INFO("JPEG decode initialized successfully.\n");
>>
>>          return 0;
>>   }
>> @@ -549,7 +549,7 @@ static const struct amdgpu_ring_funcs jpeg_v5_0_0_dec_ring_vm_funcs = {
>>   static void jpeg_v5_0_0_set_dec_ring_funcs(struct amdgpu_device *adev)
>>   {
>>          adev->jpeg.inst->ring_dec->funcs = &jpeg_v5_0_0_dec_ring_vm_funcs;
>> -       DRM_DEV_INFO(adev->dev, "JPEG decode is enabled in VM mode\n");
>> +       DRM_INFO("JPEG decode is enabled in VM mode\n");
>>   }
>>
>>   static const struct amdgpu_irq_src_funcs jpeg_v5_0_0_irq_funcs = {
>> --
>> 2.34.1
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20240509/ae47d619/attachment.htm>