[PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
pixel formats denoted by 4ccs. The 4cc 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>
---
 Documentation/core-api/printk-formats.rst | 11 +++++++++
 lib/vsprintf.c                            | 29 +++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 8ebe46b1af39..b6249f513c09 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -545,6 +545,17 @@ For printing netdev_features_t.
 
 Passed by reference.
 
+V4L2 and DRM fourcc code (pixel format)
+---------------------------------------
+
+::
+
+	%ppf
+
+Print a 4cc code used by V4L2 or DRM.
+
+Passed by reference.
+
 Thanks
 ======
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7c488a1ce318..b39f0ac317c5 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1721,6 +1721,32 @@ char *netdev_bits(char *buf, char *end, const void *addr,
 	return special_hex_number(buf, end, num, size);
 }
 
+static noinline_for_stack
+char *pixel_format_string(char *buf, char *end, const u32 *fourcc,
+			  struct printf_spec spec, const char *fmt)
+{
+	char ch[2] = { 0 };
+	unsigned int i;
+
+	if (check_pointer(&buf, end, fourcc, spec))
+		return buf;
+
+	switch (fmt[1]) {
+	case 'f':
+		for (i = 0; i < sizeof(*fourcc); i++) {
+			ch[0] = *fourcc >> (i << 3);
+			buf = string(buf, end, ch, spec);
+		}
+
+		if (*fourcc & BIT(31))
+			buf = string(buf, end, "-BE", spec);
+
+		return buf;
+	default:
+		return error_string(buf, end, "(%pp?)", spec);
+	}
+}
+
 static noinline_for_stack
 char *address_val(char *buf, char *end, const void *addr,
 		  struct printf_spec spec, const char *fmt)
@@ -2131,6 +2157,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
+ * - 'pf' V4L2 or DRM pixel format.
  * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
  *            a certain separator (' ' by default):
  *              C colon
@@ -2223,6 +2250,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 'p':
+		return pixel_format_string(buf, end, ptr, spec, fmt);
 	case 'a':
 		return address_val(buf, end, ptr, spec, fmt);
 	case 'd':
-- 
2.20.1

[PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
pixel formats denoted by 4ccs. The 4cc 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>
---
 Documentation/core-api/printk-formats.rst | 11 +++++++++
 lib/vsprintf.c                            | 29 +++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 8ebe46b1af39..b6249f513c09 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -545,6 +545,17 @@ For printing netdev_features_t.
 
 Passed by reference.
 
+V4L2 and DRM fourcc code (pixel format)
+---------------------------------------
+
+::
+
+	%ppf
+
+Print a 4cc code used by V4L2 or DRM.
+
+Passed by reference.
+
 Thanks
 ======
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7c488a1ce318..b39f0ac317c5 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1721,6 +1721,32 @@ char *netdev_bits(char *buf, char *end, const void *addr,
 	return special_hex_number(buf, end, num, size);
 }
 
+static noinline_for_stack
+char *pixel_format_string(char *buf, char *end, const u32 *fourcc,
+			  struct printf_spec spec, const char *fmt)
+{
+	char ch[2] = { 0 };
+	unsigned int i;
+
+	if (check_pointer(&buf, end, fourcc, spec))
+		return buf;
+
+	switch (fmt[1]) {
+	case 'f':
+		for (i = 0; i < sizeof(*fourcc); i++) {
+			ch[0] = *fourcc >> (i << 3);
+			buf = string(buf, end, ch, spec);
+		}
+
+		if (*fourcc & BIT(31))
+			buf = string(buf, end, "-BE", spec);
+
+		return buf;
+	default:
+		return error_string(buf, end, "(%pp?)", spec);
+	}
+}
+
 static noinline_for_stack
 char *address_val(char *buf, char *end, const void *addr,
 		  struct printf_spec spec, const char *fmt)
@@ -2131,6 +2157,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
+ * - 'pf' V4L2 or DRM pixel format.
  * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
  *            a certain separator (' ' by default):
  *              C colon
@@ -2223,6 +2250,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 'p':
+		return pixel_format_string(buf, end, ptr, spec, fmt);
 	case 'a':
 		return address_val(buf, end, ptr, spec, fmt);
 	case 'd':
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Hans Verkuil]

On 4/1/20 4:05 PM, Sakari Ailus wrote:
> Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> pixel formats denoted by 4ccs. The 4cc 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>
> ---
>  Documentation/core-api/printk-formats.rst | 11 +++++++++
>  lib/vsprintf.c                            | 29 +++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 8ebe46b1af39..b6249f513c09 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -545,6 +545,17 @@ For printing netdev_features_t.
>  
>  Passed by reference.
>  
> +V4L2 and DRM fourcc code (pixel format)
> +---------------------------------------
> +
> +::
> +
> +	%ppf
> +
> +Print a 4cc code used by V4L2 or DRM.

FourCC appears to be the more-or-less official name (https://en.wikipedia.org/wiki/FourCC)

I would explain about the -BE suffix for bigendian fourcc variants.

> +
> +Passed by reference.
> +
>  Thanks
>  ======
>  
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 7c488a1ce318..b39f0ac317c5 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1721,6 +1721,32 @@ char *netdev_bits(char *buf, char *end, const void *addr,
>  	return special_hex_number(buf, end, num, size);
>  }
>  
> +static noinline_for_stack
> +char *pixel_format_string(char *buf, char *end, const u32 *fourcc,
> +			  struct printf_spec spec, const char *fmt)
> +{
> +	char ch[2] = { 0 };

This can just be '{ };'

> +	unsigned int i;
> +
> +	if (check_pointer(&buf, end, fourcc, spec))
> +		return buf;
> +
> +	switch (fmt[1]) {
> +	case 'f':
> +		for (i = 0; i < sizeof(*fourcc); i++) {
> +			ch[0] = *fourcc >> (i << 3);

You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set)
will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't
used except to indicate a BE variant.

> +			buf = string(buf, end, ch, spec);
> +		}
> +
> +		if (*fourcc & BIT(31))
> +			buf = string(buf, end, "-BE", spec);
> +
> +		return buf;
> +	default:
> +		return error_string(buf, end, "(%pp?)", spec);
> +	}
> +}
> +
>  static noinline_for_stack
>  char *address_val(char *buf, char *end, const void *addr,
>  		  struct printf_spec spec, const char *fmt)
> @@ -2131,6 +2157,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
> + * - 'pf' V4L2 or DRM pixel format.

I'd say 'FourCC format' instead of 'pixel format'.

>   * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
>   *            a certain separator (' ' by default):
>   *              C colon
> @@ -2223,6 +2250,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 'p':
> +		return pixel_format_string(buf, end, ptr, spec, fmt);
>  	case 'a':
>  		return address_val(buf, end, ptr, spec, fmt);
>  	case 'd':
> 

Regards,

	Hans

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Hans Verkuil]

On 4/1/20 4:05 PM, Sakari Ailus wrote:
> Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> pixel formats denoted by 4ccs. The 4cc 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>
> ---
>  Documentation/core-api/printk-formats.rst | 11 +++++++++
>  lib/vsprintf.c                            | 29 +++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 8ebe46b1af39..b6249f513c09 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -545,6 +545,17 @@ For printing netdev_features_t.
>  
>  Passed by reference.
>  
> +V4L2 and DRM fourcc code (pixel format)
> +---------------------------------------
> +
> +::
> +
> +	%ppf
> +
> +Print a 4cc code used by V4L2 or DRM.

FourCC appears to be the more-or-less official name (https://en.wikipedia.org/wiki/FourCC)

I would explain about the -BE suffix for bigendian fourcc variants.

> +
> +Passed by reference.
> +
>  Thanks
>  ======
>  
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 7c488a1ce318..b39f0ac317c5 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1721,6 +1721,32 @@ char *netdev_bits(char *buf, char *end, const void *addr,
>  	return special_hex_number(buf, end, num, size);
>  }
>  
> +static noinline_for_stack
> +char *pixel_format_string(char *buf, char *end, const u32 *fourcc,
> +			  struct printf_spec spec, const char *fmt)
> +{
> +	char ch[2] = { 0 };

This can just be '{ };'

> +	unsigned int i;
> +
> +	if (check_pointer(&buf, end, fourcc, spec))
> +		return buf;
> +
> +	switch (fmt[1]) {
> +	case 'f':
> +		for (i = 0; i < sizeof(*fourcc); i++) {
> +			ch[0] = *fourcc >> (i << 3);

You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set)
will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't
used except to indicate a BE variant.

> +			buf = string(buf, end, ch, spec);
> +		}
> +
> +		if (*fourcc & BIT(31))
> +			buf = string(buf, end, "-BE", spec);
> +
> +		return buf;
> +	default:
> +		return error_string(buf, end, "(%pp?)", spec);
> +	}
> +}
> +
>  static noinline_for_stack
>  char *address_val(char *buf, char *end, const void *addr,
>  		  struct printf_spec spec, const char *fmt)
> @@ -2131,6 +2157,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
> + * - 'pf' V4L2 or DRM pixel format.

I'd say 'FourCC format' instead of 'pixel format'.

>   * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
>   *            a certain separator (' ' by default):
>   *              C colon
> @@ -2223,6 +2250,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 'p':
> +		return pixel_format_string(buf, end, ptr, spec, fmt);
>  	case 'a':
>  		return address_val(buf, end, ptr, spec, fmt);
>  	case 'd':
> 

Regards,

	Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Andy Shevchenko]

On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote:
> On 4/1/20 4:05 PM, Sakari Ailus wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.

%p4cc ?

> > +	char ch[2] = { 0 };
> 
> This can just be '{ };'

The latter is GCC extension, while above is C standard. Former is slightly
better I think. Though see below.

> > +	unsigned int i;
> > +
> > +	if (check_pointer(&buf, end, fourcc, spec))
> > +		return buf;
> > +
> > +	switch (fmt[1]) {
> > +	case 'f':

> > +		for (i = 0; i < sizeof(*fourcc); i++) {
> > +			ch[0] = *fourcc >> (i << 3);
> 
> You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set)
> will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't
> used except to indicate a BE variant.

Why not to do it once by a flag and do reset it once?

	u32 tmp = *fourcc;
	bool be4cc = tmp & BIT(31);

	tmp &= BIT(31);

On top of that, as promised above, why not simple do it in a simpler way, i.e.
using standard idiom:

	for (i = 0; i < sizeof(*fourcc); i++) {
		if (buf < end)
			*buf = tmp >> (i * 8);
		buf++;
	}
?

> > +			buf = string(buf, end, ch, spec);
> > +		}
> > +
> > +		if (*fourcc & BIT(31))
> > +			buf = string(buf, end, "-BE", spec);

Another possibility

	u8 ch[8];

	if (*fourcc & BIT(31)) {
		put_unaligned_be32(tmp, &ch[0]);
		strcpy(&ch[4], "-BE");
	} else {
		put_unaligned_le32(tmp, &ch[0]);
		strcpy(&ch[4], "-LE");
	}
	return string(buf, end, &ch[0], spec);

> > +		return buf;
> > +	default:
> > +		return error_string(buf, end, "(%pp?)", spec);
> > +	}
> > +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Andy Shevchenko]

On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote:
> On 4/1/20 4:05 PM, Sakari Ailus wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.

%p4cc ?

> > +	char ch[2] = { 0 };
> 
> This can just be '{ };'

The latter is GCC extension, while above is C standard. Former is slightly
better I think. Though see below.

> > +	unsigned int i;
> > +
> > +	if (check_pointer(&buf, end, fourcc, spec))
> > +		return buf;
> > +
> > +	switch (fmt[1]) {
> > +	case 'f':

> > +		for (i = 0; i < sizeof(*fourcc); i++) {
> > +			ch[0] = *fourcc >> (i << 3);
> 
> You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set)
> will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't
> used except to indicate a BE variant.

Why not to do it once by a flag and do reset it once?

	u32 tmp = *fourcc;
	bool be4cc = tmp & BIT(31);

	tmp &= BIT(31);

On top of that, as promised above, why not simple do it in a simpler way, i.e.
using standard idiom:

	for (i = 0; i < sizeof(*fourcc); i++) {
		if (buf < end)
			*buf = tmp >> (i * 8);
		buf++;
	}
?

> > +			buf = string(buf, end, ch, spec);
> > +		}
> > +
> > +		if (*fourcc & BIT(31))
> > +			buf = string(buf, end, "-BE", spec);

Another possibility

	u8 ch[8];

	if (*fourcc & BIT(31)) {
		put_unaligned_be32(tmp, &ch[0]);
		strcpy(&ch[4], "-BE");
	} else {
		put_unaligned_le32(tmp, &ch[0]);
		strcpy(&ch[4], "-LE");
	}
	return string(buf, end, &ch[0], spec);

> > +		return buf;
> > +	default:
> > +		return error_string(buf, end, "(%pp?)", spec);
> > +	}
> > +}

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

Hi Andy,

Thanks for the review.

On Wed, Apr 01, 2020 at 06:13:32PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote:
> > On 4/1/20 4:05 PM, Sakari Ailus wrote:
> > > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > > the same implementation can be used.
> 
> %p4cc ?

Sounds good. Numbers have special handling but AFAIR only right after %
sign, so this should be possible.

> 
> > > +	char ch[2] = { 0 };
> > 
> > This can just be '{ };'
> 
> The latter is GCC extension, while above is C standard. Former is slightly
> better I think. Though see below.
> 
> > > +	unsigned int i;
> > > +
> > > +	if (check_pointer(&buf, end, fourcc, spec))
> > > +		return buf;
> > > +
> > > +	switch (fmt[1]) {
> > > +	case 'f':
> 
> > > +		for (i = 0; i < sizeof(*fourcc); i++) {
> > > +			ch[0] = *fourcc >> (i << 3);
> > 
> > You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set)
> > will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't
> > used except to indicate a BE variant.
> 
> Why not to do it once by a flag and do reset it once?
> 
> 	u32 tmp = *fourcc;
> 	bool be4cc = tmp & BIT(31);
> 
> 	tmp &= BIT(31);

I had two extra temporary variables in a version I didn't send but I
figured they could be removed. :-)

> 
> On top of that, as promised above, why not simple do it in a simpler way, i.e.
> using standard idiom:
> 
> 	for (i = 0; i < sizeof(*fourcc); i++) {
> 		if (buf < end)
> 			*buf = tmp >> (i * 8);
> 		buf++;
> 	}
> ?

I guess that's at least more efficient, and comparing buf to end is
trivial. I'll do that in v2.

> 
> > > +			buf = string(buf, end, ch, spec);
> > > +		}
> > > +
> > > +		if (*fourcc & BIT(31))
> > > +			buf = string(buf, end, "-BE", spec);
> 
> Another possibility
> 
> 	u8 ch[8];
> 
> 	if (*fourcc & BIT(31)) {
> 		put_unaligned_be32(tmp, &ch[0]);
> 		strcpy(&ch[4], "-BE");
> 	} else {
> 		put_unaligned_le32(tmp, &ch[0]);
> 		strcpy(&ch[4], "-LE");
> 	}
> 	return string(buf, end, &ch[0], spec);

I think I prefer the loop. I figured you can only call string once,
otherwise field width handling will be broken. Let's see.

> 
> > > +		return buf;
> > > +	default:
> > > +		return error_string(buf, end, "(%pp?)", spec);
> > > +	}
> > > +}
> 

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

Hi Andy,

Thanks for the review.

On Wed, Apr 01, 2020 at 06:13:32PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote:
> > On 4/1/20 4:05 PM, Sakari Ailus wrote:
> > > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > > the same implementation can be used.
> 
> %p4cc ?

Sounds good. Numbers have special handling but AFAIR only right after %
sign, so this should be possible.

> 
> > > +	char ch[2] = { 0 };
> > 
> > This can just be '{ };'
> 
> The latter is GCC extension, while above is C standard. Former is slightly
> better I think. Though see below.
> 
> > > +	unsigned int i;
> > > +
> > > +	if (check_pointer(&buf, end, fourcc, spec))
> > > +		return buf;
> > > +
> > > +	switch (fmt[1]) {
> > > +	case 'f':
> 
> > > +		for (i = 0; i < sizeof(*fourcc); i++) {
> > > +			ch[0] = *fourcc >> (i << 3);
> > 
> > You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set)
> > will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't
> > used except to indicate a BE variant.
> 
> Why not to do it once by a flag and do reset it once?
> 
> 	u32 tmp = *fourcc;
> 	bool be4cc = tmp & BIT(31);
> 
> 	tmp &= BIT(31);

I had two extra temporary variables in a version I didn't send but I
figured they could be removed. :-)

> 
> On top of that, as promised above, why not simple do it in a simpler way, i.e.
> using standard idiom:
> 
> 	for (i = 0; i < sizeof(*fourcc); i++) {
> 		if (buf < end)
> 			*buf = tmp >> (i * 8);
> 		buf++;
> 	}
> ?

I guess that's at least more efficient, and comparing buf to end is
trivial. I'll do that in v2.

> 
> > > +			buf = string(buf, end, ch, spec);
> > > +		}
> > > +
> > > +		if (*fourcc & BIT(31))
> > > +			buf = string(buf, end, "-BE", spec);
> 
> Another possibility
> 
> 	u8 ch[8];
> 
> 	if (*fourcc & BIT(31)) {
> 		put_unaligned_be32(tmp, &ch[0]);
> 		strcpy(&ch[4], "-BE");
> 	} else {
> 		put_unaligned_le32(tmp, &ch[0]);
> 		strcpy(&ch[4], "-LE");
> 	}
> 	return string(buf, end, &ch[0], spec);

I think I prefer the loop. I figured you can only call string once,
otherwise field width handling will be broken. Let's see.

> 
> > > +		return buf;
> > > +	default:
> > > +		return error_string(buf, end, "(%pp?)", spec);
> > > +	}
> > > +}
> 

-- 
Kind regards,

Sakari Ailus
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 1115 bytes --]

On Wed, 1 Apr 2020 18:13:32 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote:
> > On 4/1/20 4:05 PM, Sakari Ailus wrote:  
> > > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > > the same implementation can be used.  
> 
> %p4cc ?
> 

> Another possibility
> 
> 	u8 ch[8];
> 
> 	if (*fourcc & BIT(31)) {
> 		put_unaligned_be32(tmp, &ch[0]);
> 		strcpy(&ch[4], "-BE");
> 	} else {
> 		put_unaligned_le32(tmp, &ch[0]);
> 		strcpy(&ch[4], "-LE");
> 	}
> 	return string(buf, end, &ch[0], spec);
> 

Hi,

mind, if I guess right what that code does, I think this would confuse
the fourcc code endianness and the pixel data endianness. I think fourcc
codes are always crafted machine-endian, regardless of how the pixel
data is. At least, that is what drm_fourcc.h seems to be doing with
fourcc_code(). That has nothing to do with DRM_FORMAT_BIG_ENDIAN which
refers to the pixel data.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 1115 bytes --]

On Wed, 1 Apr 2020 18:13:32 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote:
> > On 4/1/20 4:05 PM, Sakari Ailus wrote:  
> > > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > > the same implementation can be used.  
> 
> %p4cc ?
> 

> Another possibility
> 
> 	u8 ch[8];
> 
> 	if (*fourcc & BIT(31)) {
> 		put_unaligned_be32(tmp, &ch[0]);
> 		strcpy(&ch[4], "-BE");
> 	} else {
> 		put_unaligned_le32(tmp, &ch[0]);
> 		strcpy(&ch[4], "-LE");
> 	}
> 	return string(buf, end, &ch[0], spec);
> 

Hi,

mind, if I guess right what that code does, I think this would confuse
the fourcc code endianness and the pixel data endianness. I think fourcc
codes are always crafted machine-endian, regardless of how the pixel
data is. At least, that is what drm_fourcc.h seems to be doing with
fourcc_code(). That has nothing to do with DRM_FORMAT_BIG_ENDIAN which
refers to the pixel data.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

Hi Hans,

Thank you for the review.

On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote:
> On 4/1/20 4:05 PM, Sakari Ailus wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc 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>
> > ---
> >  Documentation/core-api/printk-formats.rst | 11 +++++++++
> >  lib/vsprintf.c                            | 29 +++++++++++++++++++++++
> >  2 files changed, 40 insertions(+)
> > 
> > diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> > index 8ebe46b1af39..b6249f513c09 100644
> > --- a/Documentation/core-api/printk-formats.rst
> > +++ b/Documentation/core-api/printk-formats.rst
> > @@ -545,6 +545,17 @@ For printing netdev_features_t.
> >  
> >  Passed by reference.
> >  
> > +V4L2 and DRM fourcc code (pixel format)
> > +---------------------------------------
> > +
> > +::
> > +
> > +	%ppf
> > +
> > +Print a 4cc code used by V4L2 or DRM.
> 
> FourCC appears to be the more-or-less official name (https://en.wikipedia.org/wiki/FourCC)
> 
> I would explain about the -BE suffix for bigendian fourcc variants.

Agreed, I'll address these in v2.

> 
> > +
> > +Passed by reference.
> > +
> >  Thanks
> >  ======
> >  
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 7c488a1ce318..b39f0ac317c5 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1721,6 +1721,32 @@ char *netdev_bits(char *buf, char *end, const void *addr,
> >  	return special_hex_number(buf, end, num, size);
> >  }
> >  
> > +static noinline_for_stack
> > +char *pixel_format_string(char *buf, char *end, const u32 *fourcc,
> > +			  struct printf_spec spec, const char *fmt)
> > +{
> > +	char ch[2] = { 0 };
> 
> This can just be '{ };'

As Andy, I also do prefer { 0 }.

> 
> > +	unsigned int i;
> > +
> > +	if (check_pointer(&buf, end, fourcc, spec))
> > +		return buf;
> > +
> > +	switch (fmt[1]) {
> > +	case 'f':
> > +		for (i = 0; i < sizeof(*fourcc); i++) {
> > +			ch[0] = *fourcc >> (i << 3);
> 
> You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set)
> will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't
> used except to indicate a BE variant.

Good point, will fix for v2.

> 
> > +			buf = string(buf, end, ch, spec);
> > +		}
> > +
> > +		if (*fourcc & BIT(31))
> > +			buf = string(buf, end, "-BE", spec);
> > +
> > +		return buf;
> > +	default:
> > +		return error_string(buf, end, "(%pp?)", spec);
> > +	}
> > +}
> > +
> >  static noinline_for_stack
> >  char *address_val(char *buf, char *end, const void *addr,
> >  		  struct printf_spec spec, const char *fmt)
> > @@ -2131,6 +2157,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
> > + * - 'pf' V4L2 or DRM pixel format.
> 
> I'd say 'FourCC format' instead of 'pixel format'.

Will fix.

-- 
Regards,

Sakari Ailus

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

Hi Hans,

Thank you for the review.

On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote:
> On 4/1/20 4:05 PM, Sakari Ailus wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc 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>
> > ---
> >  Documentation/core-api/printk-formats.rst | 11 +++++++++
> >  lib/vsprintf.c                            | 29 +++++++++++++++++++++++
> >  2 files changed, 40 insertions(+)
> > 
> > diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> > index 8ebe46b1af39..b6249f513c09 100644
> > --- a/Documentation/core-api/printk-formats.rst
> > +++ b/Documentation/core-api/printk-formats.rst
> > @@ -545,6 +545,17 @@ For printing netdev_features_t.
> >  
> >  Passed by reference.
> >  
> > +V4L2 and DRM fourcc code (pixel format)
> > +---------------------------------------
> > +
> > +::
> > +
> > +	%ppf
> > +
> > +Print a 4cc code used by V4L2 or DRM.
> 
> FourCC appears to be the more-or-less official name (https://en.wikipedia.org/wiki/FourCC)
> 
> I would explain about the -BE suffix for bigendian fourcc variants.

Agreed, I'll address these in v2.

> 
> > +
> > +Passed by reference.
> > +
> >  Thanks
> >  ======
> >  
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 7c488a1ce318..b39f0ac317c5 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1721,6 +1721,32 @@ char *netdev_bits(char *buf, char *end, const void *addr,
> >  	return special_hex_number(buf, end, num, size);
> >  }
> >  
> > +static noinline_for_stack
> > +char *pixel_format_string(char *buf, char *end, const u32 *fourcc,
> > +			  struct printf_spec spec, const char *fmt)
> > +{
> > +	char ch[2] = { 0 };
> 
> This can just be '{ };'

As Andy, I also do prefer { 0 }.

> 
> > +	unsigned int i;
> > +
> > +	if (check_pointer(&buf, end, fourcc, spec))
> > +		return buf;
> > +
> > +	switch (fmt[1]) {
> > +	case 'f':
> > +		for (i = 0; i < sizeof(*fourcc); i++) {
> > +			ch[0] = *fourcc >> (i << 3);
> 
> You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set)
> will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't
> used except to indicate a BE variant.

Good point, will fix for v2.

> 
> > +			buf = string(buf, end, ch, spec);
> > +		}
> > +
> > +		if (*fourcc & BIT(31))
> > +			buf = string(buf, end, "-BE", spec);
> > +
> > +		return buf;
> > +	default:
> > +		return error_string(buf, end, "(%pp?)", spec);
> > +	}
> > +}
> > +
> >  static noinline_for_stack
> >  char *address_val(char *buf, char *end, const void *addr,
> >  		  struct printf_spec spec, const char *fmt)
> > @@ -2131,6 +2157,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
> > + * - 'pf' V4L2 or DRM pixel format.
> 
> I'd say 'FourCC format' instead of 'pixel format'.

