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_YWq4ry6Y-Jgvjq@gmail.com>
Date: Wed, 9 Apr 2025 08:41:47 +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 08:11:23AM +0200, Javier Martinez Canillas wrote:

[...]
> >> > +static int st7571_set_pixel_format(struct st7571_device *st7571,
> >> > +				   u32 pixel_format)
> >> > +{
> >> > +	switch (pixel_format) {
> >> > +	case DRM_FORMAT_C1:
> >> > +		return st7571_set_color_mode(st7571, ST7571_COLOR_MODE_BLACKWHITE);
> >> > +	case DRM_FORMAT_C2:
> >> > +		return st7571_set_color_mode(st7571, ST7571_COLOR_MODE_GRAY);
> >> > +	default:
> >> > +		return -EINVAL;
> >> > +	}
> >> 
> >> These should be DRM_FORMAT_R1 and DRM_FORMAT_R2 and not C{1,2}. The former
> >> is for displays have a single color (i.e: grey) while the latter is when a
> >> pixel can have different color, whose values are defined by a CLUT table.
> >> 
> >
> > I see.
> > Does fbdev only works with CLUT formats? I get this error when I switch
> > to DRM_FORMAT_R{1,2}:
> >
> > [drm] Initialized st7571 1.0.0 for 0-003f on minor 0
> > st7571 0-003f: [drm] format C1   little-endian (0x20203143) not supported
> > st7571 0-003f: [drm] No compatible format found
> > st7571 0-003f: [drm] *ERROR* fbdev: Failed to setup emulation (ret=-22)
> >
> >
> 
> 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.

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?

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

Best regards,
Marcus Folkesson

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