linux-kernel - Re: [PATCH v3 2/3] drm/st7571-i2c: add support for Sitronix ST7571 LCD controller

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z_Z1UOan6Qu5d3VM@gmail.com>
Date: Wed, 9 Apr 2025 15:25:36 +0200
From: Marcus Folkesson <marcus.folkesson@...il.com>
To: Javier Martinez Canillas <javierm@...hat.com>
Cc: David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, dri-devel@...ts.freedesktop.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	Thomas Zimmermann <tzimmrmann@...e.de>
Subject: Re: [PATCH v3 2/3] drm/st7571-i2c: add support for Sitronix ST7571
 LCD controller

Hello Javier,

On Wed, Apr 09, 2025 at 11:43:54AM +0200, Javier Martinez Canillas wrote:
> Marcus Folkesson <marcus.folkesson@...il.com> writes:
> 
> Hello Marcus,
> 
> [...]
> 
> >> 
> >> That's a god question, I don't really know...
> >> 
> >> But fbdev does support XRGB8888, which may be another good reason to add
> >> it and make it the default format. Yes, it will cause an unnecessary pixel
> >> format conversion but the I2C transport is so slow anyways that compute is
> >> not the bottleneck when using these small displays.
> >
> > Hrm, I now realised that I have another issue.
> > Not all LCDs that will be attached to the ST7571 controller will be
> > grayscale.
> > The display I've attached to the ST7571 is a monochrome LCD for example.
> >
> 
> Oh, that's very interesting. This means that vendors are using a more capable IC
> (i.e: ST7571) for display controllers + LCD panels board designs, even where they
> could had used a less capable one (i.e: ST7765). That is, using an IC that supports
> 2-bit grayscale when they could just used one that only supported monochrome pixels.
> 
> From a quick search, I found for example this one from SinoCrystal:
> 
> https://displaysino.com/product_details/SC128128012-V01.html
> 
> > Maybe the right way to do it is to only support XRGB8888 and specify 
> > if the display is monochrome or grayscale in the device tree.
> >
> > Or do you have any good suggestions?
> >
> 
> I don't know the proper way to handle this, but what I would do is to include
> the actual boards as entries in the OF device ID table instead of just the ICs.
> 
> And then for each entry you can specify what formats are supported, e.g:
> 
> static const uint32_t monochrome_formats[] = {
> 	DRM_FORMAT_XRGB8888,
>         DRM_FORMAT_R1
> };
> 
> static const uint32_t grayscale_formats[] = {
> 	DRM_FORMAT_XRGB8888,
>         DRM_FORMAT_R1
>         DRM_FORMAT_R2
> };
> 
> static const struct of_device_id st7571_of_match[] = {
> 	/* monochrome displays */
> 	{
> 		.compatible = "sinocrystal,sc128128012-v01",
> 		.data = monochrome_formats,
> 	},
> ...
>         /* grayscale displays */
> 	{
> 		.compatible = "foo,bar",
> 		.data = grayscale_formats,
> 	},
> };
> 
> and then in your probe callback, you can get the correct format list for
> the device matched. Something like the following for example:
> 
> static int st7571_probe(struct i2c_client *client) {
>        const uint32_t *formats = device_get_match_data(client->dev);
>        ...
> 
>        ret = drm_universal_plane_init(..., formats, ...);
>        ...
> };
> 
> Likely you will need to define more stuff to be specific for each entry, maybe
> you will need different primary plane update handlers too. Similar to how I had  
> to do it the ssd130x driver to support all the Solomon OLED controller families:
> 
> https://elixir.bootlin.com/linux/v6.11/source/drivers/gpu/drm/solomon/ssd130x.c#L1439

Thanks, that sounds like a good idea.

I've now experimenting with XRGB8888, and, well, it works. I guess.
The thresholds levels in the all conversion steps for  XRGB8888 -> 8 bit grayscale -> monochrome
makes my penguin look a bit boring.

But I guess that is expected.

Please compare
https://www.marcusfolkesson.se/xrgb8888.png
and
https://www.marcusfolkesson.se/c1.png

> -- 
> Best regards,
> 
> Javier Martinez Canillas
> Core Platforms
> Red Hat
> 

Best regards,
Marcus Folkesson

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)