[PATCH v2 0/6] drm/st7571-i2c: add support for grayscale xrgb8888

[PATCH v2 0/6] drm/st7571-i2c: add support for grayscale xrgb8888

[Marcus Folkesson]

The goal with this series is to add support for 2bit grayscale with
the xrgb8888 pixel format for the st7571 display controller.

The first patch only corrects a comment of the pixel format.

The next four patches adds support for inverting pixels. This is
necessary as the connected display may or may not use the "right" (0 =>
black, 1 => white) pixel format expected by the supported formats
(R1/R2/XRGB8888).

The fifth patch adds a helper function (drm_fb_xrgb8888_to_gray2) to
convert xrgb8888 to gray2.

The last path adds support for gray2 in the st7571 driver.
Compare the mono [1] and the gray2 [2] variants of our penguin.

[1] https://www.marcusfolkesson.se/gray2.png
[2] https://www.marcusfolkesson.se/xrgb8888.png

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
Changes in v2:
- Do not share code between _to_mono() and _to_gray2()
- Correct formatting (remove  "|") in the dt bindings
- Implement support for inverting pixels for st7567
- Link to v1: https://lore.kernel.org/r/20250714-st7571-format-v1-0-a27e5112baff@gmail.com

---
Marcus Folkesson (6):
      drm/st7571-i2c: correct pixel data format description
      dt-bindings: display: sitronix,st7571: add optional inverted property
      dt-bindings: display: sitronix,st7567: add optional inverted property
      drm/st7571-i2c: add support for inverted pixel format
      drm/format-helper: introduce drm_fb_xrgb8888_to_gray2()
      drm/st7571-i2c: add support for 2bit grayscale for XRGB8888

 .../bindings/display/sitronix,st7567.yaml          |   5 +
 .../bindings/display/sitronix,st7571.yaml          |   5 +
 drivers/gpu/drm/drm_format_helper.c                | 108 +++++++++++++++++++++
 drivers/gpu/drm/sitronix/st7571-i2c.c              |  41 ++++----
 include/drm/drm_format_helper.h                    |   4 +
 5 files changed, 144 insertions(+), 19 deletions(-)
---
base-commit: ca2a6abdaee43808034cdb218428d2ed85fd3db8
change-id: 20250520-st7571-format-2ce6badc48c6

Best regards,
-- 
Marcus Folkesson <marcus.folkesson@gmail.com>

[PATCH v2 1/6] drm/st7571-i2c: correct pixel data format description

[Marcus Folkesson]

The comment describes the pixel data format as stated in
the st7571 datasheet, which is not necessary the same
as for the connected display.

