linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
@ 2020-11-13 10:54 Sakari Ailus
  2020-11-20 15:57 ` Petr Mladek
  2020-12-10  8:34 ` Petr Mladek
  0 siblings, 2 replies; 8+ messages in thread
From: Sakari Ailus @ 2020-11-13 10:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-media, Andy Shevchenko, Petr Mladek, Dave Stevenson,
	dri-devel, hverkuil, laurent.pinchart, mchehab,
	Sergey Senozhatsky, Steven Rostedt, Joe Perches, Jani Nikula,
	Rasmus Villemoes

Add a printk modifier %p4cc (for pixel format) for printing V4L2 and DRM
pixel formats denoted by fourccs. The fourcc encoding is the same for both
so the same implementation can be used.

Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v4:

- Use correct destination for error string (was broken in v4)

- Remove unneeded comment "/* subtract parenthesis and the space from the
  size */".

 Documentation/core-api/printk-formats.rst | 16 +++++++
 lib/test_printf.c                         | 17 ++++++++
 lib/vsprintf.c                            | 51 +++++++++++++++++++++++
 3 files changed, 84 insertions(+)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 6d26c5c6ac48..080262d2e030 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -565,6 +565,22 @@ For printing netdev_features_t.
 
 Passed by reference.
 
+V4L2 and DRM FourCC code (pixel format)
+---------------------------------------
+
+::
+
+	%p4cc
+
+Print a FourCC code used by V4L2 or DRM, including format endianness and
+its numerical value as hexadecimal.
+
+Passed by reference.
+
+Examples::
+
+	%p4cc	BG12 little-endian (0x32314742)
+
 Thanks
 ======
 
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 7ac87f18a10f..7fb042542660 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -649,6 +649,22 @@ static void __init fwnode_pointer(void)
 	software_node_unregister(&softnodes[0]);
 }
 
+static void __init fourcc_pointer(void)
+{
+	struct {
+		u32 code;
+		char *str;
+	} const try[] = {
+		{ 0x20104646, "FF(10) little-endian (0x20104646)", },
+		{ 0xa0104646, "FF(10) big-endian (0xa0104646)", },
+		{ 0x10111213, "(13)(12)(11)(10) little-endian (0x10111213)", },
+	};
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(try); i++)
+		test(try[i].str, "%p4cc", &try[i].code);
+}
+
 static void __init
 errptr(void)
 {
@@ -694,6 +710,7 @@ test_pointer(void)
 	flags();
 	errptr();
 	fwnode_pointer();
+	fourcc_pointer();
 }
 
 static void __init selftest(void)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 14c9a6af1b23..b07ee5b7de06 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1733,6 +1733,54 @@ char *netdev_bits(char *buf, char *end, const void *addr,
 	return special_hex_number(buf, end, num, size);
 }
 
+static noinline_for_stack
+char *fourcc_string(char *buf, char *end, const u32 *fourcc,
+		    struct printf_spec spec, const char *fmt)
+{
+	char output[sizeof("(xx)(xx)(xx)(xx) little-endian (0x01234567)")];
+	char *p = output;
+	unsigned int i;
+	u32 val;
+
+	if (fmt[1] != 'c' || fmt[2] != 'c')
+		return error_string(buf, end, "(%p4?)", spec);
+
+	if (check_pointer(&buf, end, fourcc, spec))
+		return buf;
+
+	val = *fourcc & ~BIT(31);
+
+	for (i = 0; i < sizeof(*fourcc); i++) {
+		unsigned char c = val >> (i * 8);
+
+		/* Weed out spaces */
+		if (c == ' ')
+			continue;
+
+		/* Print non-control ASCII characters as-is */
+		if (isascii(c) && isprint(c)) {
+			*p++ = c;
+			continue;
+		}
+
+		*p++ = '(';
+		p = hex_byte_pack(p, c);
+		*p++ = ')';
+	}
+
+	strcpy(p, *fourcc & BIT(31) ? " big-endian" : " little-endian");
+	p += strlen(p);
+
+	*p++ = ' ';
+	*p++ = '(';
+	p = special_hex_number(p, output + sizeof(output) - 2, *fourcc,
+			       sizeof(u32));
+	*p++ = ')';
+	*p = '\0';
+
+	return string(buf, end, output, spec);
+}
+
 static noinline_for_stack
 char *address_val(char *buf, char *end, const void *addr,
 		  struct printf_spec spec, const char *fmt)