Will fix.

-- 
Regards,

Sakari Ailus
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Jani Nikula]

On Wed, 01 Apr 2020, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> the same implementation can be used.

I'm not going to take a strong stand in one way or the other regarding
the patch at hand, but I do think at some point we have to draw a line
what should be included in printk formats. Arguably they should be
reserved to things that are generally useful across large parts of the
kernel, right?

I think the more specialized you get, the more you should think about
just using the plain old %s, and your own helpers. Because frankly, the
kernel printk specifiers also start getting more than a little obscure.

Or could we conceive of a way to make this locally extensible yet safe,
letting callers use something like %{foo}, as well as providing a
locally relevant function to do the conversion?


BR,
Jani.


>
> Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/core-api/printk-formats.rst | 11 +++++++++
>  lib/vsprintf.c                            | 29 +++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 8ebe46b1af39..b6249f513c09 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -545,6 +545,17 @@ For printing netdev_features_t.
>  
>  Passed by reference.
>  
> +V4L2 and DRM fourcc code (pixel format)
> +---------------------------------------
> +
> +::
> +
> +	%ppf
> +
> +Print a 4cc code used by V4L2 or DRM.
> +
> +Passed by reference.
> +
>  Thanks
>  ======
>  
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 7c488a1ce318..b39f0ac317c5 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1721,6 +1721,32 @@ char *netdev_bits(char *buf, char *end, const void *addr,
>  	return special_hex_number(buf, end, num, size);
>  }
>  
> +static noinline_for_stack
> +char *pixel_format_string(char *buf, char *end, const u32 *fourcc,
> +			  struct printf_spec spec, const char *fmt)
> +{
> +	char ch[2] = { 0 };
> +	unsigned int i;
> +
> +	if (check_pointer(&buf, end, fourcc, spec))
> +		return buf;
> +
> +	switch (fmt[1]) {
> +	case 'f':
> +		for (i = 0; i < sizeof(*fourcc); i++) {
> +			ch[0] = *fourcc >> (i << 3);
> +			buf = string(buf, end, ch, spec);
> +		}
> +
> +		if (*fourcc & BIT(31))
> +			buf = string(buf, end, "-BE", spec);
> +
> +		return buf;
> +	default:
> +		return error_string(buf, end, "(%pp?)", spec);
> +	}
> +}
> +
>  static noinline_for_stack
>  char *address_val(char *buf, char *end, const void *addr,
>  		  struct printf_spec spec, const char *fmt)
> @@ -2131,6 +2157,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
> + * - 'pf' V4L2 or DRM pixel format.
>   * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
>   *            a certain separator (' ' by default):
>   *              C colon
> @@ -2223,6 +2250,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 'p':
> +		return pixel_format_string(buf, end, ptr, spec, fmt);
>  	case 'a':
>  		return address_val(buf, end, ptr, spec, fmt);
>  	case 'd':

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Jani Nikula]

On Wed, 01 Apr 2020, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> the same implementation can be used.

I'm not going to take a strong stand in one way or the other regarding
the patch at hand, but I do think at some point we have to draw a line
what should be included in printk formats. Arguably they should be
reserved to things that are generally useful across large parts of the
kernel, right?

I think the more specialized you get, the more you should think about
just using the plain old %s, and your own helpers. Because frankly, the
kernel printk specifiers also start getting more than a little obscure.

Or could we conceive of a way to make this locally extensible yet safe,
letting callers use something like %{foo}, as well as providing a
locally relevant function to do the conversion?


BR,
Jani.


>
> Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/core-api/printk-formats.rst | 11 +++++++++
>  lib/vsprintf.c                            | 29 +++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 8ebe46b1af39..b6249f513c09 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -545,6 +545,17 @@ For printing netdev_features_t.
>  
>  Passed by reference.
>  
> +V4L2 and DRM fourcc code (pixel format)
> +---------------------------------------
> +
> +::
> +
> +	%ppf
> +
> +Print a 4cc code used by V4L2 or DRM.
> +
> +Passed by reference.
> +
>  Thanks
>  ======
>  
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 7c488a1ce318..b39f0ac317c5 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1721,6 +1721,32 @@ char *netdev_bits(char *buf, char *end, const void *addr,
>  	return special_hex_number(buf, end, num, size);
>  }
>  
> +static noinline_for_stack
> +char *pixel_format_string(char *buf, char *end, const u32 *fourcc,
> +			  struct printf_spec spec, const char *fmt)
> +{
> +	char ch[2] = { 0 };
> +	unsigned int i;
> +
> +	if (check_pointer(&buf, end, fourcc, spec))
> +		return buf;
> +
> +	switch (fmt[1]) {
> +	case 'f':
> +		for (i = 0; i < sizeof(*fourcc); i++) {
> +			ch[0] = *fourcc >> (i << 3);
> +			buf = string(buf, end, ch, spec);
> +		}
> +
> +		if (*fourcc & BIT(31))
> +			buf = string(buf, end, "-BE", spec);
> +
> +		return buf;
> +	default:
> +		return error_string(buf, end, "(%pp?)", spec);
> +	}
> +}
> +
>  static noinline_for_stack
>  char *address_val(char *buf, char *end, const void *addr,
>  		  struct printf_spec spec, const char *fmt)
> @@ -2131,6 +2157,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
> + * - 'pf' V4L2 or DRM pixel format.
>   * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
>   *            a certain separator (' ' by default):
>   *              C colon
> @@ -2223,6 +2250,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 'p':
> +		return pixel_format_string(buf, end, ptr, spec, fmt);
>  	case 'a':
>  		return address_val(buf, end, ptr, spec, fmt);
>  	case 'd':

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