Instead, describe the expected pixel data format which is used for
R1/R2/XRGB8888.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/gpu/drm/sitronix/st7571-i2c.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sitronix/st7571-i2c.c b/drivers/gpu/drm/sitronix/st7571-i2c.c
index 453eb7e045e5fb5942720d6020b6279a4b7315d7..dfdd0fa4ff24090c3cbe7ab162285be45464d6a6 100644
--- a/drivers/gpu/drm/sitronix/st7571-i2c.c
+++ b/drivers/gpu/drm/sitronix/st7571-i2c.c
@@ -386,10 +386,10 @@ static int st7571_fb_update_rect_grayscale(struct drm_framebuffer *fb, struct dr
 			 * even if the format is monochrome.
 			 *
 			 * The bit values maps to the following grayscale:
-			 * 0 0 = White
-			 * 0 1 = Light gray
-			 * 1 0 = Dark gray
-			 * 1 1 = Black
+			 * 0 0 = Black
+			 * 0 1 = Dark gray
+			 * 1 0 = Light gray
+			 * 1 1 = White
 			 *
 			 * For monochrome formats, write the same value twice to get
 			 * either a black or white pixel.

-- 
2.49.0

[PATCH v2 2/6] dt-bindings: display: sitronix,st7571: add optional inverted property

[Marcus Folkesson]

Depending on which display that is connected to the controller, an "1"
means either a black or a white pixel.

The supported formats (R1/R2/XRGB8888) expects the pixels
to map against (4bit):
00 => Black
01 => Dark Gray
10 => Light Gray
11 => White

If this is not what the display map against, the controller has support
to invert these values.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 Documentation/devicetree/bindings/display/sitronix,st7571.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/sitronix,st7571.yaml b/Documentation/devicetree/bindings/display/sitronix,st7571.yaml
index 4fea782fccd701f5095a08290c13722a12a58b52..b83721eb4b7f8d258b4e845f107b056696b8d4a8 100644
--- a/Documentation/devicetree/bindings/display/sitronix,st7571.yaml
+++ b/Documentation/devicetree/bindings/display/sitronix,st7571.yaml
@@ -28,6 +28,11 @@ properties:
     description:
       Display supports 4-level grayscale.
 
+  sitronix,inverted:
+    type: boolean
+    description:
+      Display pixels are inverted, i.e. 0 is white and 1 is black.
+
   reset-gpios: true
   width-mm: true
   height-mm: true

-- 
2.49.0

[PATCH v2 3/6] dt-bindings: display: sitronix,st7567: add optional inverted property

[Marcus Folkesson]

Depending on which display that is connected to the controller, an "1"
means either a black or a white pixel.

The supported format (R1) expects the pixels to map against:
    0 => Black
    1 => White

If this is not what the display map against, the controller has support
to invert these values.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 Documentation/devicetree/bindings/display/sitronix,st7567.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/sitronix,st7567.yaml b/Documentation/devicetree/bindings/display/sitronix,st7567.yaml
index e8a5b8ad18fe01429146b20a0b8237a164a7dd47..2eb6d00b5a25632a1ce121c1b43a92b2a4010fde 100644
--- a/Documentation/devicetree/bindings/display/sitronix,st7567.yaml
+++ b/Documentation/devicetree/bindings/display/sitronix,st7567.yaml
@@ -23,6 +23,11 @@ properties:
   reg:
     maxItems: 1
 
+  sitronix,inverted:
+    type: boolean
+    description:
+      Display pixels are inverted, i.e. 0 is white and 1 is black.
+
   width-mm: true
   height-mm: true
   panel-timing: true

-- 
2.49.0

[PATCH v2 4/6] drm/st7571-i2c: add support for inverted pixel format

[Marcus Folkesson]

Depending on which display that is connected to the controller, an
"1" means either a black or a white pixel.

The supported formats (R1/R2/XRGB8888) expects the pixels
to map against (4bit):
    00 => Black
    01 => Dark Gray
    10 => Light Gray
    11 => White

If this is not what the display map against, make it possible to invert
the pixels.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/gpu/drm/sitronix/st7571-i2c.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sitronix/st7571-i2c.c b/drivers/gpu/drm/sitronix/st7571-i2c.c
index dfdd0fa4ff24090c3cbe7ab162285be45464d6a6..9f2de057ce9d990fdd77e395a6c32ba1e2f36137 100644
--- a/drivers/gpu/drm/sitronix/st7571-i2c.c
+++ b/drivers/gpu/drm/sitronix/st7571-i2c.c
@@ -151,6 +151,7 @@ struct st7571_device {
 	bool ignore_nak;
 
 	bool grayscale;
+	bool inverted;
 	u32 height_mm;
 	u32 width_mm;
 	u32 startline;
@@ -792,6 +793,7 @@ static int st7567_parse_dt(struct st7571_device *st7567)
 
 	of_property_read_u32(np, "width-mm", &st7567->width_mm);
 	of_property_read_u32(np, "height-mm", &st7567->height_mm);
+	st7567->inverted = of_property_read_bool(np, "sitronix,inverted");
 
 	st7567->pformat = &st7571_monochrome;
 	st7567->bpp = 1;
@@ -819,6 +821,7 @@ static int st7571_parse_dt(struct st7571_device *st7571)
 	of_property_read_u32(np, "width-mm", &st7571->width_mm);
 	of_property_read_u32(np, "height-mm", &st7571->height_mm);
 	st7571->grayscale = of_property_read_bool(np, "sitronix,grayscale");
+	st7571->inverted = of_property_read_bool(np, "sitronix,inverted");
 
 	if (st7571->grayscale) {
 		st7571->pformat = &st7571_grayscale;
@@ -873,7 +876,7 @@ static int st7567_lcd_init(struct st7571_device *st7567)
 		ST7571_SET_POWER(0x6),	/* Power Control, VC: ON, VR: ON, VF: OFF */
 		ST7571_SET_POWER(0x7),	/* Power Control, VC: ON, VR: ON, VF: ON */
 
-		ST7571_SET_REVERSE(0),
+		ST7571_SET_REVERSE(st7567->inverted ? 1 : 0),
 		ST7571_SET_ENTIRE_DISPLAY_ON(0),
 	};
 
@@ -917,7 +920,7 @@ static int st7571_lcd_init(struct st7571_device *st7571)
 		ST7571_SET_COLOR_MODE(st7571->pformat->mode),
 		ST7571_COMMAND_SET_NORMAL,
 
-		ST7571_SET_REVERSE(0),
+		ST7571_SET_REVERSE(st7571->inverted ? 1 : 0),
 		ST7571_SET_ENTIRE_DISPLAY_ON(0),
 	};
 

-- 
2.49.0

[PATCH v2 5/6] drm/format-helper: introduce drm_fb_xrgb8888_to_gray2()

[Marcus Folkesson]

Convert XRGB8888 to 2bit grayscale.

It uses drm_fb_xrgb8888_to_gray8() to convert the pixels to gray8 as an
intermediate step before converting to gray2.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/gpu/drm/drm_format_helper.c | 108 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_format_helper.h     |   4 ++
 2 files changed, 112 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 8f3daf38ca639d3d39742c2c9fa0c54a3a9297a5..bfeaa4db6929859ea029192a868e6d42741b31f2 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -1253,6 +1253,25 @@ int drm_fb_blit(struct iosys_map *dst, const unsigned int *dst_pitch, uint32_t d
 }
 EXPORT_SYMBOL(drm_fb_blit);
 
+static void drm_fb_gray8_to_gray2_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	u8 *dbuf8 = dbuf;
+	const u8 *sbuf8 = sbuf;
+	u8 px;
+
+	while (pixels) {
+		unsigned int i, bits = min(pixels, 4U);
+		u8 byte = 0;
+
+		for (i = 0; i < bits; i++, pixels--) {
+			byte >>= 2;
+			px = (*sbuf8++ * 3 + 127) / 255;
+			byte |= (px &= 0x03) << 6;
+		}
+		*dbuf8++ = byte;
+	}
+}
+
 static void drm_fb_gray8_to_mono_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	u8 *dbuf8 = dbuf;
