RE: [PATCH v4 2/7] drm: rcar-du: lvds: Improve identification of panels

2019-12-17 Thread Fabrizio Castro
Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart 
> Sent: 13 December 2019 21:22
> Subject: Re: [PATCH v4 2/7] drm: rcar-du: lvds: Improve identification of 
> panels
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Dec 06, 2019 at 04:32:49PM +, Fabrizio Castro wrote:
> > Dual-LVDS panels are mistakenly identified as bridges, this
> > commit replaces the current logic with a call to
> > drm_of_find_panel_or_bridge to sort that out.
> >
> > Signed-off-by: Fabrizio Castro 
> >
> > ---
> > v3->v4:
> > * New patch extracted from patch:
> >   "drm: rcar-du: lvds: Add dual-LVDS panels support"
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 45 
> > +
> >  1 file changed, 10 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> > b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 8c6c172..3cb0a83 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -21,6 +21,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >
> > @@ -705,10 +706,7 @@ static int rcar_lvds_parse_dt_companion(struct 
> > rcar_lvds *lvds)
> >  static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >  {
> > struct device_node *local_output = NULL;
> > -   struct device_node *remote_input = NULL;
> > struct device_node *remote = NULL;
> > -   struct device_node *node;
> > -   bool is_bridge = false;
> > int ret = 0;
> >
> > local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
> > @@ -736,45 +734,22 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> > goto done;
> > }
> 
> I think you can remove the calls above this too.
> drm_of_find_panel_or_bridge() calls of_graph_get_remote_node(), which in
> turn calls of_graph_get_endpoint_by_regs(),
> of_graph_get_remote_port_parent() and checks the status of the remote
> with of_device_is_available().

Will take those out in v5

Thanks,
Fab

