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

[PATCH 0/5] 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 two 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 forth patch adds a helper function (drm_fb_xrgb8888_to_gray2) to
convert xrgb8888 to gray2.
drm_fb_xrgb8888_to_mono() has been changed splitted into two functions
where the common part is used by both drm_fb_xrgb8888_to_mono and
drm_fb_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>
---
Marcus Folkesson (5):
      drm/st7571-i2c: correct pixel data format description
      dt-bindings: display: sitronix,st7571: add optional inverted property
      drm/st7571-i2c: add support 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,st7571.yaml          |   5 +
 drivers/gpu/drm/drm_format_helper.c                | 148 +++++++++++++++------
 drivers/gpu/drm/sitronix/st7571-i2c.c              |  38 +++---
 include/drm/drm_format_helper.h                    |   4 +
 4 files changed, 135 insertions(+), 60 deletions(-)
---
base-commit: a6cfa4c8833944f8912c1fa7f95795753f6376ea
change-id: 20250520-st7571-format-2ce6badc48c6

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

[PATCH 1/5] 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.

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 eec846892962dce2d0b86c9c5c226d44135f2b0d..3024c94c890497d1ddbbf0f7f81ec4664f48d580 100644
--- a/drivers/gpu/drm/sitronix/st7571-i2c.c
+++ b/drivers/gpu/drm/sitronix/st7571-i2c.c
@@ -382,10 +382,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

Re: [PATCH 1/5] drm/st7571-i2c: correct pixel data format description

[Javier Martinez Canillas]

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

> 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.
>
> 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

[PATCH 2/5] 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.

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..065d61b718dc92e04419056b1e2d73fd0b2cb345 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

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

[Javier Martinez Canillas]

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

> 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.
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---

Could you please rebase on top of my series that adds the ST7567? Because
I see that this controller also have a "Inverse Display" command, so this
property will also apply.

Or do you prefer for this to land before and then I could post a v3 on top?

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

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

[Marcus Folkesson]

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

On Mon, Jul 14, 2025 at 12:51:12PM +0200, Javier Martinez Canillas wrote:
> Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> 
> > 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.
> >
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > ---
> 
> Could you please rebase on top of my series that adds the ST7567? Because
> I see that this controller also have a "Inverse Display" command, so this
> property will also apply.

Sure, I will rebase on top of your series and include support for the
inverse display command for the ST7567 controller as well.

> 
> Or do you prefer for this to land before and then I could post a v3 on top?
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> -- 
> Best regards,
> 
> Javier Martinez Canillas
> Core Platforms
> Red Hat

Best regards,
Marcus Folkesson

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

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

[Rob Herring]

On Mon, Jul 14, 2025 at 10:04:00AM +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.
> 
> 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..065d61b718dc92e04419056b1e2d73fd0b2cb345 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: |

Don't need '|'.

> +      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 3/5] drm/st7571-i2c: add support 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.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/gpu/drm/sitronix/st7571-i2c.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sitronix/st7571-i2c.c b/drivers/gpu/drm/sitronix/st7571-i2c.c
index 3024c94c890497d1ddbbf0f7f81ec4664f48d580..6a5d719f8b0044ec700ff119972b580c904ffb27 100644
--- a/drivers/gpu/drm/sitronix/st7571-i2c.c
+++ b/drivers/gpu/drm/sitronix/st7571-i2c.c
@@ -147,6 +147,7 @@ struct st7571_device {
 	bool ignore_nak;
 
 	bool grayscale;
+	bool inverted;
 	u32 height_mm;
 	u32 width_mm;
 	u32 startline;
@@ -789,6 +790,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;
@@ -853,7 +855,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

Re: [PATCH 3/5] drm/st7571-i2c: add support inverted pixel format

[Javier Martinez Canillas]

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

> 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.
>
> 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

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

[Marcus Folkesson]

drm_fb_xrgb8888_to_gray2() works like and share much code with
drm_fb_xrgb8888_to_mono(), but converts XRGB8888 to
2bit grayscale instead.

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 | 148 ++++++++++++++++++++++++++----------
 include/drm/drm_format_helper.h     |   4 +
 2 files changed, 110 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 8f3daf38ca639d3d39742c2c9fa0c54a3a9297a5..2f2171b3df609263cc9ce6809bf6144028c25380 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;
@@ -1270,40 +1289,11 @@ static void drm_fb_gray8_to_mono_line(void *dbuf, const void *sbuf, unsigned int
 	}
 }
 