@@ -1359,3 +1378,92 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 	}
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono);
+
+/**
+ * drm_fb_xrgb8888_to_gray2 - Convert XRGB8888 to gray2
+ * @dst: Array of gray2 destination buffer
+ * @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). The first pixel (upper left corner of the clip rectangle) will
+ * be converted and copied to the two first bits (LSB) in the first byte of the gray2
+ * destination buffer. If the caller requires that the first pixel in a byte must
+ * be located at an x-coordinate that is a multiple of 8, then the caller must take
+ * care itself of supplying a suitable clip rectangle.
+ *
+ * DRM doesn't have native gray2 support. Drivers can use this function for
+ * gray2 devices that don't support XRGB8888 natively. Such drivers can
+ * announce the commonly supported XR24 format to userspace and use this function
+ * to convert to the native format.
+ *
+ */
+void drm_fb_xrgb8888_to_gray2(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 unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
+		0, 0, 0, 0
+	};
+	unsigned int linepixels = drm_rect_width(clip);
+	unsigned int lines = drm_rect_height(clip);
+	unsigned int cpp = fb->format->cpp[0];
+	unsigned int len_src32 = linepixels * cpp;
+	struct drm_device *dev = fb->dev;
+	void *vaddr = src[0].vaddr;
+	unsigned int dst_pitch_0;
+	unsigned int y;
+	u8 *gray2 = dst[0].vaddr, *gray8;
+	u32 *src32;
+
+	if (drm_WARN_ON(dev, fb->format->format != DRM_FORMAT_XRGB8888))
+		return;
+
+	if (!dst_pitch)
+		dst_pitch = default_dst_pitch;
+	dst_pitch_0 = dst_pitch[0];
+
+	/*
+	 * The gray2 destination buffer contains 2 bit per pixel
+	 */
+	if (!dst_pitch_0)
+		dst_pitch_0 = DIV_ROUND_UP(linepixels, 4);
+
+	/*
+	 * The dma memory is write-combined so reads are uncached.
+	 * Speed up by fetching one line at a time.
+	 *
+	 * Also, format conversion from XR24 to gray2 are done
+	 * line-by-line but are converted to 8-bit grayscale as an
+	 * intermediate step.
+	 *
+	 * Allocate a buffer to be used for both copying from the cma
+	 * memory and to store the intermediate grayscale line pixels.
+	 */
+	src32 = drm_format_conv_state_reserve(state, len_src32 + linepixels, GFP_KERNEL);
+	if (!src32)
+		return;
+
+	gray8 = (u8 *)src32 + len_src32;
+
+	vaddr += clip_offset(clip, fb->pitches[0], cpp);
+	for (y = 0; y < lines; y++) {
+		src32 = memcpy(src32, vaddr, len_src32);
+		drm_fb_xrgb8888_to_gray8_line(gray8, src32, linepixels);
+		drm_fb_gray8_to_gray2_line(gray2, gray8, linepixels);
+		vaddr += fb->pitches[0];
+		gray2 += dst_pitch_0;
+	}
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray2);
+
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 562bc383ece4e90d96aa92b47b4f69609f825a6e..8488befafb7e0e0311f87bd2fef5011bab45065b 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -136,4 +136,8 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 			     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_gray2(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);
+
 #endif /* __LINUX_DRM_FORMAT_HELPER_H */

