2021-02-08 21:19:04

by Sakari Ailus

[permalink] [raw]
Subject: [PATCH v6 0/3] Add %p4cc printk modifier for V4L2 and DRM fourcc codes

Hi all,

This set adds support for %p4cc printk modifier for printing V4L2 and DRM
fourcc codes. The codes are cumbersome to print manually and by adding the
modifier, this task is saved from the V4L2 and DRM frameworks as well as
related drivers. DRM actually had it handled in a way (see 3rd patch) but
the printk modifier makes printing the format easier even there. On V4L2
side it saves quite a few lines of repeating different implementations of
printing the 4cc codes.

Further work will include converting the V4L2 drivers doing the same, as
well as converting DRM drivers from drm_get_format_name() to plain %p4cc.
I left these out from this version since individual drivers are easier
changed without dealing with multiple trees.

If DRM folks would prefer to convert drivers to %p4cc directly instead I
have no problem dropping the 3rd patch. Nearly all uses in DRM are in
printk family of functions that can readily use %p4cc instead of the
current arrangement that relies on caller-allocated temporary buffer.

Since v5:

- Added V4L2 core conversion to %p4cc, as well as change the DRM
fourcc printing function to use %p4cc.

- Add missing checkpatch.pl checks for %p4cc modifier.

Sakari Ailus (3):
lib/vsprintf: Add support for printing V4L2 and DRM fourccs
v4l: ioctl: Use %p4cc printk modifier to print FourCC codes
drm/fourcc: Switch to %p4cc format modifier

Documentation/core-api/printk-formats.rst | 16 +++++
drivers/gpu/drm/drm_fourcc.c | 16 +----
drivers/media/v4l2-core/v4l2-ioctl.c | 85 ++++++-----------------
lib/test_printf.c | 17 +++++
lib/vsprintf.c | 51 ++++++++++++++
scripts/checkpatch.pl | 6 +-
6 files changed, 112 insertions(+), 79 deletions(-)

--
2.29.2


2021-02-08 21:21:42

by Sakari Ailus

[permalink] [raw]
Subject: [PATCH v6 3/3] drm/fourcc: Switch to %p4cc format modifier

Instead of constructing the FourCC code manually, use the %p4cc printk
modifier to print it. Also leave a message to avoid using this function.

The next step would be to convert the users to use %p4cc directly instead
and removing the function.

Signed-off-by: Sakari Ailus <[email protected]>
---
drivers/gpu/drm/drm_fourcc.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 03262472059c..4ff40f2f27c0 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -30,11 +30,6 @@
#include <drm/drm_device.h>
#include <drm/drm_fourcc.h>

-static char printable_char(int c)
-{
- return isascii(c) && isprint(c) ? c : '?';
-}
-
/**
* drm_mode_legacy_fb_format - compute drm fourcc code from legacy description
* @bpp: bits per pixels
@@ -134,17 +129,12 @@ EXPORT_SYMBOL(drm_driver_legacy_fb_format);
* drm_get_format_name - fill a string with a drm fourcc format's name
* @format: format to compute name of
* @buf: caller-supplied buffer
+ *
+ * Please use %p4cc printk format modifier instead of this function.
*/
const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf)
{
- snprintf(buf->str, sizeof(buf->str),
- "%c%c%c%c %s-endian (0x%08x)",
- printable_char(format & 0xff),
- printable_char((format >> 8) & 0xff),
- printable_char((format >> 16) & 0xff),
- printable_char((format >> 24) & 0x7f),
- format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
- format);
+ snprintf(buf->str, sizeof(buf->str), "%p4cc", &format);

return buf->str;
}
--
2.29.2

2021-02-09 07:32:13

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH v6 3/3] drm/fourcc: Switch to %p4cc format modifier

On Mon, Feb 8, 2021 at 9:20 PM Sakari Ailus
<[email protected]> wrote:
>
> Instead of constructing the FourCC code manually, use the %p4cc printk
> modifier to print it. Also leave a message to avoid using this function.
>
> The next step would be to convert the users to use %p4cc directly instead
> and removing the function.
>
> Signed-off-by: Sakari Ailus <[email protected]>
> ---
> drivers/gpu/drm/drm_fourcc.c | 16 +++-------------
> 1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 03262472059c..4ff40f2f27c0 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -30,11 +30,6 @@
> #include <drm/drm_device.h>
> #include <drm/drm_fourcc.h>
>
> -static char printable_char(int c)
> -{
> - return isascii(c) && isprint(c) ? c : '?';
> -}
> -
> /**
> * drm_mode_legacy_fb_format - compute drm fourcc code from legacy description
> * @bpp: bits per pixels
> @@ -134,17 +129,12 @@ EXPORT_SYMBOL(drm_driver_legacy_fb_format);
> * drm_get_format_name - fill a string with a drm fourcc format's name
> * @format: format to compute name of
> * @buf: caller-supplied buffer
> + *
> + * Please use %p4cc printk format modifier instead of this function.

I think would be nice if we could roll this out and outright delete
this one here ... Quick git grep says there's not that many, and %p4cc
is quite a bit shorter than what we have now.
-Daniel

> */
> const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf)
> {
> - snprintf(buf->str, sizeof(buf->str),
> - "%c%c%c%c %s-endian (0x%08x)",
> - printable_char(format & 0xff),
> - printable_char((format >> 8) & 0xff),
> - printable_char((format >> 16) & 0xff),
> - printable_char((format >> 24) & 0x7f),
> - format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
> - format);
> + snprintf(buf->str, sizeof(buf->str), "%p4cc", &format);
>
> return buf->str;
> }
> --
> 2.29.2
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2021-02-09 09:12:03

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH v6 3/3] drm/fourcc: Switch to %p4cc format modifier

Hi Daniel,

Thanks for the comments.

On Tue, Feb 09, 2021 at 08:27:10AM +0100, Daniel Vetter wrote:
> On Mon, Feb 8, 2021 at 9:20 PM Sakari Ailus
> <[email protected]> wrote:
> >
> > Instead of constructing the FourCC code manually, use the %p4cc printk
> > modifier to print it. Also leave a message to avoid using this function.
> >
> > The next step would be to convert the users to use %p4cc directly instead
> > and removing the function.
> >
> > Signed-off-by: Sakari Ailus <[email protected]>
> > ---
> > drivers/gpu/drm/drm_fourcc.c | 16 +++-------------
> > 1 file changed, 3 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index 03262472059c..4ff40f2f27c0 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -30,11 +30,6 @@
> > #include <drm/drm_device.h>
> > #include <drm/drm_fourcc.h>
> >
> > -static char printable_char(int c)
> > -{
> > - return isascii(c) && isprint(c) ? c : '?';
> > -}
> > -
> > /**
> > * drm_mode_legacy_fb_format - compute drm fourcc code from legacy description
> > * @bpp: bits per pixels
> > @@ -134,17 +129,12 @@ EXPORT_SYMBOL(drm_driver_legacy_fb_format);
> > * drm_get_format_name - fill a string with a drm fourcc format's name
> > * @format: format to compute name of
> > * @buf: caller-supplied buffer
> > + *
> > + * Please use %p4cc printk format modifier instead of this function.
>
> I think would be nice if we could roll this out and outright delete
> this one here ... Quick git grep says there's not that many, and %p4cc
> is quite a bit shorter than what we have now.

Sounds good; I can submit patches for that but I think I'll do that once we
have the %p4cc modifier in.

--
Regards,

Sakari Ailus