-/**
- * drm_fb_xrgb8888_to_mono - Convert XRGB8888 to monochrome
- * @dst: Array of monochrome destination buffers (0=black, 1=white)
- * @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 first bit (LSB) in the first byte of the monochrome
- * 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 monochrome support. Drivers can use this function for
- * monochrome 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.
- *
- * This function uses drm_fb_xrgb8888_to_gray8() to convert to grayscale and
- * then the result is converted from grayscale to monochrome.
- */
-void drm_fb_xrgb8888_to_mono(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 void drm_fb_xrgb8888_to_gray(u8 *dst, const unsigned int *dst_pitch,
+			     u8 *src, const struct drm_framebuffer *fb,
+			     const struct drm_rect *clip, struct drm_format_conv_state *state,
+			     u8 bpp,
+			     void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
 {
 	static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
 		0, 0, 0, 0
@@ -1313,11 +1303,10 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 	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 *mono = dst[0].vaddr, *gray8;
 	u32 *src32;
+	u8 *gray8;
 
 	if (drm_WARN_ON(dev, fb->format->format != DRM_FORMAT_XRGB8888))
 		return;
@@ -1330,7 +1319,7 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 	 * The mono destination buffer contains 1 bit per pixel
 	 */
 	if (!dst_pitch_0)
-		dst_pitch_0 = DIV_ROUND_UP(linepixels, 8);
+		dst_pitch_0 = DIV_ROUND_UP(linepixels, 8 / bpp);
 
 	/*
 	 * The dma memory is write-combined so reads are uncached.
@@ -1349,13 +1338,88 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
 
 	gray8 = (u8 *)src32 + len_src32;
 
-	vaddr += clip_offset(clip, fb->pitches[0], cpp);
+	src += clip_offset(clip, fb->pitches[0], cpp);
 	for (y = 0; y < lines; y++) {
-		src32 = memcpy(src32, vaddr, len_src32);
+		src32 = memcpy(src32, src, len_src32);
 		drm_fb_xrgb8888_to_gray8_line(gray8, src32, linepixels);
-		drm_fb_gray8_to_mono_line(mono, gray8, linepixels);
-		vaddr += fb->pitches[0];
-		mono += dst_pitch_0;
+		xfrm_line(dst, gray8, linepixels);
+		src += fb->pitches[0];
+		dst += dst_pitch_0;
 	}
 }
+
+/**
+ * drm_fb_xrgb8888_to_mono - Convert XRGB8888 to monochrome
+ * @dst: Array of monochrome destination buffers (0=black, 1=white)
+ * @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
+ *
+ * DRM doesn't have native monochrome support. Drivers can use this function for
+ * monochrome 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.
+ *
+ * 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 first bit (LSB) in the first byte of the monochrome
+ * 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.
+ *
+ *
+ * This function uses drm_fb_xrgb8888_to_gray8() to convert to grayscale and
+ * then the result is converted from grayscale to monochrome.
+ */
+
+
+void drm_fb_xrgb8888_to_mono(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)
+{
+	drm_fb_xrgb8888_to_gray(dst[0].vaddr, dst_pitch, src[0].vaddr, fb, clip, state,
+				1, drm_fb_gray8_to_mono_line);
+}
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono);
+
+/**
+ * drm_fb_xrgb8888_to_gray2 - Convert XRGB8888 to gray2
+ * @dst: Array of gray2 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.
+ *
+ * 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.
+ *
+ * This function uses drm_fb_xrgb8888_to_gray8() to convert to grayscale and
+ * then the result is converted from grayscale to gray2.
+ */
+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)
+{
+	drm_fb_xrgb8888_to_gray(dst[0].vaddr, dst_pitch, src[0].vaddr, fb, clip, state,
+				2, drm_fb_gray8_to_gray2_line);
+}
+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

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

[Thomas Zimmermann]

Hi

Am 14.07.25 um 10:04 schrieb Marcus Folkesson:
> drm_fb_xrgb8888_to_gray2() works like and share much code with
> drm_fb_xrgb8888_to_mono(), but converts XRGB8888 to
> 2bit grayscale instead.
>
> It uses drm_fb_xrgb8888_to_gray8() to convert the pixels to gray8 as an
> intermediate step before converting to gray2.

Please don't share code here.  This needs a serious rework anyway. The 
best thing for now is to add a new, separate function for _gray2().

Best regards
Thomas


>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
>   drivers/gpu/drm/drm_format_helper.c | 148 ++++++++++++++++++++++++++----------
>   include/drm/drm_format_helper.h     |   4 +
>   2 files changed, 110 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 8f3daf38ca639d3d39742c2c9fa0c54a3a9297a5..2f2171b3df609263cc9ce6809bf6144028c25380 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;
> @@ -1270,40 +1289,11 @@ static void drm_fb_gray8_to_mono_line(void *dbuf, const void *sbuf, unsigned int
>   	}
>   }
>   
> -/**
> - * drm_fb_xrgb8888_to_mono - Convert XRGB8888 to monochrome
> - * @dst: Array of monochrome destination buffers (0=black, 1=white)
> - * @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 first bit (LSB) in the first byte of the monochrome
> - * 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 monochrome support. Drivers can use this function for
> - * monochrome 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.
> - *
> - * This function uses drm_fb_xrgb8888_to_gray8() to convert to grayscale and
> - * then the result is converted from grayscale to monochrome.
> - */
> -void drm_fb_xrgb8888_to_mono(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 void drm_fb_xrgb8888_to_gray(u8 *dst, const unsigned int *dst_pitch,
> +			     u8 *src, const struct drm_framebuffer *fb,
> +			     const struct drm_rect *clip, struct drm_format_conv_state *state,
> +			     u8 bpp,
> +			     void (*xfrm_line)(void *dbuf, const void *sbuf, unsigned int npixels))
>   {
>   	static const unsigned int default_dst_pitch[DRM_FORMAT_MAX_PLANES] = {
>   		0, 0, 0, 0
> @@ -1313,11 +1303,10 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
>   	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 *mono = dst[0].vaddr, *gray8;
>   	u32 *src32;
> +	u8 *gray8;
>   
>   	if (drm_WARN_ON(dev, fb->format->format != DRM_FORMAT_XRGB8888))
>   		return;
> @@ -1330,7 +1319,7 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
>   	 * The mono destination buffer contains 1 bit per pixel
>   	 */
>   	if (!dst_pitch_0)
> -		dst_pitch_0 = DIV_ROUND_UP(linepixels, 8);
> +		dst_pitch_0 = DIV_ROUND_UP(linepixels, 8 / bpp);
>   
>   	/*
>   	 * The dma memory is write-combined so reads are uncached.
> @@ -1349,13 +1338,88 @@ void drm_fb_xrgb8888_to_mono(struct iosys_map *dst, const unsigned int *dst_pitc
>   
>   	gray8 = (u8 *)src32 + len_src32;
>   
> -	vaddr += clip_offset(clip, fb->pitches[0], cpp);
> +	src += clip_offset(clip, fb->pitches[0], cpp);
>   	for (y = 0; y < lines; y++) {
> -		src32 = memcpy(src32, vaddr, len_src32);
> +		src32 = memcpy(src32, src, len_src32);
>   		drm_fb_xrgb8888_to_gray8_line(gray8, src32, linepixels);
> -		drm_fb_gray8_to_mono_line(mono, gray8, linepixels);
> -		vaddr += fb->pitches[0];
> -		mono += dst_pitch_0;
> +		xfrm_line(dst, gray8, linepixels);
> +		src += fb->pitches[0];
> +		dst += dst_pitch_0;
>   	}
>   }
> +
> +/**
> + * drm_fb_xrgb8888_to_mono - Convert XRGB8888 to monochrome
> + * @dst: Array of monochrome destination buffers (0=black, 1=white)
> + * @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
> + *
> + * DRM doesn't have native monochrome support. Drivers can use this function for
> + * monochrome 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.
> + *
> + * 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 first bit (LSB) in the first byte of the monochrome
> + * 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.
> + *
> + *
> + * This function uses drm_fb_xrgb8888_to_gray8() to convert to grayscale and
> + * then the result is converted from grayscale to monochrome.
> + */
> +
> +
> +void drm_fb_xrgb8888_to_mono(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)
> +{
> +	drm_fb_xrgb8888_to_gray(dst[0].vaddr, dst_pitch, src[0].vaddr, fb, clip, state,
> +				1, drm_fb_gray8_to_mono_line);
> +}
>   EXPORT_SYMBOL(drm_fb_xrgb8888_to_mono);
> +
> +/**
> + * drm_fb_xrgb8888_to_gray2 - Convert XRGB8888 to gray2
> + * @dst: Array of gray2 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.
> + *
> + * 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.
> + *
> + * This function uses drm_fb_xrgb8888_to_gray8() to convert to grayscale and
> + * then the result is converted from grayscale to gray2.
> + */
> +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)
> +{
> +	drm_fb_xrgb8888_to_gray(dst[0].vaddr, dst_pitch, src[0].vaddr, fb, clip, state,
> +				2, drm_fb_gray8_to_gray2_line);
> +}
> +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 */
>

-- 
--
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 4/5] drm/format-helper: introduce drm_fb_xrgb8888_to_gray2()

[Marcus Folkesson]

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

Hi Thomas,


Thank you for your comments!

On Mon, Jul 14, 2025 at 11:13:04AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 14.07.25 um 10:04 schrieb Marcus Folkesson:
> > drm_fb_xrgb8888_to_gray2() works like and share much code with
> > drm_fb_xrgb8888_to_mono(), but converts XRGB8888 to
> > 2bit grayscale instead.
> > 
> > It uses drm_fb_xrgb8888_to_gray8() to convert the pixels to gray8 as an
> > intermediate step before converting to gray2.
> 
> Please don't share code here.  This needs a serious rework anyway. The best
> thing for now is to add a new, separate function for _gray2().

I see. I will instead add a separate _gray() function, even though it
will look very similar to _mono.

> 
> Best regards
> Thomas

Best regards,
Marcus Folkesson

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

[PATCH 5/5] 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.

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 6a5d719f8b0044ec700ff119972b580c904ffb27..2af800a4c5e4960f3dfcebfeda4554b56015915f 100644
--- a/drivers/gpu/drm/sitronix/st7571-i2c.c
+++ b/drivers/gpu/drm/sitronix/st7571-i2c.c
@@ -215,10 +215,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
@@ -233,7 +234,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);
@@ -300,11 +301,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:
@@ -330,7 +331,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);
 
@@ -355,14 +356,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;
@@ -370,7 +370,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);
 
@@ -391,7 +391,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 5/5] drm/st7571-i2c: add support for 2bit grayscale for XRGB8888

[Javier Martinez Canillas]

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

> Add support for 2bit grayscale and use it for XRGB8888 when grayscale is
> supported.
>
> 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