-- 
2.49.0

[PATCH v2 6/6] drm/st7571-i2c: add support for 2bit grayscale for XRGB8888

[Marcus Folkesson]

Add support for 2bit grayscale and use it for XRGB8888 when grayscale is
supported.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/gpu/drm/sitronix/st7571-i2c.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/sitronix/st7571-i2c.c b/drivers/gpu/drm/sitronix/st7571-i2c.c
index 9f2de057ce9d990fdd77e395a6c32ba1e2f36137..6c5935c37a2abf99116f8c2f67eec25bad90c8a8 100644
--- a/drivers/gpu/drm/sitronix/st7571-i2c.c
+++ b/drivers/gpu/drm/sitronix/st7571-i2c.c
@@ -219,10 +219,11 @@ static int st7571_send_command_list(struct st7571_device *st7571,
 	return ret;
 }
 
-static inline u8 st7571_transform_xy(const char *p, int x, int y)
+static inline u8 st7571_transform_xy(const char *p, int x, int y, u8 bpp)
 {
 	int xrest = x % 8;
 	u8 result = 0;
+	u8 row_len = 16 * bpp;
 
 	/*
 	 * Transforms an (x, y) pixel coordinate into a vertical 8-bit
@@ -237,7 +238,7 @@ static inline u8 st7571_transform_xy(const char *p, int x, int y)
 
 	for (int i = 0; i < 8; i++) {
 		int row_idx = y + i;
-		u8 byte = p[row_idx * 16 + x];
+		u8 byte = p[row_idx * row_len + x];
 		u8 bit = (byte >> xrest) & 1;
 
 		result |= (bit << i);
@@ -304,11 +305,11 @@ static void st7571_prepare_buffer_grayscale(struct st7571_device *st7571,
 	struct iosys_map dst;
 
 	switch (fb->format->format) {
-	case DRM_FORMAT_XRGB8888: /* Only support XRGB8888 in monochrome mode */
-		dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8);
+	case DRM_FORMAT_XRGB8888:
+		dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 4);
 		iosys_map_set_vaddr(&dst, st7571->hwbuf);
 
