Re: [PATCH v2 09/14] drm/solomon: ssd16xx: Expose color mode as plane property

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/solomon: ssd16xx: 3-color mode with partial refresh corrupts R1 pixels
- [Medium] drm/solomon: ssd16xx: Commit message contradicts color_mode property creation
--

commit 50bd4d2129cba69722affa1695b7f8c68a0578a8
Author: Devarsh Thakkar <devarsht@ti.com>

drm/solomon: ssd16xx: Expose color mode as plane property

This commit introduces a new DRM plane property called "color_mode" to support
panels with 3-color (black/white/red) display modes. It allows switching
between standard black/white representation and a 3-color representation where
the R1 format designates red ink on a white background.

> Add a DRM plane property "color_mode" for panels that support
> 3-color (black/white/red) display modes.
> 
> The property allows runtime selection between:
>   - bw (black/white): R1 format represents black (0) and white (1)
>   - 3color: R1 format represents red ink (1) on white background
> 
> This property is only created on panels with red_supported = true.

[Severity: Medium]
The commit message states the property is only created on panels where red
is supported.

However, the code in ssd16xx_plane_create_properties() appears to create
it unconditionally for all panels (while restricting the selectable enum
options if unsupported).

Is this description accurate?

> The default is set from the panel configuration's default_color_mode
> field (typically BW for monochrome panels).
> 
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>

diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
index dc2b8fe536af3..3815af0cee5c5 100644
--- a/drivers/gpu/drm/solomon/ssd16xx.c
+++ b/drivers/gpu/drm/solomon/ssd16xx.c
[ ... ]
@@ -1568,6 +1587,7 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
 	}
 
 	device->refresh_mode = new_ssd->refresh_mode;
+	device->color_mode = new_ssd->color_mode;

[Severity: High]
When a user sets color_mode to 3COLOR and refresh_mode to PARTIAL,
device->color_mode is updated here before calling ssd16xx_fb_dirty().

However, looking at ssd16xx_fb_dirty():

drivers/gpu/drm/solomon/ssd16xx.c:ssd16xx_fb_dirty() {
    ...
    if (device->color_mode == SSD16XX_COLOR_MODE_3COLOR &&
        (device->refresh_mode == SSD16XX_REFRESH_FULL ||
         device->refresh_mode == SSD16XX_REFRESH_FAST)) {
            red_buffer = device->tx_red_buf;
    }
    ...
    if (red_buffer)
            ssd16xx_convert_fb_to_3color(mono_buffer, red_buffer, &map, fb, rect);
    else
            ssd16xx_convert_fb_to_1bpp(mono_buffer, &map, fb, rect, device->orientation);
    ...
}

Because red_buffer is only initialized for FULL or FAST refresh modes, a
PARTIAL refresh in 3-color mode leaves red_buffer as NULL.

This triggers the fallback to ssd16xx_convert_fb_to_1bpp(), which copies
R1 bits directly and interprets them as luma (0=black, 1=white) instead of
the 3-color expectation (0=white, 1=red).

Can this lead to corrupted R1 pixel colors during a partial refresh?

 	/*
 	 * When refresh_mode_init was set, use the specified mode for this first
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927182329.4193961-1-devarsht@ti.com?part=9