Moi,

On Thu, Apr 02, 2020 at 11:34:48AM +0300, Jani Nikula wrote:
> On Wed, 01 Apr 2020, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.
> 
> I'm not going to take a strong stand in one way or the other regarding
> the patch at hand, but I do think at some point we have to draw a line
> what should be included in printk formats. Arguably they should be
> reserved to things that are generally useful across large parts of the
> kernel, right?
> 
> I think the more specialized you get, the more you should think about
> just using the plain old %s, and your own helpers. Because frankly, the
> kernel printk specifiers also start getting more than a little obscure.

I don't really disagree... While this is functionality very commonly needed
in drivers, there are alternatives such as posted here:

<URL:https://lore.kernel.org/linux-media/20190916100433.24367-1-hverkuil-cisco@xs4all.nl/>

The 4cc codes added by this set is still relatively generic (while still
Linux subsystem specific and not related to e.g. hardware standards), but I
wonder how many other, possibly similar cases there could be in the kernel,
and how many new specifiers we might get with those all added.

For what it's worth, even C99 defines macros for printing some formats
such as PRIu64 for uint64_t.

-- 
Terveisin,

Sakari Ailus

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

Moi,

On Thu, Apr 02, 2020 at 11:34:48AM +0300, Jani Nikula wrote:
> On Wed, 01 Apr 2020, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.
> 
> I'm not going to take a strong stand in one way or the other regarding
> the patch at hand, but I do think at some point we have to draw a line
> what should be included in printk formats. Arguably they should be
> reserved to things that are generally useful across large parts of the
> kernel, right?
> 
> I think the more specialized you get, the more you should think about
> just using the plain old %s, and your own helpers. Because frankly, the
> kernel printk specifiers also start getting more than a little obscure.

I don't really disagree... While this is functionality very commonly needed
in drivers, there are alternatives such as posted here:

<URL:https://lore.kernel.org/linux-media/20190916100433.24367-1-hverkuil-cisco@xs4all.nl/>

The 4cc codes added by this set is still relatively generic (while still
Linux subsystem specific and not related to e.g. hardware standards), but I
wonder how many other, possibly similar cases there could be in the kernel,
and how many new specifiers we might get with those all added.

For what it's worth, even C99 defines macros for printing some formats
such as PRIu64 for uint64_t.

-- 
Terveisin,

Sakari Ailus
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Mauro Carvalho Chehab]

Em Thu, 02 Apr 2020 11:34:48 +0300
Jani Nikula <jani.nikula@linux.intel.com> escreveu:

> On Wed, 01 Apr 2020, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.  
> 
> I'm not going to take a strong stand in one way or the other regarding
> the patch at hand, but I do think at some point we have to draw a line
> what should be included in printk formats. Arguably they should be
> reserved to things that are generally useful across large parts of the
> kernel, right?
> 
> I think the more specialized you get, the more you should think about
> just using the plain old %s, and your own helpers.

As I suggested it, from my side, I'd like to have it inside printk :-)

There is a subset of formats that are subsystem-specific anyway at
printk, like the network ones. We use extensively fourcc along the
media subsystem (and you probably also use fourcc at DRM). Even some input
devices nowadays may be using V4L2 core (some multi-sensor touching
devices), with depends on it.

So, those fourcc codes are pretty common. Having it at the printk
infra makes a lot easier for people to use them.

> Because frankly, the
> kernel printk specifiers also start getting more than a little obscure.

I liked one of the suggestions of using "%p4cc" (or maybe something
similar, if having a number there is a problem, like "%pAcc" or "%pfcc")
for this printk. This would be very easy for people to identify and 
remember about its meaning.

> Or could we conceive of a way to make this locally extensible yet safe,
> letting callers use something like %{foo}, as well as providing a
> locally relevant function to do the conversion?

That's something that it makes sense to be implemented in the future,
for things that would be self-contained inside an specific subsystem.

Thanks,
Mauro

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Mauro Carvalho Chehab]

Em Thu, 02 Apr 2020 11:34:48 +0300
Jani Nikula <jani.nikula@linux.intel.com> escreveu:

> On Wed, 01 Apr 2020, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.  
> 
> I'm not going to take a strong stand in one way or the other regarding
> the patch at hand, but I do think at some point we have to draw a line
> what should be included in printk formats. Arguably they should be
> reserved to things that are generally useful across large parts of the
> kernel, right?
> 
> I think the more specialized you get, the more you should think about
> just using the plain old %s, and your own helpers.

As I suggested it, from my side, I'd like to have it inside printk :-)

There is a subset of formats that are subsystem-specific anyway at
printk, like the network ones. We use extensively fourcc along the
media subsystem (and you probably also use fourcc at DRM). Even some input
devices nowadays may be using V4L2 core (some multi-sensor touching
devices), with depends on it.

So, those fourcc codes are pretty common. Having it at the printk
infra makes a lot easier for people to use them.

> Because frankly, the
> kernel printk specifiers also start getting more than a little obscure.

I liked one of the suggestions of using "%p4cc" (or maybe something
similar, if having a number there is a problem, like "%pAcc" or "%pfcc")
for this printk. This would be very easy for people to identify and 
remember about its meaning.

> Or could we conceive of a way to make this locally extensible yet safe,
> letting callers use something like %{foo}, as well as providing a
> locally relevant function to do the conversion?

That's something that it makes sense to be implemented in the future,
for things that would be self-contained inside an specific subsystem.

Thanks,
Mauro
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Joe Perches]

On Thu, 2020-04-02 at 15:53 +0200, Mauro Carvalho Chehab wrote:

> I liked one of the suggestions of using "%p4cc" (or maybe something
> similar, if having a number there is a problem, like "%pAcc" or "%pfcc")

Using a number is not a problem.

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Joe Perches]

On Thu, 2020-04-02 at 15:53 +0200, Mauro Carvalho Chehab wrote:

> I liked one of the suggestions of using "%p4cc" (or maybe something
> similar, if having a number there is a problem, like "%pAcc" or "%pfcc")