-		drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
+		drm_fb_xrgb8888_to_gray2(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
 		break;
 
 	case DRM_FORMAT_R1:
@@ -334,7 +335,7 @@ static int st7571_fb_update_rect_monochrome(struct drm_framebuffer *fb, struct d
 
 	for (int y = rect->y1; y < rect->y2; y += ST7571_PAGE_HEIGHT) {
 		for (int x = rect->x1; x < rect->x2; x++)
-			row[x] = st7571_transform_xy(st7571->hwbuf, x, y);
+			row[x] = st7571_transform_xy(st7571->hwbuf, x, y, 1);
 
 		st7571_set_position(st7571, rect->x1, y);
 
@@ -359,14 +360,13 @@ static int st7571_fb_update_rect_grayscale(struct drm_framebuffer *fb, struct dr
 	rect->y2 = min_t(unsigned int, round_up(rect->y2, ST7571_PAGE_HEIGHT), st7571->nlines);
 
 	switch (format) {
-	case DRM_FORMAT_XRGB8888:
-		/* Threated as monochrome (R1) */
-		fallthrough;
 	case DRM_FORMAT_R1:
-		x1 = rect->x1;
-		x2 = rect->x2;
+		x1 = rect->x1 * 1;
+		x2 = rect->x2 * 1;
 		break;
 	case DRM_FORMAT_R2:
+		fallthrough;
+	case DRM_FORMAT_XRGB8888:
 		x1 = rect->x1 * 2;
 		x2 = rect->x2 * 2;
 		break;
@@ -374,7 +374,7 @@ static int st7571_fb_update_rect_grayscale(struct drm_framebuffer *fb, struct dr
 
 	for (int y = rect->y1; y < rect->y2; y += ST7571_PAGE_HEIGHT) {
 		for (int x = x1; x < x2; x++)
-			row[x] = st7571_transform_xy(st7571->hwbuf, x, y);
+			row[x] = st7571_transform_xy(st7571->hwbuf, x, y, 2);
 
 		st7571_set_position(st7571, rect->x1, y);
 
@@ -395,7 +395,7 @@ static int st7571_fb_update_rect_grayscale(struct drm_framebuffer *fb, struct dr
 			 * For monochrome formats, write the same value twice to get
 			 * either a black or white pixel.
 			 */
-			if (format == DRM_FORMAT_R1 || format == DRM_FORMAT_XRGB8888)
+			if (format == DRM_FORMAT_R1)
 				regmap_bulk_write(st7571->regmap, ST7571_DATA_MODE, row + x, 1);
 		}
 	}

-- 
2.49.0

Re: [PATCH v2 3/6] dt-bindings: display: sitronix,st7567: add optional inverted property

[Javier Martinez Canillas]

Marcus Folkesson <marcus.folkesson@gmail.com> writes:

Hello Marcus,

> Depending on which display that is connected to the controller, an "1"
> means either a black or a white pixel.
>
> The supported format (R1) expects the pixels to map against:
>     0 => Black
>     1 => White
>
> If this is not what the display map against, the controller has support
> to invert these values.
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 5/6] drm/format-helper: introduce drm_fb_xrgb8888_to_gray2()

[Javier Martinez Canillas]

Marcus Folkesson <marcus.folkesson@gmail.com> writes:

> Convert XRGB8888 to 2bit grayscale.
>
> It uses drm_fb_xrgb8888_to_gray8() to convert the pixels to gray8 as an
> intermediate step before converting to gray2.
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---

I would like Thomas to review it too, but for me the change looks good.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 2/6] dt-bindings: display: sitronix,st7571: add optional inverted property

[Rob Herring (Arm)]

On Mon, 21 Jul 2025 12:43:32 +0200, Marcus Folkesson wrote:
> Depending on which display that is connected to the controller, an "1"
> means either a black or a white pixel.
> 
> The supported formats (R1/R2/XRGB8888) expects the pixels
> to map against (4bit):
> 00 => Black
> 01 => Dark Gray
> 10 => Light Gray
> 11 => White
> 
> If this is not what the display map against, the controller has support
> to invert these values.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
>  Documentation/devicetree/bindings/display/sitronix,st7571.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v2 3/6] dt-bindings: display: sitronix,st7567: add optional inverted property

[Rob Herring (Arm)]

On Mon, 21 Jul 2025 12:43:33 +0200, Marcus Folkesson wrote:
> Depending on which display that is connected to the controller, an "1"
> means either a black or a white pixel.
> 
> The supported format (R1) expects the pixels to map against:
>     0 => Black
>     1 => White
> 
> If this is not what the display map against, the controller has support
> to invert these values.
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
>  Documentation/devicetree/bindings/display/sitronix,st7567.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v2 5/6] drm/format-helper: introduce drm_fb_xrgb8888_to_gray2()

[Marcus Folkesson]

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

On Mon, Jul 21, 2025 at 01:24:19PM +0200, Javier Martinez Canillas wrote:
> Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> 
> > Convert XRGB8888 to 2bit grayscale.
> >
> > It uses drm_fb_xrgb8888_to_gray8() to convert the pixels to gray8 as an
> > intermediate step before converting to gray2.
> >
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > ---
> 
> I would like Thomas to review it too, but for me the change looks good.

A friendly ping to Thomas.

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

Re: [PATCH v2 5/6] drm/format-helper: introduce drm_fb_xrgb8888_to_gray2()

[Thomas Zimmermann]

Hi

Am 11.08.25 um 14:06 schrieb Marcus Folkesson:
> On Mon, Jul 21, 2025 at 01:24:19PM +0200, Javier Martinez Canillas wrote:
>> Marcus Folkesson <marcus.folkesson@gmail.com> writes:
>>
>>> Convert XRGB8888 to 2bit grayscale.
>>>
>>> It uses drm_fb_xrgb8888_to_gray8() to convert the pixels to gray8 as an
>>> intermediate step before converting to gray2.
>>>
>>> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
>>> ---
>> I would like Thomas to review it too, but for me the change looks good.
> A friendly ping to Thomas.

Apologies for the late review.

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>


-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v2 5/6] drm/format-helper: introduce drm_fb_xrgb8888_to_gray2()

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Hi
>
> Am 11.08.25 um 14:06 schrieb Marcus Folkesson:
>> On Mon, Jul 21, 2025 at 01:24:19PM +0200, Javier Martinez Canillas wrote:
>>> Marcus Folkesson <marcus.folkesson@gmail.com> writes:
>>>
>>>> Convert XRGB8888 to 2bit grayscale.
>>>>
>>>> It uses drm_fb_xrgb8888_to_gray8() to convert the pixels to gray8 as an
>>>> intermediate step before converting to gray2.
>>>>
>>>> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
>>>> ---
>>> I would like Thomas to review it too, but for me the change looks good.
>> A friendly ping to Thomas.
>
> Apologies for the late review.
>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>

No worries and thanks a lot for your review.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v2 0/6] drm/st7571-i2c: add support for grayscale xrgb8888

