Re: [RFC PATCH] drm/panel: simple: panel-dpi: use bus-format to set bpc and bus_format

From: Marek Vasut <marex@denx.de>
To: Maxime Ripard <maxime@cerno.tech>,
	Max Krummenacher <max.oss.09@gmail.com>
Cc: dri-devel@lists.freedesktop.org,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Fabio Estevam <festevam@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	DenysDrozdov <denys.drozdov@toradex.com>,
	David Airlie <airlied@linux.ie>,
	Christoph Niedermaier <cniedermaier@dh-electronics.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Sam Ravnborg <sam@ravnborg.org>, Shawn Guo <shawnguo@kernel.org>,
	Daniel Vetter <daniel@ffwll.ch>,
	NXP Linux Team <linux-imx@nxp.com>,
	Max Krummenacher <max.krummenacher@toradex.com>
Subject: Re: [RFC PATCH] drm/panel: simple: panel-dpi: use bus-format to set bpc and bus_format
Date: Wed, 23 Feb 2022 14:45:30 +0100	[thread overview]
Message-ID: <b5f471f4-0712-b798-efb8-b5b481cdb898@denx.de> (raw)
In-Reply-To: <20220223134154.oo7xhf37bgtvm3ai@houat>

On 2/23/22 14:41, Maxime Ripard wrote:
> Hi,
> 
> On Tue, Feb 22, 2022 at 09:47:23AM +0100, Max Krummenacher wrote:
>> Use the new property bus-format to set the enum bus_format and bpc.
>> Completes: commit 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support")
>>
>> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
>>
>> ---
>>
>>   drivers/gpu/drm/panel/panel-simple.c | 32 ++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> Relates to the discussion: https://lore.kernel.org/all/20220201110717.3585-1-cniedermaier@dh-electronics.com/
>>
>> Max
>>
>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
>> index c5f133667a2d..5c07260de71c 100644
>> --- a/drivers/gpu/drm/panel/panel-simple.c
>> +++ b/drivers/gpu/drm/panel/panel-simple.c
>> @@ -453,6 +453,7 @@ static int panel_dpi_probe(struct device *dev,
>>   	struct panel_desc *desc;
>>   	unsigned int bus_flags;
>>   	struct videomode vm;
>> +	const char *format = "";
>>   	int ret;
>>   
>>   	np = dev->of_node;
>> @@ -477,6 +478,37 @@ static int panel_dpi_probe(struct device *dev,
>>   	of_property_read_u32(np, "width-mm", &desc->size.width);
>>   	of_property_read_u32(np, "height-mm", &desc->size.height);
>>   
>> +	of_property_read_string(np, "bus-format", &format);
>> +	if (!strcmp(format, "BGR888_1X24")) {
>> +		desc->bpc = 8;
>> +		desc->bus_format = MEDIA_BUS_FMT_BGR888_1X24;
>> +	} else if (!strcmp(format, "GBR888_1X24")) {
>> +		desc->bpc = 8;
>> +		desc->bus_format = MEDIA_BUS_FMT_GBR888_1X24;
>> +	} else if (!strcmp(format, "RBG888_1X24")) {
>> +		desc->bpc = 8;
>> +		desc->bus_format = MEDIA_BUS_FMT_RBG888_1X24;
>> +	} else if (!strcmp(format, "RGB444_1X12")) {
>> +		desc->bpc = 6;
>> +		desc->bus_format = MEDIA_BUS_FMT_RGB444_1X12;
>> +	} else if (!strcmp(format, "RGB565_1X16")) {
>> +		desc->bpc = 6;
>> +		desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
>> +	} else if (!strcmp(format, "RGB666_1X18")) {
>> +		desc->bpc = 6;
>> +		desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
>> +	} else if (!strcmp(format, "RGB666_1X24_CPADHI")) {
>> +		desc->bpc = 6;
>> +		desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI;
>> +	} else if (!strcmp(format, "RGB888_1X24")) {
>> +		desc->bpc = 8;
>> +		desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
>> +	} else {
>> +		dev_err(dev, "%pOF: missing or unknown bus-format property\n",
>> +			np);
>> +		return -EINVAL;
>> +	}
>> +
> 
> It doesn't seem right, really. We can't the bus format / bpc be inferred
> from the compatible? I'd expect two panels that don't have the same bus
> format to not be claimed as compatible.

Which compatible ?

Note that this is for panel-dpi compatible, i.e. the panel which has 
timings specified in DT (and needs bus format specified there too).

I agree this doesn't look right however, some more generic color channel 
width/shift/mapping might be better.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel