RE: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Sent: 02 August 2019 09:20
> Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
> > If the display comes with two ports, assume it supports dual
> > link.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@xxxxxxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 2d54ae5..97c51c2 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >  			ret = -EPROBE_DEFER;
> >  			goto done;
> >  		}
> > +		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> > +			lvds->dual_link = of_graph_get_endpoint_count(remote)
> > +					== 2;
> 
> This is a bit of a hack, as I think the information should be queried
> from the panel, like we do for bridges. I'd say we can live with this
> for now, but as the data swap flag should come from the panel as well,
> we will need infrastructure for that, and we can as well through the
> dual link flag there at the same time.

I totally agree, this is a nasty hack, my series is missing the infrastructure
for describing this information

> 
> I think we should use the drm_bridge_timings structure for this purpose,
> as it would make life more difficult for users of drm_bridge and
> drm_panel to have two different structures (especially when wrapping a
> drm_panel with drm_panel_bridge_add()). The structure could be renamed
> if desired.

I am not too sure using drm_bridge_timings for panels would make everybody
happy? Is there any alternative? Perhaps this calls for a new struct we could
embed in both drm_bridge_timings and some drm_panel_<whatever> data
structure?

Thanks,
Fab
 
> 
> >  	}
> >
> >  	if (lvds->dual_link) {
> 
> --
> Regards,
> 
> Laurent Pinchart