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 <[email protected]>

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 <[email protected]>

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/[email protected]?part=9

Reply via email to