[Javier Martinez Canillas]

Marcus Folkesson <marcus.folkesson@gmail.com> writes:

Hello Marcus,

> The goal with this series is to add support for 2bit grayscale with
> the xrgb8888 pixel format for the st7571 display controller.
>
> The first patch only corrects a comment of the pixel format.
>
> The next four patches adds support for inverting pixels. This is
> necessary as the connected display may or may not use the "right" (0 =>
> black, 1 => white) pixel format expected by the supported formats
> (R1/R2/XRGB8888).
>
> The fifth patch adds a helper function (drm_fb_xrgb8888_to_gray2) to
> convert xrgb8888 to gray2.
>
> The last path adds support for gray2 in the st7571 driver.
> Compare the mono [1] and the gray2 [2] variants of our penguin.
>
> [1] https://www.marcusfolkesson.se/gray2.png
> [2] https://www.marcusfolkesson.se/xrgb8888.png
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
> Changes in v2:
> - Do not share code between _to_mono() and _to_gray2()
> - Correct formatting (remove  "|") in the dt bindings
> - Implement support for inverting pixels for st7567
> - Link to v1: https://lore.kernel.org/r/20250714-st7571-format-v1-0-a27e5112baff@gmail.com
>
> ---
> Marcus Folkesson (6):
>       drm/st7571-i2c: correct pixel data format description
>       dt-bindings: display: sitronix,st7571: add optional inverted property
>       dt-bindings: display: sitronix,st7567: add optional inverted property
>       drm/st7571-i2c: add support for inverted pixel format
>       drm/format-helper: introduce drm_fb_xrgb8888_to_gray2()
>       drm/st7571-i2c: add support for 2bit grayscale for XRGB8888
>

When applying I noticed that patch #5 had the following warning [0], but
I fixed it locally before pushing to drm-misc (drm-misc-next). Thanks!

[0]: CHECK: Alignment should match open parenthesis
#86: FILE: drivers/gpu/drm/drm_format_helper.c:1415:
+void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pitch,
+                            const struct iosys_map *src, const struct drm_framebuffer *fb,

CHECK: Alignment should match open parenthesis
#153: FILE: include/drm/drm_format_helper.h:140:
+void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pitch,
+                            const struct iosys_map *src, const struct drm_framebuffer *fb,

total: 0 errors, 0 warnings, 2 checks, 125 lines checked

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat