diff mbox series

[v4,1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge

Message ID 20210322140152.101709-2-jagan@amarulasolutions.com
State New
Headers
Series drm: sun4i: dsi: Convert drm bridge |

Commit Message

Jagan Teki March 22, 2021, 2:01 p.m. UTC 
Replace of_drm_find_panel with drm_of_find_panel_or_bridge
for finding panel, this indeed help to find the bridge if
bridge support added.

Added NULL in bridge argument, same will replace with bridge
parameter once bridge supported.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v4, v3:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart March 23, 2021, 10:53 p.m. UTC | #1 
Hi Jagan,

Thank you for the patch.

On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> for finding panel, this indeed help to find the bridge if
> bridge support added.
> 
> Added NULL in bridge argument, same will replace with bridge
> parameter once bridge supported.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Looks good, there should be no functional change.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> Changes for v4, v3:
> - none
> 
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 4f5efcace68e..2e9e7b2d4145 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -21,6 +21,7 @@
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_of.h>
>  #include <drm/drm_panel.h>
>  #include <drm/drm_print.h>
>  #include <drm/drm_probe_helper.h>
> @@ -963,10 +964,14 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
>  			    struct mipi_dsi_device *device)
>  {
>  	struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
> -	struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
> +	struct drm_panel *panel;
> +	int ret;
> +
> +	ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> +					  &panel, NULL);
> +	if (ret)
> +		return ret;
>  
> -	if (IS_ERR(panel))
> -		return PTR_ERR(panel);
>  	if (!dsi->drm || !dsi->drm->registered)
>  		return -EPROBE_DEFER;
>
Samuel Holland March 24, 2021, 2:48 a.m. UTC | #2 
On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> Hi Jagan,
> 
> Thank you for the patch.
> 
> On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
>> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
>> for finding panel, this indeed help to find the bridge if
>> bridge support added.
>>
>> Added NULL in bridge argument, same will replace with bridge
>> parameter once bridge supported.
>>
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> 
> Looks good, there should be no functional change.

Actually this breaks all existing users of this driver, see below.

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
>> ---
>> Changes for v4, v3:
>> - none
>>
>>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>> index 4f5efcace68e..2e9e7b2d4145 100644
>> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>> @@ -21,6 +21,7 @@
>>  
>>  #include <drm/drm_atomic_helper.h>
>>  #include <drm/drm_mipi_dsi.h>
>> +#include <drm/drm_of.h>
>>  #include <drm/drm_panel.h>
>>  #include <drm/drm_print.h>
>>  #include <drm/drm_probe_helper.h>
>> @@ -963,10 +964,14 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
>>  			    struct mipi_dsi_device *device)
>>  {
>>  	struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
>> -	struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);

This is using the OF node of the DSI device, which is a direct child of
the DSI host's OF node. There is no OF graph involved.

>> +	struct drm_panel *panel;
>> +	int ret;
>> +
>> +	ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
>> +					  &panel, NULL);

However, this function expects to find the panel using OF graph. This
does not work with existing device trees (PinePhone, PineTab) which do
not use OF graph to connect the panel. And it cannot work, because the
DSI host's binding specifies a single port: the input port from the
display engine.

Regards,
Samuel

>> +	if (ret)
>> +		return ret;
>>  
>> -	if (IS_ERR(panel))
>> -		return PTR_ERR(panel);
>>  	if (!dsi->drm || !dsi->drm->registered)
>>  		return -EPROBE_DEFER;
>>  
>
Jagan Teki March 24, 2021, 9:14 a.m. UTC | #3 
On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland <samuel@sholland.org> wrote:
>
> On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > Hi Jagan,
> >
> > Thank you for the patch.
> >
> > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> >> for finding panel, this indeed help to find the bridge if
> >> bridge support added.
> >>
> >> Added NULL in bridge argument, same will replace with bridge
> >> parameter once bridge supported.
> >>
> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >
> > Looks good, there should be no functional change.
>
> Actually this breaks all existing users of this driver, see below.
>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> >> ---
> >> Changes for v4, v3:
> >> - none
> >>
> >>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> >>  1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> >> index 4f5efcace68e..2e9e7b2d4145 100644
> >> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> >> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> >> @@ -21,6 +21,7 @@
> >>
> >>  #include <drm/drm_atomic_helper.h>
> >>  #include <drm/drm_mipi_dsi.h>
> >> +#include <drm/drm_of.h>
> >>  #include <drm/drm_panel.h>
> >>  #include <drm/drm_print.h>
> >>  #include <drm/drm_probe_helper.h>
> >> @@ -963,10 +964,14 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
> >>                          struct mipi_dsi_device *device)
> >>  {
> >>      struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
> >> -    struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
>
> This is using the OF node of the DSI device, which is a direct child of
> the DSI host's OF node. There is no OF graph involved.
>
> >> +    struct drm_panel *panel;
> >> +    int ret;
> >> +
> >> +    ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> >> +                                      &panel, NULL);
>
> However, this function expects to find the panel using OF graph. This
> does not work with existing device trees (PinePhone, PineTab) which do
> not use OF graph to connect the panel. And it cannot work, because the
> DSI host's binding specifies a single port: the input port from the
> display engine.

Thanks for noticing this. I did understand your point and yes, I did
mention the updated pipeline in previous versions and forgot to add it
to this series.

Here is the updated pipeline to make it work:

https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/

Let me know your comments on this, so I will add a patch for the
above-affected DTS files.

Jagan.
Laurent Pinchart March 24, 2021, 9:38 a.m. UTC | #4 
Hi Jagan,

On Wed, Mar 24, 2021 at 02:44:57PM +0530, Jagan Teki wrote:
> On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland wrote:
> > On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> > >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> > >> for finding panel, this indeed help to find the bridge if
> > >> bridge support added.
> > >>
> > >> Added NULL in bridge argument, same will replace with bridge
> > >> parameter once bridge supported.
> > >>
> > >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > >
> > > Looks good, there should be no functional change.
> >
> > Actually this breaks all existing users of this driver, see below.
> >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > >> ---
> > >> Changes for v4, v3:
> > >> - none
> > >>
> > >>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> > >>  1 file changed, 8 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > >> index 4f5efcace68e..2e9e7b2d4145 100644
> > >> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > >> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > >> @@ -21,6 +21,7 @@
> > >>
> > >>  #include <drm/drm_atomic_helper.h>
> > >>  #include <drm/drm_mipi_dsi.h>
> > >> +#include <drm/drm_of.h>
> > >>  #include <drm/drm_panel.h>
> > >>  #include <drm/drm_print.h>
> > >>  #include <drm/drm_probe_helper.h>
> > >> @@ -963,10 +964,14 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
> > >>                          struct mipi_dsi_device *device)
> > >>  {
> > >>      struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
> > >> -    struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
> >
> > This is using the OF node of the DSI device, which is a direct child of
> > the DSI host's OF node. There is no OF graph involved.
> >
> > >> +    struct drm_panel *panel;
> > >> +    int ret;
> > >> +
> > >> +    ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> > >> +                                      &panel, NULL);
> >
> > However, this function expects to find the panel using OF graph. This
> > does not work with existing device trees (PinePhone, PineTab) which do
> > not use OF graph to connect the panel. And it cannot work, because the
> > DSI host's binding specifies a single port: the input port from the
> > display engine.
> 
> Thanks for noticing this. I did understand your point and yes, I did
> mention the updated pipeline in previous versions and forgot to add it
> to this series.
> 
> Here is the updated pipeline to make it work:
> 
> https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/
> 
> Let me know your comments on this, so I will add a patch for the
> above-affected DTS files.

DT is an ABI, we need to ensure backward compatibility. Changes in
kernel drivers can't break devices that have an old DT.
Jagan Teki March 24, 2021, 9:49 a.m. UTC | #5 
Hi Laurent,

On Wed, Mar 24, 2021 at 3:09 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Jagan,
>
> On Wed, Mar 24, 2021 at 02:44:57PM +0530, Jagan Teki wrote:
> > On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland wrote:
> > > On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > > > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> > > >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> > > >> for finding panel, this indeed help to find the bridge if
> > > >> bridge support added.
> > > >>
> > > >> Added NULL in bridge argument, same will replace with bridge
> > > >> parameter once bridge supported.
> > > >>
> > > >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > >
> > > > Looks good, there should be no functional change.
> > >
> > > Actually this breaks all existing users of this driver, see below.
> > >
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > >> ---
> > > >> Changes for v4, v3:
> > > >> - none
> > > >>
> > > >>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> > > >>  1 file changed, 8 insertions(+), 3 deletions(-)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > >> index 4f5efcace68e..2e9e7b2d4145 100644
> > > >> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > >> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > >> @@ -21,6 +21,7 @@
> > > >>
> > > >>  #include <drm/drm_atomic_helper.h>
> > > >>  #include <drm/drm_mipi_dsi.h>
> > > >> +#include <drm/drm_of.h>
> > > >>  #include <drm/drm_panel.h>
> > > >>  #include <drm/drm_print.h>
> > > >>  #include <drm/drm_probe_helper.h>
> > > >> @@ -963,10 +964,14 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
> > > >>                          struct mipi_dsi_device *device)
> > > >>  {
> > > >>      struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
> > > >> -    struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
> > >
> > > This is using the OF node of the DSI device, which is a direct child of
> > > the DSI host's OF node. There is no OF graph involved.
> > >
> > > >> +    struct drm_panel *panel;
> > > >> +    int ret;
> > > >> +
> > > >> +    ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> > > >> +                                      &panel, NULL);
> > >
> > > However, this function expects to find the panel using OF graph. This
> > > does not work with existing device trees (PinePhone, PineTab) which do
> > > not use OF graph to connect the panel. And it cannot work, because the
> > > DSI host's binding specifies a single port: the input port from the
> > > display engine.
> >
> > Thanks for noticing this. I did understand your point and yes, I did
> > mention the updated pipeline in previous versions and forgot to add it
> > to this series.
> >
> > Here is the updated pipeline to make it work:
> >
> > https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/
> >
> > Let me know your comments on this, so I will add a patch for the
> > above-affected DTS files.
>
> DT is an ABI, we need to ensure backward compatibility. Changes in
> kernel drivers can't break devices that have an old DT.

Thanks for your point.

So, we need to choose APIs that would compatible with the old DT and
new DT changes. Am I correct?

Jagan.
Laurent Pinchart March 24, 2021, 9:55 a.m. UTC | #6 
Hi Jagan,

On Wed, Mar 24, 2021 at 03:19:10PM +0530, Jagan Teki wrote:
> On Wed, Mar 24, 2021 at 3:09 PM Laurent Pinchart wrote:
> > On Wed, Mar 24, 2021 at 02:44:57PM +0530, Jagan Teki wrote:
> > > On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland wrote:
> > > > On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > > > > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> > > > >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> > > > >> for finding panel, this indeed help to find the bridge if
> > > > >> bridge support added.
> > > > >>
> > > > >> Added NULL in bridge argument, same will replace with bridge
> > > > >> parameter once bridge supported.
> > > > >>
> > > > >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > >
> > > > > Looks good, there should be no functional change.
> > > >
> > > > Actually this breaks all existing users of this driver, see below.
> > > >
> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > >
> > > > >> ---
> > > > >> Changes for v4, v3:
> > > > >> - none
> > > > >>
> > > > >>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> > > > >>  1 file changed, 8 insertions(+), 3 deletions(-)
> > > > >>
> > > > >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > >> index 4f5efcace68e..2e9e7b2d4145 100644
> > > > >> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > >> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > >> @@ -21,6 +21,7 @@
> > > > >>
> > > > >>  #include <drm/drm_atomic_helper.h>
> > > > >>  #include <drm/drm_mipi_dsi.h>
> > > > >> +#include <drm/drm_of.h>
> > > > >>  #include <drm/drm_panel.h>
> > > > >>  #include <drm/drm_print.h>
> > > > >>  #include <drm/drm_probe_helper.h>
> > > > >> @@ -963,10 +964,14 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
> > > > >>                          struct mipi_dsi_device *device)
> > > > >>  {
> > > > >>      struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
> > > > >> -    struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
> > > >
> > > > This is using the OF node of the DSI device, which is a direct child of
> > > > the DSI host's OF node. There is no OF graph involved.
> > > >
> > > > >> +    struct drm_panel *panel;
> > > > >> +    int ret;
> > > > >> +
> > > > >> +    ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> > > > >> +                                      &panel, NULL);
> > > >
> > > > However, this function expects to find the panel using OF graph. This
> > > > does not work with existing device trees (PinePhone, PineTab) which do
> > > > not use OF graph to connect the panel. And it cannot work, because the
> > > > DSI host's binding specifies a single port: the input port from the
> > > > display engine.
> > >
> > > Thanks for noticing this. I did understand your point and yes, I did
> > > mention the updated pipeline in previous versions and forgot to add it
> > > to this series.
> > >
> > > Here is the updated pipeline to make it work:
> > >
> > > https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/
> > >
> > > Let me know your comments on this, so I will add a patch for the
> > > above-affected DTS files.
> >
> > DT is an ABI, we need to ensure backward compatibility. Changes in
> > kernel drivers can't break devices that have an old DT.
> 
> Thanks for your point.
> 
> So, we need to choose APIs that would compatible with the old DT and
> new DT changes. Am I correct?

Yes, that's correct.
Maxime Ripard March 24, 2021, 10:11 a.m. UTC | #7 
On Wed, Mar 24, 2021 at 11:55:35AM +0200, Laurent Pinchart wrote:
> Hi Jagan,
> 
> On Wed, Mar 24, 2021 at 03:19:10PM +0530, Jagan Teki wrote:
> > On Wed, Mar 24, 2021 at 3:09 PM Laurent Pinchart wrote:
> > > On Wed, Mar 24, 2021 at 02:44:57PM +0530, Jagan Teki wrote:
> > > > On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland wrote:
> > > > > On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > > > > > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> > > > > >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> > > > > >> for finding panel, this indeed help to find the bridge if
> > > > > >> bridge support added.
> > > > > >>
> > > > > >> Added NULL in bridge argument, same will replace with bridge
> > > > > >> parameter once bridge supported.
> > > > > >>
> > > > > >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > >
> > > > > > Looks good, there should be no functional change.
> > > > >
> > > > > Actually this breaks all existing users of this driver, see below.
> > > > >
> > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > >
> > > > > >> ---
> > > > > >> Changes for v4, v3:
> > > > > >> - none
> > > > > >>
> > > > > >>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> > > > > >>  1 file changed, 8 insertions(+), 3 deletions(-)
> > > > > >>
> > > > > >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > > >> index 4f5efcace68e..2e9e7b2d4145 100644
> > > > > >> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > > >> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > > >> @@ -21,6 +21,7 @@
> > > > > >>
> > > > > >>  #include <drm/drm_atomic_helper.h>
> > > > > >>  #include <drm/drm_mipi_dsi.h>
> > > > > >> +#include <drm/drm_of.h>
> > > > > >>  #include <drm/drm_panel.h>
> > > > > >>  #include <drm/drm_print.h>
> > > > > >>  #include <drm/drm_probe_helper.h>
> > > > > >> @@ -963,10 +964,14 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
> > > > > >>                          struct mipi_dsi_device *device)
> > > > > >>  {
> > > > > >>      struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
> > > > > >> -    struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
> > > > >
> > > > > This is using the OF node of the DSI device, which is a direct child of
> > > > > the DSI host's OF node. There is no OF graph involved.
> > > > >
> > > > > >> +    struct drm_panel *panel;
> > > > > >> +    int ret;
> > > > > >> +
> > > > > >> +    ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> > > > > >> +                                      &panel, NULL);
> > > > >
> > > > > However, this function expects to find the panel using OF graph. This
> > > > > does not work with existing device trees (PinePhone, PineTab) which do
> > > > > not use OF graph to connect the panel. And it cannot work, because the
> > > > > DSI host's binding specifies a single port: the input port from the
> > > > > display engine.
> > > >
> > > > Thanks for noticing this. I did understand your point and yes, I did
> > > > mention the updated pipeline in previous versions and forgot to add it
> > > > to this series.
> > > >
> > > > Here is the updated pipeline to make it work:
> > > >
> > > > https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/
> > > >
> > > > Let me know your comments on this, so I will add a patch for the
> > > > above-affected DTS files.
> > >
> > > DT is an ABI, we need to ensure backward compatibility. Changes in
> > > kernel drivers can't break devices that have an old DT.
> > 
> > Thanks for your point.
> > 
> > So, we need to choose APIs that would compatible with the old DT and
> > new DT changes. Am I correct?
> 
> Yes, that's correct.

However, I see no particular reason to change the DT binding in this
case. The DSI devices are supposed to be described through a subnode of
their DSI controller, that's the generic binding and except for very odd
devices (and a bridge like this one is certainly not one), I see no
reason to deviate from that.

Maxime
diff mbox series

Patch

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 4f5efcace68e..2e9e7b2d4145 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -21,6 +21,7 @@ 
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_mipi_dsi.h>
+#include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
@@ -963,10 +964,14 @@  static int sun6i_dsi_attach(struct mipi_dsi_host *host,
 			    struct mipi_dsi_device *device)
 {
 	struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
-	struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
+	struct drm_panel *panel;
+	int ret;
+
+	ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
+					  &panel, NULL);
+	if (ret)
+		return ret;
 
-	if (IS_ERR(panel))
-		return PTR_ERR(panel);
 	if (!dsi->drm || !dsi->drm->registered)
 		return -EPROBE_DEFER;