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_iwspuiYAhARS6Y@gmail.com>
Date: Fri, 11 Apr 2025 08:03:30 +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,

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,
> 	},
> };

A comment for v4:

I think I will go for a property in the device tree. I've implemented
board entries as above, but I'm not satisfied for two reasons:

1. All other properties like display size and resolution are already
   specified in the device tree. If I add entries for specific boards,
   these properties will be somehow redundant and not as generic.

2. I could not find a ST7571 with a grayscale display as a off-the-shelf
   product.
> 
> -- 
> Best regards,
> 
> Javier Martinez Canillas
> Core Platforms
> Red Hat

Best regards,
Marcus Folkesson

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