Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Eric Anholt]

Rob Herring  writes:

> Convert drivers to use the new of_graph_get_remote_node() helper
> instead of parsing the endpoint node and then getting the remote device
> node. Now drivers can just specify the device node and which
> port/endpoint and get back the connected remote device node. The details
> of the graph binding are nicely abstracted into the core OF graph code.
>
> This changes some error messages to debug messages (in the graph core).
> Graph connections are often "no connects" depending on the particular
> board, so we want to avoid spurious messages. Plus the kernel is not a
> DT validator.

For vc4,

Tested-by: Eric Anholt 

Thanks for making this easier.


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

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Liviu Dudau]

Hi Rob,

On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote:
> Convert drivers to use the new of_graph_get_remote_node() helper
> instead of parsing the endpoint node and then getting the remote device
> node. Now drivers can just specify the device node and which
> port/endpoint and get back the connected remote device node. The details
> of the graph binding are nicely abstracted into the core OF graph code.
> 
> This changes some error messages to debug messages (in the graph core).
> Graph connections are often "no connects" depending on the particular
> board, so we want to avoid spurious messages. Plus the kernel is not a
> DT validator.
> 
> Signed-off-by: Rob Herring 
> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++---
>  drivers/gpu/drm/arm/malidp_drv.c| 29 ++-

For the HDLCD and Mali DP part you can also add:

Tested-by: Liviu Dudau 

Best regards,
Liviu

>  drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 +
>  drivers/gpu/drm/bridge/dumb-vga-dac.c   | 15 ++
>  drivers/gpu/drm/bridge/ti-tfp410.c  | 15 ++
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++---
>  drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +--
>  drivers/gpu/drm/mediatek/mtk_dpi.c  | 12 ++---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++
>  drivers/gpu/drm/meson/meson_drv.c   | 12 ++---
>  drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++-
>  drivers/gpu/drm/msm/dsi/dsi_host.c  |  3 +-
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 +
>  drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 
> +++--
>  drivers/gpu/drm/vc4/vc4_dpi.c   | 15 ++
>  20 files changed, 64 insertions(+), 351 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index e5f4f4a6546d..0f70f5fe9970 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -430,29 +430,13 @@ static int compare_dev(struct device *dev, void *data)
>  
>  static int hdlcd_probe(struct platform_device *pdev)
>  {
> - struct device_node *port, *ep;
> + struct device_node *port;
>   struct component_match *match = NULL;
>  
> - if (!pdev->dev.of_node)
> - return -ENODEV;
> -
>   /* there is only one output port inside each device, find it */
> - ep = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> - if (!ep)
> - return -ENODEV;
> -
> - if (!of_device_is_available(ep)) {
> - of_node_put(ep);
> + port = of_graph_get_remote_node(pdev->dev.of_node, 0, 0);
> + if (!port)
>   return -ENODEV;
> - }
> -
> - /* add the remote encoder port as component */
> - port = of_graph_get_remote_port_parent(ep);
> - of_node_put(ep);
> - if (!port || !of_device_is_available(port)) {
> - of_node_put(port);
> - return -EAGAIN;
> - }
>  
>   drm_of_component_match_add(>dev, , compare_dev, port);
>   of_node_put(port);
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c 
> b/drivers/gpu/drm/arm/malidp_drv.c
> index 32f746e31379..bfa04be7f5de 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -262,7 +262,6 @@ static int malidp_bind(struct device *dev)
>  {
>   struct resource *res;
>   struct drm_device *drm;
> - struct device_node *ep;
>   struct malidp_drm *malidp;
>   struct malidp_hw_device *hwdev;
>   struct platform_device *pdev = to_platform_device(dev);
> @@ -360,12 +359,7 @@ static int malidp_bind(struct device *dev)
>   goto init_fail;
>  
>   /* Set the CRTC's port so that the encoder component can find it */
> - ep = of_graph_get_next_endpoint(dev->of_node, NULL);
> - if (!ep) {
> - ret = -EINVAL;
> - goto port_fail;
> - }
> - malidp->crtc.port = of_get_next_parent(ep);
> + malidp->crtc.port = of_graph_get_port_by_id(dev->of_node, 0);
>  
>   ret = component_bind_all(dev, drm);
>   if (ret) {
> @@ -418,9 +412,7 @@ static int malidp_bind(struct device *dev)
>  irq_init_fail:
>   component_unbind_all(dev, drm);
>  bind_fail:
> - of_node_put(malidp->crtc.port);
>   malidp->crtc.port = NULL;
> -port_fail:
>   malidp_fini(drm);
>  init_fail:
>   drm->dev_private = NULL;
> @@ -478,29 +470,16 @@ static int malidp_compare_dev(struct device *dev, void 
> *data)
>  
>  static int malidp_platform_probe(struct platform_device *pdev)
>  {
> - 

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Liviu Dudau]

On Mon, Feb 06, 2017 at 05:34:07PM +, Russell King - ARM Linux wrote:
> On Mon, Feb 06, 2017 at 05:23:06PM +, Liviu Dudau wrote:
> > On Mon, Feb 06, 2017 at 11:09:49AM -0600, Rob Herring wrote:
> > > On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote:
> > > > On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote:
> > > > > - /* add the remote encoder port as component */
> > > > > - port = of_graph_get_remote_port_parent(ep);
> > > > > - of_node_put(ep);
> > > > > - if (!port || !of_device_is_available(port)) {
> > > > > - of_node_put(port);
> > > > > - return -EAGAIN;
> > > > 
> > > > The HDLCD change looks reasonable except for this -EAGAIN business. 
> > > > I'll have to
> > > > test your changes on my setup to see how this affects having the 
> > > > encoder as a module.
> > > 
> > > What are you expecting to happen with -EAGAIN? This one was a bit of an 
> > > oddball.
> > 
> > When both the HDLCD and the TDA998x drivers are compiled as modules, the
> > order in which they are inserted can be somewhat random (due to testing).
> 
> Not really "due to testing" but if you run a real distro, they tend to
> have a multi-threaded behaviour when loading kernel modules at boot.

Yeah, a lot of times I'm using a toy "distribution" (buildroot) as it boots
faster under ARM models than a "real" (read systemd-based) distro would.

> 
> > It is at that time when you want the probe of HDLCD to be retried on the
> > insmod-ing of the tda998x.ko rather than fail entirely.
> 
> -EAGAIN doesn't get you that, and in any case, solving that problem is
> exactly why the component API exists - so that DRM only comes up once
> all the necessary components are available.
> 
> -EAGAIN also doesn't get you that from inside a probe function - such
> an error will be reported in the kernel log, and no further action
> will be taken (the device driver probe will be failed, and not
> automatically retried.

I stand corrected on the behaviour of the driver then. That was the original
intent, to generate a re-probe of the driver.

> 
> The only case that we automatically retry is if a driver returns
> -EPROBE_DEFER.  Everything else causes a probe failure.

OK, I will fix the driver if Rob's patch still requires it.

Best regards,
Liviu

> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

-- 

| 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 3/5] drm: convert drivers to use of_graph_get_remote_node

[Russell King - ARM Linux]

On Mon, Feb 06, 2017 at 05:55:33PM +, Liviu Dudau wrote:
> OK, I will fix the driver if Rob's patch still requires it.

I don't think you ever needed it.  As Rob says, what you're testing
won't ever change unless you're using overlays - it's certainly not
dependent on the tda998x module being loaded or not, or even the
tda998x driver being bound or not.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Rob Herring]

On Mon, Feb 6, 2017 at 11:23 AM, Liviu Dudau  wrote:
> On Mon, Feb 06, 2017 at 11:09:49AM -0600, Rob Herring wrote:
>> On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote:
>> > On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote:
>> > > Convert drivers to use the new of_graph_get_remote_node() helper
>> > > instead of parsing the endpoint node and then getting the remote device
>> > > node. Now drivers can just specify the device node and which
>> > > port/endpoint and get back the connected remote device node. The details
>> > > of the graph binding are nicely abstracted into the core OF graph code.
>> > >
>> > > This changes some error messages to debug messages (in the graph core).
>> > > Graph connections are often "no connects" depending on the particular
>> > > board, so we want to avoid spurious messages. Plus the kernel is not a
>> > > DT validator.

[...]

>> > > - /* add the remote encoder port as component */
>> > > - port = of_graph_get_remote_port_parent(ep);
>> > > - of_node_put(ep);
>> > > - if (!port || !of_device_is_available(port)) {
>> > > - of_node_put(port);
>> > > - return -EAGAIN;
>> >
>> > The HDLCD change looks reasonable except for this -EAGAIN business. I'll 
>> > have to
>> > test your changes on my setup to see how this affects having the encoder 
>> > as a module.
>>
>> What are you expecting to happen with -EAGAIN? This one was a bit of an
>> oddball.
>
> When both the HDLCD and the TDA998x drivers are compiled as modules, the 
> order in which
> they are inserted can be somewhat random (due to testing). It is at that time 
> when you
> want the probe of HDLCD to be retried on the insmod-ing of the tda998x.ko 
> rather than
> fail entirely.
>
>>
>> This condition would only change if you had an overlay. That's a use
>> case that needs to be handled in a common way ('cause I don't want to
>> clean-up every driver doing overlays in their own way latter). Just
>> having "status" changing at runtime would have all sorts of implications
>> in the kernel.
>
> Hmm, not sure what you mean here with overlays. Are you thinking that the
> remote port is initially disabled and then re-enabled by an overlay? That is
> not the only way of_device_is_available() can fail, see above regarding 
> modules.

Russell pretty much answered most of this, but specifically for
of_device_is_available, the only way of_device_is_available() can
change is a DT change with "status" changing. The only way
of_graph_get_remote_port_parent changes is also from a DT change.

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

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Rob Herring]

On Mon, Feb 6, 2017 at 4:52 AM, Philipp Zabel  wrote:
> On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote:
>> Convert drivers to use the new of_graph_get_remote_node() helper
>> instead of parsing the endpoint node and then getting the remote device
>> node. Now drivers can just specify the device node and which
>> port/endpoint and get back the connected remote device node. The details
>> of the graph binding are nicely abstracted into the core OF graph code.
>>
>> This changes some error messages to debug messages (in the graph core).
>> Graph connections are often "no connects" depending on the particular
>> board, so we want to avoid spurious messages. Plus the kernel is not a
>> DT validator.
>>
>> Signed-off-by: Rob Herring 
>> ---
>>  drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++---
>>  drivers/gpu/drm/arm/malidp_drv.c| 29 ++-
>>  drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 +
>>  drivers/gpu/drm/bridge/dumb-vga-dac.c   | 15 ++
>>  drivers/gpu/drm/bridge/ti-tfp410.c  | 15 ++
>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +-
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++---
>>  drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +-
>>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++
>>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +--
>>  drivers/gpu/drm/mediatek/mtk_dpi.c  | 12 ++---
>>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++
>>  drivers/gpu/drm/meson/meson_drv.c   | 12 ++---
>>  drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++-
>>  drivers/gpu/drm/msm/dsi/dsi_host.c  |  3 +-
>>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +--
>>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++
>>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 +
>>  drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 
>> +++--
>>  drivers/gpu/drm/vc4/vc4_dpi.c   | 15 ++
>>  20 files changed, 64 insertions(+), 351 deletions(-)
>>
> [...]
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
>> b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> index 90fb831ef031..dbd554c09a39 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> @@ -661,7 +661,7 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>>   struct device *dev = >dev;
>>   struct mtk_dpi *dpi;
>>   struct resource *mem;
>> - struct device_node *ep, *bridge_node = NULL;
>> + struct device_node *bridge_node;
>>   int comp_id;
>>   int ret;
>>
>> @@ -706,15 +706,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>>   return -EINVAL;
>>   }
>>
>> - ep = of_graph_get_next_endpoint(dev->of_node, NULL);
>> - if (ep) {
>> - bridge_node = of_graph_get_remote_port_parent(ep);
>> - of_node_put(ep);
>> - }
>> - if (!bridge_node) {
>> - dev_err(dev, "Failed to find bridge node\n");
>> + bridge_node = of_graph_get_remote_node(dev->of_node, 0, 0);
>
> Note that before this change, of_graph_get_next_endpoint would just
> choose the first port/endpoint without restrictions to their reg
> properties, whereas the new code requires reg to be either not set or
> set to zero.
> As the former is the case for the mt8173 dpi->hdmi link, which is the
> only use of this driver, this should be fine.

Yes, that is on purpose. A given node should have known port numbering
and know what type of port each port is. The binding doc should
outline the ports and endpoints.

>
> [...]
>> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c 
>> b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> index 0e8c4d9af340..f14e472812ce 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> @@ -1433,7 +1433,7 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi 
>> *hdmi,
>>  {
>>   struct device *dev = >dev;
>>   struct device_node *np = dev->of_node;
>> - struct device_node *cec_np, *port, *ep, *remote, *i2c_np;
>> + struct device_node *cec_np, *remote, *i2c_np;
>>   struct platform_device *cec_pdev;
>>   struct regmap *regmap;
>>   struct resource *mem;
>> @@ -1485,29 +1485,9 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi 
>> *hdmi,
>>   if (IS_ERR(hdmi->regs))
>>   return PTR_ERR(hdmi->regs);
>>
>> - port = of_graph_get_port_by_id(np, 1);
>> - if (!port) {
>> - dev_err(dev, "Missing output port node\n");
>> + remote = of_graph_get_remote_node(np, 1, 0);
>> + if (!remote)
>>   return -EINVAL;
>> - }
>> -
>> - ep = of_get_child_by_name(port, "endpoint");
>> - if (!ep) {
>> - dev_err(dev, "Missing endpoint node in port %s\n",
>> - port->full_name);
>> - 

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Rob Herring]

On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote:
> On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote:
> > Convert drivers to use the new of_graph_get_remote_node() helper
> > instead of parsing the endpoint node and then getting the remote device
> > node. Now drivers can just specify the device node and which
> > port/endpoint and get back the connected remote device node. The details
> > of the graph binding are nicely abstracted into the core OF graph code.
> > 
> > This changes some error messages to debug messages (in the graph core).
> > Graph connections are often "no connects" depending on the particular
> > board, so we want to avoid spurious messages. Plus the kernel is not a
> > DT validator.
> > 
> > Signed-off-by: Rob Herring 
> > ---
> >  drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++---
> >  drivers/gpu/drm/arm/malidp_drv.c| 29 ++-
> >  drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 +
> >  drivers/gpu/drm/bridge/dumb-vga-dac.c   | 15 ++
> >  drivers/gpu/drm/bridge/ti-tfp410.c  | 15 ++
> >  drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +-
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++---
> >  drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +-
> >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +--
> >  drivers/gpu/drm/mediatek/mtk_dpi.c  | 12 ++---
> >  drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++
> >  drivers/gpu/drm/meson/meson_drv.c   | 12 ++---
> >  drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++-
> >  drivers/gpu/drm/msm/dsi/dsi_host.c  |  3 +-
> >  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +--
> >  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++
> >  drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 +
> >  drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 
> > +++--
> >  drivers/gpu/drm/vc4/vc4_dpi.c   | 15 ++
> >  20 files changed, 64 insertions(+), 351 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c 
> > b/drivers/gpu/drm/arm/hdlcd_drv.c
> > index e5f4f4a6546d..0f70f5fe9970 100644
> > --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> > @@ -430,29 +430,13 @@ static int compare_dev(struct device *dev, void *data)
> >  
> >  static int hdlcd_probe(struct platform_device *pdev)
> >  {
> > -   struct device_node *port, *ep;
> > +   struct device_node *port;
> > struct component_match *match = NULL;
> >  
> > -   if (!pdev->dev.of_node)
> > -   return -ENODEV;
> > -
> > /* there is only one output port inside each device, find it */
> > -   ep = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> > -   if (!ep)
> > -   return -ENODEV;
> > -
> > -   if (!of_device_is_available(ep)) {
> > -   of_node_put(ep);
> > +   port = of_graph_get_remote_node(pdev->dev.of_node, 0, 0);
> > +   if (!port)
> > return -ENODEV;
> > -   }
> > -
> > -   /* add the remote encoder port as component */
> > -   port = of_graph_get_remote_port_parent(ep);
> > -   of_node_put(ep);
> > -   if (!port || !of_device_is_available(port)) {
> > -   of_node_put(port);
> > -   return -EAGAIN;
> 
> The HDLCD change looks reasonable except for this -EAGAIN business. I'll have 
> to
> test your changes on my setup to see how this affects having the encoder as a 
> module.

What are you expecting to happen with -EAGAIN? This one was a bit of an 
oddball. 

This condition would only change if you had an overlay. That's a use 
case that needs to be handled in a common way ('cause I don't want to 
clean-up every driver doing overlays in their own way latter). Just 
having "status" changing at runtime would have all sorts of implications 
in the kernel.

> 
> > -   }
> >  
> > drm_of_component_match_add(>dev, , compare_dev, port);
> > of_node_put(port);
> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c 
> > b/drivers/gpu/drm/arm/malidp_drv.c
> > index 32f746e31379..bfa04be7f5de 100644
> > --- a/drivers/gpu/drm/arm/malidp_drv.c
> > +++ b/drivers/gpu/drm/arm/malidp_drv.c
> > @@ -262,7 +262,6 @@ static int malidp_bind(struct device *dev)
> >  {
> > struct resource *res;
> > struct drm_device *drm;
> > -   struct device_node *ep;
> > struct malidp_drm *malidp;
> > struct malidp_hw_device *hwdev;
> > struct platform_device *pdev = to_platform_device(dev);
> > @@ -360,12 +359,7 @@ static int malidp_bind(struct device *dev)
> > goto init_fail;
> >  
> > /* Set the CRTC's port so that the encoder component can find it */
> > -   ep = of_graph_get_next_endpoint(dev->of_node, NULL);
> > -   if (!ep) {
> > -   ret = -EINVAL;
> > -   goto port_fail;
> > -   }
> > -   malidp->crtc.port = 

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Neil Armstrong]

On 02/04/2017 04:36 AM, Rob Herring wrote:
> Convert drivers to use the new of_graph_get_remote_node() helper
> instead of parsing the endpoint node and then getting the remote device
> node. Now drivers can just specify the device node and which
> port/endpoint and get back the connected remote device node. The details
> of the graph binding are nicely abstracted into the core OF graph code.
> 
> This changes some error messages to debug messages (in the graph core).
> Graph connections are often "no connects" depending on the particular
> board, so we want to avoid spurious messages. Plus the kernel is not a
> DT validator.
> 
> Signed-off-by: Rob Herring 
> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++---
>  drivers/gpu/drm/arm/malidp_drv.c| 29 ++-
>  drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 +
>  drivers/gpu/drm/bridge/dumb-vga-dac.c   | 15 ++
>  drivers/gpu/drm/bridge/ti-tfp410.c  | 15 ++
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++---
>  drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +--
>  drivers/gpu/drm/mediatek/mtk_dpi.c  | 12 ++---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++
>  drivers/gpu/drm/meson/meson_drv.c   | 12 ++---
>  drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++-
>  drivers/gpu/drm/msm/dsi/dsi_host.c  |  3 +-
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 +
>  drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 
> +++--
>  drivers/gpu/drm/vc4/vc4_dpi.c   | 15 ++
>  20 files changed, 64 insertions(+), 351 deletions(-)
> 
[...]
> diff --git a/drivers/gpu/drm/meson/meson_drv.c 
> b/drivers/gpu/drm/meson/meson_drv.c
> index ff1f6019b97b..37cb9c755ed7 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -163,14 +163,14 @@ static struct drm_driver meson_driver = {
>  
>  static bool meson_vpu_has_available_connectors(struct device *dev)
>  {
> - struct device_node *ep, *remote;
> + struct device_node *remote;
> + int i;
>  
> - /* Parses each endpoint and check if remote exists */
> - for_each_endpoint_of_node(dev->of_node, ep) {
> - /* If the endpoint node exists, consider it enabled */
> - remote = of_graph_get_remote_port(ep);
> - if (remote)
> + for_each_of_graph_remote_node(dev->of_node, remote, i, 2) {
> + if (remote) {
> + of_node_put(remote);
>   return true;
> + }
>   }
>  
>   return false;
> diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c 
> b/drivers/gpu/drm/meson/meson_venc_cvbs.c
> index a2bcc70a03ef..8566de2edb62 100644
> --- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
> +++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c
> @@ -217,25 +217,14 @@ static const struct drm_encoder_helper_funcs
>  
>  static bool meson_venc_cvbs_connector_is_available(struct meson_drm *priv)
>  {
> - struct device_node *ep, *remote;
> + struct device_node *remote;
>  
> - /* CVBS VDAC output is on the first port, first endpoint */
> - ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> - if (!ep)
> + remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0);
> + if (!remote)
>   return false;
>  
> -
> - /* If the endpoint node exists, consider it enabled */
> - remote = of_graph_get_remote_port(ep);
> - if (remote) {
> - of_node_put(ep);
> - return true;
> - }
> -
> - of_node_put(ep);
>   of_node_put(remote);
> -
> - return false;
> + return true;
>  }
>  
>  int meson_venc_cvbs_create(struct meson_drm *priv)
[...]

Hi Rob,

For the meson changes :
Acked-by: Neil Armstrong 

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

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Jyri Sarha]

Thanks Rob, for nice cleanup, but ...

On 02/04/17 05:36, Rob Herring wrote:
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 6dfdb145f3bb..e74cc236a79b 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -1013,16 +1013,7 @@ int tilcdc_crtc_create(struct drm_device *dev)
>   drm_crtc_helper_add(crtc, _crtc_helper_funcs);
>  
>   if (priv->is_componentized) {
> - struct device_node *ports =
> - of_get_child_by_name(dev->dev->of_node, "ports");
> -
> - if (ports) {
> - crtc->port = of_get_child_by_name(ports, "port");
> - of_node_put(ports);
> - } else {
> - crtc->port =
> - of_get_child_by_name(dev->dev->of_node, "port");
> - }
> + crtc->port = of_graph_get_port_by_id(dev->dev->of_node, 0, 0);
>   if (!crtc->port) { /* This should never happen */
>   dev_err(dev->dev, "Port node not found in %s\n",
>   dev->dev->of_node->full_name);
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index c67d7cd7d57e..b7523dce4e8a 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -187,39 +187,6 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct 
> drm_bridge *bridge)
>   return ret;
>  }
>  
> -static int tilcdc_node_has_port(struct device_node *dev_node)
> -{
> - struct device_node *node;
> -
> - node = of_get_child_by_name(dev_node, "ports");
> - if (!node)
> - node = of_get_child_by_name(dev_node, "port");
> - if (!node)
> - return 0;
> - of_node_put(node);
> -
> - return 1;
> -}
> -
> -static
> -struct device_node *tilcdc_get_remote_node(struct device_node *node)
> -{
> - struct device_node *ep;
> - struct device_node *parent;
> -
> - if (!tilcdc_node_has_port(node))
> - return NULL;
> -
> - ep = of_graph_get_next_endpoint(node, NULL);
> - if (!ep)
> - return NULL;
> -
> - parent = of_graph_get_remote_port_parent(ep);
> - of_node_put(ep);
> -
> - return parent;
> -}
> -
>  int tilcdc_attach_external_device(struct drm_device *ddev)
>  {
>   struct tilcdc_drm_private *priv = ddev->dev_private;
> @@ -227,7 +194,7 @@ int tilcdc_attach_external_device(struct drm_device *ddev)
>   struct drm_bridge *bridge;
>   int ret;
>  
> - remote_node = tilcdc_get_remote_node(ddev->dev->of_node);
> + remote_node = of_graph_get_remote_node(ddev->dev->of_node, 0, 0);
>   if (!remote_node)
>   return 0;
>  
> @@ -266,35 +233,18 @@ int tilcdc_get_external_components(struct device *dev,
>  struct component_match **match)
>  {
>   struct device_node *node;
> - struct device_node *ep = NULL;
> - int count = 0;
> - int ret = 0;
>  
> - if (!tilcdc_node_has_port(dev->of_node))


> + if (!match)
>   return 0;

This will break tilcdc on setups that use the old tilcdc internal panel-
or tfp410- support. The driver uses tilcdc_get_external_components()
with match == NULL to check if the setup uses graph binding or legacy
internal panel/tfp410 support.

My intention is to get rid off the legacy internal encoder and connector
support (I even did that once already, but DRM had lot of infrastructure
changes in that area and my changes became obsolete), but we still need it.

>  
> - while ((ep = of_graph_get_next_endpoint(dev->of_node, ep))) {
> - node = of_graph_get_remote_port_parent(ep);
> - if (!node || !of_device_is_available(node)) {
> - of_node_put(node);
> - continue;
> - }
> -
> - dev_dbg(dev, "Subdevice node '%s' found\n", node->name);
> -
> - if (of_device_is_compatible(node, "nxp,tda998x")) {
> - if (match)
> - drm_of_component_match_add(dev, match,
> -dev_match_of, node);
> - ret = 1;
> - }
> + node = of_graph_get_remote_node(dev->of_node, 0, 0);
>  
> + if (!of_device_is_compatible(node, "nxp,tda998x")) {
>   of_node_put(node);
> - if (count++ > 1) {
> - dev_err(dev, "Only one port is supported\n");
> - return -EINVAL;
> - }
> + return 0;
>   }
>  
> - return ret;

Simply remove the above mentioned if statement add and this here:
if (match)
> + drm_of_component_match_add(dev, match, dev_match_of, node);
> + of_node_put(node);
> + return 1;
>  }

Best regards,
Jyri

___
dri-devel mailing 

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Liviu Dudau]

On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote:
> Convert drivers to use the new of_graph_get_remote_node() helper
> instead of parsing the endpoint node and then getting the remote device
> node. Now drivers can just specify the device node and which
> port/endpoint and get back the connected remote device node. The details
> of the graph binding are nicely abstracted into the core OF graph code.
> 
> This changes some error messages to debug messages (in the graph core).
> Graph connections are often "no connects" depending on the particular
> board, so we want to avoid spurious messages. Plus the kernel is not a
> DT validator.
> 
> Signed-off-by: Rob Herring 
> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++---
>  drivers/gpu/drm/arm/malidp_drv.c| 29 ++-
>  drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 +
>  drivers/gpu/drm/bridge/dumb-vga-dac.c   | 15 ++
>  drivers/gpu/drm/bridge/ti-tfp410.c  | 15 ++
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++---
>  drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +--
>  drivers/gpu/drm/mediatek/mtk_dpi.c  | 12 ++---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++
>  drivers/gpu/drm/meson/meson_drv.c   | 12 ++---
>  drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++-
>  drivers/gpu/drm/msm/dsi/dsi_host.c  |  3 +-
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 +
>  drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 
> +++--
>  drivers/gpu/drm/vc4/vc4_dpi.c   | 15 ++
>  20 files changed, 64 insertions(+), 351 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index e5f4f4a6546d..0f70f5fe9970 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -430,29 +430,13 @@ static int compare_dev(struct device *dev, void *data)
>  
>  static int hdlcd_probe(struct platform_device *pdev)
>  {
> - struct device_node *port, *ep;
> + struct device_node *port;
>   struct component_match *match = NULL;
>  
> - if (!pdev->dev.of_node)
> - return -ENODEV;
> -
>   /* there is only one output port inside each device, find it */
> - ep = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> - if (!ep)
> - return -ENODEV;
> -
> - if (!of_device_is_available(ep)) {
> - of_node_put(ep);
> + port = of_graph_get_remote_node(pdev->dev.of_node, 0, 0);
> + if (!port)
>   return -ENODEV;
> - }
> -
> - /* add the remote encoder port as component */
> - port = of_graph_get_remote_port_parent(ep);
> - of_node_put(ep);
> - if (!port || !of_device_is_available(port)) {
> - of_node_put(port);
> - return -EAGAIN;

The HDLCD change looks reasonable except for this -EAGAIN business. I'll have to
test your changes on my setup to see how this affects having the encoder as a 
module.

> - }
>  
>   drm_of_component_match_add(>dev, , compare_dev, port);
>   of_node_put(port);
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c 
> b/drivers/gpu/drm/arm/malidp_drv.c
> index 32f746e31379..bfa04be7f5de 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -262,7 +262,6 @@ static int malidp_bind(struct device *dev)
>  {
>   struct resource *res;
>   struct drm_device *drm;
> - struct device_node *ep;
>   struct malidp_drm *malidp;
>   struct malidp_hw_device *hwdev;
>   struct platform_device *pdev = to_platform_device(dev);
> @@ -360,12 +359,7 @@ static int malidp_bind(struct device *dev)
>   goto init_fail;
>  
>   /* Set the CRTC's port so that the encoder component can find it */
> - ep = of_graph_get_next_endpoint(dev->of_node, NULL);
> - if (!ep) {
> - ret = -EINVAL;
> - goto port_fail;
> - }
> - malidp->crtc.port = of_get_next_parent(ep);
> + malidp->crtc.port = of_graph_get_port_by_id(dev->of_node, 0);
>  
>   ret = component_bind_all(dev, drm);
>   if (ret) {
> @@ -418,9 +412,7 @@ static int malidp_bind(struct device *dev)
>  irq_init_fail:
>   component_unbind_all(dev, drm);
>  bind_fail:
> - of_node_put(malidp->crtc.port);

Why removing this line? AFAICT this is still needed, according to 
of_graph_get_port_by_id()
documentation.

>   malidp->crtc.port = NULL;
> -port_fail:
>   malidp_fini(drm);
>  init_fail:
>   drm->dev_private = NULL;
> @@ -478,29 +470,16 

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Russell King - ARM Linux]

On Mon, Feb 06, 2017 at 05:23:06PM +, Liviu Dudau wrote:
> On Mon, Feb 06, 2017 at 11:09:49AM -0600, Rob Herring wrote:
> > On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote:
> > > On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote:
> > > > -   /* add the remote encoder port as component */
> > > > -   port = of_graph_get_remote_port_parent(ep);
> > > > -   of_node_put(ep);
> > > > -   if (!port || !of_device_is_available(port)) {
> > > > -   of_node_put(port);
> > > > -   return -EAGAIN;
> > > 
> > > The HDLCD change looks reasonable except for this -EAGAIN business. I'll 
> > > have to
> > > test your changes on my setup to see how this affects having the encoder 
> > > as a module.
> > 
> > What are you expecting to happen with -EAGAIN? This one was a bit of an 
> > oddball.
> 
> When both the HDLCD and the TDA998x drivers are compiled as modules, the
> order in which they are inserted can be somewhat random (due to testing).

Not really "due to testing" but if you run a real distro, they tend to
have a multi-threaded behaviour when loading kernel modules at boot.

> It is at that time when you want the probe of HDLCD to be retried on the
> insmod-ing of the tda998x.ko rather than fail entirely.

-EAGAIN doesn't get you that, and in any case, solving that problem is
exactly why the component API exists - so that DRM only comes up once
all the necessary components are available.

-EAGAIN also doesn't get you that from inside a probe function - such
an error will be reported in the kernel log, and no further action
will be taken (the device driver probe will be failed, and not
automatically retried.

The only case that we automatically retry is if a driver returns
-EPROBE_DEFER.  Everything else causes a probe failure.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Philipp Zabel]

On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote:
> Convert drivers to use the new of_graph_get_remote_node() helper
> instead of parsing the endpoint node and then getting the remote device
> node. Now drivers can just specify the device node and which
> port/endpoint and get back the connected remote device node. The details
> of the graph binding are nicely abstracted into the core OF graph code.
> 
> This changes some error messages to debug messages (in the graph core).
> Graph connections are often "no connects" depending on the particular
> board, so we want to avoid spurious messages. Plus the kernel is not a
> DT validator.
> 
> Signed-off-by: Rob Herring 
> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++---
>  drivers/gpu/drm/arm/malidp_drv.c| 29 ++-
>  drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 +
>  drivers/gpu/drm/bridge/dumb-vga-dac.c   | 15 ++
>  drivers/gpu/drm/bridge/ti-tfp410.c  | 15 ++
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++---
>  drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +-
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +--
>  drivers/gpu/drm/mediatek/mtk_dpi.c  | 12 ++---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++
>  drivers/gpu/drm/meson/meson_drv.c   | 12 ++---
>  drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++-
>  drivers/gpu/drm/msm/dsi/dsi_host.c  |  3 +-
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 +
>  drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 
> +++--
>  drivers/gpu/drm/vc4/vc4_dpi.c   | 15 ++
>  20 files changed, 64 insertions(+), 351 deletions(-)
> 
[...] 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c 
> b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 90fb831ef031..dbd554c09a39 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -661,7 +661,7 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>   struct device *dev = >dev;
>   struct mtk_dpi *dpi;
>   struct resource *mem;
> - struct device_node *ep, *bridge_node = NULL;
> + struct device_node *bridge_node;
>   int comp_id;
>   int ret;
>  
> @@ -706,15 +706,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>   return -EINVAL;
>   }
>  
> - ep = of_graph_get_next_endpoint(dev->of_node, NULL);
> - if (ep) {
> - bridge_node = of_graph_get_remote_port_parent(ep);
> - of_node_put(ep);
> - }
> - if (!bridge_node) {
> - dev_err(dev, "Failed to find bridge node\n");
> + bridge_node = of_graph_get_remote_node(dev->of_node, 0, 0);

Note that before this change, of_graph_get_next_endpoint would just
choose the first port/endpoint without restrictions to their reg
properties, whereas the new code requires reg to be either not set or
set to zero.
As the former is the case for the mt8173 dpi->hdmi link, which is the
only use of this driver, this should be fine.

[...]
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c 
> b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 0e8c4d9af340..f14e472812ce 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1433,7 +1433,7 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi 
> *hdmi,
>  {
>   struct device *dev = >dev;
>   struct device_node *np = dev->of_node;
> - struct device_node *cec_np, *port, *ep, *remote, *i2c_np;
> + struct device_node *cec_np, *remote, *i2c_np;
>   struct platform_device *cec_pdev;
>   struct regmap *regmap;
>   struct resource *mem;
> @@ -1485,29 +1485,9 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi 
> *hdmi,
>   if (IS_ERR(hdmi->regs))
>   return PTR_ERR(hdmi->regs);
>  
> - port = of_graph_get_port_by_id(np, 1);
> - if (!port) {
> - dev_err(dev, "Missing output port node\n");
> + remote = of_graph_get_remote_node(np, 1, 0);
> + if (!remote)
>   return -EINVAL;
> - }
> -
> - ep = of_get_child_by_name(port, "endpoint");
> - if (!ep) {
> - dev_err(dev, "Missing endpoint node in port %s\n",
> - port->full_name);
> - of_node_put(port);
> - return -EINVAL;
> - }
> - of_node_put(port);
> -
> - remote = of_graph_get_remote_port_parent(ep);
> - if (!remote) {
> - dev_err(dev, "Missing connector/bridge node for endpoint %s\n",
> - ep->full_name);
> - of_node_put(ep);
> - return -EINVAL;
> - 

Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Liviu Dudau]

On Mon, Feb 06, 2017 at 11:09:49AM -0600, Rob Herring wrote:
> On Mon, Feb 06, 2017 at 10:29:33AM +, Liviu Dudau wrote:
> > On Fri, Feb 03, 2017 at 09:36:33PM -0600, Rob Herring wrote:
> > > Convert drivers to use the new of_graph_get_remote_node() helper
> > > instead of parsing the endpoint node and then getting the remote device
> > > node. Now drivers can just specify the device node and which
> > > port/endpoint and get back the connected remote device node. The details
> > > of the graph binding are nicely abstracted into the core OF graph code.
> > > 
> > > This changes some error messages to debug messages (in the graph core).
> > > Graph connections are often "no connects" depending on the particular
> > > board, so we want to avoid spurious messages. Plus the kernel is not a
> > > DT validator.
> > > 
> > > Signed-off-by: Rob Herring 
> > > ---
> > >  drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++---
> > >  drivers/gpu/drm/arm/malidp_drv.c| 29 ++-
> > >  drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 +
> > >  drivers/gpu/drm/bridge/dumb-vga-dac.c   | 15 ++
> > >  drivers/gpu/drm/bridge/ti-tfp410.c  | 15 ++
> > >  drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +-
> > >  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++---
> > >  drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +-
> > >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++
> > >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +--
> > >  drivers/gpu/drm/mediatek/mtk_dpi.c  | 12 ++---
> > >  drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++
> > >  drivers/gpu/drm/meson/meson_drv.c   | 12 ++---
> > >  drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++-
> > >  drivers/gpu/drm/msm/dsi/dsi_host.c  |  3 +-
> > >  drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +--
> > >  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++
> > >  drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 +
> > >  drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 
> > > +++--
> > >  drivers/gpu/drm/vc4/vc4_dpi.c   | 15 ++
> > >  20 files changed, 64 insertions(+), 351 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c 
> > > b/drivers/gpu/drm/arm/hdlcd_drv.c
> > > index e5f4f4a6546d..0f70f5fe9970 100644
> > > --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> > > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> > > @@ -430,29 +430,13 @@ static int compare_dev(struct device *dev, void 
> > > *data)
> > >  
> > >  static int hdlcd_probe(struct platform_device *pdev)
> > >  {
> > > - struct device_node *port, *ep;
> > > + struct device_node *port;
> > >   struct component_match *match = NULL;
> > >  
> > > - if (!pdev->dev.of_node)
> > > - return -ENODEV;
> > > -
> > >   /* there is only one output port inside each device, find it */
> > > - ep = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> > > - if (!ep)
> > > - return -ENODEV;
> > > -
> > > - if (!of_device_is_available(ep)) {
> > > - of_node_put(ep);
> > > + port = of_graph_get_remote_node(pdev->dev.of_node, 0, 0);
> > > + if (!port)
> > >   return -ENODEV;
> > > - }
> > > -
> > > - /* add the remote encoder port as component */
> > > - port = of_graph_get_remote_port_parent(ep);
> > > - of_node_put(ep);
> > > - if (!port || !of_device_is_available(port)) {
> > > - of_node_put(port);
> > > - return -EAGAIN;
> > 
> > The HDLCD change looks reasonable except for this -EAGAIN business. I'll 
> > have to
> > test your changes on my setup to see how this affects having the encoder as 
> > a module.
> 
> What are you expecting to happen with -EAGAIN? This one was a bit of an 
> oddball.

When both the HDLCD and the TDA998x drivers are compiled as modules, the order 
in which
they are inserted can be somewhat random (due to testing). It is at that time 
when you
want the probe of HDLCD to be retried on the insmod-ing of the tda998x.ko 
rather than
fail entirely.

> 
> This condition would only change if you had an overlay. That's a use 
> case that needs to be handled in a common way ('cause I don't want to 
> clean-up every driver doing overlays in their own way latter). Just 
> having "status" changing at runtime would have all sorts of implications 
> in the kernel.

Hmm, not sure what you mean here with overlays. Are you thinking that the
remote port is initially disabled and then re-enabled by an overlay? That is
not the only way of_device_is_available() can fail, see above regarding modules.

Best regards,
Liviu

> 
> > 
> > > - }
> > >  
> > >   drm_of_component_match_add(>dev, , compare_dev, port);
> > >   of_node_put(port);
> > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c 
> > > b/drivers/gpu/drm/arm/malidp_drv.c
> > > index 32f746e31379..bfa04be7f5de 100644
> > > --- 

[PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node

[Rob Herring]

Convert drivers to use the new of_graph_get_remote_node() helper
instead of parsing the endpoint node and then getting the remote device
node. Now drivers can just specify the device node and which
port/endpoint and get back the connected remote device node. The details
of the graph binding are nicely abstracted into the core OF graph code.

This changes some error messages to debug messages (in the graph core).
Graph connections are often "no connects" depending on the particular
board, so we want to avoid spurious messages. Plus the kernel is not a
DT validator.

Signed-off-by: Rob Herring 
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 22 ++---
 drivers/gpu/drm/arm/malidp_drv.c| 29 ++-
 drivers/gpu/drm/bridge/adv7511/adv7533.c| 12 +
 drivers/gpu/drm/bridge/dumb-vga-dac.c   | 15 ++
 drivers/gpu/drm/bridge/ti-tfp410.c  | 15 ++
 drivers/gpu/drm/exynos/exynos_drm_dpi.c | 16 +-
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++---
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 27 +-
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c| 26 ++
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 30 +--
 drivers/gpu/drm/mediatek/mtk_dpi.c  | 12 ++---
 drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 ++
 drivers/gpu/drm/meson/meson_drv.c   | 12 ++---
 drivers/gpu/drm/meson/meson_venc_cvbs.c | 19 ++-
 drivers/gpu/drm/msm/dsi/dsi_host.c  |  3 +-
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +--
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 18 +++
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 11 +
 drivers/gpu/drm/tilcdc/tilcdc_external.c| 66 +++--
 drivers/gpu/drm/vc4/vc4_dpi.c   | 15 ++
 20 files changed, 64 insertions(+), 351 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index e5f4f4a6546d..0f70f5fe9970 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -430,29 +430,13 @@ static int compare_dev(struct device *dev, void *data)
 
 static int hdlcd_probe(struct platform_device *pdev)
 {
-   struct device_node *port, *ep;
+   struct device_node *port;
struct component_match *match = NULL;
 
-   if (!pdev->dev.of_node)
-   return -ENODEV;
-
/* there is only one output port inside each device, find it */
-   ep = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
-   if (!ep)
-   return -ENODEV;
-
-   if (!of_device_is_available(ep)) {
-   of_node_put(ep);
+   port = of_graph_get_remote_node(pdev->dev.of_node, 0, 0);
+   if (!port)
return -ENODEV;
-   }
-
-   /* add the remote encoder port as component */
-   port = of_graph_get_remote_port_parent(ep);
-   of_node_put(ep);
-   if (!port || !of_device_is_available(port)) {
-   of_node_put(port);
-   return -EAGAIN;
-   }
 
drm_of_component_match_add(>dev, , compare_dev, port);
of_node_put(port);
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 32f746e31379..bfa04be7f5de 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -262,7 +262,6 @@ static int malidp_bind(struct device *dev)
 {
struct resource *res;
struct drm_device *drm;
-   struct device_node *ep;
struct malidp_drm *malidp;
struct malidp_hw_device *hwdev;
struct platform_device *pdev = to_platform_device(dev);
@@ -360,12 +359,7 @@ static int malidp_bind(struct device *dev)
goto init_fail;
 
/* Set the CRTC's port so that the encoder component can find it */
-   ep = of_graph_get_next_endpoint(dev->of_node, NULL);
-   if (!ep) {
-   ret = -EINVAL;
-   goto port_fail;
-   }
-   malidp->crtc.port = of_get_next_parent(ep);
+   malidp->crtc.port = of_graph_get_port_by_id(dev->of_node, 0);
 
ret = component_bind_all(dev, drm);
if (ret) {
@@ -418,9 +412,7 @@ static int malidp_bind(struct device *dev)
 irq_init_fail:
component_unbind_all(dev, drm);
 bind_fail:
-   of_node_put(malidp->crtc.port);
malidp->crtc.port = NULL;
-port_fail:
malidp_fini(drm);
 init_fail:
drm->dev_private = NULL;
@@ -478,29 +470,16 @@ static int malidp_compare_dev(struct device *dev, void 
*data)
 
 static int malidp_platform_probe(struct platform_device *pdev)
 {
-   struct device_node *port, *ep;
+   struct device_node *port;
struct component_match *match = NULL;
 
if (!pdev->dev.of_node)
return -ENODEV;
 
/* there is only one output port inside each device, find it */
-   ep = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
-   if