[PATCH 2/2] drm/appletbdm: use %p4cl instead of %p4cc

Aditya Garg posted 2 patches 9 months, 1 week ago
There is a newer version of this series
[PATCH 2/2] drm/appletbdm: use %p4cl instead of %p4cc
Posted by Aditya Garg 9 months, 1 week ago
From: Aditya Garg <gargaditya08@live.com>

Due to lack of a proper printk format, %p4cc was being used instead of
%p4cl for the purpose of printing FourCCs. But the disadvange was that
they were being printed in a reverse order. %p4cl should correct this
issue.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 drivers/gpu/drm/tiny/appletbdrm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
index 703b9a41a..751b05753 100644
--- a/drivers/gpu/drm/tiny/appletbdrm.c
+++ b/drivers/gpu/drm/tiny/appletbdrm.c
@@ -212,7 +212,7 @@ static int appletbdrm_read_response(struct appletbdrm_device *adev,
 	}
 
 	if (response->msg != expected_response) {
-		drm_err(drm, "Unexpected response from device (expected %p4cc found %p4cc)\n",
+		drm_err(drm, "Unexpected response from device (expected %p4cl found %p4cl)\n",
 			&expected_response, &response->msg);
 		return -EIO;
 	}
@@ -286,7 +286,7 @@ static int appletbdrm_get_information(struct appletbdrm_device *adev)
 	}
 
 	if (pixel_format != APPLETBDRM_PIXEL_FORMAT) {
-		drm_err(drm, "Encountered unknown pixel format (%p4cc)\n", &pixel_format);
+		drm_err(drm, "Encountered unknown pixel format (%p4cl)\n", &pixel_format);
 		ret = -EINVAL;
 		goto free_info;
 	}
-- 
2.47.1
Re: [PATCH 2/2] drm/appletbdm: use %p4cl instead of %p4cc
Posted by Thomas Zimmermann 9 months, 1 week ago
Hi

Am 12.03.25 um 10:06 schrieb Aditya Garg:
> From: Aditya Garg <gargaditya08@live.com>
>
> Due to lack of a proper printk format, %p4cc was being used instead of
> %p4cl for the purpose of printing FourCCs. But the disadvange was that
> they were being printed in a reverse order. %p4cl should correct this
> issue.
>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---
>   drivers/gpu/drm/tiny/appletbdrm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
> index 703b9a41a..751b05753 100644
> --- a/drivers/gpu/drm/tiny/appletbdrm.c
> +++ b/drivers/gpu/drm/tiny/appletbdrm.c
> @@ -212,7 +212,7 @@ static int appletbdrm_read_response(struct appletbdrm_device *adev,
>   	}
>   
>   	if (response->msg != expected_response) {
> -		drm_err(drm, "Unexpected response from device (expected %p4cc found %p4cc)\n",
> +		drm_err(drm, "Unexpected response from device (expected %p4cl found %p4cl)\n",
>   			&expected_response, &response->msg);

If you want to print out protocol-header tokens, why not use formatting 
macros as we do with DRM mode? There's one for the format string [1] and 
one for the argument. [2] It's not as fancy as the conversion code, but 
you'll get something for your driver that is easily understandable.

[1] 
https://elixir.bootlin.com/linux/v6.14-rc4/source/include/drm/drm_modes.h#L425
[2] 
https://elixir.bootlin.com/linux/v6.14-rc4/source/include/drm/drm_modes.h#L431

Best regards
Thomas

>   		return -EIO;
>   	}
> @@ -286,7 +286,7 @@ static int appletbdrm_get_information(struct appletbdrm_device *adev)
>   	}
>   
>   	if (pixel_format != APPLETBDRM_PIXEL_FORMAT) {
> -		drm_err(drm, "Encountered unknown pixel format (%p4cc)\n", &pixel_format);
> +		drm_err(drm, "Encountered unknown pixel format (%p4cl)\n", &pixel_format);
>   		ret = -EINVAL;
>   		goto free_info;
>   	}

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [PATCH 2/2] drm/appletbdm: use %p4cl instead of %p4cc
Posted by andriy.shevchenko@linux.intel.com 9 months, 1 week ago
On Wed, Mar 12, 2025 at 12:51:33PM +0100, Thomas Zimmermann wrote:
> Am 12.03.25 um 10:06 schrieb Aditya Garg:

...

> If you want to print out protocol-header tokens, why not use formatting
> macros as we do with DRM mode? There's one for the format string [1] and one
> for the argument. [2] It's not as fancy as the conversion code, but you'll
> get something for your driver that is easily understandable.

The benefit of the specific code formatters as in this patch at least in the
stack usage and hence a lot of code generated again and again. I believe you
can get rid of dozens of (kilo?) bytes in DRM on top of compensating this in
the printf() implementation. This backs us to the discussion on how the best
would be to implement custom printf() specifiers...

-- 
With Best Regards,
Andy Shevchenko