Re: [PATCH v1 1/4] fbtft: Unorphan the driver

From: Thomas Zimmermann <tzimmermann@suse.de>
To: Javier Martinez Canillas <javierm@redhat.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy@kernel.org>,
	linux-fbdev@vger.kernel.org,
	Michael Hennerich <michael.hennerich@analog.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Helge Deller <deller@gmx.de>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org,
	Phillip Potter <phil@philpotter.co.uk>,
	Carlis <zhangxuezhi1@yulong.com>,
	Lee Jones <lee.jones@linaro.org>,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH v1 1/4] fbtft: Unorphan the driver
Date: Mon, 31 Jan 2022 11:28:23 +0100	[thread overview]
Message-ID: <e7fbef3c-2f87-15f9-b24d-34ffaa5a2853@suse.de> (raw)
In-Reply-To: <840ec74d-60c6-9480-709c-8cd597c6f5b0@redhat.com>

[-- Attachment #1.1: Type: text/plain, Size: 2364 bytes --]

Hi

Am 31.01.22 um 11:18 schrieb Javier Martinez Canillas:
> Hello Thomas,
> 
> On 1/31/22 10:18, Thomas Zimmermann wrote:
> 
> [snip]
> 
>>> There are some hacks in the driver though. For example it exposes an XRGB8888
>>> format even thought the OLED display is monochromatic and has 1 bit per pixel.
>>>
>>> The driver then goes and converts the XRGB8888 pixels first to grayscale and
>>> then to reverse mono. I took that idea from the repaper driver but that gives
>>> us the multiple copies that Geert was complaining about.
>>
>> This requires to update the console code for 1-bit BW output. The fbcon
>> side already supports this AFAIK. DRM's fbdev needs a few more branches
>> and something like a DRM_FORMAT_C1 fourcc. The XRGB8888 is really a
>> userspace requirement that is imposed by modern desktops. If DRM's
>> console has been updated, you could leave it out entirely.
>>
>> I could imagine that some simple userspace, such as Weston, comes with
>> support for palette formats and BW. Or there could be an entirely
>> separate program that puts graphics onto these displays.
>>
> 
> Yes, I understand the rationale of why the repaper driver is doing that way
> but was just pointing out because Geert mentioned that is not efficient.

It's a fair point, I think. People are concerned about resource 
consumption on low-end devices.

>   
> Maybe in the meantime we can add a drm_fb_gray8_to_mono_reversed() helper to
> drivers/gpu/drm/drm_format_helper.c since there is more than one driver that
> does the same ?

Sure.

> 
> It's not a big issue for this device really since the I2C bus is slow anyways
> and the multiple copies are not a bottleneck AFAICT.
> 
> I believe is worth to propose this driver as is and then try to optimize later.

Absolutely. If you post a cleaned-up version of the patch, I'd take a look.

> 
> Another thing that's missing is a DRM_MODE_CONNECTOR_I2C, because I used for
> now a DRM_MODE_CONNECTOR_Unknown.

That might have implications on userspace. Maybe ask around. (Not that 
we actually run userspace on the device).

Best regards
Thomas

> 
> Best regards,

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]