> 
> >
> > -   remote_input = of_graph_get_remote_endpoint(local_output);
> > -
> > -   for_each_endpoint_of_node(remote, node) {
> > -   if (node != remote_input) {
> > -   /*
> > -* We've found one endpoint other than the input, this
> > -* must be a bridge.
> > -*/
> > -   is_bridge = true;
> > -   of_node_put(node);
> > -   break;
> > -   }
> > -   }
> > -
> > -   if (is_bridge) {
> > -   lvds->next_bridge = of_drm_find_bridge(remote);
> > -   if (!lvds->next_bridge) {
> > -   ret = -EPROBE_DEFER;
> > -   goto done;
> > -   }
> > +   ret = drm_of_find_panel_or_bridge(lvds->dev->of_node, 1, 0,
> > + &lvds->panel, &lvds->next_bridge);
> > +   if (ret)
> > +   goto done;
> >
> > -   if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> > -   lvds->dual_link = lvds->next_bridge->timings
> > -   ? lvds->next_bridge->timings->dual_link
> > -   : false;
> > -   } else {
> > -   lvds->panel = of_drm_find_panel(remote);
> > -   if (IS_ERR(lvds->panel)) {
> > -   ret = PTR_ERR(lvds->panel);
> > -   goto done;
> > -   }
> > -   }
> > +   if ((lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) &&
> > +   lvds->next_bridge)
> > +   lvds->dual_link = lvds->next_bridge->timings
> > +   ? lvds->next_bridge->timings->dual_link
> > +   : false;
> >
> > if (lvds->dual_link)
> > ret = rcar_lvds_parse_dt_companion(lvds);
> >
> >  done:
> > of_node_put(local_output);
> > -   of_node_put(remote_input);
> > of_node_put(remote);
> >
> > /*
> 
> --
> Regards,
> 
> Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v4 2/7] drm: rcar-du: lvds: Improve identification of panels

2019-12-13 Thread Laurent Pinchart
Hi Fabrizio,

Thank you for the patch.

On Fri, Dec 06, 2019 at 04:32:49PM +, Fabrizio Castro wrote:
> Dual-LVDS panels are mistakenly identified as bridges, this
> commit replaces the current logic with a call to
> drm_of_find_panel_or_bridge to sort that out.
> 
> Signed-off-by: Fabrizio Castro 
> 
> ---
> v3->v4:
> * New patch extracted from patch:
>   "drm: rcar-du: lvds: Add dual-LVDS panels support"
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 45 
> +
>  1 file changed, 10 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 8c6c172..3cb0a83 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -705,10 +706,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds 
> *lvds)
>  static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  {
>   struct device_node *local_output = NULL;
> - struct device_node *remote_input = NULL;
>   struct device_node *remote = NULL;
> - struct device_node *node;
> - bool is_bridge = false;
>   int ret = 0;
>  
>   local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
> @@ -736,45 +734,22 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>   goto done;
>   }

I think you can remove the calls above this too.
drm_of_find_panel_or_bridge() calls of_graph_get_remote_node(), which in
turn calls of_graph_get_endpoint_by_regs(),
of_graph_get_remote_port_parent() and checks the status of the remote
with of_device_is_available().

>  
> - remote_input = of_graph_get_remote_endpoint(local_output);
> -
> - for_each_endpoint_of_node(remote, node) {
> - if (node != remote_input) {
> - /*
> -  * We've found one endpoint other than the input, this
> -  * must be a bridge.
> -  */
> - is_bridge = true;
> - of_node_put(node);
> - break;
> - }
> - }
> -
> - if (is_bridge) {
> - lvds->next_bridge = of_drm_find_bridge(remote);
> - if (!lvds->next_bridge) {
> - ret = -EPROBE_DEFER;
> - goto done;
> - }
> + ret = drm_of_find_panel_or_bridge(lvds->dev->of_node, 1, 0,
> +   &lvds->panel, &lvds->next_bridge);
> + if (ret)
> + goto done;
>  
> - if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> - lvds->dual_link = lvds->next_bridge->timings
> - ? lvds->next_bridge->timings->dual_link
> - : false;
> - } else {
> - lvds->panel = of_drm_find_panel(remote);
> - if (IS_ERR(lvds->panel)) {
> - ret = PTR_ERR(lvds->panel);
> - goto done;
> - }
> - }
> + if ((lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) &&
> + lvds->next_bridge)
> + lvds->dual_link = lvds->next_bridge->timings
> + ? lvds->next_bridge->timings->dual_link
> + : false;
>  
>   if (lvds->dual_link)
>   ret = rcar_lvds_parse_dt_companion(lvds);
>  
>  done:
>   of_node_put(local_output);
> - of_node_put(remote_input);
>   of_node_put(remote);
>  
>   /*

-- 
Regards,

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


[PATCH v4 2/7] drm: rcar-du: lvds: Improve identification of panels

2019-12-09 Thread Fabrizio Castro
Dual-LVDS panels are mistakenly identified as bridges, this
commit replaces the current logic with a call to
drm_of_find_panel_or_bridge to sort that out.

Signed-off-by: Fabrizio Castro 

---
v3->v4:
* New patch extracted from patch:
  "drm: rcar-du: lvds: Add dual-LVDS panels support"
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 45 +
 1 file changed, 10 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 8c6c172..3cb0a83 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -705,10 +706,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds 
*lvds)
 static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 {
struct device_node *local_output = NULL;
-   struct device_node *remote_input = NULL;
struct device_node *remote = NULL;
-   struct device_node *node;
-   bool is_bridge = false;
int ret = 0;
 
local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
@@ -736,45 +734,22 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
goto done;
}
 
-   remote_input = of_graph_get_remote_endpoint(local_output);
-
-   for_each_endpoint_of_node(remote, node) {
-   if (node != remote_input) {
-   /*
-* We've found one endpoint other than the input, this
-* must be a bridge.
-*/
-   is_bridge = true;
-   of_node_put(node);
-   break;
-   }
-   }
-
-   if (is_bridge) {
-   lvds->next_bridge = of_drm_find_bridge(remote);
-   if (!lvds->next_bridge) {
-   ret = -EPROBE_DEFER;
-   goto done;
-   }
+   ret = drm_of_find_panel_or_bridge(lvds->dev->of_node, 1, 0,
+ &lvds->panel, &lvds->next_bridge);
+   if (ret)
+   goto done;
 
-   if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
-   lvds->dual_link = lvds->next_bridge->timings
-   ? lvds->next_bridge->timings->dual_link
-   : false;
-   } else {
-   lvds->panel = of_drm_find_panel(remote);
-   if (IS_ERR(lvds->panel)) {
-   ret = PTR_ERR(lvds->panel);
-   goto done;
-   }
-   }
+   if ((lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) &&
+   lvds->next_bridge)
+   lvds->dual_link = lvds->next_bridge->timings
+   ? lvds->next_bridge->timings->dual_link
+   : false;
 
if (lvds->dual_link)
ret = rcar_lvds_parse_dt_companion(lvds);
 
 done:
of_node_put(local_output);
-   of_node_put(remote_input);
of_node_put(remote);
 
/*
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel