diff mbox series

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

Message ID 1564731249-22671-8-git-send-email-fabrizio.castro@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Kieran Bingham
Headers show
Series Add dual-LVDS panel support to EK874 | expand

Commit Message

Fabrizio Castro Aug. 2, 2019, 7:34 a.m. UTC
If the display comes with two ports, assume it supports dual
link.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Laurent Pinchart Aug. 2, 2019, 8:20 a.m. UTC | #1
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@bp.renesas.com>
> ---
>  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 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.

>  	}
>  
>  	if (lvds->dual_link) {
Fabrizio Castro Aug. 5, 2019, 9:12 a.m. UTC | #2
Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 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@bp.renesas.com>
> > ---
> >  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
Laurent Pinchart Aug. 5, 2019, 9:48 a.m. UTC | #3
Hi Fabrizio,

On Mon, Aug 05, 2019 at 09:12:34AM +0000, Fabrizio Castro wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 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@bp.renesas.com>
> > > ---
> > >  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?

I think we could simply rename the structure, all its fields apply to
panels too.

> > >  	}
> > >
> > >  	if (lvds->dual_link) {
Fabrizio Castro Aug. 5, 2019, 10:07 a.m. UTC | #4
Hi Laurent,

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 05 August 2019 10:49
> Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
> 
> Hi Fabrizio,
> 
> On Mon, Aug 05, 2019 at 09:12:34AM +0000, Fabrizio Castro wrote:
> > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 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@bp.renesas.com>
> > > > ---
> > > >  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?
> 
> I think we could simply rename the structure, all its fields apply to
> panels too.

Ok, will give this a try.

Thanks,
Fab

> 
> > > >  	}
> > > >
> > > >  	if (lvds->dual_link) {
> 
> --
> Regards,
> 
> Laurent Pinchart
diff mbox series

Patch

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;
 	}
 
 	if (lvds->dual_link) {