[PATCH v2 1/3] drm/format-helper: Add conversion from XRGB8888 to BGR888

[PATCH v2 1/3] drm/format-helper: Add conversion from XRGB8888 to BGR888

[Aditya Garg]

From: Kerem Karabay <kekrby@gmail.com>

Add XRGB8888 emulation helper for devices that only support BGR888.

Signed-off-by: Kerem Karabay <kekrby@gmail.com>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---
v2 -> Fix incorrect description
 drivers/gpu/drm/drm_format_helper.c           | 54 +++++++++++++
 .../gpu/drm/tests/drm_format_helper_test.c    | 81 +++++++++++++++++++
 include/drm/drm_format_helper.h               |  3 +
 3 files changed, 138 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index b1be458ed..4f60c8d8f 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -702,6 +702,57 @@ void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pi
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888);
 
+static void drm_fb_xrgb8888_to_bgr888_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	u8 *dbuf8 = dbuf;
+	const __le32 *sbuf32 = sbuf;
+	unsigned int x;
+	u32 pix;
+
+	for (x = 0; x < pixels; x++) {
+		pix = le32_to_cpu(sbuf32[x]);
+		/* write red-green-blue to output in little endianness */
+		*dbuf8++ = (pix & 0x00ff0000) >> 16;
+		*dbuf8++ = (pix & 0x0000ff00) >> 8;
+		*dbuf8++ = (pix & 0x000000ff) >> 0;
+	}
+}
+
+/**
+ * drm_fb_xrgb8888_to_bgr888 - Convert XRGB8888 to BGR888 clip buffer
+ * @dst: Array of BGR888 destination buffers
+ * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
+ *             within @dst; can be NULL if scanlines are stored next to each other.
+ * @src: Array of XRGB8888 source buffers
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy
+ * @state: Transform and conversion state
+ *
+ * This function copies parts of a framebuffer to display memory and converts the
+ * color format during the process. Destination and framebuffer formats must match. The
+ * parameters @dst, @dst_pitch and @src refer to arrays. Each array must have at
+ * least as many entries as there are planes in @fb's format. Each entry stores the
+ * value for the format's respective color plane at the same index.
+ *
+ * This function does not apply clipping on @dst (i.e. the destination is at the
+ * top-left corner).
+ *
+ * Drivers can use this function for BGR888 devices that don't natively
+ * support XRGB8888.
+ */
+void drm_fb_xrgb8888_to_bgr888(struct iosys_map *dst, const unsigned int *dst_pitch,
+			       const struct iosys_map *src, const struct drm_framebuffer *fb,
+			       const struct drm_rect *clip, struct drm_format_conv_state *state)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		3,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, state,
+		    drm_fb_xrgb8888_to_bgr888_line);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_bgr888);
+
 static void drm_fb_xrgb8888_to_argb8888_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	__le32 *dbuf32 = dbuf;
@@ -1035,6 +1086,9 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 		} else if (dst_format == DRM_FORMAT_RGB888) {
 			drm_fb_xrgb8888_to_rgb888(dst, dst_pitch, src, fb, clip, state);
 			return 0;
+		} else if (dst_format == DRM_FORMAT_BGR888) {
+			drm_fb_xrgb8888_to_bgr888(dst, dst_pitch, src, fb, clip, state);
+			return 0;
 		} else if (dst_format == DRM_FORMAT_ARGB8888) {
 			drm_fb_xrgb8888_to_argb8888(dst, dst_pitch, src, fb, clip, state);
 			return 0;
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 08992636e..35cd3405d 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -60,6 +60,11 @@ struct convert_to_rgb888_result {
 	const u8 expected[TEST_BUF_SIZE];
 };
 
+struct convert_to_bgr888_result {
+	unsigned int dst_pitch;
+	const u8 expected[TEST_BUF_SIZE];
+};
+
 struct convert_to_argb8888_result {
 	unsigned int dst_pitch;
 	const u32 expected[TEST_BUF_SIZE];
@@ -107,6 +112,7 @@ struct convert_xrgb8888_case {
 	struct convert_to_argb1555_result argb1555_result;
 	struct convert_to_rgba5551_result rgba5551_result;
 	struct convert_to_rgb888_result rgb888_result;
+	struct convert_to_bgr888_result bgr888_result;
 	struct convert_to_argb8888_result argb8888_result;
 	struct convert_to_xrgb2101010_result xrgb2101010_result;
 	struct convert_to_argb2101010_result argb2101010_result;
@@ -151,6 +157,10 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 			.dst_pitch = TEST_USE_DEFAULT_PITCH,
 			.expected = { 0x00, 0x00, 0xFF },
 		},
+		.bgr888_result = {
+			.dst_pitch = TEST_USE_DEFAULT_PITCH,
+			.expected = { 0xFF, 0x00, 0x00 },
+		},
 		.argb8888_result = {
 			.dst_pitch = TEST_USE_DEFAULT_PITCH,
 			.expected = { 0xFFFF0000 },
@@ -217,6 +227,10 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 			.dst_pitch = TEST_USE_DEFAULT_PITCH,
 			.expected = { 0x00, 0x00, 0xFF },
 		},
+		.bgr888_result = {
+			.dst_pitch = TEST_USE_DEFAULT_PITCH,
+			.expected = { 0xFF, 0x00, 0x00 },
+		},
 		.argb8888_result = {
 			.dst_pitch = TEST_USE_DEFAULT_PITCH,
 			.expected = { 0xFFFF0000 },
@@ -330,6 +344,15 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 				0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0x00,
 			},
 		},
+		.bgr888_result = {
+			.dst_pitch = TEST_USE_DEFAULT_PITCH,
+			.expected = {
+				0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00,
+				0xFF, 0x00, 0x00, 0x00, 0xFF, 0x00,
+				0x00, 0x00, 0xFF, 0xFF, 0x00, 0xFF,
+				0xFF, 0xFF, 0x00, 0x00, 0xFF, 0xFF,
+			},
+		},
 		.argb8888_result = {
 			.dst_pitch = TEST_USE_DEFAULT_PITCH,
 			.expected = {
@@ -468,6 +491,17 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
 				0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 			},
 		},
+		.bgr888_result = {
+			.dst_pitch = 15,
+			.expected = {
+				0x0E, 0x44, 0x9C, 0x11, 0x4D, 0x05, 0xA8, 0xF3, 0x03,
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+				0x6C, 0xF0, 0x73, 0x0E, 0x44, 0x9C, 0x11, 0x4D, 0x05,
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+				0xA8, 0x03, 0x03, 0x6C, 0xF0, 0x73, 0x0E, 0x44, 0x9C,
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+			},
+		},
 		.argb8888_result = {
 			.dst_pitch = 20,
 			.expected = {
@@ -914,6 +948,52 @@ static void drm_test_fb_xrgb8888_to_rgb888(struct kunit *test)
 	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
 }
 
+static void drm_test_fb_xrgb8888_to_bgr888(struct kunit *test)
+{
+	const struct convert_xrgb8888_case *params = test->param_value;
+	const struct convert_to_bgr888_result *result = &params->bgr888_result;
+	size_t dst_size;
+	u8 *buf = NULL;
+	__le32 *xrgb8888 = NULL;
+	struct iosys_map dst, src;
+
+	struct drm_framebuffer fb = {
+		.format = drm_format_info(DRM_FORMAT_XRGB8888),
+		.pitches = { params->pitch, 0, 0 },
+	};
+
+	dst_size = conversion_buf_size(DRM_FORMAT_BGR888, result->dst_pitch,
+				       &params->clip, 0);
+	KUNIT_ASSERT_GT(test, dst_size, 0);
+
+	buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+	iosys_map_set_vaddr(&dst, buf);
+
+	xrgb8888 = cpubuf_to_le32(test, params->xrgb8888, TEST_BUF_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb8888);
+	iosys_map_set_vaddr(&src, xrgb8888);
+
+	/*
+	 * BGR888 expected results are already in little-endian
+	 * order, so there's no need to convert the test output.
+	 */
+	drm_fb_xrgb8888_to_bgr888(&dst, &result->dst_pitch, &src, &fb, &params->clip,
+				  &fmtcnv_state);
+	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
+
+	buf = dst.vaddr; /* restore original value of buf */
+	memset(buf, 0, dst_size);
+
+	int blit_result = 0;
+
+	blit_result = drm_fb_blit(&dst, &result->dst_pitch, DRM_FORMAT_BGR888, &src, &fb, &params->clip,
+				  &fmtcnv_state);
+
+	KUNIT_EXPECT_FALSE(test, blit_result);
+	KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
+}
+
 static void drm_test_fb_xrgb8888_to_argb8888(struct kunit *test)
 {
 	const struct convert_xrgb8888_case *params = test->param_value;
@@ -1851,6 +1931,7 @@ static struct kunit_case drm_format_helper_test_cases[] = {
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb1555, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgba5551, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_rgb888, convert_xrgb8888_gen_params),
+	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_bgr888, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb8888, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_xrgb2101010, convert_xrgb8888_gen_params),
 	KUNIT_CASE_PARAM(drm_test_fb_xrgb8888_to_argb2101010, convert_xrgb8888_gen_params),
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 428d81afe..aa1604d92 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -96,6 +96,9 @@ void drm_fb_xrgb8888_to_rgba5551(struct iosys_map *dst, const unsigned int *dst_
 void drm_fb_xrgb8888_to_rgb888(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
 			       const struct drm_rect *clip, struct drm_format_conv_state *state);
+void drm_fb_xrgb8888_to_bgr888(struct iosys_map *dst, const unsigned int *dst_pitch,
+			       const struct iosys_map *src, const struct drm_framebuffer *fb,
+			       const struct drm_rect *clip, struct drm_format_conv_state *state);
 void drm_fb_xrgb8888_to_argb8888(struct iosys_map *dst, const unsigned int *dst_pitch,
 				 const struct iosys_map *src, const struct drm_framebuffer *fb,
 				 const struct drm_rect *clip, struct drm_format_conv_state *state);
-- 
2.43.0

[PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

From: Hector Martin <marcan@marcan.st>

%p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
it's useful to be able to print generic 4-character codes formatted as
an integer. Extend it to add format specifiers for printing generic
32-bit FOURCCs with various endian semantics:

%p4ch   Host-endian
%p4cl	Little-endian
%p4cb	Big-endian
%p4cr	Reverse-endian

The endianness determines how bytes are interpreted as a u32, and the
FOURCC is then always printed MSByte-first (this is the opposite of
V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
allow printing LSByte-first FOURCCs stored in host endian order
(other than the hex form being in character order, not the integer
value).

Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
v2 -> Add this patch
 Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
 lib/test_printf.c                         | 39 +++++++++++++++++++----
 lib/vsprintf.c                            | 38 ++++++++++++++++++----
 scripts/checkpatch.pl                     |  2 +-
 4 files changed, 97 insertions(+), 14 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index ecccc0473..9982861fa 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -648,6 +648,38 @@ Examples::
 	%p4cc	Y10  little-endian (0x20303159)
 	%p4cc	NV12 big-endian (0xb231564e)
 
+Generic FourCC code
+-------------------
+
+::
+	%p4c[hrbl]	gP00 (0x67503030)
+
+Print a generic FourCC code, as both ASCII characters and its numerical
+value as hexadecimal.
+
+The additional ``h``, ``r``, ``b``, and ``l`` specifiers are used to specify
+host, reversed, big or little endian order data respectively. Host endian
+order means the data is interpreted as a 32-bit integer and the most
+significant byte is printed first; that is, the character code as printed
+matches the byte order stored in memory on big-endian systems, and is reversed
+on little-endian systems.
+
+Passed by reference.
+
+Examples for a little-endian machine, given &(u32)0x67503030::
+
+	%p4ch	gP00 (0x67503030)
+	%p4cr	00Pg (0x30305067)
+	%p4cb	00Pg (0x30305067)
+	%p4cl	gP00 (0x67503030)
+
+Examples for a big-endian machine, given &(u32)0x67503030::
+
+	%p4ch	gP00 (0x67503030)
+	%p4cr	00Pg (0x30305067)
+	%p4cb	gP00 (0x67503030)
+	%p4cl	00Pg (0x30305067)
+
 Rust
 ----
 
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 59dbe4f9a..ee860327e 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
 	software_node_unregister_node_group(group);
 }
 
+struct fourcc_struct {
+	u32 code;
+	const char *str;
+};
+
+static void __init fourcc_pointer_test(const struct fourcc_struct *fc, size_t n,
+				       const char *fmt)
+{
+	size_t i;
+
+	for (i = 0; i < n; i++)
+		test(fc[i].str, fmt, &fc[i].code);
+}
+
 static void __init fourcc_pointer(void)
 {
-	struct {
-		u32 code;
-		char *str;
-	} const try[] = {
+	struct fourcc_struct const try_cc[] = {
 		{ 0x3231564e, "NV12 little-endian (0x3231564e)", },
 		{ 0xb231564e, "NV12 big-endian (0xb231564e)", },
 		{ 0x10111213, ".... little-endian (0x10111213)", },
 		{ 0x20303159, "Y10  little-endian (0x20303159)", },
 	};
-	unsigned int i;
+	struct fourcc_struct const try_ch = {
+		0x41424344, "ABCD (0x41424344)",
+	};
+	struct fourcc_struct const try_cr = {
+		0x41424344, "DCBA (0x44434241)",
+	};
+	struct fourcc_struct const try_cl = {
+		le32_to_cpu(0x41424344), "ABCD (0x41424344)",
+	};
+	struct fourcc_struct const try_cb = {
+		be32_to_cpu(0x41424344), "ABCD (0x41424344)",
+	};
 
-	for (i = 0; i < ARRAY_SIZE(try); i++)
-		test(try[i].str, "%p4cc", &try[i].code);
+	fourcc_pointer_test(try_cc, ARRAY_SIZE(try_cc), "%p4cc");
+	fourcc_pointer_test(&try_ch, 1, "%p4ch");
+	fourcc_pointer_test(&try_cr, 1, "%p4cr");
+	fourcc_pointer_test(&try_cl, 1, "%p4cl");
+	fourcc_pointer_test(&try_cb, 1, "%p4cb");
 }
 
 static void __init
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 56fe96319..13733a4da 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1781,27 +1781,53 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
 	char output[sizeof("0123 little-endian (0x01234567)")];
 	char *p = output;
 	unsigned int i;
+	bool pixel_fmt = false;
 	u32 orig, val;
 
-	if (fmt[1] != 'c' || fmt[2] != 'c')
+	if (fmt[1] != 'c')
 		return error_string(buf, end, "(%p4?)", spec);
 
 	if (check_pointer(&buf, end, fourcc, spec))
 		return buf;
 
 	orig = get_unaligned(fourcc);
-	val = orig & ~BIT(31);
+	switch (fmt[2]) {
+	case 'h':
+		val = orig;
+		break;
+	case 'r':
+		orig = swab32(orig);
+		val = orig;
+		break;
+	case 'l':
+		orig = le32_to_cpu(orig);
+		val = orig;
+		break;
+	case 'b':
+		orig = be32_to_cpu(orig);
+		val = orig;
+		break;
+	case 'c':
+		/* Pixel formats are printed LSB-first */
+		val = swab32(orig & ~BIT(31));
+		pixel_fmt = true;
+		break;
+	default:
+		return error_string(buf, end, "(%p4?)", spec);
+	}
 
 	for (i = 0; i < sizeof(u32); i++) {
-		unsigned char c = val >> (i * 8);
+		unsigned char c = val >> ((3 - i) * 8);
 
 		/* Print non-control ASCII characters as-is, dot otherwise */
 		*p++ = isascii(c) && isprint(c) ? c : '.';
 	}
 
-	*p++ = ' ';
-	strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
-	p += strlen(p);
+	if (pixel_fmt) {
+		*p++ = ' ';
+		strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
+		p += strlen(p);
+	}
 
 	*p++ = ' ';
 	*p++ = '(';
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7b28ad331..21516f753 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6904,7 +6904,7 @@ sub process {
 					    ($extension eq "f" &&
 					     defined $qualifier && $qualifier !~ /^w/) ||
 					    ($extension eq "4" &&
-					     defined $qualifier && $qualifier !~ /^cc/)) {
+					     defined $qualifier && $qualifier !~ /^c[chlbr]/)) {
 						$bad_specifier = $specifier;
 						last;
 					}
-- 
2.43.0

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Rasmus Villemoes]

On Thu, Feb 20 2025, Aditya Garg <gargaditya08@live.com> wrote:

> v2 -> Add this patch
>  Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
>  lib/test_printf.c                         | 39 +++++++++++++++++++----
>  lib/vsprintf.c                            | 38 ++++++++++++++++++----

Yay! Thanks for remembering to include test cases.

>  
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 59dbe4f9a..ee860327e 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
>  	software_node_unregister_node_group(group);
>  }
>  
> +struct fourcc_struct {
> +	u32 code;
> +	const char *str;
> +};
> +
> +static void __init fourcc_pointer_test(const struct fourcc_struct *fc, size_t n,
> +				       const char *fmt)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < n; i++)
> +		test(fc[i].str, fmt, &fc[i].code);
> +}
> +
>  static void __init fourcc_pointer(void)
>  {
> -	struct {
> -		u32 code;
> -		char *str;
> -	} const try[] = {
> +	struct fourcc_struct const try_cc[] = {

I know it matches the code it replaces, but kernel style seems to be
"const struct foo" rather than "struct foo const" (at around 130:1) -
just as you use in the new helper function.

Also, please consider changing the array, and the newly added instances,
to be static instead of automatic (our le32_to_cpu should be usable also
for static initializers).

This will conflict with the conversion-to-kunit which is in flight, but
the conflict should be trivial to resolve.

Rasmus

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

Hi

> 
>> 
>> diff --git a/lib/test_printf.c b/lib/test_printf.c
>> index 59dbe4f9a..ee860327e 100644
>> --- a/lib/test_printf.c
>> +++ b/lib/test_printf.c
>> @@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
>> software_node_unregister_node_group(group);
>> }
>> 
>> +struct fourcc_struct {
>> + u32 code;
>> + const char *str;
>> +};
>> +
>> +static void __init fourcc_pointer_test(const struct fourcc_struct *fc, size_t n,
>> +       const char *fmt)
>> +{
>> + size_t i;
>> +
>> + for (i = 0; i < n; i++)
>> + test(fc[i].str, fmt, &fc[i].code);
>> +}
>> +
>> static void __init fourcc_pointer(void)
>> {
>> - struct {
>> - u32 code;
>> - char *str;
>> - } const try[] = {
>> + struct fourcc_struct const try_cc[] = {
> 
> I know it matches the code it replaces, but kernel style seems to be
> "const struct foo" rather than "struct foo const" (at around 130:1) -
> just as you use in the new helper function.
> 
> Also, please consider changing the array, and the newly added instances,
> to be static instead of automatic (our le32_to_cpu should be usable also
> for static initializers).
> 

V3 sent here:
https://lore.kernel.org/dri-devel/98289BC4-D5E1-41B8-AC89-632DBD2C2789@live.com/T/#mfa1dac647c9517674649a50301b122a524cc364c

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[andriy.shevchenko]

On Thu, Feb 20, 2025 at 04:39:23PM +0000, Aditya Garg wrote:
> From: Hector Martin <marcan@marcan.st>
> 
> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
> it's useful to be able to print generic 4-character codes formatted as
> an integer. Extend it to add format specifiers for printing generic
> 32-bit FOURCCs with various endian semantics:
> 
> %p4ch   Host-endian
> %p4cl	Little-endian
> %p4cb	Big-endian
> %p4cr	Reverse-endian
> 
> The endianness determines how bytes are interpreted as a u32, and the
> FOURCC is then always printed MSByte-first (this is the opposite of
> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
> allow printing LSByte-first FOURCCs stored in host endian order
> (other than the hex form being in character order, not the integer
> value).

...

>  	orig = get_unaligned(fourcc);
> -	val = orig & ~BIT(31);
> +	switch (fmt[2]) {
> +	case 'h':
> +		val = orig;
> +		break;
> +	case 'r':
> +		orig = swab32(orig);
> +		val = orig;
> +		break;
> +	case 'l':

> +		orig = le32_to_cpu(orig);
> +		val = orig;
> +		break;
> +	case 'b':
> +		orig = be32_to_cpu(orig);

I do not see that orig is a union of different types. Have you run sparse?
It will definitely complain on this code.

> +		val = orig;
> +		break;
> +	case 'c':
> +		/* Pixel formats are printed LSB-first */
> +		val = swab32(orig & ~BIT(31));
> +		pixel_fmt = true;
> +		break;
> +	default:
> +		return error_string(buf, end, "(%p4?)", spec);
> +	}

...

> -	*p++ = ' ';
> -	strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> -	p += strlen(p);

> +	if (pixel_fmt) {

Technically we can avoid a boolean by checking fmt[2] again here, but I'm okay
with a temporary holder.

> +		*p++ = ' ';
> +		strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> +		p += strlen(p);
> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> On 21 Feb 2025, at 8:57 PM, andriy.shevchenko@linux.intel.com wrote:
> 
> On Thu, Feb 20, 2025 at 04:39:23PM +0000, Aditya Garg wrote:
>> From: Hector Martin <marcan@marcan.st>
>> 
>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
>> it's useful to be able to print generic 4-character codes formatted as
>> an integer. Extend it to add format specifiers for printing generic
>> 32-bit FOURCCs with various endian semantics:
>> 
>> %p4ch   Host-endian
>> %p4cl Little-endian
>> %p4cb Big-endian
>> %p4cr Reverse-endian
>> 
>> The endianness determines how bytes are interpreted as a u32, and the
>> FOURCC is then always printed MSByte-first (this is the opposite of
>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
>> allow printing LSByte-first FOURCCs stored in host endian order
>> (other than the hex form being in character order, not the integer
>> value).
> 
> ...
> 
>> orig = get_unaligned(fourcc);
>> - val = orig & ~BIT(31);
>> + switch (fmt[2]) {
>> + case 'h':
>> + val = orig;
>> + break;
>> + case 'r':
>> + orig = swab32(orig);
>> + val = orig;
>> + break;
>> + case 'l':
> 
>> + orig = le32_to_cpu(orig);
>> + val = orig;
>> + break;
>> + case 'b':
>> + orig = be32_to_cpu(orig);
> 
> I do not see that orig is a union of different types. Have you run sparse?
> It will definitely complain on this code.

Does this look good now? Made orig a union.

char *fourcc_string(char *buf, char *end, const u32 *fourcc, const char *fmt, struct printf_spec spec) 
{
char output[sizeof("0123 little-endian (0x01234567)")];
char *p = output;
unsigned int i;
bool pixel_fmt = false;
u32 val;

union {
u32 raw;
__le32 le;
__be32 be;
} orig;

if (fmt[1] != 'c')
return error_string(buf, end, "(%p4?)", spec);

if (check_pointer(&buf, end, fourcc, spec))
return buf;

orig.raw = get_unaligned(fourcc);

switch (fmt[2]) {
case 'h':
val = orig.raw;
break;
case 'r':
val = swab32(orig.raw);
break;
case 'l':
val = le32_to_cpu(orig.le);
break;
case 'b':
val = be32_to_cpu(orig.be);
break;
case 'c':
val = swab32(orig.raw & ~BIT(31));
pixel_fmt = true;
break;
default:
return error_string(buf, end, "(%p4?)", spec);
}

for (i = 0; i < sizeof(u32); i++) {
unsigned char c = val >> ((3 - i) * 8);
*p++ = isascii(c) && isprint(c) ? c : '.';
}

if (pixel_fmt) {
*p++ = ' ';
strcpy(p, orig.raw & BIT(31) ? "big-endian" : "little-endian");
p += strlen(p);
}

*p++ = ' ';
*p++ = '(';
p += sprintf(p, "0x%08x", orig.raw);
*p++ = ')';
*p = '\0';

return string_nocheck(buf, end, output, spec);
}

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> Does this look good now? Made orig a union.

Wait, it's messier. Maybe declare data type of val separately in each case?
> 
> char *fourcc_string(char *buf, char *end, const u32 *fourcc, const char *fmt, struct printf_spec spec)
> {
> char output[sizeof("0123 little-endian (0x01234567)")];
> char *p = output;
> unsigned int i;
> bool pixel_fmt = false;
> u32 val;
> 
> union {
> u32 raw;
> __le32 le;
> __be32 be;
> } orig;
> 
> if (fmt[1] != 'c')
> return error_string(buf, end, "(%p4?)", spec);
> 
> if (check_pointer(&buf, end, fourcc, spec))
> return buf;
> 
> orig.raw = get_unaligned(fourcc);
> 
> switch (fmt[2]) {
> case 'h':
> val = orig.raw;
> break;
> case 'r':
> val = swab32(orig.raw);
> break;
> case 'l':
> val = le32_to_cpu(orig.le);
> break;
> case 'b':
> val = be32_to_cpu(orig.be);
> break;
> case 'c':
> val = swab32(orig.raw & ~BIT(31));
> pixel_fmt = true;
> break;
> default:
> return error_string(buf, end, "(%p4?)", spec);
> }
> 
> for (i = 0; i < sizeof(u32); i++) {
> unsigned char c = val >> ((3 - i) * 8);
> *p++ = isascii(c) && isprint(c) ? c : '.';
> }
> 
> if (pixel_fmt) {
> *p++ = ' ';
> strcpy(p, orig.raw & BIT(31) ? "big-endian" : "little-endian");
> p += strlen(p);
> }
> 
> *p++ = ' ';
> *p++ = '(';
> p += sprintf(p, "0x%08x", orig.raw);
> *p++ = ')';
> *p = '\0';
> 
> return string_nocheck(buf, end, output, spec);
> }
> 

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[andriy.shevchenko]

On Fri, Feb 21, 2025 at 08:06:51PM +0000, Aditya Garg wrote:
> 
> > Does this look good now? Made orig a union.
> 
> Wait, it's messier. Maybe declare data type of val separately in each case?

Yes, this sounds better.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> On 21 Feb 2025, at 8:57 PM, andriy.shevchenko@linux.intel.com wrote:
> 
> On Thu, Feb 20, 2025 at 04:39:23PM +0000, Aditya Garg wrote:
>> From: Hector Martin <marcan@marcan.st>
>> 
>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
>> it's useful to be able to print generic 4-character codes formatted as
>> an integer. Extend it to add format specifiers for printing generic
>> 32-bit FOURCCs with various endian semantics:
>> 
>> %p4ch   Host-endian
>> %p4cl Little-endian
>> %p4cb Big-endian
>> %p4cr Reverse-endian
>> 
>> The endianness determines how bytes are interpreted as a u32, and the
>> FOURCC is then always printed MSByte-first (this is the opposite of
>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
>> allow printing LSByte-first FOURCCs stored in host endian order
>> (other than the hex form being in character order, not the integer
>> value).
> 
> ...
> 
>> orig = get_unaligned(fourcc);
>> - val = orig & ~BIT(31);
>> + switch (fmt[2]) {
>> + case 'h':
>> + val = orig;
>> + break;
>> + case 'r':
>> + orig = swab32(orig);
>> + val = orig;
>> + break;
>> + case 'l':
> 
>> + orig = le32_to_cpu(orig);
>> + val = orig;
>> + break;
>> + case 'b':
>> + orig = be32_to_cpu(orig);
> 
> I do not see that orig is a union of different types. Have you run sparse?
> It will definitely complain on this code.

After messing around with this, what I’ve noticed is that orig and val used in this struct should be u32. Now in case of little endian and big endian, that things are messy. The original code by Hector was using le32_to_cpu on orig, which itself is declared as a u32 here (maybe was done with the intention to convert le32 orig to u32 orig?).

Anyways, what I have done is that:

1. Declare new variable, orig_le which is __le32.
2. Instead of doing orig = le32_to_cpu(orig); , we can do orig_le = cpu_to_le32(orig). This fixes the sparse warning: cast to restricted __le32
3. Now the original code was intending to use val=orig=le32_to_cpu(orig) at the bottom part of this struct. Those parts also require val and orig to be u32. For that, we are now using le32_to_cpu(orig_le). Since val is same as orig, in case these cases, instead of making a val_le, I’ve simply used orig_le there as well.

Similar changes done for big endian.

So, the struct looks like this now:

static noinline_for_stack
char *fourcc_string(char *buf, char *end, const u32 *fourcc,
		    struct printf_spec spec, const char *fmt)
{
	char output[sizeof("0123 little-endian (0x01234567)")];
	char *p = output;
	unsigned int i;
	unsigned char c;
	bool pixel_fmt = false;
	u32 orig, val;
	__le32 orig_le;
	__be32 orig_be;

	if (fmt[1] != 'c')
		return error_string(buf, end, "(%p4?)", spec);

	if (check_pointer(&buf, end, fourcc, spec))
		return buf;

	orig = get_unaligned(fourcc);
	switch (fmt[2]) {
	case 'h':
		val = orig;
		break;
	case 'r':
		orig = swab32(orig);
		val = orig;
		break;
	case 'l':
		orig_le = cpu_to_le32(orig);
		break;
	case 'b':
		orig_be = cpu_to_be32(orig);
		break;
	case 'c':
		/* Pixel formats are printed LSB-first */
		val = swab32(orig & ~BIT(31));
		pixel_fmt = true;
		break;
	default:
		return error_string(buf, end, "(%p4?)", spec);
	}

	for (i = 0; i < sizeof(u32); i++) {
		switch (fmt[2]) {
		case 'h':
		case 'r':
		case 'c':
			c = val >> ((3 - i) * 8);
			break;
		case 'l':
			c = le32_to_cpu(orig_le) >> ((3 - i) * 8);
			break;
		case 'b':
			c = be32_to_cpu(orig_be) >> ((3 - i) * 8);
			break;
		}

		/* Print non-control ASCII characters as-is, dot otherwise */
		*p++ = isascii(c) && isprint(c) ? c : '.';
	}

	if (pixel_fmt) {
		*p++ = ' ';
		strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
		p += strlen(p);
	}

	*p++ = ' ';
	*p++ = '(';

	switch (fmt[2]) {
	case 'h':
	case 'r':
	case 'c':
		p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32));
		break;
	case 'l':
		p = special_hex_number(p, output + sizeof(output) - 2, le32_to_cpu(orig_le), sizeof(u32));
		break;
	case 'b':
		p = special_hex_number(p, output + sizeof(output) - 2, be32_to_cpu(orig_be), sizeof(u32));
		break;
	}

	*p++ = ')';
	*p = '\0';

	return string(buf, end, output, spec);
}

Andy, could you verify this?

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> From: Hector Martin <marcan@marcan.st>
> 
> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
> it's useful to be able to print generic 4-character codes formatted as
> an integer. Extend it to add format specifiers for printing generic
> 32-bit FOURCCs with various endian semantics:
> 
> %p4ch   Host-endian
> %p4cl Little-endian
> %p4cb Big-endian
> %p4cr Reverse-endian
> 
> The endianness determines how bytes are interpreted as a u32, and the
> FOURCC is then always printed MSByte-first (this is the opposite of
> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
> allow printing LSByte-first FOURCCs stored in host endian order
> (other than the hex form being in character order, not the integer
> value).
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>

BTW, after looking at the comments by Martin [1], its actually better to use existing specifiers for the appletbdrm driver.
The driver needs the host endian as proposed by this patch, so instead of that, we can use %.4s

[1]: https://lore.kernel.org/asahi/E753B391-D2CB-4213-AF82-678ADD5A7644@cutebit.org/

Alternatively we could add a host endian only. Other endians are not really used by any driver AFAIK. The host endian is being used by appletbdrm and Asahi Linux’ SMC driver only.
> 
> 
> ---
> - *p++ = ' ';
> - strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> - p += strlen(p);
> + if (pixel_fmt) {
> + *p++ = ' ';
> + strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> + p += strlen(p);
> + }

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[andriy.shevchenko]

On Sat, Feb 22, 2025 at 03:46:03PM +0000, Aditya Garg wrote:
> > On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:
> > 
> > %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
> > it's useful to be able to print generic 4-character codes formatted as
> > an integer. Extend it to add format specifiers for printing generic
> > 32-bit FOURCCs with various endian semantics:
> > 
> > %p4ch   Host-endian
> > %p4cl Little-endian
> > %p4cb Big-endian
> > %p4cr Reverse-endian
> > 
> > The endianness determines how bytes are interpreted as a u32, and the
> > FOURCC is then always printed MSByte-first (this is the opposite of
> > V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
> > allow printing LSByte-first FOURCCs stored in host endian order
> > (other than the hex form being in character order, not the integer
> > value).

...

> BTW, after looking at the comments by Martin [1], its actually better to use
> existing specifiers for the appletbdrm driver.  The driver needs the host
> endian as proposed by this patch, so instead of that, we can use %.4s

Do you mean this patch will not be needed? If this a case, that would be the
best solution.

> [1]: https://lore.kernel.org/asahi/E753B391-D2CB-4213-AF82-678ADD5A7644@cutebit.org/
> 
> Alternatively we could add a host endian only. Other endians are not really
> used by any driver AFAIK. The host endian is being used by appletbdrm and
> Asahi Linux’ SMC driver only.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> On 24 Feb 2025, at 3:28 PM, andriy.shevchenko@linux.intel.com wrote:
> 
> On Sat, Feb 22, 2025 at 03:46:03PM +0000, Aditya Garg wrote:
>>>> On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:
>>> 
>>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
>>> it's useful to be able to print generic 4-character codes formatted as
>>> an integer. Extend it to add format specifiers for printing generic
>>> 32-bit FOURCCs with various endian semantics:
>>> 
>>> %p4ch   Host-endian
>>> %p4cl Little-endian
>>> %p4cb Big-endian
>>> %p4cr Reverse-endian
>>> 
>>> The endianness determines how bytes are interpreted as a u32, and the
>>> FOURCC is then always printed MSByte-first (this is the opposite of
>>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
>>> allow printing LSByte-first FOURCCs stored in host endian order
>>> (other than the hex form being in character order, not the integer
>>> value).
> 
> ...
> 
>> BTW, after looking at the comments by Martin [1], its actually better to use
>> existing specifiers for the appletbdrm driver.  The driver needs the host
>> endian as proposed by this patch, so instead of that, we can use %.4s
> 
> Do you mean this patch will not be needed? If this a case, that would be the
> best solution.

I tested with %4pE, and the results are different from expected. So this would be preferred. Kindly see my latest email with a proposed workaround for the sparse warnings.
> 
>> [1]: https://lore.kernel.org/asahi/E753B391-D2CB-4213-AF82-678ADD5A7644@cutebit.org/
>> 
>> Alternatively we could add a host endian only. Other endians are not really
>> used by any driver AFAIK. The host endian is being used by appletbdrm and
>> Asahi Linux’ SMC driver only.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[andriy.shevchenko]

On Mon, Feb 24, 2025 at 10:18:48AM +0000, Aditya Garg wrote:
> 
> 
> > On 24 Feb 2025, at 3:28 PM, andriy.shevchenko@linux.intel.com wrote:
> > 
> > On Sat, Feb 22, 2025 at 03:46:03PM +0000, Aditya Garg wrote:
> >>>> On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:
> >>> 
> >>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
> >>> it's useful to be able to print generic 4-character codes formatted as
> >>> an integer. Extend it to add format specifiers for printing generic
> >>> 32-bit FOURCCs with various endian semantics:
> >>> 
> >>> %p4ch   Host-endian
> >>> %p4cl Little-endian
> >>> %p4cb Big-endian
> >>> %p4cr Reverse-endian
> >>> 
> >>> The endianness determines how bytes are interpreted as a u32, and the
> >>> FOURCC is then always printed MSByte-first (this is the opposite of
> >>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
> >>> allow printing LSByte-first FOURCCs stored in host endian order
> >>> (other than the hex form being in character order, not the integer
> >>> value).
> > 
> > ...
> > 
> >> BTW, after looking at the comments by Martin [1], its actually better to use
> >> existing specifiers for the appletbdrm driver.  The driver needs the host
> >> endian as proposed by this patch, so instead of that, we can use %.4s
> > 
> > Do you mean this patch will not be needed? If this a case, that would be the
> > best solution.
> 
> I tested with %4pE, and the results are different from expected. So this
> would be preferred. Kindly see my latest email with a proposed workaround for
> the sparse warnings.

%.4s sounded okay, but %4pE is always about escaping and the result may occupy
%4x memory (octal escaping of non-printable characters). Of course, you may vary
the escaping classes, but IIRC the octal or hex escaping is unconditional.

> >> [1]: https://lore.kernel.org/asahi/E753B391-D2CB-4213-AF82-678ADD5A7644@cutebit.org/
> >> 
> >> Alternatively we could add a host endian only. Other endians are not really
> >> used by any driver AFAIK. The host endian is being used by appletbdrm and
> >> Asahi Linux’ SMC driver only.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> On 24 Feb 2025, at 3:54 PM, andriy.shevchenko@linux.intel.com wrote:
> 
> On Mon, Feb 24, 2025 at 10:18:48AM +0000, Aditya Garg wrote:
>> 
>> 
>>>> On 24 Feb 2025, at 3:28 PM, andriy.shevchenko@linux.intel.com wrote:
>>> 
>>> On Sat, Feb 22, 2025 at 03:46:03PM +0000, Aditya Garg wrote:
>>>>>> On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:
>>>>> 
>>>>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
>>>>> it's useful to be able to print generic 4-character codes formatted as
>>>>> an integer. Extend it to add format specifiers for printing generic
>>>>> 32-bit FOURCCs with various endian semantics:
>>>>> 
>>>>> %p4ch   Host-endian
>>>>> %p4cl Little-endian
>>>>> %p4cb Big-endian
>>>>> %p4cr Reverse-endian
>>>>> 
>>>>> The endianness determines how bytes are interpreted as a u32, and the
>>>>> FOURCC is then always printed MSByte-first (this is the opposite of
>>>>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
>>>>> allow printing LSByte-first FOURCCs stored in host endian order
>>>>> (other than the hex form being in character order, not the integer
>>>>> value).
>>> 
>>> ...
>>> 
>>>> BTW, after looking at the comments by Martin [1], its actually better to use
>>>> existing specifiers for the appletbdrm driver.  The driver needs the host
>>>> endian as proposed by this patch, so instead of that, we can use %.4s
>>> 
>>> Do you mean this patch will not be needed? If this a case, that would be the
>>> best solution.
>> 
>> I tested with %4pE, and the results are different from expected. So this
>> would be preferred. Kindly see my latest email with a proposed workaround for
>> the sparse warnings.
> 
> %.4s sounded okay, but %4pE is always about escaping and the result may occupy
> %4x memory (octal escaping of non-printable characters). Of course, you may vary
> the escaping classes, but IIRC the octal or hex escaping is unconditional.

%.4s is used for unsigned int iirc, here it's __le32.
> 
>>>> [1]: https://lore.kernel.org/asahi/E753B391-D2CB-4213-AF82-678ADD5A7644@cutebit.org/
>>>> 
>>>> Alternatively we could add a host endian only. Other endians are not really
>>>> used by any driver AFAIK. The host endian is being used by appletbdrm and
>>>> Asahi Linux’ SMC driver only.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[andriy.shevchenko]

On Mon, Feb 24, 2025 at 10:32:27AM +0000, Aditya Garg wrote:
> > On 24 Feb 2025, at 3:54 PM, andriy.shevchenko@linux.intel.com wrote:
> > On Mon, Feb 24, 2025 at 10:18:48AM +0000, Aditya Garg wrote:
> >>>> On 24 Feb 2025, at 3:28 PM, andriy.shevchenko@linux.intel.com wrote:
> >>> On Sat, Feb 22, 2025 at 03:46:03PM +0000, Aditya Garg wrote:
> >>>>>> On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:

...

> >>>>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
> >>>>> it's useful to be able to print generic 4-character codes formatted as
> >>>>> an integer. Extend it to add format specifiers for printing generic
> >>>>> 32-bit FOURCCs with various endian semantics:
> >>>>> 
> >>>>> %p4ch   Host-endian
> >>>>> %p4cl Little-endian
> >>>>> %p4cb Big-endian
> >>>>> %p4cr Reverse-endian
> >>>>> 
> >>>>> The endianness determines how bytes are interpreted as a u32, and the
> >>>>> FOURCC is then always printed MSByte-first (this is the opposite of
> >>>>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
> >>>>> allow printing LSByte-first FOURCCs stored in host endian order
> >>>>> (other than the hex form being in character order, not the integer
> >>>>> value).
> >>> 
> >>> ...
> >>> 
> >>>> BTW, after looking at the comments by Martin [1], its actually better to use
> >>>> existing specifiers for the appletbdrm driver.  The driver needs the host
> >>>> endian as proposed by this patch, so instead of that, we can use %.4s
> >>> 
> >>> Do you mean this patch will not be needed? If this a case, that would be the
> >>> best solution.
> >> 
> >> I tested with %4pE, and the results are different from expected. So this
> >> would be preferred. Kindly see my latest email with a proposed workaround for
> >> the sparse warnings.
> > 
> > %.4s sounded okay, but %4pE is always about escaping and the result may occupy
> > %4x memory (octal escaping of non-printable characters). Of course, you may vary
> > the escaping classes, but IIRC the octal or hex escaping is unconditional.
> 
> %.4s is used for unsigned int iirc, here it's __le32.

No, it's used to 'char *'. in case one may guarantee that it at least is
four characters long.

> >>>> [1]: https://lore.kernel.org/asahi/E753B391-D2CB-4213-AF82-678ADD5A7644@cutebit.org/
> >>>> 
> >>>> Alternatively we could add a host endian only. Other endians are not really
> >>>> used by any driver AFAIK. The host endian is being used by appletbdrm and
> >>>> Asahi Linux’ SMC driver only.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> On 24 Feb 2025, at 4:11 PM, andriy.shevchenko@linux.intel.com wrote:
> 
> On Mon, Feb 24, 2025 at 10:32:27AM +0000, Aditya Garg wrote:
>>>> On 24 Feb 2025, at 3:54 PM, andriy.shevchenko@linux.intel.com wrote:
>>> On Mon, Feb 24, 2025 at 10:18:48AM +0000, Aditya Garg wrote:
>>>>>> On 24 Feb 2025, at 3:28 PM, andriy.shevchenko@linux.intel.com wrote:
>>>>> On Sat, Feb 22, 2025 at 03:46:03PM +0000, Aditya Garg wrote:
>>>>>>>> On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> ...
> 
>>>>>>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
>>>>>>> it's useful to be able to print generic 4-character codes formatted as
>>>>>>> an integer. Extend it to add format specifiers for printing generic
>>>>>>> 32-bit FOURCCs with various endian semantics:
>>>>>>> 
>>>>>>> %p4ch   Host-endian
>>>>>>> %p4cl Little-endian
>>>>>>> %p4cb Big-endian
>>>>>>> %p4cr Reverse-endian
>>>>>>> 
>>>>>>> The endianness determines how bytes are interpreted as a u32, and the
>>>>>>> FOURCC is then always printed MSByte-first (this is the opposite of
>>>>>>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
>>>>>>> allow printing LSByte-first FOURCCs stored in host endian order
>>>>>>> (other than the hex form being in character order, not the integer
>>>>>>> value).
>>>>> 
>>>>> ...
>>>>> 
>>>>>> BTW, after looking at the comments by Martin [1], its actually better to use
>>>>>> existing specifiers for the appletbdrm driver.  The driver needs the host
>>>>>> endian as proposed by this patch, so instead of that, we can use %.4s
>>>>> 
>>>>> Do you mean this patch will not be needed? If this a case, that would be the
>>>>> best solution.
>>>> 
>>>> I tested with %4pE, and the results are different from expected. So this
>>>> would be preferred. Kindly see my latest email with a proposed workaround for
>>>> the sparse warnings.
>>> 
>>> %.4s sounded okay, but %4pE is always about escaping and the result may occupy
>>> %4x memory (octal escaping of non-printable characters). Of course, you may vary
>>> the escaping classes, but IIRC the octal or hex escaping is unconditional.
>> 
>> %.4s is used for unsigned int iirc, here it's __le32.
> 
> No, it's used to 'char *'. in case one may guarantee that it at least is
> four characters long.

Still the argument here is __le32. %p4h is showing reverse values than what %4pE as well as %.4s shows.
> 
>>>>>> [1]: https://lore.kernel.org/asahi/E753B391-D2CB-4213-AF82-678ADD5A7644@cutebit.org/
>>>>>> 
>>>>>> Alternatively we could add a host endian only. Other endians are not really
>>>>>> used by any driver AFAIK. The host endian is being used by appletbdrm and
>>>>>> Asahi Linux’ SMC driver only.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[andriy.shevchenko]

On Mon, Feb 24, 2025 at 10:43:54AM +0000, Aditya Garg wrote:
> > On 24 Feb 2025, at 4:11 PM, andriy.shevchenko@linux.intel.com wrote:
> > On Mon, Feb 24, 2025 at 10:32:27AM +0000, Aditya Garg wrote:
> >>>> On 24 Feb 2025, at 3:54 PM, andriy.shevchenko@linux.intel.com wrote:
> >>> On Mon, Feb 24, 2025 at 10:18:48AM +0000, Aditya Garg wrote:
> >>>>>> On 24 Feb 2025, at 3:28 PM, andriy.shevchenko@linux.intel.com wrote:
> >>>>> On Sat, Feb 22, 2025 at 03:46:03PM +0000, Aditya Garg wrote:
> >>>>>>>> On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:

...

> >>>>>>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
> >>>>>>> it's useful to be able to print generic 4-character codes formatted as
> >>>>>>> an integer. Extend it to add format specifiers for printing generic
> >>>>>>> 32-bit FOURCCs with various endian semantics:
> >>>>>>> 
> >>>>>>> %p4ch   Host-endian
> >>>>>>> %p4cl Little-endian
> >>>>>>> %p4cb Big-endian
> >>>>>>> %p4cr Reverse-endian
> >>>>>>> 
> >>>>>>> The endianness determines how bytes are interpreted as a u32, and the
> >>>>>>> FOURCC is then always printed MSByte-first (this is the opposite of
> >>>>>>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
> >>>>>>> allow printing LSByte-first FOURCCs stored in host endian order
> >>>>>>> (other than the hex form being in character order, not the integer
> >>>>>>> value).
> >>>>> 
> >>>>> ...
> >>>>> 
> >>>>>> BTW, after looking at the comments by Martin [1], its actually better to use
> >>>>>> existing specifiers for the appletbdrm driver.  The driver needs the host
> >>>>>> endian as proposed by this patch, so instead of that, we can use %.4s
> >>>>> 
> >>>>> Do you mean this patch will not be needed? If this a case, that would be the
> >>>>> best solution.
> >>>> 
> >>>> I tested with %4pE, and the results are different from expected. So this
> >>>> would be preferred. Kindly see my latest email with a proposed workaround for
> >>>> the sparse warnings.
> >>> 
> >>> %.4s sounded okay, but %4pE is always about escaping and the result may occupy
> >>> %4x memory (octal escaping of non-printable characters). Of course, you may vary
> >>> the escaping classes, but IIRC the octal or hex escaping is unconditional.
> >> 
> >> %.4s is used for unsigned int iirc, here it's __le32.
> > 
> > No, it's used to 'char *'. in case one may guarantee that it at least is
> > four characters long.

> Still the argument here is __le32. %p4h is showing reverse values than what
> %4pE as well as %.4s shows.

For __le32 the %.4s will print from LSB to MSB and otherwise for __be32.
You can provide any conversion you want to have it stable between
the endianesses. In any case looking at the DRM patch it might be that
the entire driver got the endianess wrong. Have you checked my comment
there?

> >>>>>> [1]: https://lore.kernel.org/asahi/E753B391-D2CB-4213-AF82-678ADD5A7644@cutebit.org/
> >>>>>> 
> >>>>>> Alternatively we could add a host endian only. Other endians are not really
> >>>>>> used by any driver AFAIK. The host endian is being used by appletbdrm and
> >>>>>> Asahi Linux’ SMC driver only.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> On 24 Feb 2025, at 4:19 PM, andriy.shevchenko@linux.intel.com wrote:
> 
> On Mon, Feb 24, 2025 at 10:43:54AM +0000, Aditya Garg wrote:
>>>> On 24 Feb 2025, at 4:11 PM, andriy.shevchenko@linux.intel.com wrote:
>>> On Mon, Feb 24, 2025 at 10:32:27AM +0000, Aditya Garg wrote:
>>>>>> On 24 Feb 2025, at 3:54 PM, andriy.shevchenko@linux.intel.com wrote:
>>>>> On Mon, Feb 24, 2025 at 10:18:48AM +0000, Aditya Garg wrote:
>>>>>>>> On 24 Feb 2025, at 3:28 PM, andriy.shevchenko@linux.intel.com wrote:
>>>>>>> On Sat, Feb 22, 2025 at 03:46:03PM +0000, Aditya Garg wrote:
>>>>>>>>>> On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> ...
> 
>>>>>>>>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
>>>>>>>>> it's useful to be able to print generic 4-character codes formatted as
>>>>>>>>> an integer. Extend it to add format specifiers for printing generic
>>>>>>>>> 32-bit FOURCCs with various endian semantics:
>>>>>>>>> 
>>>>>>>>> %p4ch   Host-endian
>>>>>>>>> %p4cl Little-endian
>>>>>>>>> %p4cb Big-endian
>>>>>>>>> %p4cr Reverse-endian
>>>>>>>>> 
>>>>>>>>> The endianness determines how bytes are interpreted as a u32, and the
>>>>>>>>> FOURCC is then always printed MSByte-first (this is the opposite of
>>>>>>>>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
>>>>>>>>> allow printing LSByte-first FOURCCs stored in host endian order
>>>>>>>>> (other than the hex form being in character order, not the integer
>>>>>>>>> value).
>>>>>>> 
>>>>>>> ...
>>>>>>> 
>>>>>>>> BTW, after looking at the comments by Martin [1], its actually better to use
>>>>>>>> existing specifiers for the appletbdrm driver.  The driver needs the host
>>>>>>>> endian as proposed by this patch, so instead of that, we can use %.4s
>>>>>>> 
>>>>>>> Do you mean this patch will not be needed? If this a case, that would be the
>>>>>>> best solution.
>>>>>> 
>>>>>> I tested with %4pE, and the results are different from expected. So this
>>>>>> would be preferred. Kindly see my latest email with a proposed workaround for
>>>>>> the sparse warnings.
>>>>> 
>>>>> %.4s sounded okay, but %4pE is always about escaping and the result may occupy
>>>>> %4x memory (octal escaping of non-printable characters). Of course, you may vary
>>>>> the escaping classes, but IIRC the octal or hex escaping is unconditional.
>>>> 
>>>> %.4s is used for unsigned int iirc, here it's __le32.
>>> 
>>> No, it's used to 'char *'. in case one may guarantee that it at least is
>>> four characters long.
> 
>> Still the argument here is __le32. %p4h is showing reverse values than what
>> %4pE as well as %.4s shows.
> 
> For __le32 the %.4s will print from LSB to MSB and otherwise for __be32.
> You can provide any conversion you want to have it stable between
> the endianesses. In any case looking at the DRM patch it might be that
> the entire driver got the endianess wrong. Have you checked my comment
> there?

drm drivers have a %p4cc already upstream. I think we need that then.
> 
>>>>>>>> [1]: https://lore.kernel.org/asahi/E753B391-D2CB-4213-AF82-678ADD5A7644@cutebit.org/
>>>>>>>> 
>>>>>>>> Alternatively we could add a host endian only. Other endians are not really
>>>>>>>> used by any driver AFAIK. The host endian is being used by appletbdrm and
>>>>>>>> Asahi Linux’ SMC driver only.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

I request the printk maintainers for their views on whether if they are ok with the sparse errors in this original patch.

> On 20 Feb 2025, at 10:09 PM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> From: Hector Martin <marcan@marcan.st>
> 
> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
> it's useful to be able to print generic 4-character codes formatted as
> an integer. Extend it to add format specifiers for printing generic
> 32-bit FOURCCs with various endian semantics:
> 
> %p4ch   Host-endian
> %p4cl    Little-endian
> %p4cb    Big-endian
> %p4cr    Reverse-endian
> 
> The endianness determines how bytes are interpreted as a u32, and the
> FOURCC is then always printed MSByte-first (this is the opposite of
> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
> allow printing LSByte-first FOURCCs stored in host endian order
> (other than the hex form being in character order, not the integer
> value).
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---
> v2 -> Add this patch
> Documentation/core-api/printk-formats.rst | 32 +++++++++++++++++++
> lib/test_printf.c                         | 39 +++++++++++++++++++----
> lib/vsprintf.c                            | 38 ++++++++++++++++++----
> scripts/checkpatch.pl                     |  2 +-
> 4 files changed, 97 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index ecccc0473..9982861fa 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -648,6 +648,38 @@ Examples::
>    %p4cc    Y10  little-endian (0x20303159)
>    %p4cc    NV12 big-endian (0xb231564e)
> 
> +Generic FourCC code
> +-------------------
> +
> +::
> +    %p4c[hrbl]    gP00 (0x67503030)
> +
> +Print a generic FourCC code, as both ASCII characters and its numerical
> +value as hexadecimal.
> +
> +The additional ``h``, ``r``, ``b``, and ``l`` specifiers are used to specify
> +host, reversed, big or little endian order data respectively. Host endian
> +order means the data is interpreted as a 32-bit integer and the most
> +significant byte is printed first; that is, the character code as printed
> +matches the byte order stored in memory on big-endian systems, and is reversed
> +on little-endian systems.
> +
> +Passed by reference.
> +
> +Examples for a little-endian machine, given &(u32)0x67503030::
> +
> +    %p4ch    gP00 (0x67503030)
> +    %p4cr    00Pg (0x30305067)
> +    %p4cb    00Pg (0x30305067)
> +    %p4cl    gP00 (0x67503030)
> +
> +Examples for a big-endian machine, given &(u32)0x67503030::
> +
> +    %p4ch    gP00 (0x67503030)
> +    %p4cr    00Pg (0x30305067)
> +    %p4cb    gP00 (0x67503030)
> +    %p4cl    00Pg (0x30305067)
> +
> Rust
> ----
> 
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 59dbe4f9a..ee860327e 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -776,21 +776,46 @@ static void __init fwnode_pointer(void)
>    software_node_unregister_node_group(group);
> }
> 
> +struct fourcc_struct {
> +    u32 code;
> +    const char *str;
> +};
> +
> +static void __init fourcc_pointer_test(const struct fourcc_struct *fc, size_t n,
> +                       const char *fmt)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < n; i++)
> +        test(fc[i].str, fmt, &fc[i].code);
> +}
> +
> static void __init fourcc_pointer(void)
> {
> -    struct {
> -        u32 code;
> -        char *str;
> -    } const try[] = {
> +    struct fourcc_struct const try_cc[] = {
>        { 0x3231564e, "NV12 little-endian (0x3231564e)", },
>        { 0xb231564e, "NV12 big-endian (0xb231564e)", },
>        { 0x10111213, ".... little-endian (0x10111213)", },
>        { 0x20303159, "Y10  little-endian (0x20303159)", },
>    };
> -    unsigned int i;
> +    struct fourcc_struct const try_ch = {
> +        0x41424344, "ABCD (0x41424344)",
> +    };
> +    struct fourcc_struct const try_cr = {
> +        0x41424344, "DCBA (0x44434241)",
> +    };
> +    struct fourcc_struct const try_cl = {
> +        le32_to_cpu(0x41424344), "ABCD (0x41424344)",
> +    };
> +    struct fourcc_struct const try_cb = {
> +        be32_to_cpu(0x41424344), "ABCD (0x41424344)",
> +    };
> 
> -    for (i = 0; i < ARRAY_SIZE(try); i++)
> -        test(try[i].str, "%p4cc", &try[i].code);
> +    fourcc_pointer_test(try_cc, ARRAY_SIZE(try_cc), "%p4cc");
> +    fourcc_pointer_test(&try_ch, 1, "%p4ch");
> +    fourcc_pointer_test(&try_cr, 1, "%p4cr");
> +    fourcc_pointer_test(&try_cl, 1, "%p4cl");
> +    fourcc_pointer_test(&try_cb, 1, "%p4cb");
> }
> 
> static void __init
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 56fe96319..13733a4da 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1781,27 +1781,53 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc,
>    char output[sizeof("0123 little-endian (0x01234567)")];
>    char *p = output;
>    unsigned int i;
> +    bool pixel_fmt = false;
>    u32 orig, val;
> 
> -    if (fmt[1] != 'c' || fmt[2] != 'c')
> +    if (fmt[1] != 'c')
>        return error_string(buf, end, "(%p4?)", spec);
> 
>    if (check_pointer(&buf, end, fourcc, spec))
>        return buf;
> 
>    orig = get_unaligned(fourcc);
> -    val = orig & ~BIT(31);
> +    switch (fmt[2]) {
> +    case 'h':
> +        val = orig;
> +        break;
> +    case 'r':
> +        orig = swab32(orig);
> +        val = orig;
> +        break;
> +    case 'l':
> +        orig = le32_to_cpu(orig);
> +        val = orig;
> +        break;
> +    case 'b':
> +        orig = be32_to_cpu(orig);
> +        val = orig;
> +        break;
> +    case 'c':
> +        /* Pixel formats are printed LSB-first */
> +        val = swab32(orig & ~BIT(31));
> +        pixel_fmt = true;
> +        break;
> +    default:
> +        return error_string(buf, end, "(%p4?)", spec);
> +    }
> 
>    for (i = 0; i < sizeof(u32); i++) {
> -        unsigned char c = val >> (i * 8);
> +        unsigned char c = val >> ((3 - i) * 8);
> 
>        /* Print non-control ASCII characters as-is, dot otherwise */
>        *p++ = isascii(c) && isprint(c) ? c : '.';
>    }
> 
> -    *p++ = ' ';
> -    strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> -    p += strlen(p);
> +    if (pixel_fmt) {
> +        *p++ = ' ';
> +        strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> +        p += strlen(p);
> +    }
> 
>    *p++ = ' ';
>    *p++ = '(';
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7b28ad331..21516f753 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6904,7 +6904,7 @@ sub process {
>                        ($extension eq "f" &&
>                         defined $qualifier && $qualifier !~ /^w/) ||
>                        ($extension eq "4" &&
> -                         defined $qualifier && $qualifier !~ /^cc/)) {
> +                         defined $qualifier && $qualifier !~ /^c[chlbr]/)) {
>                        $bad_specifier = $specifier;
>                        last;
>                    }
> --
> 2.43.0
> 

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> On 24 Feb 2025, at 9:47 PM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> I request the printk maintainers for their views on whether if they are ok with the sparse errors in this original patch.

FWIW, I read a bit about FOURCC and also investigated the cpu_to_le32 and similar macros. I think the v4 I sent should work well, without the sparse warnings. I've also made the v4 separate from the DRM patch set, so as to avoid multiple tree complications and hindering the DRM driver upstream process. For now %p4cc was the best format helper I could find upstream, but I would prefer using %p4cl (little endian) instead for appletbdrm. And this patch imo is needed simply because we need better format helpers, rather than using workarounds to swap bits and using other format helpers.

[PATCH v2 3/3] drm/tiny: add driver for Apple Touch Bars in x86 Macs

[Aditya Garg]

From: Kerem Karabay <kekrby@gmail.com>

The Touch Bars found on x86 Macs support two USB configurations: one
where the device presents itself as a HID keyboard and can display
predefined sets of keys, and one where the operating system has full
control over what is displayed.

This commit adds support for the display functionality of the second
configuration. Functionality for the first configuration has been
merged in the HID tree.

Note that this driver has only been tested on T2 Macs, and only includes
the USB device ID for these devices. Testing on T1 Macs would be
appreciated.

Credit goes to @imbushuo on GitHub for reverse engineering most of the
protocol.

Signed-off-by: Kerem Karabay <kekrby@gmail.com>
Co-developed-by: Atharva Tiwari <evepolonium@gmail.com>
Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
Co-developed-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Aun-Ali Zaidi <admin@kodeit.net>
---
v2 ->
- Add the driver to MAINTAINERS.
- Allocate memory for request and response in plane's atomic-check helper
- Void the use of drm_fb_blit()
- Implement atomic_disable
- Make PRIME work
- Remove the date field from struct drm_driver 
- intersect damage with dst_clip
- Register DRM device in appletbdrm_probe
- Clear the display as the final call in probe
 MAINTAINERS                       |   7 +
 drivers/gpu/drm/tiny/Kconfig      |  14 +
 drivers/gpu/drm/tiny/Makefile     |   1 +
 drivers/gpu/drm/tiny/appletbdrm.c | 806 ++++++++++++++++++++++++++++++
 4 files changed, 828 insertions(+)
 create mode 100644 drivers/gpu/drm/tiny/appletbdrm.c

diff --git a/MAINTAINERS b/MAINTAINERS
index efee40ea5..43fafaab3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7148,6 +7148,13 @@ S:	Supported
 T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
 F:	drivers/gpu/drm/sun4i/sun8i*
 
+DRM DRIVER FOR APPLE TOUCH BARS
+M:	Aun-Ali Zaidi <admin@kodeit.net>
+L:	dri-devel@lists.freedesktop.org
+S:	Maintained
+T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
+F:	drivers/gpu/drm/tiny/appletbdrm.c
+
 DRM DRIVER FOR ARM PL111 CLCD
 M:	Linus Walleij <linus.walleij@linaro.org>
 S:	Maintained
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 94cbdb133..25471791c 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -1,5 +1,19 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
+config DRM_APPLETBDRM
+	tristate "DRM support for Apple Touch Bars"
+	depends on DRM && USB && MMU
+	select DRM_GEM_SHMEM_HELPER
+	select DRM_KMS_HELPER
+	select HID_APPLETB_BL
+	select HID_MULTITOUCH
+	help
+	  Say Y here if you want support for the display of Touch Bars on x86
+	  MacBook Pros.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called appletbdrm.
+
 config DRM_ARCPGU
 	tristate "ARC PGU"
 	depends on DRM && OF
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 60816d2eb..0a3a7837a 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
+obj-$(CONFIG_DRM_APPLETBDRM)		+= appletbdrm.o
 obj-$(CONFIG_DRM_ARCPGU)		+= arcpgu.o
 obj-$(CONFIG_DRM_BOCHS)			+= bochs.o
 obj-$(CONFIG_DRM_CIRRUS_QEMU)		+= cirrus-qemu.o
diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
new file mode 100644
index 000000000..a17a3ecc3
--- /dev/null
+++ b/drivers/gpu/drm/tiny/appletbdrm.c
@@ -0,0 +1,806 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Apple Touch Bar DRM Driver
+ *
+ * Copyright (c) 2023 Kerem Karabay <kekrby@gmail.com>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/unaligned.h>
+#include <linux/usb.h>
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_format_helper.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_plane.h>
+#include <drm/drm_probe_helper.h>
+
+#define __APPLETBDRM_MSG_STR4(str4)	((__le32 __force)((str4[0] << 24) | (str4[1] << 16) | (str4[2] << 8) | str4[3]))
+#define __APPLETBDRM_MSG_TOK4(tok4)	__APPLETBDRM_MSG_STR4(#tok4)
+
+#define APPLETBDRM_PIXEL_FORMAT		__APPLETBDRM_MSG_TOK4(RGBA) /* The actual format is BGR888 */
+#define APPLETBDRM_BITS_PER_PIXEL	24
+
+#define APPLETBDRM_MSG_CLEAR_DISPLAY	__APPLETBDRM_MSG_TOK4(CLRD)
+#define APPLETBDRM_MSG_GET_INFORMATION	__APPLETBDRM_MSG_TOK4(GINF)
+#define APPLETBDRM_MSG_UPDATE_COMPLETE	__APPLETBDRM_MSG_TOK4(UDCL)
+#define APPLETBDRM_MSG_SIGNAL_READINESS	__APPLETBDRM_MSG_TOK4(REDY)
+
+#define APPLETBDRM_BULK_MSG_TIMEOUT	1000
+
+#define drm_to_adev(_drm)		container_of(_drm, struct appletbdrm_device, drm)
+#define adev_to_udev(adev)		interface_to_usbdev(to_usb_interface(adev->dev))
+
+struct appletbdrm_msg_request_header {
+	__le16 unk_00;
+	__le16 unk_02;
+	__le32 unk_04;
+	__le32 unk_08;
+	__le32 size;
+} __packed;
+
+struct appletbdrm_msg_response_header {
+	u8 unk_00[16];
+	__le32 msg;
+} __packed;
+
+struct appletbdrm_msg_simple_request {
+	struct appletbdrm_msg_request_header header;
+	__le32 msg;
+	u8 unk_14[8];
+	__le32 size;
+} __packed;
+
+struct appletbdrm_msg_information {
+	struct appletbdrm_msg_response_header header;
+	u8 unk_14[12];
+	__le32 width;
+	__le32 height;
+	u8 bits_per_pixel;
+	__le32 bytes_per_row;
+	__le32 orientation;
+	__le32 bitmap_info;
+	__le32 pixel_format;
+	__le32 width_inches;	/* floating point */
+	__le32 height_inches;	/* floating point */
+} __packed;
+
+struct appletbdrm_frame {
+	__le16 begin_x;
+	__le16 begin_y;
+	__le16 width;
+	__le16 height;
+	__le32 buf_size;
+	u8 buf[];
+} __packed;
+
+struct appletbdrm_fb_request_footer {
+	u8 unk_00[12];
+	__le32 unk_0c;
+	u8 unk_10[12];
+	__le32 unk_1c;
+	__le64 timestamp;
+	u8 unk_28[12];
+	__le32 unk_34;
+	u8 unk_38[20];
+	__le32 unk_4c;
+} __packed;
+
+struct appletbdrm_fb_request {
+	struct appletbdrm_msg_request_header header;
+	__le16 unk_10;
+	u8 msg_id;
+	u8 unk_13[29];
+	/*
+	 * Contents of `data`:
+	 * - struct appletbdrm_frame frames[];
+	 * - struct appletbdrm_fb_request_footer footer;
+	 * - padding to make the total size a multiple of 16
+	 */
+	u8 data[];
+} __packed;
+
+struct appletbdrm_fb_request_response {
+	struct appletbdrm_msg_response_header header;
+	u8 unk_14[12];
+	__le64 timestamp;
+} __packed;
+
+struct appletbdrm_device {
+	struct device *dev;
+	struct device *dmadev;
+
+	unsigned int in_ep;
+	unsigned int out_ep;
+
+	unsigned int width;
+	unsigned int height;
+
+	struct drm_device drm;
+	struct drm_display_mode mode;
+	struct drm_connector connector;
+	struct drm_plane primary_plane;
+	struct drm_crtc crtc;
+	struct drm_encoder encoder;
+};
+
+struct appletbdrm_plane_state {
+	struct drm_shadow_plane_state base;
+	struct appletbdrm_fb_request *request;
+	struct appletbdrm_fb_request_response *response;
+	size_t request_size;
+	size_t frames_size;
+};
+
+static inline struct appletbdrm_plane_state *to_appletbdrm_plane_state(struct drm_plane_state *state)
+{
+	return container_of(state, struct appletbdrm_plane_state, base.base);
+}
+
+static int appletbdrm_send_request(struct appletbdrm_device *adev,
+				   struct appletbdrm_msg_request_header *request, size_t size)
+{
+	struct usb_device *udev = adev_to_udev(adev);
+	struct drm_device *drm = &adev->drm;
+	int ret, actual_size;
+
+	ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, adev->out_ep),
+			   request, size, &actual_size, APPLETBDRM_BULK_MSG_TIMEOUT);
+	if (ret) {
+		drm_err(drm, "Failed to send message (%d)\n", ret);
+		return ret;
+	}
+
+	if (actual_size != size) {
+		drm_err(drm, "Actual size (%d) doesn't match expected size (%lu)\n",
+			actual_size, size);
+		return -EIO;
+	}
+
+	return ret;
+}
+
+static int appletbdrm_read_response(struct appletbdrm_device *adev,
+				    struct appletbdrm_msg_response_header *response,
+				    size_t size, u32 expected_response)
+{
+	struct usb_device *udev = adev_to_udev(adev);
+	struct drm_device *drm = &adev->drm;
+	int ret, actual_size;
+	bool readiness_signal_received = false;
+
+retry:
+	ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, adev->in_ep),
+			   response, size, &actual_size, APPLETBDRM_BULK_MSG_TIMEOUT);
+	if (ret) {
+		drm_err(drm, "Failed to read response (%d)\n", ret);
+		return ret;
+	}
+
+	/*
+	 * The device responds to the first request sent in a particular
+	 * timeframe after the USB device configuration is set with a readiness
+	 * signal, in which case the response should be read again
+	 */
+	if (response->msg == APPLETBDRM_MSG_SIGNAL_READINESS) {
+		if (!readiness_signal_received) {
+			readiness_signal_received = true;
+			goto retry;
+		}
+
+		drm_err(drm, "Encountered unexpected readiness signal\n");
+		return -EIO;
+	}
+
+	if (actual_size != size) {
+		drm_err(drm, "Actual size (%d) doesn't match expected size (%lu)\n",
+			actual_size, size);
+		return -EIO;
+	}
+
+	if (response->msg != expected_response) {
+		drm_err(drm, "Unexpected response from device (expected %p4ch found %p4ch)\n",
+			&expected_response, &response->msg);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int appletbdrm_send_msg(struct appletbdrm_device *adev, u32 msg)
+{
+	struct appletbdrm_msg_simple_request *request;
+	int ret;
+
+	request = kzalloc(sizeof(*request), GFP_KERNEL);
+	if (!request)
+		return -ENOMEM;
+
+	request->header.unk_00 = cpu_to_le16(2);
+	request->header.unk_02 = cpu_to_le16(0x1512);
+	request->header.size = cpu_to_le32(sizeof(*request) - sizeof(request->header));
+	request->msg = msg;
+	request->size = request->header.size;
+
+	ret = appletbdrm_send_request(adev, &request->header, sizeof(*request));
+
+	kfree(request);
+
+	return ret;
+}
+
+static int appletbdrm_clear_display(struct appletbdrm_device *adev)
+{
+	return appletbdrm_send_msg(adev, APPLETBDRM_MSG_CLEAR_DISPLAY);
+}
+
+static int appletbdrm_signal_readiness(struct appletbdrm_device *adev)
+{
+	return appletbdrm_send_msg(adev, APPLETBDRM_MSG_SIGNAL_READINESS);
+}
+
+static int appletbdrm_get_information(struct appletbdrm_device *adev)
+{
+	struct appletbdrm_msg_information *info;
+	struct drm_device *drm = &adev->drm;
+	u8 bits_per_pixel;
+	u32 pixel_format;
+	int ret;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	ret = appletbdrm_send_msg(adev, APPLETBDRM_MSG_GET_INFORMATION);
+	if (ret)
+		return ret;
+
+	ret = appletbdrm_read_response(adev, &info->header, sizeof(*info),
+				       APPLETBDRM_MSG_GET_INFORMATION);
+	if (ret)
+		goto free_info;
+
+	bits_per_pixel = info->bits_per_pixel;
+	pixel_format = get_unaligned(&info->pixel_format);
+
+	adev->width = get_unaligned_le32(&info->width);
+	adev->height = get_unaligned_le32(&info->height);
+
+	if (bits_per_pixel != APPLETBDRM_BITS_PER_PIXEL) {
+		drm_err(drm, "Encountered unexpected bits per pixel value (%d)\n", bits_per_pixel);
+		ret = -EINVAL;
+		goto free_info;
+	}
+
+	if (pixel_format != APPLETBDRM_PIXEL_FORMAT) {
+		drm_err(drm, "Encountered unknown pixel format (%p4ch)\n", &pixel_format);
+		ret = -EINVAL;
+		goto free_info;
+	}
+
+free_info:
+	kfree(info);
+
+	return ret;
+}
+
+static u32 rect_size(struct drm_rect *rect)
+{
+	return drm_rect_width(rect) * drm_rect_height(rect) * (APPLETBDRM_BITS_PER_PIXEL / 8);
+}
+
+static int appletbdrm_connector_helper_get_modes(struct drm_connector *connector)
+{
+	struct appletbdrm_device *adev = drm_to_adev(connector->dev);
+
+	return drm_connector_helper_get_modes_fixed(connector, &adev->mode);
+}
+
+static const u32 appletbdrm_primary_plane_formats[] = {
+	DRM_FORMAT_BGR888,
+	DRM_FORMAT_XRGB8888, /* emulated */
+};
+
+static int appletbdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
+						   struct drm_atomic_state *state)
+{
+	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+	struct drm_crtc *new_crtc = new_plane_state->crtc;
+	struct drm_crtc_state *new_crtc_state = NULL;
+	struct appletbdrm_plane_state *appletbdrm_state = to_appletbdrm_plane_state(new_plane_state);
+	struct drm_atomic_helper_damage_iter iter;
+	struct drm_rect damage;
+	size_t frames_size = 0;
+	size_t request_size;
+	int ret;
+
+	if (new_crtc)
+		new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
+
+	ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
+						  DRM_PLANE_NO_SCALING,
+						  DRM_PLANE_NO_SCALING,
+						  false, false);
+	if (ret)
+		return ret;
+	else if (!new_plane_state->visible)
+		return 0;
+
+	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, new_plane_state);
+	drm_atomic_for_each_plane_damage(&iter, &damage) {
+		frames_size += struct_size((struct appletbdrm_frame *)0, buf, rect_size(&damage));
+	}
+
+	if (!frames_size)
+		return 0;
+
+	request_size = ALIGN(sizeof(struct appletbdrm_fb_request) +
+		       frames_size +
+		       sizeof(struct appletbdrm_fb_request_footer), 16);
+
+	appletbdrm_state->request = kzalloc(request_size, GFP_KERNEL);
+
+	if (!appletbdrm_state->request)
+		return -ENOMEM;
+
+	appletbdrm_state->response = kzalloc(sizeof(*appletbdrm_state->response), GFP_KERNEL);
+
+	if (!appletbdrm_state->response)
+		return -ENOMEM;
+
+	appletbdrm_state->request_size = request_size;
+	appletbdrm_state->frames_size = frames_size;
+
+	return 0;
+}
+
+static int appletbdrm_flush_damage(struct appletbdrm_device *adev,
+				   struct drm_plane_state *old_state,
+				   struct drm_plane_state *state)
+{
+	struct appletbdrm_plane_state *appletbdrm_state = to_appletbdrm_plane_state(state);
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
+	struct appletbdrm_fb_request_response *response = appletbdrm_state->response;
+	struct appletbdrm_fb_request_footer *footer;
+	struct drm_atomic_helper_damage_iter iter;
+	struct drm_framebuffer *fb = state->fb;
+	struct appletbdrm_fb_request *request = appletbdrm_state->request;
+	struct drm_device *drm = &adev->drm;
+	struct appletbdrm_frame *frame;
+	u64 timestamp = ktime_get_ns();
+	struct drm_rect damage;
+	size_t frames_size = appletbdrm_state->frames_size;
+	size_t request_size = appletbdrm_state->request_size;
+	int ret;
+
+	if (!frames_size)
+		return 0;
+
+	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+	if (ret) {
+		drm_err(drm, "Failed to start CPU framebuffer access (%d)\n", ret);
+		goto end_fb_cpu_access;
+	}
+
+	request->header.unk_00 = cpu_to_le16(2);
+	request->header.unk_02 = cpu_to_le16(0x12);
+	request->header.unk_04 = cpu_to_le32(9);
+	request->header.size = cpu_to_le32(request_size - sizeof(request->header));
+	request->unk_10 = cpu_to_le16(1);
+	request->msg_id = timestamp & 0xff;
+
+	frame = (struct appletbdrm_frame *)request->data;
+
+	drm_atomic_helper_damage_iter_init(&iter, old_state, state);
+	drm_atomic_for_each_plane_damage(&iter, &damage) {
+		struct drm_rect dst_clip = state->dst;
+		struct iosys_map dst = IOSYS_MAP_INIT_VADDR(frame->buf);
+		u32 buf_size = rect_size(&damage);
+
+		if (!drm_rect_intersect(&dst_clip, &damage))
+			continue;
+
+		/*
+		 * The coordinates need to be translated to the coordinate
+		 * system the device expects, see the comment in
+		 * appletbdrm_setup_mode_config
+		 */
+		frame->begin_x = cpu_to_le16(damage.y1);
+		frame->begin_y = cpu_to_le16(adev->height - damage.x2);
+		frame->width = cpu_to_le16(drm_rect_height(&damage));
+		frame->height = cpu_to_le16(drm_rect_width(&damage));
+		frame->buf_size = cpu_to_le32(buf_size);
+
+		switch (fb->format->format) {
+		case DRM_FORMAT_XRGB8888:
+			drm_fb_xrgb8888_to_bgr888(&dst, NULL, &shadow_plane_state->data[0], fb, &damage, &shadow_plane_state->fmtcnv_state);
+			break;
+		default:
+			drm_fb_memcpy(&dst, NULL, &shadow_plane_state->data[0], fb, &damage);
+			break;
+		}
+
+		frame = (void *)frame + struct_size(frame, buf, buf_size);
+	}
+
+	footer = (struct appletbdrm_fb_request_footer *)&request->data[frames_size];
+
+	footer->unk_0c = cpu_to_le32(0xfffe);
+	footer->unk_1c = cpu_to_le32(0x80001);
+	footer->unk_34 = cpu_to_le32(0x80002);
+	footer->unk_4c = cpu_to_le32(0xffff);
+	footer->timestamp = cpu_to_le64(timestamp);
+
+	ret = appletbdrm_send_request(adev, &request->header, request_size);
+	if (ret)
+		goto end_fb_cpu_access;
+
+	ret = appletbdrm_read_response(adev, &response->header, sizeof(*response),
+				       APPLETBDRM_MSG_UPDATE_COMPLETE);
+	if (ret)
+		goto end_fb_cpu_access;
+
+	if (response->timestamp != footer->timestamp) {
+		drm_err(drm, "Response timestamp (%llu) doesn't match request timestamp (%llu)\n",
+			le64_to_cpu(response->timestamp), timestamp);
+		goto end_fb_cpu_access;
+	}
+
+end_fb_cpu_access:
+	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+
+	return ret;
+}
+
+static void appletbdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
+						     struct drm_atomic_state *old_state)
+{
+	struct appletbdrm_device *adev = drm_to_adev(plane->dev);
+	struct drm_device *drm = plane->dev;
+	struct drm_plane_state *plane_state = plane->state;
+	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(old_state, plane);
+	int idx;
+
+	if (!drm_dev_enter(drm, &idx))
+		return;
+
+	appletbdrm_flush_damage(adev, old_plane_state, plane_state);
+
+	drm_dev_exit(idx);
+}
+
+static void appletbdrm_primary_plane_helper_atomic_disable(struct drm_plane *plane,
+							   struct drm_atomic_state *state)
+{
+	struct drm_device *dev = plane->dev;
+	struct appletbdrm_device *adev = drm_to_adev(dev);
+	int idx;
+
+	if (!drm_dev_enter(dev, &idx))
+		return;
+
+	appletbdrm_clear_display(adev);
+
+	drm_dev_exit(idx);
+}
+
+static void appletbdrm_primary_plane_reset(struct drm_plane *plane)
+{
+	struct appletbdrm_plane_state *appletbdrm_state;
+
+	WARN_ON(plane->state);
+
+	appletbdrm_state = kzalloc(sizeof(*appletbdrm_state), GFP_KERNEL);
+	if (!appletbdrm_state)
+		return;
+
+	__drm_gem_reset_shadow_plane(plane, &appletbdrm_state->base);
+}
+
+static struct drm_plane_state *appletbdrm_primary_plane_duplicate_state(struct drm_plane *plane)
+{
+	struct drm_shadow_plane_state *new_shadow_plane_state;
+	struct appletbdrm_plane_state *old_appletbdrm_state;
+	struct appletbdrm_plane_state *appletbdrm_state;
+
+	if (WARN_ON(!plane->state))
+		return NULL;
+
+	old_appletbdrm_state = to_appletbdrm_plane_state(plane->state);
+	appletbdrm_state = kmemdup(old_appletbdrm_state, sizeof(*appletbdrm_state), GFP_KERNEL);
+	if (!appletbdrm_state)
+		return NULL;
+
+	/* Request and response are not duplicated and are allocated in .atomic_check */
+	appletbdrm_state->request = NULL;
+	appletbdrm_state->response = NULL;
+
+	new_shadow_plane_state = &appletbdrm_state->base;
+
+	__drm_gem_duplicate_shadow_plane_state(plane, new_shadow_plane_state);
+
+	return &new_shadow_plane_state->base;
+}
+
+static void appletbdrm_primary_plane_destroy_state(struct drm_plane *plane,
+						   struct drm_plane_state *state)
+{
+	struct appletbdrm_plane_state *appletbdrm_state = to_appletbdrm_plane_state(state);
+
+	kfree(appletbdrm_state->request);
+	kfree(appletbdrm_state->response);
+
+	__drm_gem_destroy_shadow_plane_state(&appletbdrm_state->base);
+
+	kfree(appletbdrm_state);
+}
+
+static const struct drm_plane_helper_funcs appletbdrm_primary_plane_helper_funcs = {
+	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+	.atomic_check = appletbdrm_primary_plane_helper_atomic_check,
+	.atomic_update = appletbdrm_primary_plane_helper_atomic_update,
+	.atomic_disable = appletbdrm_primary_plane_helper_atomic_disable,
+};
+
+static const struct drm_plane_funcs appletbdrm_primary_plane_funcs = {
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.reset = appletbdrm_primary_plane_reset,
+	.atomic_duplicate_state = appletbdrm_primary_plane_duplicate_state,
+	.atomic_destroy_state = appletbdrm_primary_plane_destroy_state,
+	.destroy = drm_plane_cleanup,
+};
+
+static enum drm_mode_status appletbdrm_crtc_helper_mode_valid(struct drm_crtc *crtc,
+							  const struct drm_display_mode *mode)
+{
+	struct appletbdrm_device *adev = drm_to_adev(crtc->dev);
+
+	return drm_crtc_helper_mode_valid_fixed(crtc, mode, &adev->mode);
+}
+
+static const struct drm_mode_config_funcs appletbdrm_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create_with_dirty,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+static const struct drm_connector_funcs appletbdrm_connector_funcs = {
+	.reset = drm_atomic_helper_connector_reset,
+	.destroy = drm_connector_cleanup,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+};
+
+static const struct drm_connector_helper_funcs appletbdrm_connector_helper_funcs = {
+	.get_modes = appletbdrm_connector_helper_get_modes,
+};
+
+static const struct drm_crtc_helper_funcs appletbdrm_crtc_helper_funcs = {
+	.mode_valid = appletbdrm_crtc_helper_mode_valid,
+};
+
+static const struct drm_crtc_funcs appletbdrm_crtc_funcs = {
+	.reset = drm_atomic_helper_crtc_reset,
+	.destroy = drm_crtc_cleanup,
+	.set_config = drm_atomic_helper_set_config,
+	.page_flip = drm_atomic_helper_page_flip,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+};
+
+static const struct drm_encoder_funcs appletbdrm_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static struct drm_gem_object *appletbdrm_driver_gem_prime_import(struct drm_device *dev,
+								 struct dma_buf *dma_buf)
+{
+	struct appletbdrm_device *adev = drm_to_adev(dev);
+
+	if (!adev->dmadev)
+		return ERR_PTR(-ENODEV);
+
+	return drm_gem_prime_import_dev(dev, dma_buf, adev->dmadev);
+}
+
+DEFINE_DRM_GEM_FOPS(appletbdrm_drm_fops);
+
+static const struct drm_driver appletbdrm_drm_driver = {
+	DRM_GEM_SHMEM_DRIVER_OPS,
+	.gem_prime_import	= appletbdrm_driver_gem_prime_import,
+	.name			= "appletbdrm",
+	.desc			= "Apple Touch Bar DRM Driver",
+	.major			= 1,
+	.minor			= 0,
+	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
+	.fops			= &appletbdrm_drm_fops,
+};
+
+static int appletbdrm_setup_mode_config(struct appletbdrm_device *adev)
+{
+	struct drm_connector *connector = &adev->connector;
+	struct drm_plane *primary_plane;
+	struct drm_crtc *crtc;
+	struct drm_encoder *encoder;
+	struct drm_device *drm = &adev->drm;
+	struct device *dev = adev->dev;
+	int ret;
+
+	ret = drmm_mode_config_init(drm);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize mode configuration\n");
+
+	primary_plane = &adev->primary_plane;
+	ret = drm_universal_plane_init(drm, primary_plane, 0,
+				       &appletbdrm_primary_plane_funcs,
+				       appletbdrm_primary_plane_formats,
+				       ARRAY_SIZE(appletbdrm_primary_plane_formats),
+				       NULL,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize universal plane object\n");
+	drm_plane_helper_add(primary_plane, &appletbdrm_primary_plane_helper_funcs);
+	drm_plane_enable_fb_damage_clips(primary_plane);
+
+	crtc = &adev->crtc;
+	ret = drm_crtc_init_with_planes(drm, crtc, primary_plane, NULL,
+					&appletbdrm_crtc_funcs, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize CRTC object\n");
+	drm_crtc_helper_add(crtc, &appletbdrm_crtc_helper_funcs);
+
+	encoder = &adev->encoder;
+	ret = drm_encoder_init(drm, encoder, &appletbdrm_encoder_funcs,
+			       DRM_MODE_ENCODER_DAC, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize encoder\n");
+	encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+	/*
+	 * The coordinate system used by the device is different from the
+	 * coordinate system of the framebuffer in that the x and y axes are
+	 * swapped, and that the y axis is inverted; so what the device reports
+	 * as the height is actually the width of the framebuffer and vice
+	 * versa
+	 */
+	drm->mode_config.min_width = 0;
+	drm->mode_config.min_height = 0;
+	drm->mode_config.max_width = max(adev->height, DRM_SHADOW_PLANE_MAX_WIDTH);
+	drm->mode_config.max_height = max(adev->width, DRM_SHADOW_PLANE_MAX_HEIGHT);
+	drm->mode_config.preferred_depth = APPLETBDRM_BITS_PER_PIXEL;
+	drm->mode_config.funcs = &appletbdrm_mode_config_funcs;
+
+	adev->mode = (struct drm_display_mode) {
+		DRM_MODE_INIT(60, adev->height, adev->width,
+			      DRM_MODE_RES_MM(adev->height, 218),
+			      DRM_MODE_RES_MM(adev->width, 218))
+	};
+
+	ret = drm_connector_init(drm, connector,
+				 &appletbdrm_connector_funcs, DRM_MODE_CONNECTOR_USB);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize connector\n");
+
+	drm_connector_helper_add(connector, &appletbdrm_connector_helper_funcs);
+
+	ret = drm_connector_set_panel_orientation(connector,
+						  DRM_MODE_PANEL_ORIENTATION_RIGHT_UP);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set panel orientation\n");
+
+	connector->display_info.non_desktop = true;
+	ret = drm_object_property_set_value(&connector->base,
+					    drm->mode_config.non_desktop_property, true);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set non-desktop property\n");
+
+	ret = drm_connector_attach_encoder(connector, encoder);
+
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize simple display pipe\n");
+
+	drm_mode_config_reset(drm);
+
+	return 0;
+}
+
+static int appletbdrm_probe(struct usb_interface *intf,
+			    const struct usb_device_id *id)
+{
+	struct usb_endpoint_descriptor *bulk_in, *bulk_out;
+	struct device *dev = &intf->dev;
+	struct appletbdrm_device *adev;
+	struct drm_device *drm;
+	int ret;
+
+	ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to find bulk endpoints\n");
+
+	adev = devm_drm_dev_alloc(dev, &appletbdrm_drm_driver, struct appletbdrm_device, drm);
+	if (IS_ERR(adev))
+		return PTR_ERR(adev);
+
+	adev->dev = dev;
+	adev->in_ep = bulk_in->bEndpointAddress;
+	adev->out_ep = bulk_out->bEndpointAddress;
+
+	drm = &adev->drm;
+
+	usb_set_intfdata(intf, adev);
+
+	ret = appletbdrm_get_information(adev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get display information\n");
+
+	ret = appletbdrm_signal_readiness(adev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to signal readiness\n");
+
+	ret = appletbdrm_setup_mode_config(adev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to setup mode config\n");
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register DRM device\n");
+
+	ret = appletbdrm_clear_display(adev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to clear display\n");
+
+	return 0;
+}
+
+static void appletbdrm_disconnect(struct usb_interface *intf)
+{
+	struct appletbdrm_device *adev = usb_get_intfdata(intf);
+	struct drm_device *drm = &adev->drm;
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+}
+
+static void appletbdrm_shutdown(struct usb_interface *intf)
+{
+	struct appletbdrm_device *adev = usb_get_intfdata(intf);
+
+	/*
+	 * The framebuffer needs to be cleared on shutdown since its content
+	 * persists across boots
+	 */
+	drm_atomic_helper_shutdown(&adev->drm);
+}
+
+static const struct usb_device_id appletbdrm_usb_id_table[] = {
+	{ USB_DEVICE_INTERFACE_CLASS(0x05ac, 0x8302, USB_CLASS_AUDIO_VIDEO) },
+	{}
+};
+MODULE_DEVICE_TABLE(usb, appletbdrm_usb_id_table);
+
+static struct usb_driver appletbdrm_usb_driver = {
+	.name		= "appletbdrm",
+	.probe		= appletbdrm_probe,
+	.disconnect	= appletbdrm_disconnect,
+	.shutdown	= appletbdrm_shutdown,
+	.id_table	= appletbdrm_usb_id_table,
+};
+module_usb_driver(appletbdrm_usb_driver);
+
+MODULE_AUTHOR("Kerem Karabay <kekrby@gmail.com>");
+MODULE_DESCRIPTION("Apple Touch Bar DRM Driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v2 3/3] drm/tiny: add driver for Apple Touch Bars in x86 Macs

[Neal Gompa]

On Thu, Feb 20, 2025 at 11:47 AM Aditya Garg <gargaditya08@live.com> wrote:
>
> From: Kerem Karabay <kekrby@gmail.com>
>
> The Touch Bars found on x86 Macs support two USB configurations: one
> where the device presents itself as a HID keyboard and can display
> predefined sets of keys, and one where the operating system has full
> control over what is displayed.
>
> This commit adds support for the display functionality of the second
> configuration. Functionality for the first configuration has been
> merged in the HID tree.
>
> Note that this driver has only been tested on T2 Macs, and only includes
> the USB device ID for these devices. Testing on T1 Macs would be
> appreciated.
>
> Credit goes to @imbushuo on GitHub for reverse engineering most of the
> protocol.
>

Can we credit them as "Ben (Bingxing) Wang" instead? That's the name
on their GitHub profile.


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH v2 3/3] drm/tiny: add driver for Apple Touch Bars in x86 Macs

[Aditya Garg]

Hi

> On 21 Feb 2025, at 12:04 AM, Neal Gompa <neal@gompa.dev> wrote:
> 
> On Thu, Feb 20, 2025 at 11:47 AM Aditya Garg <gargaditya08@live.com> wrote:
>> 
>> From: Kerem Karabay <kekrby@gmail.com>
>> 
>> The Touch Bars found on x86 Macs support two USB configurations: one
>> where the device presents itself as a HID keyboard and can display
>> predefined sets of keys, and one where the operating system has full
>> control over what is displayed.
>> 
>> This commit adds support for the display functionality of the second
>> configuration. Functionality for the first configuration has been
>> merged in the HID tree.
>> 
>> Note that this driver has only been tested on T2 Macs, and only includes
>> the USB device ID for these devices. Testing on T1 Macs would be
>> appreciated.
>> 
>> Credit goes to @imbushuo on GitHub for reverse engineering most of the
>> protocol.
>> 
> 
> Can we credit them as "Ben (Bingxing) Wang" instead? That's the name
> on their GitHub profile.

Sure I can change that. I’ll wait for a review on the driver itself as well, and incorporate this change along with other changes requested (if any).
> 
> 
> -- 
> 真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> On 22 Feb 2025, at 5:41 PM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> 
> 
>> On 21 Feb 2025, at 8:57 PM, andriy.shevchenko@linux.intel.com wrote:
>>> On Thu, Feb 20, 2025 at 04:39:23PM +0000, Aditya Garg wrote:
>>> From: Hector Martin <marcan@marcan.st>
>>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
>>> it's useful to be able to print generic 4-character codes formatted as
>>> an integer. Extend it to add format specifiers for printing generic
>>> 32-bit FOURCCs with various endian semantics:
>>> %p4ch   Host-endian
>>> %p4cl Little-endian
>>> %p4cb Big-endian
>>> %p4cr Reverse-endian
>>> The endianness determines how bytes are interpreted as a u32, and the
>>> FOURCC is then always printed MSByte-first (this is the opposite of
>>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
>>> allow printing LSByte-first FOURCCs stored in host endian order
>>> (other than the hex form being in character order, not the integer
>>> value).
>> ...
>>> orig = get_unaligned(fourcc);
>>> - val = orig & ~BIT(31);
>>> + switch (fmt[2]) {
>>> + case 'h':
>>> + val = orig;
>>> + break;
>>> + case 'r':
>>> + orig = swab32(orig);
>>> + val = orig;
>>> + break;
>>> + case 'l':
>>> + orig = le32_to_cpu(orig);
>>> + val = orig;
>>> + break;
>>> + case 'b':
>>> + orig = be32_to_cpu(orig);
>> I do not see that orig is a union of different types. Have you run sparse?
>> It will definitely complain on this code.
> 
> After messing around with this, what I’ve noticed is that orig and val used in this struct should be u32. Now in case of little endian and big endian, that things are messy. The original code by Hector was using le32_to_cpu on orig, which itself is declared as a u32 here (maybe was done with the intention to convert le32 orig to u32 orig?).
> 
> Anyways, what I have done is that:
> 
> 1. Declare new variable, orig_le which is __le32.
> 2. Instead of doing orig = le32_to_cpu(orig); , we can do orig_le = cpu_to_le32(orig). This fixes the sparse warning: cast to restricted __le32
> 3. Now the original code was intending to use val=orig=le32_to_cpu(orig) at the bottom part of this struct. Those parts also require val and orig to be u32. For that, we are now using le32_to_cpu(orig_le). Since val is same as orig, in case these cases, instead of making a val_le, I’ve simply used orig_le there as well.
> 
> Similar changes done for big endian.
> 
> So, the struct looks like this now:
> 
> static noinline_for_stack
> char *fourcc_string(char *buf, char *end, const u32 *fourcc,
>           struct printf_spec spec, const char *fmt)
> {
>   char output[sizeof("0123 little-endian (0x01234567)")];
>   char *p = output;
>   unsigned int i;
>   unsigned char c;
>   bool pixel_fmt = false;
>   u32 orig, val;
>   __le32 orig_le;
>   __be32 orig_be;
> 
>   if (fmt[1] != 'c')
>       return error_string(buf, end, "(%p4?)", spec);
> 
>   if (check_pointer(&buf, end, fourcc, spec))
>       return buf;
> 
>   orig = get_unaligned(fourcc);
>   switch (fmt[2]) {
>   case 'h':
>       val = orig;
>       break;
>   case 'r':
>       orig = swab32(orig);
>       val = orig;
>       break;
>   case 'l':
>       orig_le = cpu_to_le32(orig);
>       break;
>   case 'b':
>       orig_be = cpu_to_be32(orig);
>       break;
>   case 'c':
>       /* Pixel formats are printed LSB-first */
>       val = swab32(orig & ~BIT(31));
>       pixel_fmt = true;
>       break;
>   default:
>       return error_string(buf, end, "(%p4?)", spec);
>   }
> 
>   for (i = 0; i < sizeof(u32); i++) {
>       switch (fmt[2]) {
>       case 'h':
>       case 'r':
>       case 'c':
>           c = val >> ((3 - i) * 8);
>           break;
>       case 'l':
>           c = le32_to_cpu(orig_le) >> ((3 - i) * 8);
>           break;
>       case 'b':
>           c = be32_to_cpu(orig_be) >> ((3 - i) * 8);
>           break;
>       }
> 
>       /* Print non-control ASCII characters as-is, dot otherwise */
>       *p++ = isascii(c) && isprint(c) ? c : '.';
>   }
> 
>   if (pixel_fmt) {
>       *p++ = ' ';
>       strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
>       p += strlen(p);
>   }
> 
>   *p++ = ' ';
>   *p++ = '(';
> 
>   switch (fmt[2]) {
>   case 'h':
>   case 'r':
>   case 'c':
>       p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32));
>       break;
>   case 'l':
>       p = special_hex_number(p, output + sizeof(output) - 2, le32_to_cpu(orig_le), sizeof(u32));
>       break;
>   case 'b':
>       p = special_hex_number(p, output + sizeof(output) - 2, be32_to_cpu(orig_be), sizeof(u32));
>       break;
>   }
> 
>   *p++ = ')';
>   *p = '\0';
> 
>   return string(buf, end, output, spec);
> }
> 
> Andy, could you verify this?

Looking at the header files, it looks like doing cpu_to_le32 on that variable and doing le32_to_cpu will actually reverse the order twice, on big endian systems, thus technically all way would not swap the order at all.

I'm not really sure how to manage the sparse warnings here.

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[andriy.shevchenko]

On Sun, Feb 23, 2025 at 06:39:15AM +0000, Aditya Garg wrote:
> > On 22 Feb 2025, at 5:41 PM, Aditya Garg <gargaditya08@live.com> wrote:
> >> On 21 Feb 2025, at 8:57 PM, andriy.shevchenko@linux.intel.com wrote:
> >>> On Thu, Feb 20, 2025 at 04:39:23PM +0000, Aditya Garg wrote:

...

> >>> %p4cc is designed for DRM/V4L2 FOURCCs with their specific quirks, but
> >>> it's useful to be able to print generic 4-character codes formatted as
> >>> an integer. Extend it to add format specifiers for printing generic
> >>> 32-bit FOURCCs with various endian semantics:
> >>> %p4ch   Host-endian
> >>> %p4cl Little-endian
> >>> %p4cb Big-endian
> >>> %p4cr Reverse-endian
> >>> The endianness determines how bytes are interpreted as a u32, and the
> >>> FOURCC is then always printed MSByte-first (this is the opposite of
> >>> V4L/DRM FOURCCs). This covers most practical cases, e.g. %p4cr would
> >>> allow printing LSByte-first FOURCCs stored in host endian order
> >>> (other than the hex form being in character order, not the integer
> >>> value).
> >> ...
> >>> orig = get_unaligned(fourcc);
> >>> - val = orig & ~BIT(31);
> >>> + switch (fmt[2]) {
> >>> + case 'h':
> >>> + val = orig;
> >>> + break;
> >>> + case 'r':
> >>> + orig = swab32(orig);
> >>> + val = orig;
> >>> + break;
> >>> + case 'l':
> >>> + orig = le32_to_cpu(orig);
> >>> + val = orig;
> >>> + break;
> >>> + case 'b':
> >>> + orig = be32_to_cpu(orig);
> >> I do not see that orig is a union of different types. Have you run sparse?
> >> It will definitely complain on this code.
> > 
> > After messing around with this, what I’ve noticed is that orig and val used
> > in this struct should be u32. Now in case of little endian and big endian,
> > that things are messy. The original code by Hector was using le32_to_cpu on
> > orig, which itself is declared as a u32 here (maybe was done with the
> > intention to convert le32 orig to u32 orig?).
> > 
> > Anyways, what I have done is that:
> > 
> > 1. Declare new variable, orig_le which is __le32.
> > 2. Instead of doing orig = le32_to_cpu(orig); , we can do orig_le =
> > cpu_to_le32(orig). This fixes the sparse warning: cast to restricted __le32
> > 3. Now the original code was intending to use val=orig=le32_to_cpu(orig) at
> > the bottom part of this struct. Those parts also require val and orig to be
> > u32. For that, we are now using le32_to_cpu(orig_le). Since val is same as
> > orig, in case these cases, instead of making a val_le, I’ve simply used
> > orig_le there as well.
> > 
> > Similar changes done for big endian.
> > 
> > So, the struct looks like this now:
> > 
> > static noinline_for_stack
> > char *fourcc_string(char *buf, char *end, const u32 *fourcc,
> >           struct printf_spec spec, const char *fmt)
> > {
> >   char output[sizeof("0123 little-endian (0x01234567)")];
> >   char *p = output;
> >   unsigned int i;
> >   unsigned char c;
> >   bool pixel_fmt = false;
> >   u32 orig, val;
> >   __le32 orig_le;
> >   __be32 orig_be;
> > 
> >   if (fmt[1] != 'c')
> >       return error_string(buf, end, "(%p4?)", spec);
> > 
> >   if (check_pointer(&buf, end, fourcc, spec))
> >       return buf;
> > 
> >   orig = get_unaligned(fourcc);
> >   switch (fmt[2]) {
> >   case 'h':
> >       val = orig;
> >       break;
> >   case 'r':
> >       orig = swab32(orig);
> >       val = orig;
> >       break;
> >   case 'l':
> >       orig_le = cpu_to_le32(orig);
> >       break;
> >   case 'b':
> >       orig_be = cpu_to_be32(orig);
> >       break;
> >   case 'c':
> >       /* Pixel formats are printed LSB-first */
> >       val = swab32(orig & ~BIT(31));
> >       pixel_fmt = true;
> >       break;
> >   default:
> >       return error_string(buf, end, "(%p4?)", spec);
> >   }
> > 
> >   for (i = 0; i < sizeof(u32); i++) {
> >       switch (fmt[2]) {
> >       case 'h':
> >       case 'r':
> >       case 'c':
> >           c = val >> ((3 - i) * 8);
> >           break;

> >       case 'l':
> >           c = le32_to_cpu(orig_le) >> ((3 - i) * 8);
> >           break;
> >       case 'b':
> >           c = be32_to_cpu(orig_be) >> ((3 - i) * 8);
> >           break;

This doesn't look right. It's basically two conversions from and to orig,
it's like using orig here, but that's wrong for the respective endianess,
'l'/BE should be LE, same for 'b'/LE which should be BE.

> >       }
> > 
> >       /* Print non-control ASCII characters as-is, dot otherwise */
> >       *p++ = isascii(c) && isprint(c) ? c : '.';
> >   }
> > 
> >   if (pixel_fmt) {
> >       *p++ = ' ';
> >       strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian");
> >       p += strlen(p);
> >   }
> > 
> >   *p++ = ' ';
> >   *p++ = '(';
> > 
> >   switch (fmt[2]) {
> >   case 'h':
> >   case 'r':
> >   case 'c':
> >       p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32));
> >       break;
> >   case 'l':
> >       p = special_hex_number(p, output + sizeof(output) - 2, le32_to_cpu(orig_le), sizeof(u32));
> >       break;
> >   case 'b':
> >       p = special_hex_number(p, output + sizeof(output) - 2, be32_to_cpu(orig_be), sizeof(u32));
> >       break;
> >   }
> > 
> >   *p++ = ')';
> >   *p = '\0';
> > 
> >   return string(buf, end, output, spec);
> > }
> > 
> > Andy, could you verify this?
> 
> Looking at the header files, it looks like doing cpu_to_le32 on that variable
> and doing le32_to_cpu will actually reverse the order twice, on big endian
> systems, thus technically all way would not swap the order at all.
> 
> I'm not really sure how to manage the sparse warnings here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[Aditya Garg]

> 
> Looking at the header files, it looks like doing cpu_to_le32 on that variable and doing le32_to_cpu will actually reverse the order twice, on big endian systems, thus technically all way would not swap the order at all.
> 
> I'm not really sure how to manage the sparse warnings here.



Not sure whether the maintainers would like it, but we can do something like this:

	case 'l’:
#ifdef __LITTLE_ENDIAN
		val = orig;
#else
		orig = swab32(orig);
		val = orig;
#endif
		break;

	case 'b’:
#ifdef __LITTLE_ENDIAN
		orig = swab32(orig);
		val = orig;
#else
		val = orig;
#endif
		break;

Re: [PATCH v2 2/3] lib/vsprintf: Add support for generic FOURCCs by extending %p4cc

[andriy.shevchenko]

On Sun, Feb 23, 2025 at 03:16:28PM +0000, Aditya Garg wrote:

> > Looking at the header files, it looks like doing cpu_to_le32 on that variable and doing le32_to_cpu will actually reverse the order twice, on big endian systems, thus technically all way would not swap the order at all.
> > 
> > I'm not really sure how to manage the sparse warnings here.
> 
> Not sure whether the maintainers would like it, but we can do something like this:

This is not what we want, I believe. And this looks like a reinventing a wheel
of cpu_to_*() and *_to_cpu() or similar macros.

> 	case 'l’:
> #ifdef __LITTLE_ENDIAN
> 		val = orig;
> #else
> 		orig = swab32(orig);
> 		val = orig;
> #endif
> 		break;
> 
> 	case 'b’:
> #ifdef __LITTLE_ENDIAN
> 		orig = swab32(orig);
> 		val = orig;
> #else
> 		val = orig;
> #endif
> 		break;

-- 
With Best Regards,
Andy Shevchenko