Re: [PATCH 1/2] drm/panel: Add and fill drm_panel type field

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Nicolas.Ferre@microchip.com
Cc: alexandre.belloni@bootlin.com, narmstrong@baylibre.com,
	agx@sigxcpu.org, dri-devel@lists.freedesktop.org,
	paul@crapouillou.net, thierry.reding@gmail.com, sam@ravnborg.org,
	stefan@olimex.com, tomi.valkeinen@ti.com,
	Tudor.Ambarus@microchip.com, Ludovic.Desroches@microchip.com,
	boris.brezillon@collabora.com, jagan@amarulasolutions.com,
	robert.chiras@nxp.com, alexandre.torgue@st.com, jonas@kwiboo.se,
	jsarha@ti.com, vincent.abriou@st.com, jernej.skrabec@siol.net,
	kernel@puri.sm, philippe.cornu@st.com, yannick.fertre@st.com,
	kieran.bingham+renesas@ideasonboard.com,
	mcoquelin.stm32@gmail.com, Claudiu.Beznea@microchip.com
Subject: Re: [PATCH 1/2] drm/panel: Add and fill drm_panel type field
Date: Fri, 23 Aug 2019 14:10:13 +0300	[thread overview]
Message-ID: <20190823111013.GA5028@pendragon.ideasonboard.com> (raw)
In-Reply-To: <01887051-7cf7-cf52-65d1-85d60c3dd02c@microchip.com>

Hi Nicolas,

On Fri, Aug 23, 2019 at 07:30:07AM +0000, Nicolas.Ferre@microchip.com wrote:
> On 23/08/2019 at 03:40, Laurent Pinchart wrote:
> > Add a type field to the drm_panel structure to report the panel type,
> > using DRM_MODE_CONNECTOR_* macros (the values that make sense are LVDS,
> > eDP, DSI and DPI). This will be used to initialise the corresponding
> > connector type.
> 
> With Microchip/Atmel driver, we mainly (only) use the "Unknown" type of 
> connector because our hardware simply uses RGB wires in parallel.

That's called DPI (Display Pixel Interface, sometimes also referred to
as Display Parallel Interface) :-)

> Should we move to another connector type (maybe now that it's created 
> and it was not, back when we chose the "Unknown" one)?

I think DRM_MODE_CONNECTOR_DPI would be best, yes.

> What would be the consequences if we move (silently?) to another type 
> and particularly on the command line argument like the ones we currently 
> use: "Unknown-1:800x480-16"?

That will be nasty to handle :-( As much as I'd love to ignore that
issue, I don't think we can. At the same time it shouldn't prevent us
from moving forward and exposing the real connector type. One option
could be to keep the extra type argument to drm_panel_bridge_add() to
force the connector type, and add another variant of the function that
would derive it automatically from the panel type. Drivers could then
decide to switch to the new variant on a case-by-case basis. Still, that
won't solve your issue, as I'm not sure how you would be able to decide
which variant to call if we want newer systems to expose the right panel
type while keeping backward compatibility.

Another option would be to hack the command line argument parsing to
convert Unknown-* to a panel type when there's no connector with an
unknown type, but that seems fragile.

Any other proposal ? :-)

> > Update all panel drivers to fill the new field.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel