| 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 |
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) {
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
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) {
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 --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) {
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(+)