linux-kernel - Re: [PATCH v3 4/4] drm/ssd130x: Add support for the SSD133x OLED controller family

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87y1dm4lzb.fsf@minerva.mail-host-address-is-not-set>
Date: Fri, 22 Dec 2023 10:36:56 +0100
From: Javier Martinez Canillas <javierm@...hat.com>
To: Jocelyn Falempe <jfalempe@...hat.com>, linux-kernel@...r.kernel.org
Cc: Geert Uytterhoeven <geert@...ux-m68k.org>, Maxime Ripard
 <mripard@...nel.org>, Peter Robinson <pbrobinson@...il.com>, Rob Herring
 <robh@...nel.org>, Conor Dooley <conor@...nel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski@...aro.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 Daniel Vetter <daniel@...ll.ch>, David Airlie <airlied@...il.com>, Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>,
 dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v3 4/4] drm/ssd130x: Add support for the SSD133x OLED
 controller family

Jocelyn Falempe <jfalempe@...hat.com> writes:

Hello Jocelyn,

Thanks a lot for your review!

> On 19/12/2023 21:34, Javier Martinez Canillas wrote:
>> The Solomon SSD133x controllers (such as the SSD1331) are used by RGB dot
>> matrix OLED panels, add a modesetting pipeline to support the chip family.
>> 
>> The SSD133x controllers support 256 (8-bit) and 65k (16-bit) color depths
>> but only the former is implemented for now. This is because the 256 color
>> depth format matches a fourcc code already present in DRM (RGB8), but the
>> 65k pixel format does not match the existing RG16 fourcc code format.
>> 
>> Instead of a R:G:B 5:6:5, the controller expects the 16-bit pixels to be
>> R:G:B 6:5:6, and so a new fourcc needs to be added to support this format.
>
> small typo here, R:G:B 6:5:6 => that's 17 bits
>

Oh, tanks for pointing that out.

It seems to be a typo in the SSD1331 controller datasheet itself:

https://cdn-shop.adafruit.com/datasheets/SSD1331_1.2.pdf

"Each pixel has 16-bit data. Three sub-pixels for color A, B and C have 6
bits, 5 bits and 6 bits respectively."

I blindly copied what the datasheet said without relizing that it was 17
bits indeed!

So looking again at "Table 9 - Data bus usage under different bus width
and color depth mode" in the datasheet shared above, it seems the format
has the same sub-pixel layout than DRM_FORMAT_RGB565. But I tested with
that format and the colors were off, and the same for DRM_FORMAT_BGR565.

Now, even when only using 256 colors the images are pretty decent as you
can see in https://fosstodon.org/@javierm/111591985174504541

I'll reword the commit message and drop the comment about that RGB format
and explain that only DRM_FORMAT_RGB332 is supported for now.

> other than that, it looks good to me, feel free to add:
> Reviewed-by: Jocelyn Falempe <jfalempe@...hat.com>
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat