* [PATCH] drm/arcpgu: Get rid of "encoder-slave" property
@ 2017-03-03 15:19 Alexey Brodkin
2017-03-03 16:28 ` Liviu Dudau
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Brodkin @ 2017-03-03 15:19 UTC (permalink / raw)
To: dri-devel
Cc: linux-snps-arc, linux-kernel, devicetree, Alexey Brodkin,
Liviu Dudau, Daniel Vetter, David Airlie, Rob Herring
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property
2017-03-03 15:19 [PATCH] drm/arcpgu: Get rid of "encoder-slave" property Alexey Brodkin
@ 2017-03-03 16:28 ` Liviu Dudau
2017-03-03 17:48 ` Alexey Brodkin
0 siblings, 1 reply; 6+ messages in thread
From: Liviu Dudau @ 2017-03-03 16:28 UTC (permalink / raw)
To: Alexey Brodkin
Cc: dri-devel, linux-snps-arc, linux-kernel, devicetree,
Daniel Vetter, David Airlie, Rob Herring
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! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property
2017-03-03 16:28 ` Liviu Dudau
@ 2017-03-03 17:48 ` Alexey Brodkin
2017-03-03 18:21 ` Liviu.Dudau
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Brodkin @ 2017-03-03 17:48 UTC (permalink / raw)
To: Liviu.Dudau
Cc: dri-devel, linux-kernel, daniel, airlied, robh+dt,
linux-snps-arc, devicetree
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property
2017-03-03 17:48 ` Alexey Brodkin
@ 2017-03-03 18:21 ` Liviu.Dudau
2017-03-29 13:34 ` Alexey Brodkin
0 siblings, 1 reply; 6+ messages in thread
From: Liviu.Dudau @ 2017-03-03 18:21 UTC (permalink / raw)
To: Alexey Brodkin
Cc: Rob Herring, dri-devel, linux-kernel, daniel, airlied, robh+dt,
linux-snps-arc, devicetree
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! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property
2017-03-03 18:21 ` Liviu.Dudau
@ 2017-03-29 13:34 ` Alexey Brodkin
2017-03-29 13:48 ` Liviu.Dudau
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Brodkin @ 2017-03-29 13:34 UTC (permalink / raw)
To: Liviu.Dudau, robh
Cc: linux-kernel, robh+dt, devicetree, linux-snps-arc,
Eugeniy Paltsev, daniel, dri-devel, airlied
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property
2017-03-29 13:34 ` Alexey Brodkin
@ 2017-03-29 13:48 ` Liviu.Dudau
0 siblings, 0 replies; 6+ messages in thread
From: Liviu.Dudau @ 2017-03-29 13:48 UTC (permalink / raw)
To: Alexey Brodkin
Cc: robh, linux-kernel, robh+dt, devicetree, linux-snps-arc,
Eugeniy Paltsev, daniel, dri-devel, airlied
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! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-29 13:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-03 15:19 [PATCH] drm/arcpgu: Get rid of "encoder-slave" property Alexey Brodkin
2017-03-03 16:28 ` Liviu Dudau
2017-03-03 17:48 ` Alexey Brodkin
2017-03-03 18:21 ` Liviu.Dudau
2017-03-29 13:34 ` Alexey Brodkin
2017-03-29 13:48 ` Liviu.Dudau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).