[PATCH] drm/arcpgu: Get rid of "encoder-slave" property

[PATCH] drm/arcpgu: Get rid of "encoder-slave" property

[Alexey Brodkin]

We used to use "encoder-slave" property in PGU's
Device Tree node to refer to the encoder, but since there's
a way to find it with some code smarts we get rid of
obviously extra complication in PGU node.

Again inspired by ARM's HDLCD code.

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@linux.ie>
Cc: Rob Herring <robh+dt@kernel.org>
---
 arch/arc/boot/dts/axs10x_mb.dtsi |  1 -
 drivers/gpu/drm/arc/arcpgu_drv.c | 23 +++++++++++++++++------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi
index 41cfb29b62c1..2fe030186b9d 100644
--- a/arch/arc/boot/dts/axs10x_mb.dtsi
+++ b/arch/arc/boot/dts/axs10x_mb.dtsi
@@ -287,7 +287,6 @@
 		pgu@17000 {
 			compatible = "snps,arcpgu";
 			reg = <0x17000 0x400>;
-			encoder-slave = <&adv7511>;
 			clocks = <&pguclk>;
 			clock-names = "pxlclk";
 			memory-region = <&frame_buffer>;
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 5c82f52fba80..b1b2286bda95 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -19,6 +19,7 @@
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_atomic_helper.h>
+#include <linux/of_graph.h>
 #include <linux/of_reserved_mem.h>
 
 #include "arcpgu.h"
@@ -83,7 +84,7 @@ static int arcpgu_load(struct drm_device *drm)
 {
 	struct platform_device *pdev = to_platform_device(drm->dev);
 	struct arcpgu_drm_private *arcpgu;
-	struct device_node *encoder_node;
+	struct device_node *encoder, *port;
 	struct resource *res;
 	int ret;
 
@@ -118,14 +119,24 @@ static int arcpgu_load(struct drm_device *drm)
 	if (arc_pgu_setup_crtc(drm) < 0)
 		return -ENODEV;
 
-	/* find the encoder node and initialize it */
-	encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0);
-	if (encoder_node) {
-		ret = arcpgu_drm_hdmi_init(drm, encoder_node);
-		of_node_put(encoder_node);
+	/* There is only one output port inside each device, find it */
+	port = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
+
+	if (port) {
+		if (of_device_is_available(port))
+			encoder = of_graph_get_remote_port_parent(port);
+		of_node_put(port);
+	}
+
+	if (encoder && of_device_is_available(encoder)) {
+		dev_info(drm->dev, "Found encoder node %s, proceeding with it\n",
+			 encoder->name);
+		ret = arcpgu_drm_hdmi_init(drm, encoder);
+		of_node_put(encoder);
 		if (ret < 0)
 			return ret;
 	} else {
+		dev_info(drm->dev, "No encoder node, assume simulation\n");
 		ret = arcpgu_drm_sim_init(drm, NULL);
 		if (ret < 0)
 			return ret;
-- 
2.7.4

Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property

[Liviu Dudau]

On Fri, Mar 03, 2017 at 06:19:24PM +0300, Alexey Brodkin wrote:
> We used to use "encoder-slave" property in PGU's
> Device Tree node to refer to the encoder, but since there's
> a way to find it with some code smarts we get rid of
> obviously extra complication in PGU node.
> 
> Again inspired by ARM's HDLCD code.

Again, Reviewed-by: Liviu Dudau <Liviu.Dudau@arm.com>  :)

> 
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Rob Herring <robh+dt@kernel.org>
> ---
>  arch/arc/boot/dts/axs10x_mb.dtsi |  1 -
>  drivers/gpu/drm/arc/arcpgu_drv.c | 23 +++++++++++++++++------
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi
> index 41cfb29b62c1..2fe030186b9d 100644
> --- a/arch/arc/boot/dts/axs10x_mb.dtsi
> +++ b/arch/arc/boot/dts/axs10x_mb.dtsi
> @@ -287,7 +287,6 @@
>  		pgu@17000 {
>  			compatible = "snps,arcpgu";
>  			reg = <0x17000 0x400>;
> -			encoder-slave = <&adv7511>;
>  			clocks = <&pguclk>;
>  			clock-names = "pxlclk";
>  			memory-region = <&frame_buffer>;
> diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
> index 5c82f52fba80..b1b2286bda95 100644
> --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> @@ -19,6 +19,7 @@
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <linux/of_graph.h>
>  #include <linux/of_reserved_mem.h>
>  
>  #include "arcpgu.h"
> @@ -83,7 +84,7 @@ static int arcpgu_load(struct drm_device *drm)
>  {
>  	struct platform_device *pdev = to_platform_device(drm->dev);
>  	struct arcpgu_drm_private *arcpgu;
> -	struct device_node *encoder_node;
> +	struct device_node *encoder, *port;
>  	struct resource *res;
>  	int ret;
>  
> @@ -118,14 +119,24 @@ static int arcpgu_load(struct drm_device *drm)
>  	if (arc_pgu_setup_crtc(drm) < 0)
>  		return -ENODEV;
>  
> -	/* find the encoder node and initialize it */
> -	encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0);
> -	if (encoder_node) {
> -		ret = arcpgu_drm_hdmi_init(drm, encoder_node);
> -		of_node_put(encoder_node);
> +	/* There is only one output port inside each device, find it */
> +	port = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> +
> +	if (port) {
> +		if (of_device_is_available(port))
> +			encoder = of_graph_get_remote_port_parent(port);
> +		of_node_put(port);
> +	}

You must've been looking at some old version. Current version in -next uses
of_graph_get_remote_node() to replace all those lines you have added (see Rob
Herring's series to introduce of_graph_get_remote_node() function)

Best regards,
Liviu


> +
> +	if (encoder && of_device_is_available(encoder)) {
> +		dev_info(drm->dev, "Found encoder node %s, proceeding with it\n",
> +			 encoder->name);
> +		ret = arcpgu_drm_hdmi_init(drm, encoder);
> +		of_node_put(encoder);
>  		if (ret < 0)
>  			return ret;
>  	} else {
> +		dev_info(drm->dev, "No encoder node, assume simulation\n");
>  		ret = arcpgu_drm_sim_init(drm, NULL);
>  		if (ret < 0)
>  			return ret;
> -- 
> 2.7.4
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property

[Alexey Brodkin]

Hi Liviu,

On Fri, 2017-03-03 at 16:28 +0000, Liviu Dudau wrote:
> On Fri, Mar 03, 2017 at 06:19:24PM +0300, Alexey Brodkin wrote:
> > 
> > -	/* find the encoder node and initialize it */
> > -	encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0);
> > -	if (encoder_node) {
> > -		ret = arcpgu_drm_hdmi_init(drm, encoder_node);
> > -		of_node_put(encoder_node);
> > +	/* There is only one output port inside each device, find it */
> > +	port = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> > +
> > +	if (port) {
> > +		if (of_device_is_available(port))
> > +			encoder = of_graph_get_remote_port_parent(port);
> > +		of_node_put(port);
> > +	}
> 
> You must've been looking at some old version. Current version in -next uses
> of_graph_get_remote_node() to replace all those lines you have added (see Rob
> Herring's series to introduce of_graph_get_remote_node() function)

Hm, I'm not on Linus' master tree [1] and so I thought I was quite up to date :)
Still I made a check of linux-next and don't see any changes in
"drivers/gpu/drm/arm" compared to Linus' tree.

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/arm?id=e4563f6ba71792c77aeccb2092cc23149b44e642
[2] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/arm?id=e4563f6ba71792c77aeccb2092cc23149b44e642

Could you please clarify which exact tree did you mean?

Anyways I just tried to rebase my patch on top of linux-next tree and now
video output is broken for me - I only see some garbage on top of the screen
so I'll need to investigate it first before moving forward with stuff you
proposed :)

-Alexey
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property

[Liviu.Dudau]

On Fri, Mar 03, 2017 at 05:48:19PM +0000, Alexey Brodkin wrote:
> Hi Liviu,
> 
> On Fri, 2017-03-03 at 16:28 +0000, Liviu Dudau wrote:
> > On Fri, Mar 03, 2017 at 06:19:24PM +0300, Alexey Brodkin wrote:
> > > 
> > > -	/* find the encoder node and initialize it */
> > > -	encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0);
> > > -	if (encoder_node) {
> > > -		ret = arcpgu_drm_hdmi_init(drm, encoder_node);
> > > -		of_node_put(encoder_node);
> > > +	/* There is only one output port inside each device, find it */
> > > +	port = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> > > +
> > > +	if (port) {
> > > +		if (of_device_is_available(port))
> > > +			encoder = of_graph_get_remote_port_parent(port);
> > > +		of_node_put(port);
> > > +	}
> > 
> > You must've been looking at some old version. Current version in -next uses
> > of_graph_get_remote_node() to replace all those lines you have added (see Rob
> > Herring's series to introduce of_graph_get_remote_node() function)
> 
> Hm, I'm not on Linus' master tree [1] and so I thought I was quite up to date :)
> Still I made a check of linux-next and don't see any changes in
> "drivers/gpu/drm/arm" compared to Linus' tree.
> 
> [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/arm?id=e4563f6ba71792c77aeccb2092cc23149b44e642
> [2] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/arm?id=e4563f6ba71792c77aeccb2092cc23149b44e642
> 
> Could you please clarify which exact tree did you mean?

Sorry, I thought the series got pulled by one of the DRM trees, but it looks like
I was wrong. I was carrying a private copy in my internal tree, waiting for the
moment when it got pulled into drm-next or drm-misc-next.

Rob, do you have an update on your series introducing of_graph_get_remote_node() ?

Best regards,
Liviu

> 
> Anyways I just tried to rebase my patch on top of linux-next tree and now
> video output is broken for me - I only see some garbage on top of the screen
> so I'll need to investigate it first before moving forward with stuff you
> proposed :)
> 
> -Alexey

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property

[Alexey Brodkin]

Hi Liviu, Rob,

On Fri, 2017-03-03 at 18:21 +0000, Liviu.Dudau@arm.com wrote:
> On Fri, Mar 03, 2017 at 05:48:19PM +0000, Alexey Brodkin wrote:
> > 
> > Hi Liviu,
> > 
> > On Fri, 2017-03-03 at 16:28 +0000, Liviu Dudau wrote:
> > > 
> > > On Fri, Mar 03, 2017 at 06:19:24PM +0300, Alexey Brodkin wrote:
> > > > 
> > > > 
> > > > -	/* find the encoder node and initialize it */
> > > > -	encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0);
> > > > -	if (encoder_node) {
> > > > -		ret = arcpgu_drm_hdmi_init(drm, encoder_node);
> > > > -		of_node_put(encoder_node);
> > > > +	/* There is only one output port inside each device, find it */
> > > > +	port = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> > > > +
> > > > +	if (port) {
> > > > +		if (of_device_is_available(port))
> > > > +			encoder = of_graph_get_remote_port_parent(port);
> > > > +		of_node_put(port);
> > > > +	}
> > > 
> > > You must've been looking at some old version. Current version in -next uses
> > > of_graph_get_remote_node() to replace all those lines you have added (see Rob
> > > Herring's series to introduce of_graph_get_remote_node() function)
> > 
> > Hm, I'm not on Linus' master tree [1] and so I thought I was quite up to date :)
> > Still I made a check of linux-next and don't see any changes in
> > "drivers/gpu/drm/arm" compared to Linus' tree.
> > 
> > [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__git.kernel.org_cgit_linux_kernel_git_torvalds_linux.git_commit_drivers_gpu_drm_arm-3Fid-3D
> > e4563f6ba71792c77aeccb2092cc23149b44e642&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=SI66ngnnXy33ncb8m5H4La2
> > T1SzSEiiP7hc_XsRahEc&s=uaswjVXcjYDrUosOkO_UpTMqJMWTT-LLPrg5JE6-t-8&e= 
> > [2] https://urldefense.proofpoint.com/v2/url?u=http-3A__git.kernel.org_cgit_linux_kernel_git_next_linux-2Dnext.git_commit_drivers_gpu_drm_arm-3Fid
> > -3De4563f6ba71792c77aeccb2092cc23149b44e642&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=SI66ngnnXy33ncb8m5H4
> > La2T1SzSEiiP7hc_XsRahEc&s=hl9Y6s3K9LwLL1M2WnL3ODax_V-ZRh8k1iTiyctIqU4&e= 
> > 
> > Could you please clarify which exact tree did you mean?
> 
> Sorry, I thought the series got pulled by one of the DRM trees, but it looks like
> I was wrong. I was carrying a private copy in my internal tree, waiting for the
> moment when it got pulled into drm-next or drm-misc-next.
> 
> Rob, do you have an update on your series introducing of_graph_get_remote_node() ?

For some reason I cannot find any relevant commits in linux-next tree even today.
Could you please point me to either any random git tree with mentioned above change or
maybe just mailing list where this patch was sent?

I'd like to implement the same fix in ARCPGU and call it a day finally.

-Alexey

Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property

[Liviu.Dudau]

On Wed, Mar 29, 2017 at 01:34:00PM +0000, Alexey Brodkin wrote:
> Hi Liviu, Rob,

Hi Alexey,

> 
> On Fri, 2017-03-03 at 18:21 +0000, Liviu.Dudau@arm.com wrote:
> > On Fri, Mar 03, 2017 at 05:48:19PM +0000, Alexey Brodkin wrote:
> > > 
> > > Hi Liviu,
> > > 
> > > On Fri, 2017-03-03 at 16:28 +0000, Liviu Dudau wrote:
> > > > 
> > > > On Fri, Mar 03, 2017 at 06:19:24PM +0300, Alexey Brodkin wrote:
> > > > > 
> > > > > 
> > > > > -	/* find the encoder node and initialize it */
> > > > > -	encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0);
> > > > > -	if (encoder_node) {
> > > > > -		ret = arcpgu_drm_hdmi_init(drm, encoder_node);
> > > > > -		of_node_put(encoder_node);
> > > > > +	/* There is only one output port inside each device, find it */
> > > > > +	port = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> > > > > +
> > > > > +	if (port) {
> > > > > +		if (of_device_is_available(port))
> > > > > +			encoder = of_graph_get_remote_port_parent(port);
> > > > > +		of_node_put(port);
> > > > > +	}
> > > > 
> > > > You must've been looking at some old version. Current version in -next uses
> > > > of_graph_get_remote_node() to replace all those lines you have added (see Rob
> > > > Herring's series to introduce of_graph_get_remote_node() function)
> > > 
> > > Hm, I'm not on Linus' master tree [1] and so I thought I was quite up to date :)
> > > Still I made a check of linux-next and don't see any changes in
> > > "drivers/gpu/drm/arm" compared to Linus' tree.
> > > 
> > > [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__git.kernel.org_cgit_linux_kernel_git_torvalds_linux.git_commit_drivers_gpu_drm_arm-3Fid-3D
> > > e4563f6ba71792c77aeccb2092cc23149b44e642&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=SI66ngnnXy33ncb8m5H4La2
> > > T1SzSEiiP7hc_XsRahEc&s=uaswjVXcjYDrUosOkO_UpTMqJMWTT-LLPrg5JE6-t-8&e= 
> > > [2] https://urldefense.proofpoint.com/v2/url?u=http-3A__git.kernel.org_cgit_linux_kernel_git_next_linux-2Dnext.git_commit_drivers_gpu_drm_arm-3Fid
> > > -3De4563f6ba71792c77aeccb2092cc23149b44e642&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=SI66ngnnXy33ncb8m5H4
> > > La2T1SzSEiiP7hc_XsRahEc&s=hl9Y6s3K9LwLL1M2WnL3ODax_V-ZRh8k1iTiyctIqU4&e= 
> > > 
> > > Could you please clarify which exact tree did you mean?
> > 
> > Sorry, I thought the series got pulled by one of the DRM trees, but it looks like
> > I was wrong. I was carrying a private copy in my internal tree, waiting for the
> > moment when it got pulled into drm-next or drm-misc-next.
> > 
> > Rob, do you have an update on your series introducing of_graph_get_remote_node() ?
> 
> For some reason I cannot find any relevant commits in linux-next tree even today.
> Could you please point me to either any random git tree with mentioned above change or
> maybe just mailing list where this patch was sent?

Not sure why Rob hasn't added it to linux-next, but (according to Rob) this is the latest version:

https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/log/?h=of-graph-helpers

Best regards,
Liviu

> 
> I'd like to implement the same fix in ARCPGU and call it a day finally.
> 
> -Alexey

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