[RFC][PATCH] Revert "drm/panel-simple: drop use of data-mapping property"

[Laurent Pinchart]

Hi Christoph,

Thank you for the patch.

On Tue, Feb 01, 2022 at 12:07:17PM +0100, Christoph Niedermaier wrote:
> Without the data-mapping devicetree property my display won't
> work properly. It is flickering, because the bus flags won't
> be assigned without a defined bus format by the imx parallel
> display driver. There was a discussion about the removal [1]
> and an agreement that a better solution is needed, but it is
> missing so far. So what would be the better approach?
>
> [1] https://patchwork.freedesktop.org/patch/357659/?series=74705&rev=1
> 
> This reverts commit d021d751c14752a0266865700f6f212fab40a18c.
> 
> Signed-off-by: Christoph Niedermaier <cniedermaier at dh-electronics.com>
> Cc: Marek Vasut <marex at denx.de>
> Cc: Sam Ravnborg <sam at ravnborg.org>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Maxime Ripard <mripard at kernel.org>
> Cc: Philipp Zabel <p.zabel at pengutronix.de>
> Cc: David Airlie <airlied at linux.ie>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: Shawn Guo <shawnguo at kernel.org>
> Cc: Sascha Hauer <s.hauer at pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel at pengutronix.de>
> Cc: Fabio Estevam <festevam at gmail.com>
> Cc: NXP Linux Team <linux-imx at nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org
> To: dri-devel at lists.freedesktop.org
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 3c08f9827acf..2c683d94a3f3 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 *mapping;
>  	int ret;
>  
>  	np = dev->of_node;
> @@ -477,6 +478,16 @@ 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, "data-mapping", &mapping);
> +	if (!strcmp(mapping, "rgb24"))
> +		desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +	else if (!strcmp(mapping, "rgb565"))
> +		desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16;
> +	else if (!strcmp(mapping, "bgr666"))
> +		desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18;
> +	else if (!strcmp(mapping, "lvds666"))
> +		desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI;

You're right that there's an issue, but a revert isn't the right option.
The commit you're reverting never made it in a stable release, because
it was deemed to not be a good enough option.

First of all, any attempt to fix this should include an update to the DT
binding. Second, as this is about DPI panels, the LVDS option should be
dropped. Finally, I've shared some initial thoughts in [1], maybe you
can reply to that e-mail to continue the discussion there ?

https://lore.kernel.org/all/20200303185531.GJ11333@pendragon.ideasonboard.com/

> +
>  	/* Extract bus_flags from display_timing */
>  	bus_flags = 0;
>  	vm.flags = timing->flags;

-- 
Regards,

Laurent Pinchart