Using a number is not a problem.


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Joe Perches]

On Thu, 2020-04-02 at 11:34 +0300, Jani Nikula wrote:
> On Wed, 01 Apr 2020, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.
> 
> I'm not going to take a strong stand in one way or the other regarding
> the patch at hand, but I do think at some point we have to draw a line
> what should be included in printk formats. Arguably they should be
> reserved to things that are generally useful across large parts of the
> kernel, right?

Definitely yes.

> I think the more specialized you get, the more you should think about
> just using the plain old %s, and your own helpers. Because frankly, the
> kernel printk specifiers also start getting more than a little obscure.
> 
> Or could we conceive of a way to make this locally extensible yet safe,
> letting callers use something like %{foo}, as well as providing a
> locally relevant function to do the conversion?

No.  printf validation would be broken.

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Joe Perches]

On Thu, 2020-04-02 at 11:34 +0300, Jani Nikula wrote:
> On Wed, 01 Apr 2020, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.
> 
> I'm not going to take a strong stand in one way or the other regarding
> the patch at hand, but I do think at some point we have to draw a line
> what should be included in printk formats. Arguably they should be
> reserved to things that are generally useful across large parts of the
> kernel, right?

Definitely yes.

> I think the more specialized you get, the more you should think about
> just using the plain old %s, and your own helpers. Because frankly, the
> kernel printk specifiers also start getting more than a little obscure.
> 
> Or could we conceive of a way to make this locally extensible yet safe,
> letting callers use something like %{foo}, as well as providing a
> locally relevant function to do the conversion?

No.  printf validation would be broken.


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Jani Nikula]

On Thu, 02 Apr 2020, Joe Perches <joe@perches.com> wrote:
> On Thu, 2020-04-02 at 11:34 +0300, Jani Nikula wrote:
>> Or could we conceive of a way to make this locally extensible yet safe,
>> letting callers use something like %{foo}, as well as providing a
>> locally relevant function to do the conversion?
>
> No.  printf validation would be broken.

I tossed the idea on a whim, and thinking further I could probably come
up with a number of challenges, but care to elaborate on what you see as
the problem in validation?

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Jani Nikula]

On Thu, 02 Apr 2020, Joe Perches <joe@perches.com> wrote:
> On Thu, 2020-04-02 at 11:34 +0300, Jani Nikula wrote:
>> Or could we conceive of a way to make this locally extensible yet safe,
>> letting callers use something like %{foo}, as well as providing a
>> locally relevant function to do the conversion?
>
> No.  printf validation would be broken.

I tossed the idea on a whim, and thinking further I could probably come
up with a number of challenges, but care to elaborate on what you see as
the problem in validation?

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Joe Perches]

On Fri, 2020-04-03 at 09:37 +0300, Jani Nikula wrote:
> On Thu, 02 Apr 2020, Joe Perches <joe@perches.com> wrote:
> > On Thu, 2020-04-02 at 11:34 +0300, Jani Nikula wrote:
> > > Or could we conceive of a way to make this locally extensible yet safe,
> > > letting callers use something like %{foo}, as well as providing a
> > > locally relevant function to do the conversion?
> > 
> > No.  printf validation would be broken.
> 
> I tossed the idea on a whim, and thinking further I could probably come
> up with a number of challenges, but care to elaborate on what you see as
> the problem in validation?

I understand you to want to add something like

	%<m> where m is a non-standard format specifier

so using using gcc's extension of

__attribute__((__format__(printf, string_index, first_to_check))

could not validate the argument type against use of the %<m>
in the format string.

	printk("%a\n", a);

Compiler bleats.

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Joe Perches]

On Fri, 2020-04-03 at 09:37 +0300, Jani Nikula wrote:
> On Thu, 02 Apr 2020, Joe Perches <joe@perches.com> wrote:
> > On Thu, 2020-04-02 at 11:34 +0300, Jani Nikula wrote:
> > > Or could we conceive of a way to make this locally extensible yet safe,
> > > letting callers use something like %{foo}, as well as providing a
> > > locally relevant function to do the conversion?
> > 
> > No.  printf validation would be broken.
> 
> I tossed the idea on a whim, and thinking further I could probably come
> up with a number of challenges, but care to elaborate on what you see as
> the problem in validation?

I understand you to want to add something like

	%<m> where m is a non-standard format specifier

so using using gcc's extension of

__attribute__((__format__(printf, string_index, first_to_check))

could not validate the argument type against use of the %<m>
in the format string.

	printk("%a\n", a);

Compiler bleats.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

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>
---
 Documentation/core-api/printk-formats.rst | 12 ++++
 lib/test_printf.c                         | 17 +++++
 lib/vsprintf.c                            | 86 +++++++++++++++++++++++
 3 files changed, 115 insertions(+)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 8ebe46b1af39..7aa0451e06fb 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -545,6 +545,18 @@ 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.
+
 Thanks
 ======
 
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 2d9f520d2f27..a14754086707 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -624,6 +624,22 @@ static void __init fwnode_pointer(void)
 	software_node_unregister_nodes(softnodes);
 }
 
+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)
 {
@@ -668,6 +684,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 7c488a1ce318..02e7906619c0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1721,6 +1721,89 @@ 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)
+{
+#define FOURCC_HEX_CHAR_STR		"(xx)"
+#define FOURCC_BIG_ENDIAN_STR		" big-endian"
+#define FOURCC_LITTLE_ENDIAN_STR	" little-endian"
+#define FOURCC_HEX_NUMBER		" (0x01234567)"
+#define FOURCC_STRING_MAX						\
+	FOURCC_HEX_CHAR_STR FOURCC_HEX_CHAR_STR FOURCC_HEX_CHAR_STR	\
+	FOURCC_HEX_CHAR_STR FOURCC_LITTLE_ENDIAN_STR FOURCC_HEX_NUMBER
+	struct printf_spec my_spec = {
+		.type = FORMAT_TYPE_UINT,
+		.field_width = 2,
+		.flags = SMALL,
+		.base = 16,
+		.precision = -1,
+	};
+	char __s[sizeof(FOURCC_STRING_MAX)];
+	char *s = __s;
+	unsigned int i;
+	/*
+	 * The 31st bit defines the endianness of the data, so save its printing
+	 * for later.
+	 */
+	u32 fourcc = *__fourcc & ~BIT(31);
+	int ret;
+
+	if (check_pointer(&buf, end, __fourcc, spec))
+		return buf;
+
+	if (fmt[1] != 'c' || fmt[2] != 'c')
+		return error_string(buf, end, "(%p4?)", spec);
+
+	for (i = 0; i < sizeof(fourcc); i++, fourcc >>= 8) {
+		unsigned char c = fourcc;
+
+		/* Weed out spaces */
+		if (c == ' ')
+			continue;
+
+		/* Print non-control characters as-is */
+		if (c > ' ') {
+			*s = c;
+			s++;
+			continue;
+		}
+
+		if (WARN_ON_ONCE(sizeof(__s) <
+				 (s - __s) + sizeof(FOURCC_HEX_CHAR_STR)))
+			break;
+
+		*s = '(';
+		s++;
+		s = number(s, s + 2, c, my_spec);
+		*s = ')';
+		s++;
+	}
+
+	ret = strscpy(s, *__fourcc & BIT(31) ? FOURCC_BIG_ENDIAN_STR
+					     : FOURCC_LITTLE_ENDIAN_STR,
+		      sizeof(__s) - (s - __s));
+	if (!WARN_ON_ONCE(ret < 0))
+		s += ret;
+
+	if (!WARN_ON_ONCE(sizeof(__s) <
+			  (s - __s) + sizeof(FOURCC_HEX_NUMBER))) {
+		*s = ' ';
+		s++;
+		*s = '(';
+		s++;
+		/* subtract parentheses and the space from the size */
+		special_hex_number(s, s + sizeof(FOURCC_HEX_NUMBER) - 3,
+				   *__fourcc, sizeof(u32));
+		s += sizeof(u32) * 2 + 2 /* 0x */;
+		*s = ')';
+		s++;
+		*s = '\0';
+	}
+
+	return string(buf, end, __s, spec);
+}
+
 static noinline_for_stack
 char *address_val(char *buf, char *end, const void *addr,
 		  struct printf_spec spec, const char *fmt)
@@ -2131,6 +2214,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
@@ -2223,6 +2307,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.20.1

[PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

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>
---
 Documentation/core-api/printk-formats.rst | 12 ++++
 lib/test_printf.c                         | 17 +++++
 lib/vsprintf.c                            | 86 +++++++++++++++++++++++
 3 files changed, 115 insertions(+)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 8ebe46b1af39..7aa0451e06fb 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -545,6 +545,18 @@ 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.
+
 Thanks
 ======
 
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 2d9f520d2f27..a14754086707 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -624,6 +624,22 @@ static void __init fwnode_pointer(void)
 	software_node_unregister_nodes(softnodes);
 }
 
+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)
 {
@@ -668,6 +684,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 7c488a1ce318..02e7906619c0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1721,6 +1721,89 @@ 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)
+{
+#define FOURCC_HEX_CHAR_STR		"(xx)"
+#define FOURCC_BIG_ENDIAN_STR		" big-endian"
+#define FOURCC_LITTLE_ENDIAN_STR	" little-endian"
+#define FOURCC_HEX_NUMBER		" (0x01234567)"
+#define FOURCC_STRING_MAX						\
+	FOURCC_HEX_CHAR_STR FOURCC_HEX_CHAR_STR FOURCC_HEX_CHAR_STR	\
+	FOURCC_HEX_CHAR_STR FOURCC_LITTLE_ENDIAN_STR FOURCC_HEX_NUMBER
+	struct printf_spec my_spec = {
+		.type = FORMAT_TYPE_UINT,
+		.field_width = 2,
+		.flags = SMALL,
+		.base = 16,
+		.precision = -1,
+	};
+	char __s[sizeof(FOURCC_STRING_MAX)];
+	char *s = __s;
+	unsigned int i;
+	/*
+	 * The 31st bit defines the endianness of the data, so save its printing
+	 * for later.
+	 */
+	u32 fourcc = *__fourcc & ~BIT(31);
+	int ret;
+
+	if (check_pointer(&buf, end, __fourcc, spec))
+		return buf;
+
+	if (fmt[1] != 'c' || fmt[2] != 'c')
+		return error_string(buf, end, "(%p4?)", spec);
+
+	for (i = 0; i < sizeof(fourcc); i++, fourcc >>= 8) {
+		unsigned char c = fourcc;
+
+		/* Weed out spaces */
+		if (c == ' ')
+			continue;
+
+		/* Print non-control characters as-is */
+		if (c > ' ') {
+			*s = c;
+			s++;
+			continue;
+		}
+
+		if (WARN_ON_ONCE(sizeof(__s) <
+				 (s - __s) + sizeof(FOURCC_HEX_CHAR_STR)))
+			break;
+
+		*s = '(';
+		s++;
+		s = number(s, s + 2, c, my_spec);
+		*s = ')';
+		s++;
+	}
+
+	ret = strscpy(s, *__fourcc & BIT(31) ? FOURCC_BIG_ENDIAN_STR
+					     : FOURCC_LITTLE_ENDIAN_STR,
+		      sizeof(__s) - (s - __s));
+	if (!WARN_ON_ONCE(ret < 0))
+		s += ret;
+
+	if (!WARN_ON_ONCE(sizeof(__s) <
+			  (s - __s) + sizeof(FOURCC_HEX_NUMBER))) {
+		*s = ' ';
+		s++;
+		*s = '(';
+		s++;
+		/* subtract parentheses and the space from the size */
+		special_hex_number(s, s + sizeof(FOURCC_HEX_NUMBER) - 3,
+				   *__fourcc, sizeof(u32));
+		s += sizeof(u32) * 2 + 2 /* 0x */;
+		*s = ')';
+		s++;
+		*s = '\0';
+	}
+
+	return string(buf, end, __s, spec);
+}
+
 static noinline_for_stack
 char *address_val(char *buf, char *end, const void *addr,
 		  struct printf_spec spec, const char *fmt)
@@ -2131,6 +2214,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
@@ -2223,6 +2307,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.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

On Mon, Apr 27, 2020 at 05:50:07PM +0300, 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>

Please ignore this one.

-- 
Sakari Ailus

Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs

[Sakari Ailus]

On Mon, Apr 27, 2020 at 05:50:07PM +0300, 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>

Please ignore this one.

-- 
Sakari Ailus
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel