diff mbox series

[1/2] dt-bindings: drm: bridge: adi,adv7533: Document adi,disable-lanes-override property

Message ID 20220309151109.20957-1-biju.das.jz@bp.renesas.com (mailing list archive)
State Not Applicable
Delegated to: Kieran Bingham
Headers show
Series [1/2] dt-bindings: drm: bridge: adi,adv7533: Document adi,disable-lanes-override property | expand

Commit Message

Biju Das March 9, 2022, 3:11 p.m. UTC
On Renesas RZ/{G2L,V2L} platforms changing the lanes from 4 to 3 at
lower frequencies causes display instability. On such platforms, it
is better to avoid switching lanes from 4 to 3 as it needs different
set of PLL parameter constraints to make the display stable with 3
lanes.

This patch introduces 'adi,disable-lanes-override' property to disable
lane switching at lower frequencies.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../devicetree/bindings/display/bridge/adi,adv7533.yaml      | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Krzysztof Kozlowski March 10, 2022, 1:05 p.m. UTC | #1
On 09/03/2022 16:11, Biju Das wrote:
> On Renesas RZ/{G2L,V2L} platforms changing the lanes from 4 to 3 at
> lower frequencies causes display instability. On such platforms, it
> is better to avoid switching lanes from 4 to 3 as it needs different
> set of PLL parameter constraints to make the display stable with 3
> lanes.
> 
> This patch introduces 'adi,disable-lanes-override' property to disable
> lane switching at lower frequencies.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  .../devicetree/bindings/display/bridge/adi,adv7533.yaml      | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> index f36209137c8a..2dc378039d21 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> @@ -84,6 +84,11 @@ properties:
>        timings for HDMI output.
>      type: boolean
>  
> +  adi,disable-lanes-override:
> +    description:
> +      Disables the overriding lanes at lower frequencies.
> +    type: boolean

The bindings should not focus on desired feature/functionality of
driver, but hardware. You are now encoding the driver behavior in the
bindings.

Best regards,
Krzysztof
Laurent Pinchart March 10, 2022, 4:35 p.m. UTC | #2
Hi Biju,

Thank you for the patch.

On Wed, Mar 09, 2022 at 03:11:08PM +0000, Biju Das wrote:
> On Renesas RZ/{G2L,V2L} platforms changing the lanes from 4 to 3 at
> lower frequencies causes display instability. On such platforms, it
> is better to avoid switching lanes from 4 to 3 as it needs different
> set of PLL parameter constraints to make the display stable with 3
> lanes.

Is this because the PLL calculation code doesn't work properly, or
because the hardware can't support this ?

> This patch introduces 'adi,disable-lanes-override' property to disable
> lane switching at lower frequencies.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  .../devicetree/bindings/display/bridge/adi,adv7533.yaml      | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> index f36209137c8a..2dc378039d21 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> @@ -84,6 +84,11 @@ properties:
>        timings for HDMI output.
>      type: boolean
>  
> +  adi,disable-lanes-override:
> +    description:
> +      Disables the overriding lanes at lower frequencies.
> +    type: boolean
> +
>    adi,dsi-lanes:
>      description: Number of DSI data lanes connected to the DSI host.
>      $ref: /schemas/types.yaml#/definitions/uint32
Biju Das March 10, 2022, 4:56 p.m. UTC | #3
Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH 1/2] dt-bindings: drm: bridge: adi,adv7533: Document
> adi,disable-lanes-override property
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Wed, Mar 09, 2022 at 03:11:08PM +0000, Biju Das wrote:
> > On Renesas RZ/{G2L,V2L} platforms changing the lanes from 4 to 3 at
> > lower frequencies causes display instability. On such platforms, it is
> > better to avoid switching lanes from 4 to 3 as it needs different set
> > of PLL parameter constraints to make the display stable with 3 lanes.
> 
> Is this because the PLL calculation code doesn't work properly, or because
> the hardware can't support this ?

PLL Calculation is correct, that is the reason it works for all resolution
with 4 lanes. There are 2 clocks generated by PLL5 which is connected to a mux
with clock sources, namely 'FOUTPOSTDIV' and 'FOUT1PH0'

This Mux is connected to DSI dividers. 

'FOUTPOSTDIV' should be selected if (PLL_INPUT_FREQ/VCLK) is odd and 'FOUT1PH0' should
be selected if it is even.

The PLL calculation makes use of even selection('FOUT1PH0') and video works for all frequencies
with 4 lanes.

With 'FOUT1PH0' as clock source, if I switch to 3 lanes for lanes it doesn't work. 
But it work with 4lanes on all frequencies.

HW can support 3 lanes, if I set parameter to make (PLL_INPUT_FREQ/VCLK) odd and select
'FOUTPOSTDIV' as clk source to DSI divider.

I am not sure about the rational behind the constraint to Switch to 3 lanes for
lower frequency for this ADV7535 chip, as on our platform it can work with 4lanes
on all frequencies.

Cheers,
Biju

> 
> > This patch introduces 'adi,disable-lanes-override' property to disable
> > lane switching at lower frequencies.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  .../devicetree/bindings/display/bridge/adi,adv7533.yaml      | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> > b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> > index f36209137c8a..2dc378039d21 100644
> > ---
> > a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yam
> > +++ l
> > @@ -84,6 +84,11 @@ properties:
> >        timings for HDMI output.
> >      type: boolean
> >
> > +  adi,disable-lanes-override:
> > +    description:
> > +      Disables the overriding lanes at lower frequencies.
> > +    type: boolean
> > +
> >    adi,dsi-lanes:
> >      description: Number of DSI data lanes connected to the DSI host.
> >      $ref: /schemas/types.yaml#/definitions/uint32
> 
> --
> Regards,
> 
> Laurent Pinchart
Biju Das April 10, 2023, 4:24 p.m. UTC | #4
Hi All,

I would like to drop this patch as[1] and [2] fixes the issue

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/bridge/adv7511/adv7533.c?h=next-20230406&id=9a0cdcd6649b76f0b7ceec0e55b0a718321e34d3

[2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/bridge/adv7511/adv7533.c?h=next-20230406&id=ee0285e13455fdbce5de315bdbe91b5f198a2a06

Cheers,
Biju


> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Wednesday, March 9, 2022 3:11 PM
> To: David Airlie <airlied@linux.ie>; Daniel Vetter <daniel@ffwll.ch>; Rob
> Herring <robh+dt@kernel.org>
> Cc: Biju Das <biju.das.jz@bp.renesas.com>; Ricardo CaƱuelo
> <ricardo.canuelo@collabora.com>; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>; dri-devel@lists.freedesktop.org;
> devicetree@vger.kernel.org; Geert Uytterhoeven <geert+renesas@glider.be>;
> Chris Paterson <Chris.Paterson2@renesas.com>; Biju Das
> <biju.das@bp.renesas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@bp.renesas.com>; linux-renesas-soc@vger.kernel.org
> Subject: [PATCH 1/2] dt-bindings: drm: bridge: adi,adv7533: Document
> adi,disable-lanes-override property
> 
> On Renesas RZ/{G2L,V2L} platforms changing the lanes from 4 to 3 at lower
> frequencies causes display instability. On such platforms, it is better to
> avoid switching lanes from 4 to 3 as it needs different set of PLL parameter
> constraints to make the display stable with 3 lanes.
> 
> This patch introduces 'adi,disable-lanes-override' property to disable lane
> switching at lower frequencies.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  .../devicetree/bindings/display/bridge/adi,adv7533.yaml      | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> index f36209137c8a..2dc378039d21 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> @@ -84,6 +84,11 @@ properties:
>        timings for HDMI output.
>      type: boolean
> 
> +  adi,disable-lanes-override:
> +    description:
> +      Disables the overriding lanes at lower frequencies.
> +    type: boolean
> +
>    adi,dsi-lanes:
>      description: Number of DSI data lanes connected to the DSI host.
>      $ref: /schemas/types.yaml#/definitions/uint32
> --
> 2.17.1
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
index f36209137c8a..2dc378039d21 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
@@ -84,6 +84,11 @@  properties:
       timings for HDMI output.
     type: boolean
 
+  adi,disable-lanes-override:
+    description:
+      Disables the overriding lanes at lower frequencies.
+    type: boolean
+
   adi,dsi-lanes:
     description: Number of DSI data lanes connected to the DSI host.
     $ref: /schemas/types.yaml#/definitions/uint32