@@ -2162,6 +2210,7 @@ char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
  *       correctness of the format string and va_list arguments.
  * - 'K' For a kernel pointer that should be hidden from unprivileged users
  * - 'NF' For a netdev_features_t
+ * - '4cc' V4L2 or DRM FourCC code, with endianness and raw numerical value.
  * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
  *            a certain separator (' ' by default):
  *              C colon
@@ -2259,6 +2308,8 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		return restricted_pointer(buf, end, ptr, spec);
 	case 'N':
 		return netdev_bits(buf, end, ptr, spec, fmt);
+	case '4':
+		return fourcc_string(buf, end, ptr, spec, fmt);
 	case 'a':
 		return address_val(buf, end, ptr, spec, fmt);
 	case 'd':
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
  2020-11-13 10:54 [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs Sakari Ailus
@ 2020-11-20 15:57 ` Petr Mladek
  2020-11-21  3:05   ` Sergey Senozhatsky
  2020-12-10  8:34 ` Petr Mladek
  1 sibling, 1 reply; 8+ messages in thread
From: Petr Mladek @ 2020-11-20 15:57 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-kernel, linux-media, Andy Shevchenko, Dave Stevenson,
	dri-devel, hverkuil, laurent.pinchart, mchehab,
	Sergey Senozhatsky, Steven Rostedt, Joe Perches, Jani Nikula,
	Rasmus Villemoes

On Fri 2020-11-13 12:54:41, Sakari Ailus wrote:
> Add a printk modifier %p4cc (for pixel format) for printing V4L2 and DRM
> pixel formats denoted by fourccs. The fourcc encoding is the same for both
> so the same implementation can be used.
> 
> Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

The last version looks fine to me.

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
  2020-11-20 15:57 ` Petr Mladek
@ 2020-11-21  3:05   ` Sergey Senozhatsky
  0 siblings, 0 replies; 8+ messages in thread
From: Sergey Senozhatsky @ 2020-11-21  3:05 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sakari Ailus, linux-kernel, linux-media, Andy Shevchenko,
	Dave Stevenson, dri-devel, hverkuil, laurent.pinchart, mchehab,
	Sergey Senozhatsky, Steven Rostedt, Joe Perches, Jani Nikula,
	Rasmus Villemoes

On (20/11/20 16:57), Petr Mladek wrote:
> On Fri 2020-11-13 12:54:41, Sakari Ailus wrote:
> > Add a printk modifier %p4cc (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by fourccs. The fourcc encoding is the same for both
> > so the same implementation can be used.
> > 
> > Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> The last version looks fine to me.
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
  2020-11-13 10:54 [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs Sakari Ailus
  2020-11-20 15:57 ` Petr Mladek
@ 2020-12-10  8:34 ` Petr Mladek
  2020-12-10 13:05   ` Andy Shevchenko
  1 sibling, 1 reply; 8+ messages in thread
From: Petr Mladek @ 2020-12-10  8:34 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-kernel, linux-media, Andy Shevchenko, Dave Stevenson,
	dri-devel, hverkuil, laurent.pinchart, mchehab,
	Sergey Senozhatsky, Steven Rostedt, Joe Perches, Jani Nikula,
	Rasmus Villemoes

On Fri 2020-11-13 12:54:41, Sakari Ailus wrote:
> Add a printk modifier %p4cc (for pixel format) for printing V4L2 and DRM
> pixel formats denoted by fourccs. The fourcc encoding is the same for both
> so the same implementation can be used.
> 
> Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Andy, Rasmus,

the last version looks fine to me. I am going to push it.
Please, speak up if you are against it.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
  2020-12-10  8:34 ` Petr Mladek
@ 2020-12-10 13:05   ` Andy Shevchenko
  2020-12-10 13:55     ` Sakari Ailus
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2020-12-10 13:05 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sakari Ailus, Linux Kernel Mailing List,
	Linux Media Mailing List, Andy Shevchenko, Dave Stevenson,
	dri-devel, Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab,
	Sergey Senozhatsky, Steven Rostedt, Joe Perches, Jani Nikula,
	Rasmus Villemoes

On Thu, Dec 10, 2020 at 2:16 PM Petr Mladek <pmladek@suse.com> wrote:
> On Fri 2020-11-13 12:54:41, Sakari Ailus wrote:
> > Add a printk modifier %p4cc (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by fourccs. The fourcc encoding is the same for both
> > so the same implementation can be used.
> >
> > Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> Andy, Rasmus,
>
> the last version looks fine to me. I am going to push it.
> Please, speak up if you are against it.

My concerns are:
- not so standard format of representation (why not to use
string_escape_mem() helper?) or is it?
- no compatibility with generic 4cc
  (I would rather have an additional specifier here for v4l2 cases.
OTOH generic %p4cc to me sounds like an equivalent to %4pEh (but we
have similar cases with MAC where %6ph is the same as %pM).

But I'm not insisting on them, consider it like just my 2 cents to the
discussion.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
  2020-12-10 13:05   ` Andy Shevchenko
@ 2020-12-10 13:55     ` Sakari Ailus
  2020-12-10 14:08       ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2020-12-10 13:55 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Petr Mladek, Linux Kernel Mailing List, Linux Media Mailing List,
	Andy Shevchenko, Dave Stevenson, dri-devel, Hans Verkuil,
	Laurent Pinchart, Mauro Carvalho Chehab, Sergey Senozhatsky,
	Steven Rostedt, Joe Perches, Jani Nikula, Rasmus Villemoes

Hi Andy,

On Thu, Dec 10, 2020 at 03:05:02PM +0200, Andy Shevchenko wrote:
> On Thu, Dec 10, 2020 at 2:16 PM Petr Mladek <pmladek@suse.com> wrote:
> > On Fri 2020-11-13 12:54:41, Sakari Ailus wrote:
> > > Add a printk modifier %p4cc (for pixel format) for printing V4L2 and DRM
> > > pixel formats denoted by fourccs. The fourcc encoding is the same for both
> > > so the same implementation can be used.
> > >
> > > Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >
> > Andy, Rasmus,
> >
> > the last version looks fine to me. I am going to push it.
> > Please, speak up if you are against it.
> 
> My concerns are:
> - not so standard format of representation (why not to use
> string_escape_mem() helper?) or is it?

The format string may contain spaces that are not meant to be printed.
Other unprintable chacaters should not be present (at least not in V4L2
pixelformats). The hexadecimal representation is there to convey the
numerical value and that originally came from DRM, not V4L2.

> - no compatibility with generic 4cc
>   (I would rather have an additional specifier here for v4l2 cases.

What do you mean by "generic 4cc"? There are two users of 4cc codes in the
kernel that I know of: V4L2 and DRM. Something that does not refer to
in-memory pixel formats?

> OTOH generic %p4cc to me sounds like an equivalent to %4pEh (but we
> have similar cases with MAC where %6ph is the same as %pM).
> 
> But I'm not insisting on them, consider it like just my 2 cents to the
> discussion.

Ack.

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
  2020-12-10 13:55     ` Sakari Ailus
@ 2020-12-10 14:08       ` Andy Shevchenko
  2020-12-11 12:20         ` Petr Mladek
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2020-12-10 14:08 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Petr Mladek, Linux Kernel Mailing List, Linux Media Mailing List,
	Dave Stevenson, dri-devel, Hans Verkuil, Laurent Pinchart,
	Mauro Carvalho Chehab, Sergey Senozhatsky, Steven Rostedt,
	Joe Perches, Jani Nikula, Rasmus Villemoes

On Thu, Dec 10, 2020 at 03:55:27PM +0200, Sakari Ailus wrote:
> On Thu, Dec 10, 2020 at 03:05:02PM +0200, Andy Shevchenko wrote:
> > On Thu, Dec 10, 2020 at 2:16 PM Petr Mladek <pmladek@suse.com> wrote:
> > > On Fri 2020-11-13 12:54:41, Sakari Ailus wrote:
> > > > Add a printk modifier %p4cc (for pixel format) for printing V4L2 and DRM
> > > > pixel formats denoted by fourccs. The fourcc encoding is the same for both
> > > > so the same implementation can be used.
> > > >
> > > > Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > >
> > > Andy, Rasmus,
> > >
> > > the last version looks fine to me. I am going to push it.
> > > Please, speak up if you are against it.
> > 
> > My concerns are:
> > - not so standard format of representation (why not to use
> > string_escape_mem() helper?) or is it?
> 
> The format string may contain spaces that are not meant to be printed.
> Other unprintable chacaters should not be present (at least not in V4L2
> pixelformats). The hexadecimal representation is there to convey the
> numerical value and that originally came from DRM, not V4L2.

Yes, but I mean that we usually anticipate the escaped characters in a form of
'\xNN' (hex) or '\NNN' (octal). The format '(NN)' is quite unusual to me.

> > - no compatibility with generic 4cc
> >   (I would rather have an additional specifier here for v4l2 cases.
> 
> What do you mean by "generic 4cc"? There are two users of 4cc codes in the
> kernel that I know of: V4L2 and DRM. Something that does not refer to
> in-memory pixel formats?

Of course. Everything else. 4cc is a generic term to describe something which
is of 4 characters long [1]. It's not limited by media file formats. And
moreover some (chip) vendors are using it as well (Synopsys).
Microsoft uses 4cc in CSRT ACPI table for vendor field and so on...

[1]: https://en.wikipedia.org/wiki/FourCC

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs
  2020-12-10 14:08       ` Andy Shevchenko
@ 2020-12-11 12:20         ` Petr Mladek
  0 siblings, 0 replies; 8+ messages in thread
From: Petr Mladek @ 2020-12-11 12:20 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Sakari Ailus, Linux Kernel Mailing List,
	Linux Media Mailing List, Dave Stevenson, dri-devel,
	Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab,
	Sergey Senozhatsky, Steven Rostedt, Joe Perches, Jani Nikula,
	Rasmus Villemoes

On Thu 2020-12-10 16:08:05, Andy Shevchenko wrote:
> On Thu, Dec 10, 2020 at 03:55:27PM +0200, Sakari Ailus wrote:
> > On Thu, Dec 10, 2020 at 03:05:02PM +0200, Andy Shevchenko wrote:
> > > My concerns are:
> > > - not so standard format of representation (why not to use
> > > string_escape_mem() helper?) or is it?
> > 
> > The format string may contain spaces that are not meant to be printed.
> > Other unprintable chacaters should not be present (at least not in V4L2
> > pixelformats). The hexadecimal representation is there to convey the
> > numerical value and that originally came from DRM, not V4L2.
> 
> Yes, but I mean that we usually anticipate the escaped characters in a form of
> '\xNN' (hex) or '\NNN' (octal). The format '(NN)' is quite unusual to me.

It is true that I have been a bit confused when I saw it.


> > > - no compatibility with generic 4cc
> > >   (I would rather have an additional specifier here for v4l2 cases.
> > 
> > What do you mean by "generic 4cc"? There are two users of 4cc codes in the
> > kernel that I know of: V4L2 and DRM. Something that does not refer to
> > in-memory pixel formats?
> 
> Of course. Everything else. 4cc is a generic term to describe something which
> is of 4 characters long [1]. It's not limited by media file formats. And
> moreover some (chip) vendors are using it as well (Synopsys).
> Microsoft uses 4cc in CSRT ACPI table for vendor field and so on...

Honestly, I do not even know where exactly it is going to be used.

I did not have strong opinion. So I just followed the long discussions
about previous revisions. Some people loved it from the beginning.
Some people were concerned. Anyway, there were discussed only
implementation details in the last two revisions, so I assumed that
the idea was more or less accepted.

Would it help to send another revision with some existing DRM and
V4L2 code converted to use it?

It would help me/us to see how much different it is from the current
output. Also it will require ack from the affected subsystem
maintainers and developers.

Best Regqrds,
Petr

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-12-11 12:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13 10:54 [PATCH v5 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs Sakari Ailus
2020-11-20 15:57 ` Petr Mladek
2020-11-21  3:05   ` Sergey Senozhatsky
2020-12-10  8:34 ` Petr Mladek
2020-12-10 13:05   ` Andy Shevchenko
2020-12-10 13:55     ` Sakari Ailus
2020-12-10 14:08       ` Andy Shevchenko
2020-12-11 12:20         ` Petr Mladek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).