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

[Marcus Folkesson <marcus.folkesson@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]