[PATCH v12 0/7] drm/sun4i: Allwinner A64 MIPI-DSI support

[PATCH v12 0/7] drm/sun4i: Allwinner A64 MIPI-DSI support

[Jagan Teki]

This is v12 version for Allwinner A64 MIPI-DSI support
and here is the previous version set[1]

Changes for v12:
- use enum insted of oneOf+const
- handle bus clock using regmap attach clk
- tested on A64, A33 boards.
Changes for v11:
- fix dt-bindings for dphy
- fix dt-bindings for dsi controller
- add bus clock handling code
- tested on A64, A33 boards.
Changes for v10:
- updated dt-bindings as per .yaml format
- rebased on drm-misc/for-linux-next
Changes for v9:
- moved dsi fixes in separate series on top of A33
- rebase on linux-next
Changes for v8:
- rebased on drm-misc change along with linux-next
- reworked video start delay patch
- tested on 4 different dsi panels
- reworked commit messages
Changes for v7:
- moved vcc-dsi binding to required filed.
- drop quotes on fallback dphy bindings.
- drop min_rate clock pll-mipi patches.
- introduce dclk divider computation as like A64 BSP.
- add A64 DSI quark patches.
- fixed A64 DSI pipeline.
- add proper commit messages.
- collect Merlijn Wajer Tested-by credits.
Changes for v6:
- dropped unneeded changes, patches
- fixed all burst mode patches as per previous version comments
- rebase on master
- update proper commit message
- dropped unneeded comments
- order the patches that make review easy
Changes for v5:
- collect Rob, Acked-by
- droped "Fix VBP size calculation" patch
- updated vblk timing calculation.
- droped techstar, bananapi dsi panel drivers which may require
  bridge or other setup. it's under discussion.
Changes for v4:
- droppoed untested CCU_FEATURE_FIXED_POSTDIV check code in
  nkm min, max rate patches
- create two patches for "Add Allwinner A64 MIPI DSI support"
  one for has_mod_clk quirk and other one for A64 support
- use existing driver code construct for hblk computation
- dropped "Increase hfp packet overhead" patch [2], though BSP added
  this but we have no issues as of now.
  (no issues on panel side w/o this change)
- create separate function for vblk computation 
- enable vcc-dsi regulator in dsi_runtime_resume
- collect Rob, Acked-by
- update MAINTAINERS file for panel drivers
- cleanup commit messages
- fixed checkpatch warnings/errors

[1] https://patchwork.freedesktop.org/series/68579/

Any inputs?
Jagan.

Jagan Teki (7):
  dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller
  dt-bindings: sun6i-dsi: Add A64 DPHY compatible (w/ A31 fallback)
  drm/sun4i: dsi: Add has_mod_clk quirk
  drm/sun4i: dsi: Handle bus clock via regmap_mmio_attach_clk
  drm/sun4i: dsi: Add Allwinner A64 MIPI DSI support
  arm64: dts: allwinner: a64: Add MIPI DSI pipeline
  [DO NOT MERGE] arm64: dts: allwinner: bananapi-m64: Enable Bananapi S070WV20-CT16 DSI panel

 .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 ++++-
 .../phy/allwinner,sun6i-a31-mipi-dphy.yaml    |  6 +-
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 31 +++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 37 ++++++++
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c        | 86 +++++++++++++++----
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h        |  5 ++
 6 files changed, 164 insertions(+), 21 deletions(-)

-- 
2.18.0.321.gffc6fa0e3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v12 1/7] dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller

[Jagan Teki]

The MIPI DSI controller in Allwinner A64 is similar to A33.

But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
to have separate compatible for A64 on the same driver.

DSI_SCLK uses mod clock-names on dt-bindings, so the same
is not required for A64.

On that note
- A64 require minimum of 1 clock like the bus clock
- A33 require minimum of 2 clocks like both bus, mod clocks

So, update dt-bindings so-that it can document both A33,
A64 bindings requirements.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v12:
- Use 'enum' instead of oneOf+const

 .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
index dafc0980c4fa..b91446475f35 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
@@ -15,7 +15,9 @@ properties:
   "#size-cells": true
 
   compatible:
-    const: allwinner,sun6i-a31-mipi-dsi
+    enum:
+      - allwinner,sun6i-a31-mipi-dsi
+      - allwinner,sun50i-a64-mipi-dsi
 
   reg:
     maxItems: 1
@@ -24,6 +26,8 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 1
+    maxItems: 2
     items:
       - description: Bus Clock
       - description: Module Clock
@@ -63,13 +67,25 @@ required:
   - reg
   - interrupts
   - clocks
-  - clock-names
   - phys
   - phy-names
   - resets
   - vcc-dsi-supply
   - port
 
+allOf:
+  - if:
+      properties:
+         compatible:
+           contains:
+             const: allwinner,sun6i-a31-mipi-dsi
+      then:
+        properties:
+          clocks:
+            minItems: 2
+        required:
+          - clock-names
+
 additionalProperties: false
 
 examples:
-- 
2.18.0.321.gffc6fa0e3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v12 1/7] dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2212 bytes --]

On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> The MIPI DSI controller in Allwinner A64 is similar to A33.
>
> But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> to have separate compatible for A64 on the same driver.
>
> DSI_SCLK uses mod clock-names on dt-bindings, so the same
> is not required for A64.
>
> On that note
> - A64 require minimum of 1 clock like the bus clock
> - A33 require minimum of 2 clocks like both bus, mod clocks
>
> So, update dt-bindings so-that it can document both A33,
> A64 bindings requirements.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v12:
> - Use 'enum' instead of oneOf+const
>
>  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> index dafc0980c4fa..b91446475f35 100644
> --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> @@ -15,7 +15,9 @@ properties:
>    "#size-cells": true
>
>    compatible:
> -    const: allwinner,sun6i-a31-mipi-dsi
> +    enum:
> +      - allwinner,sun6i-a31-mipi-dsi
> +      - allwinner,sun50i-a64-mipi-dsi
>
>    reg:
>      maxItems: 1
> @@ -24,6 +26,8 @@ properties:
>      maxItems: 1
>
>    clocks:
> +    minItems: 1
> +    maxItems: 2
>      items:
>        - description: Bus Clock
>        - description: Module Clock
> @@ -63,13 +67,25 @@ required:
>    - reg
>    - interrupts
>    - clocks
> -  - clock-names
>    - phys
>    - phy-names
>    - resets
>    - vcc-dsi-supply
>    - port
>
> +allOf:
> +  - if:
> +      properties:
> +         compatible:
> +           contains:
> +             const: allwinner,sun6i-a31-mipi-dsi
> +      then:
> +        properties:
> +          clocks:
> +            minItems: 2
> +        required:
> +          - clock-names
> +

Your else condition should check that the number of clocks items is 1
on the A64

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v12 1/7] dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller

[Jagan Teki]

On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > The MIPI DSI controller in Allwinner A64 is similar to A33.
> >
> > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > to have separate compatible for A64 on the same driver.
> >
> > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > is not required for A64.
> >
> > On that note
> > - A64 require minimum of 1 clock like the bus clock
> > - A33 require minimum of 2 clocks like both bus, mod clocks
> >
> > So, update dt-bindings so-that it can document both A33,
> > A64 bindings requirements.
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v12:
> > - Use 'enum' instead of oneOf+const
> >
> >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > index dafc0980c4fa..b91446475f35 100644
> > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > @@ -15,7 +15,9 @@ properties:
> >    "#size-cells": true
> >
> >    compatible:
> > -    const: allwinner,sun6i-a31-mipi-dsi
> > +    enum:
> > +      - allwinner,sun6i-a31-mipi-dsi
> > +      - allwinner,sun50i-a64-mipi-dsi
> >
> >    reg:
> >      maxItems: 1
> > @@ -24,6 +26,8 @@ properties:
> >      maxItems: 1
> >
> >    clocks:
> > +    minItems: 1
> > +    maxItems: 2
> >      items:
> >        - description: Bus Clock
> >        - description: Module Clock
> > @@ -63,13 +67,25 @@ required:
> >    - reg
> >    - interrupts
> >    - clocks
> > -  - clock-names
> >    - phys
> >    - phy-names
> >    - resets
> >    - vcc-dsi-supply
> >    - port
> >
> > +allOf:
> > +  - if:
> > +      properties:
> > +         compatible:
> > +           contains:
> > +             const: allwinner,sun6i-a31-mipi-dsi
> > +      then:
> > +        properties:
> > +          clocks:
> > +            minItems: 2
> > +        required:
> > +          - clock-names
> > +
>
> Your else condition should check that the number of clocks items is 1
> on the A64

But the minItems mentioned as 1 in clocks, which is unchanged number
by default. doesn't it sufficient?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v12 1/7] dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3132 bytes --]

On Mon, Dec 16, 2019 at 04:37:20PM +0530, Jagan Teki wrote:
> On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > > The MIPI DSI controller in Allwinner A64 is similar to A33.
> > >
> > > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > > to have separate compatible for A64 on the same driver.
> > >
> > > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > > is not required for A64.
> > >
> > > On that note
> > > - A64 require minimum of 1 clock like the bus clock
> > > - A33 require minimum of 2 clocks like both bus, mod clocks
> > >
> > > So, update dt-bindings so-that it can document both A33,
> > > A64 bindings requirements.
> > >
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > > Changes for v12:
> > > - Use 'enum' instead of oneOf+const
> > >
> > >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > index dafc0980c4fa..b91446475f35 100644
> > > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > @@ -15,7 +15,9 @@ properties:
> > >    "#size-cells": true
> > >
> > >    compatible:
> > > -    const: allwinner,sun6i-a31-mipi-dsi
> > > +    enum:
> > > +      - allwinner,sun6i-a31-mipi-dsi
> > > +      - allwinner,sun50i-a64-mipi-dsi
> > >
> > >    reg:
> > >      maxItems: 1
> > > @@ -24,6 +26,8 @@ properties:
> > >      maxItems: 1
> > >
> > >    clocks:
> > > +    minItems: 1
> > > +    maxItems: 2
> > >      items:
> > >        - description: Bus Clock
> > >        - description: Module Clock
> > > @@ -63,13 +67,25 @@ required:
> > >    - reg
> > >    - interrupts
> > >    - clocks
> > > -  - clock-names
> > >    - phys
> > >    - phy-names
> > >    - resets
> > >    - vcc-dsi-supply
> > >    - port
> > >
> > > +allOf:
> > > +  - if:
> > > +      properties:
> > > +         compatible:
> > > +           contains:
> > > +             const: allwinner,sun6i-a31-mipi-dsi
> > > +      then:
> > > +        properties:
> > > +          clocks:
> > > +            minItems: 2
> > > +        required:
> > > +          - clock-names
> > > +
> >
> > Your else condition should check that the number of clocks items is 1
> > on the A64
>
> But the minItems mentioned as 1 in clocks, which is unchanged number
> by default. doesn't it sufficient?

In the main schema, it's said that the clocks property can have one or
two elements (to cover the A31 case that has one, and the A64 case
that has 2).

This is fine.

Later on, you enforce that the A64 has two elements, and this is fine
too.

However, you never check that on the A31 you only have one clock, and
you could very well have two and no one would notice.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v12 1/7] dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller

[Jagan Teki]

On Mon, Dec 16, 2019 at 4:50 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Mon, Dec 16, 2019 at 04:37:20PM +0530, Jagan Teki wrote:
> > On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > > > The MIPI DSI controller in Allwinner A64 is similar to A33.
> > > >
> > > > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > > > to have separate compatible for A64 on the same driver.
> > > >
> > > > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > > > is not required for A64.
> > > >
> > > > On that note
> > > > - A64 require minimum of 1 clock like the bus clock
> > > > - A33 require minimum of 2 clocks like both bus, mod clocks
> > > >
> > > > So, update dt-bindings so-that it can document both A33,
> > > > A64 bindings requirements.
> > > >
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > ---
> > > > Changes for v12:
> > > > - Use 'enum' instead of oneOf+const
> > > >
> > > >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > index dafc0980c4fa..b91446475f35 100644
> > > > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > @@ -15,7 +15,9 @@ properties:
> > > >    "#size-cells": true
> > > >
> > > >    compatible:
> > > > -    const: allwinner,sun6i-a31-mipi-dsi
> > > > +    enum:
> > > > +      - allwinner,sun6i-a31-mipi-dsi
> > > > +      - allwinner,sun50i-a64-mipi-dsi
> > > >
> > > >    reg:
> > > >      maxItems: 1
> > > > @@ -24,6 +26,8 @@ properties:
> > > >      maxItems: 1
> > > >
> > > >    clocks:
> > > > +    minItems: 1
> > > > +    maxItems: 2
> > > >      items:
> > > >        - description: Bus Clock
> > > >        - description: Module Clock
> > > > @@ -63,13 +67,25 @@ required:
> > > >    - reg
> > > >    - interrupts
> > > >    - clocks
> > > > -  - clock-names
> > > >    - phys
> > > >    - phy-names
> > > >    - resets
> > > >    - vcc-dsi-supply
> > > >    - port
> > > >
> > > > +allOf:
> > > > +  - if:
> > > > +      properties:
> > > > +         compatible:
> > > > +           contains:
> > > > +             const: allwinner,sun6i-a31-mipi-dsi
> > > > +      then:
> > > > +        properties:
> > > > +          clocks:
> > > > +            minItems: 2
> > > > +        required:
> > > > +          - clock-names
> > > > +
> > >
> > > Your else condition should check that the number of clocks items is 1
> > > on the A64
> >
> > But the minItems mentioned as 1 in clocks, which is unchanged number
> > by default. doesn't it sufficient?
>
> In the main schema, it's said that the clocks property can have one or
> two elements (to cover the A31 case that has one, and the A64 case
> that has 2).
>
> This is fine.
>
> Later on, you enforce that the A64 has two elements, and this is fine
> too.

Actually A31 case has 2 and A64 case has 1.

>
> However, you never check that on the A31 you only have one clock, and
> you could very well have two and no one would notice.

I did check A31 case for 2 but not in A64. this is what you mean? so
adding A64 check like below would fine?

allOf:
  - if:
      properties:
         compatible:
           contains:
             const: allwinner,sun6i-a31-mipi-dsi
      then:
        properties:
          clocks:
            minItems: 2
        required:
          - clock-names
  - if:
      properties:
         compatible:
           contains:
             const: allwinner,sun50i-a64-mipi-dsi
      then:
        properties:
          clocks:
            minItems: 1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v12 1/7] dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 4480 bytes --]

On Mon, Dec 16, 2019 at 10:29:08PM +0530, Jagan Teki wrote:
> On Mon, Dec 16, 2019 at 4:50 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > On Mon, Dec 16, 2019 at 04:37:20PM +0530, Jagan Teki wrote:
> > > On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > >
> > > > On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > > > > The MIPI DSI controller in Allwinner A64 is similar to A33.
> > > > >
> > > > > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > > > > to have separate compatible for A64 on the same driver.
> > > > >
> > > > > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > > > > is not required for A64.
> > > > >
> > > > > On that note
> > > > > - A64 require minimum of 1 clock like the bus clock
> > > > > - A33 require minimum of 2 clocks like both bus, mod clocks
> > > > >
> > > > > So, update dt-bindings so-that it can document both A33,
> > > > > A64 bindings requirements.
> > > > >
> > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > ---
> > > > > Changes for v12:
> > > > > - Use 'enum' instead of oneOf+const
> > > > >
> > > > >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> > > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > index dafc0980c4fa..b91446475f35 100644
> > > > > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > @@ -15,7 +15,9 @@ properties:
> > > > >    "#size-cells": true
> > > > >
> > > > >    compatible:
> > > > > -    const: allwinner,sun6i-a31-mipi-dsi
> > > > > +    enum:
> > > > > +      - allwinner,sun6i-a31-mipi-dsi
> > > > > +      - allwinner,sun50i-a64-mipi-dsi
> > > > >
> > > > >    reg:
> > > > >      maxItems: 1
> > > > > @@ -24,6 +26,8 @@ properties:
> > > > >      maxItems: 1
> > > > >
> > > > >    clocks:
> > > > > +    minItems: 1
> > > > > +    maxItems: 2
> > > > >      items:
> > > > >        - description: Bus Clock
> > > > >        - description: Module Clock
> > > > > @@ -63,13 +67,25 @@ required:
> > > > >    - reg
> > > > >    - interrupts
> > > > >    - clocks
> > > > > -  - clock-names
> > > > >    - phys
> > > > >    - phy-names
> > > > >    - resets
> > > > >    - vcc-dsi-supply
> > > > >    - port
> > > > >
> > > > > +allOf:
> > > > > +  - if:
> > > > > +      properties:
> > > > > +         compatible:
> > > > > +           contains:
> > > > > +             const: allwinner,sun6i-a31-mipi-dsi
> > > > > +      then:
> > > > > +        properties:
> > > > > +          clocks:
> > > > > +            minItems: 2
> > > > > +        required:
> > > > > +          - clock-names
> > > > > +
> > > >
> > > > Your else condition should check that the number of clocks items is 1
> > > > on the A64
> > >
> > > But the minItems mentioned as 1 in clocks, which is unchanged number
> > > by default. doesn't it sufficient?
> >
> > In the main schema, it's said that the clocks property can have one or
> > two elements (to cover the A31 case that has one, and the A64 case
> > that has 2).
> >
> > This is fine.
> >
> > Later on, you enforce that the A64 has two elements, and this is fine
> > too.
>
> Actually A31 case has 2 and A64 case has 1.
>
> >
> > However, you never check that on the A31 you only have one clock, and
> > you could very well have two and no one would notice.
>
> I did check A31 case for 2 but not in A64. this is what you mean? so
> adding A64 check like below would fine?
>
> allOf:
>   - if:
>       properties:
>          compatible:
>            contains:
>              const: allwinner,sun6i-a31-mipi-dsi

You need a new line here

>       then:

This is the wrong level of indentation, it needs to be at the same level than if

>         properties:
>           clocks:
>             minItems: 2

Newline

>         required:
>           - clock-names

Newline

>   - if:
>       properties:
>          compatible:
>            contains:
>              const: allwinner,sun50i-a64-mipi-dsi
>       then:
>         properties:
>           clocks:
>             minItems: 1

(and same thing here).

But yeah, otherwise that's what I meant

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v12 1/7] dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller

[Jagan Teki]

On Tue, Dec 17, 2019 at 12:04 AM Maxime Ripard <mripard@kernel.org> wrote:
>
> On Mon, Dec 16, 2019 at 10:29:08PM +0530, Jagan Teki wrote:
> > On Mon, Dec 16, 2019 at 4:50 PM Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > On Mon, Dec 16, 2019 at 04:37:20PM +0530, Jagan Teki wrote:
> > > > On Wed, Dec 4, 2019 at 7:06 PM Maxime Ripard <mripard@kernel.org> wrote:
> > > > >
> > > > > On Tue, Dec 03, 2019 at 07:18:10PM +0530, Jagan Teki wrote:
> > > > > > The MIPI DSI controller in Allwinner A64 is similar to A33.
> > > > > >
> > > > > > But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
> > > > > > to have separate compatible for A64 on the same driver.
> > > > > >
> > > > > > DSI_SCLK uses mod clock-names on dt-bindings, so the same
> > > > > > is not required for A64.
> > > > > >
> > > > > > On that note
> > > > > > - A64 require minimum of 1 clock like the bus clock
> > > > > > - A33 require minimum of 2 clocks like both bus, mod clocks
> > > > > >
> > > > > > So, update dt-bindings so-that it can document both A33,
> > > > > > A64 bindings requirements.
> > > > > >
> > > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > ---
> > > > > > Changes for v12:
> > > > > > - Use 'enum' instead of oneOf+const
> > > > > >
> > > > > >  .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 20 +++++++++++++++++--
> > > > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > > index dafc0980c4fa..b91446475f35 100644
> > > > > > --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
> > > > > > @@ -15,7 +15,9 @@ properties:
> > > > > >    "#size-cells": true
> > > > > >
> > > > > >    compatible:
> > > > > > -    const: allwinner,sun6i-a31-mipi-dsi
> > > > > > +    enum:
> > > > > > +      - allwinner,sun6i-a31-mipi-dsi
> > > > > > +      - allwinner,sun50i-a64-mipi-dsi
> > > > > >
> > > > > >    reg:
> > > > > >      maxItems: 1
> > > > > > @@ -24,6 +26,8 @@ properties:
> > > > > >      maxItems: 1
> > > > > >
> > > > > >    clocks:
> > > > > > +    minItems: 1
> > > > > > +    maxItems: 2
> > > > > >      items:
> > > > > >        - description: Bus Clock
> > > > > >        - description: Module Clock
> > > > > > @@ -63,13 +67,25 @@ required:
> > > > > >    - reg
> > > > > >    - interrupts
> > > > > >    - clocks
> > > > > > -  - clock-names
> > > > > >    - phys
> > > > > >    - phy-names
> > > > > >    - resets
> > > > > >    - vcc-dsi-supply
> > > > > >    - port
> > > > > >
> > > > > > +allOf:
> > > > > > +  - if:
> > > > > > +      properties:
> > > > > > +         compatible:
> > > > > > +           contains:
> > > > > > +             const: allwinner,sun6i-a31-mipi-dsi
> > > > > > +      then:
> > > > > > +        properties:
> > > > > > +          clocks:
> > > > > > +            minItems: 2
> > > > > > +        required:
> > > > > > +          - clock-names
> > > > > > +
> > > > >
> > > > > Your else condition should check that the number of clocks items is 1
> > > > > on the A64
> > > >
> > > > But the minItems mentioned as 1 in clocks, which is unchanged number
> > > > by default. doesn't it sufficient?
> > >
> > > In the main schema, it's said that the clocks property can have one or
> > > two elements (to cover the A31 case that has one, and the A64 case
> > > that has 2).
> > >
> > > This is fine.
> > >
> > > Later on, you enforce that the A64 has two elements, and this is fine
> > > too.
> >
> > Actually A31 case has 2 and A64 case has 1.
> >
> > >
> > > However, you never check that on the A31 you only have one clock, and
> > > you could very well have two and no one would notice.
> >
> > I did check A31 case for 2 but not in A64. this is what you mean? so
> > adding A64 check like below would fine?
> >
> > allOf:
> >   - if:
> >       properties:
> >          compatible:
> >            contains:
> >              const: allwinner,sun6i-a31-mipi-dsi
>
> You need a new line here

I couldn't see new line before then: in example schema -
https://elixir.bootlin.com/linux/v5.5-rc2/source/Documentation/devicetree/bindings/example-schema.yaml#L208
May be a new changes for this ML or so?

>
> >       then:
>
> This is the wrong level of indentation, it needs to be at the same level than if

True, I have seen it in example schema.

>
> >         properties:
> >           clocks:
> >             minItems: 2
>
> Newline
>
> >         required:
> >           - clock-names
>
> Newline
>
> >   - if:
> >       properties:
> >          compatible:
> >            contains:
> >              const: allwinner,sun50i-a64-mipi-dsi
> >       then:
> >         properties:
> >           clocks:
> >             minItems: 1
>
> (and same thing here).
>
> But yeah, otherwise that's what I meant

Thanks for the detailed review.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v12 2/7] dt-bindings: sun6i-dsi: Add A64 DPHY compatible (w/ A31 fallback)

[Jagan Teki]

The MIPI DSI PHY controller on Allwinner A64 is similar
on the one on A31.

Add A64 compatible and append A31 compatible as fallback.

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

 .../bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml         | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
index fa46670de299..8841938050b2 100644
--- a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
+++ b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
@@ -15,7 +15,11 @@ properties:
     const: 0
 
   compatible:
-    const: allwinner,sun6i-a31-mipi-dphy
+    oneOf:
+      - const: allwinner,sun6i-a31-mipi-dphy
+      - items:
+          - const: allwinner,sun50i-a64-mipi-dphy
+          - const: allwinner,sun6i-a31-mipi-dphy
 
   reg:
     maxItems: 1
-- 
2.18.0.321.gffc6fa0e3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v12 2/7] dt-bindings: sun6i-dsi: Add A64 DPHY compatible (w/ A31 fallback)

[Rob Herring]

On Tue, Dec 03, 2019 at 07:18:11PM +0530, Jagan Teki wrote:
> The MIPI DSI PHY controller on Allwinner A64 is similar
> on the one on A31.
> 
> Add A64 compatible and append A31 compatible as fallback.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v12:
> - none
> 
>  .../bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml         | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v12 3/7] drm/sun4i: dsi: Add has_mod_clk quirk

[Jagan Teki]

As per the user manual, look like mod clock is not mandatory
for all Allwinner MIPI DSI controllers, it is connected to
CLK_DSI_SCLK for A31 and not available in A64.

So add has_mod_clk quirk and process the mod clk accordingly.

Tested-by: Merlijn Wajer <merlijn@wizzup.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v12:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 38 ++++++++++++++++++--------
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  5 ++++
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index c958ca9bae63..8c4c541224dd 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -11,6 +11,7 @@
 #include <linux/crc-ccitt.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/phy/phy-mipi-dphy.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
@@ -1093,6 +1094,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 	dsi->dev = dev;
 	dsi->host.ops = &sun6i_dsi_host_ops;
 	dsi->host.dev = dev;
+	dsi->variant = of_device_get_match_data(dev);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(dev, res);
@@ -1120,17 +1122,20 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 		return PTR_ERR(dsi->reset);
 	}
 
-	dsi->mod_clk = devm_clk_get(dev, "mod");
-	if (IS_ERR(dsi->mod_clk)) {
-		dev_err(dev, "Couldn't get the DSI mod clock\n");
-		return PTR_ERR(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk) {
+		dsi->mod_clk = devm_clk_get(dev, "mod");
+		if (IS_ERR(dsi->mod_clk)) {
+			dev_err(dev, "Couldn't get the DSI mod clock\n");
+			return PTR_ERR(dsi->mod_clk);
+		}
 	}
 
 	/*
 	 * In order to operate properly, that clock seems to be always
 	 * set to 297MHz.
 	 */
-	clk_set_rate_exclusive(dsi->mod_clk, 297000000);
+	if (dsi->variant->has_mod_clk)
+		clk_set_rate_exclusive(dsi->mod_clk, 297000000);
 
 	dsi->dphy = devm_phy_get(dev, "dphy");
 	if (IS_ERR(dsi->dphy)) {
@@ -1160,7 +1165,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 err_pm_disable:
 	pm_runtime_disable(dev);
 err_unprotect_clk:
-	clk_rate_exclusive_put(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_rate_exclusive_put(dsi->mod_clk);
 	return ret;
 }
 
@@ -1172,7 +1178,8 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
 	component_del(&pdev->dev, &sun6i_dsi_ops);
 	mipi_dsi_host_unregister(&dsi->host);
 	pm_runtime_disable(dev);
-	clk_rate_exclusive_put(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_rate_exclusive_put(dsi->mod_clk);
 
 	return 0;
 }
@@ -1189,7 +1196,8 @@ static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev)
 	}
 
 	reset_control_deassert(dsi->reset);
-	clk_prepare_enable(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_prepare_enable(dsi->mod_clk);
 
 	/*
 	 * Enable the DSI block.
@@ -1217,7 +1225,8 @@ static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev)
 {
 	struct sun6i_dsi *dsi = dev_get_drvdata(dev);
 
-	clk_disable_unprepare(dsi->mod_clk);
+	if (dsi->variant->has_mod_clk)
+		clk_disable_unprepare(dsi->mod_clk);
 	reset_control_assert(dsi->reset);
 	regulator_disable(dsi->regulator);
 
@@ -1230,9 +1239,16 @@ static const struct dev_pm_ops sun6i_dsi_pm_ops = {
 			   NULL)
 };
 
+static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi = {
+	.has_mod_clk = true,
+};
+
 static const struct of_device_id sun6i_dsi_of_table[] = {
-	{ .compatible = "allwinner,sun6i-a31-mipi-dsi" },
-	{ }
+	{
+		.compatible = "allwinner,sun6i-a31-mipi-dsi",
+		.data = &sun6i_a31_mipi_dsi,
+	},
+	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
 
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index 3f4846f581ef..d791c9f6fccf 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -15,6 +15,10 @@
 
 #define SUN6I_DSI_TCON_DIV	4
 
+struct sun6i_dsi_variant {
+	bool			has_mod_clk;
+};
+
 struct sun6i_dsi {
 	struct drm_connector	connector;
 	struct drm_encoder	encoder;
@@ -31,6 +35,7 @@ struct sun6i_dsi {
 	struct sun4i_drv	*drv;
 	struct mipi_dsi_device	*device;
 	struct drm_panel	*panel;
+	const struct sun6i_dsi_variant	*variant;
 };
 
 static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
-- 
2.18.0.321.gffc6fa0e3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v12 3/7] drm/sun4i: dsi: Add has_mod_clk quirk

[Chen-Yu Tsai]

On Tue, Dec 3, 2019 at 9:48 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> As per the user manual, look like mod clock is not mandatory
> for all Allwinner MIPI DSI controllers, it is connected to
> CLK_DSI_SCLK for A31 and not available in A64.
>
> So add has_mod_clk quirk and process the mod clk accordingly.
>
> Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v12:
> - none
>
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 38 ++++++++++++++++++--------
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  5 ++++
>  2 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index c958ca9bae63..8c4c541224dd 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -11,6 +11,7 @@
>  #include <linux/crc-ccitt.h>
>  #include <linux/module.h>
>  #include <linux/of_address.h>
> +#include <linux/of_device.h>
>  #include <linux/phy/phy-mipi-dphy.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> @@ -1093,6 +1094,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>         dsi->dev = dev;
>         dsi->host.ops = &sun6i_dsi_host_ops;
>         dsi->host.dev = dev;
> +       dsi->variant = of_device_get_match_data(dev);
>
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         base = devm_ioremap_resource(dev, res);
> @@ -1120,17 +1122,20 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>                 return PTR_ERR(dsi->reset);
>         }
>
> -       dsi->mod_clk = devm_clk_get(dev, "mod");
> -       if (IS_ERR(dsi->mod_clk)) {
> -               dev_err(dev, "Couldn't get the DSI mod clock\n");
> -               return PTR_ERR(dsi->mod_clk);
> +       if (dsi->variant->has_mod_clk) {
> +               dsi->mod_clk = devm_clk_get(dev, "mod");
> +               if (IS_ERR(dsi->mod_clk)) {
> +                       dev_err(dev, "Couldn't get the DSI mod clock\n");
> +                       return PTR_ERR(dsi->mod_clk);
> +               }
>         }
>
>         /*
>          * In order to operate properly, that clock seems to be always
>          * set to 297MHz.
>          */
> -       clk_set_rate_exclusive(dsi->mod_clk, 297000000);
> +       if (dsi->variant->has_mod_clk)
> +               clk_set_rate_exclusive(dsi->mod_clk, 297000000);

The clk API can handle NULL pointers, so you don't need to add the if here...

>
>         dsi->dphy = devm_phy_get(dev, "dphy");
>         if (IS_ERR(dsi->dphy)) {
> @@ -1160,7 +1165,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
>  err_pm_disable:
>         pm_runtime_disable(dev);
>  err_unprotect_clk:
> -       clk_rate_exclusive_put(dsi->mod_clk);
> +       if (dsi->variant->has_mod_clk)
> +               clk_rate_exclusive_put(dsi->mod_clk);

and here...

>         return ret;
>  }
>
> @@ -1172,7 +1178,8 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
>         component_del(&pdev->dev, &sun6i_dsi_ops);
>         mipi_dsi_host_unregister(&dsi->host);
>         pm_runtime_disable(dev);
> -       clk_rate_exclusive_put(dsi->mod_clk);
> +       if (dsi->variant->has_mod_clk)
> +               clk_rate_exclusive_put(dsi->mod_clk);

and here ...

>
>         return 0;
>  }
> @@ -1189,7 +1196,8 @@ static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev)
>         }
>
>         reset_control_deassert(dsi->reset);
> -       clk_prepare_enable(dsi->mod_clk);
> +       if (dsi->variant->has_mod_clk)
> +               clk_prepare_enable(dsi->mod_clk);

and here...

>
>         /*
>          * Enable the DSI block.
> @@ -1217,7 +1225,8 @@ static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev)
>  {
>         struct sun6i_dsi *dsi = dev_get_drvdata(dev);
>
> -       clk_disable_unprepare(dsi->mod_clk);
> +       if (dsi->variant->has_mod_clk)
> +               clk_disable_unprepare(dsi->mod_clk);

and here.

>         reset_control_assert(dsi->reset);
>         regulator_disable(dsi->regulator);
>
> @@ -1230,9 +1239,16 @@ static const struct dev_pm_ops sun6i_dsi_pm_ops = {
>                            NULL)
>  };
>
> +static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi = {
> +       .has_mod_clk = true,
> +};
> +
>  static const struct of_device_id sun6i_dsi_of_table[] = {
> -       { .compatible = "allwinner,sun6i-a31-mipi-dsi" },
> -       { }
> +       {
> +               .compatible = "allwinner,sun6i-a31-mipi-dsi",
> +               .data = &sun6i_a31_mipi_dsi,
> +       },
> +       { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> index 3f4846f581ef..d791c9f6fccf 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> @@ -15,6 +15,10 @@
>
>  #define SUN6I_DSI_TCON_DIV     4
>
> +struct sun6i_dsi_variant {
> +       bool                    has_mod_clk;
> +};
> +

You could choose to put this above the probe function, since this isn't used
anywhere else, and a pointer field doesn't need the full definition.

ChenYu

>  struct sun6i_dsi {
>         struct drm_connector    connector;
>         struct drm_encoder      encoder;
> @@ -31,6 +35,7 @@ struct sun6i_dsi {
>         struct sun4i_drv        *drv;
>         struct mipi_dsi_device  *device;
>         struct drm_panel        *panel;
> +       const struct sun6i_dsi_variant  *variant;
>  };
>
>  static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
> --
> 2.18.0.321.gffc6fa0e3
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v12 3/7] drm/sun4i: dsi: Add has_mod_clk quirk

[Jagan Teki]

On Tue, Dec 3, 2019 at 8:09 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Tue, Dec 3, 2019 at 9:48 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > As per the user manual, look like mod clock is not mandatory
> > for all Allwinner MIPI DSI controllers, it is connected to
> > CLK_DSI_SCLK for A31 and not available in A64.
> >
> > So add has_mod_clk quirk and process the mod clk accordingly.
> >
> > Tested-by: Merlijn Wajer <merlijn@wizzup.org>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > Changes for v12:
> > - none
> >
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 38 ++++++++++++++++++--------
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  5 ++++
> >  2 files changed, 32 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > index c958ca9bae63..8c4c541224dd 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/crc-ccitt.h>
> >  #include <linux/module.h>
> >  #include <linux/of_address.h>
> > +#include <linux/of_device.h>
> >  #include <linux/phy/phy-mipi-dphy.h>
> >  #include <linux/phy/phy.h>
> >  #include <linux/platform_device.h>
> > @@ -1093,6 +1094,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> >         dsi->dev = dev;
> >         dsi->host.ops = &sun6i_dsi_host_ops;
> >         dsi->host.dev = dev;
> > +       dsi->variant = of_device_get_match_data(dev);
> >
> >         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >         base = devm_ioremap_resource(dev, res);
> > @@ -1120,17 +1122,20 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> >                 return PTR_ERR(dsi->reset);
> >         }
> >
> > -       dsi->mod_clk = devm_clk_get(dev, "mod");
> > -       if (IS_ERR(dsi->mod_clk)) {
> > -               dev_err(dev, "Couldn't get the DSI mod clock\n");
> > -               return PTR_ERR(dsi->mod_clk);
> > +       if (dsi->variant->has_mod_clk) {
> > +               dsi->mod_clk = devm_clk_get(dev, "mod");
> > +               if (IS_ERR(dsi->mod_clk)) {
> > +                       dev_err(dev, "Couldn't get the DSI mod clock\n");
> > +                       return PTR_ERR(dsi->mod_clk);
> > +               }
> >         }
> >
> >         /*
> >          * In order to operate properly, that clock seems to be always
> >          * set to 297MHz.
> >          */
> > -       clk_set_rate_exclusive(dsi->mod_clk, 297000000);
> > +       if (dsi->variant->has_mod_clk)
> > +               clk_set_rate_exclusive(dsi->mod_clk, 297000000);
>
> The clk API can handle NULL pointers, so you don't need to add the if here...

Ohh. I'm not aware of this. does it added recently?

>
> >
> >         dsi->dphy = devm_phy_get(dev, "dphy");
> >         if (IS_ERR(dsi->dphy)) {
> > @@ -1160,7 +1165,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> >  err_pm_disable:
> >         pm_runtime_disable(dev);
> >  err_unprotect_clk:
> > -       clk_rate_exclusive_put(dsi->mod_clk);
> > +       if (dsi->variant->has_mod_clk)
> > +               clk_rate_exclusive_put(dsi->mod_clk);
>
> and here...
>
> >         return ret;
> >  }
> >
> > @@ -1172,7 +1178,8 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
> >         component_del(&pdev->dev, &sun6i_dsi_ops);
> >         mipi_dsi_host_unregister(&dsi->host);
> >         pm_runtime_disable(dev);
> > -       clk_rate_exclusive_put(dsi->mod_clk);
> > +       if (dsi->variant->has_mod_clk)
> > +               clk_rate_exclusive_put(dsi->mod_clk);
>
> and here ...
>
> >
> >         return 0;
> >  }
> > @@ -1189,7 +1196,8 @@ static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev)
> >         }
> >
> >         reset_control_deassert(dsi->reset);
> > -       clk_prepare_enable(dsi->mod_clk);
> > +       if (dsi->variant->has_mod_clk)
> > +               clk_prepare_enable(dsi->mod_clk);
>
> and here...
>
> >
> >         /*
> >          * Enable the DSI block.
> > @@ -1217,7 +1225,8 @@ static int __maybe_unused sun6i_dsi_runtime_suspend(struct device *dev)
> >  {
> >         struct sun6i_dsi *dsi = dev_get_drvdata(dev);
> >
> > -       clk_disable_unprepare(dsi->mod_clk);
> > +       if (dsi->variant->has_mod_clk)
> > +               clk_disable_unprepare(dsi->mod_clk);
>
> and here.
>
> >         reset_control_assert(dsi->reset);
> >         regulator_disable(dsi->regulator);
> >
> > @@ -1230,9 +1239,16 @@ static const struct dev_pm_ops sun6i_dsi_pm_ops = {
> >                            NULL)
> >  };
> >
> > +static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi = {
> > +       .has_mod_clk = true,
> > +};
> > +
> >  static const struct of_device_id sun6i_dsi_of_table[] = {
> > -       { .compatible = "allwinner,sun6i-a31-mipi-dsi" },
> > -       { }
> > +       {
> > +               .compatible = "allwinner,sun6i-a31-mipi-dsi",
> > +               .data = &sun6i_a31_mipi_dsi,
> > +       },
> > +       { /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > index 3f4846f581ef..d791c9f6fccf 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > @@ -15,6 +15,10 @@
> >
> >  #define SUN6I_DSI_TCON_DIV     4
> >
> > +struct sun6i_dsi_variant {
> > +       bool                    has_mod_clk;
> > +};
> > +
>
> You could choose to put this above the probe function, since this isn't used
> anywhere else, and a pointer field doesn't need the full definition.

Okay. Will it be okay to check the compatibility directly on probe
using of_device_is_compatible?

Jagan.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v12 4/7] drm/sun4i: dsi: Handle bus clock via regmap_mmio_attach_clk

[Jagan Teki]

regmap has special API to enable the controller bus clock while
initializing register space, and current driver is using
devm_regmap_init_mmio_clk which require to specify bus
clk_id argument as "bus"

But, the usage of clocks are varies between different Allwinner
DSI controllers. Clocking in A33 would need bus and mod clocks
where as A64 would need only bus clock.

Since A64 support only single bus clock, it is optional to
specify the clock-names on the controller device tree node.
So using NULL on clk_id would get the attached clock.

To support clk_id as "bus" and "NULL" during clock enablement
between controllers, this patch add generic code to handle
the bus clock using regmap_mmio_attach_clk with associated
regmap APIs.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v12:
- get bus clock only when mod clock present
- use regmap_mmio_attach_clk

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

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 8c4c541224dd..6085ad2eafc3 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1082,6 +1082,7 @@ static const struct component_ops sun6i_dsi_ops = {
 static int sun6i_dsi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	const char *clk_name = NULL;
 	struct sun6i_dsi *dsi;
 	struct resource *res;
 	void __iomem *base;
@@ -1095,6 +1096,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 	dsi->host.ops = &sun6i_dsi_host_ops;
 	dsi->host.dev = dev;
 	dsi->variant = of_device_get_match_data(dev);
+	if (dsi->variant->has_mod_clk)
+		clk_name = "bus";
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(dev, res);
@@ -1109,24 +1112,35 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 		return PTR_ERR(dsi->regulator);
 	}
 
-	dsi->regs = devm_regmap_init_mmio_clk(dev, "bus", base,
-					      &sun6i_dsi_regmap_config);
-	if (IS_ERR(dsi->regs)) {
-		dev_err(dev, "Couldn't create the DSI encoder regmap\n");
-		return PTR_ERR(dsi->regs);
-	}
-
 	dsi->reset = devm_reset_control_get_shared(dev, NULL);
 	if (IS_ERR(dsi->reset)) {
 		dev_err(dev, "Couldn't get our reset line\n");
 		return PTR_ERR(dsi->reset);
 	}
 
+	dsi->regs = devm_regmap_init_mmio(dev, base, &sun6i_dsi_regmap_config);
+	if (IS_ERR(dsi->regs)) {
+		dev_err(dev, "Couldn't init regmap\n");
+		return PTR_ERR(dsi->regs);
+	}
+
+	dsi->bus_clk = devm_clk_get(dev, clk_name);
+	if (IS_ERR(dsi->bus_clk)) {
+		dev_err(dev, "Couldn't get the DSI bus clock\n");
+		ret = PTR_ERR(dsi->bus_clk);
+		goto err_regmap;
+	} else {
+		ret = regmap_mmio_attach_clk(dsi->regs, dsi->bus_clk);
+		if (ret)
+			goto err_bus_clk;
+	}
+
 	if (dsi->variant->has_mod_clk) {
 		dsi->mod_clk = devm_clk_get(dev, "mod");
 		if (IS_ERR(dsi->mod_clk)) {
 			dev_err(dev, "Couldn't get the DSI mod clock\n");
-			return PTR_ERR(dsi->mod_clk);
+			ret = PTR_ERR(dsi->mod_clk);
+			goto err_attach_clk;
 		}
 	}
 
@@ -1167,6 +1181,14 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
 err_unprotect_clk:
 	if (dsi->variant->has_mod_clk)
 		clk_rate_exclusive_put(dsi->mod_clk);
+err_attach_clk:
+	if (!IS_ERR(dsi->bus_clk))
+		regmap_mmio_detach_clk(dsi->regs);
+err_bus_clk:
+	if (!IS_ERR(dsi->bus_clk))
+		clk_put(dsi->bus_clk);
+err_regmap:
+	regmap_exit(dsi->regs);
 	return ret;
 }
 
@@ -1181,6 +1203,11 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
 	if (dsi->variant->has_mod_clk)
 		clk_rate_exclusive_put(dsi->mod_clk);
 
+	if (!IS_ERR(dsi->bus_clk))
+		regmap_mmio_detach_clk(dsi->regs);
+
+	regmap_exit(dsi->regs);
+
 	return 0;
 }
 
-- 
2.18.0.321.gffc6fa0e3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v12 5/7] drm/sun4i: dsi: Add Allwinner A64 MIPI DSI support

[Jagan Teki]

The MIPI DSI controller in Allwinner A64 is similar to A33.

But unlike A33, A64 doesn't have DSI_SCLK gating so add compatible
for Allwinner A64 with uninitialized has_mod_clk driver.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
Changes for v12:
- none

 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 6085ad2eafc3..34a64473dd09 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1270,11 +1270,18 @@ static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi = {
 	.has_mod_clk = true,
 };
 
+static const struct sun6i_dsi_variant sun50i_a64_mipi_dsi = {
+};
+
 static const struct of_device_id sun6i_dsi_of_table[] = {
 	{
 		.compatible = "allwinner,sun6i-a31-mipi-dsi",
 		.data = &sun6i_a31_mipi_dsi,
 	},
+	{
+		.compatible = "allwinner,sun50i-a64-mipi-dsi",
+		.data = &sun50i_a64_mipi_dsi,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
-- 
2.18.0.321.gffc6fa0e3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v12 6/7] arm64: dts: allwinner: a64: Add MIPI DSI pipeline

[Jagan Teki]

Add MIPI DSI pipeline for Allwinner A64.

- dsi node, with A64 compatible since it doesn't support
  DSI_SCLK gating unlike A33
- dphy node, with A64 compatible with A33 fallback since
  DPHY on A64 and A33 is similar
- finally, attach the dsi_in to tcon0 for complete MIPI DSI

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
Changes for v12:
- none

 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 27e48234f1c2..1db8378f59a4 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -382,6 +382,12 @@
 					#address-cells = <1>;
 					#size-cells = <0>;
 					reg = <1>;
+
+					tcon0_out_dsi: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&dsi_in_tcon0>;
+						allwinner,tcon-channel = <1>;
+					};
 				};
 			};
 		};
@@ -1014,6 +1020,37 @@
 			status = "disabled";
 		};
 
+		dsi: dsi@1ca0000 {
+			compatible = "allwinner,sun50i-a64-mipi-dsi";
+			reg = <0x01ca0000 0x1000>;
+			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_MIPI_DSI>;
+			resets = <&ccu RST_BUS_MIPI_DSI>;
+			phys = <&dphy>;
+			phy-names = "dphy";
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port {
+				dsi_in_tcon0: endpoint {
+					remote-endpoint = <&tcon0_out_dsi>;
+				};
+			};
+		};
+
+		dphy: d-phy@1ca1000 {
+			compatible = "allwinner,sun50i-a64-mipi-dphy",
+				     "allwinner,sun6i-a31-mipi-dphy";
+			reg = <0x01ca1000 0x1000>;
+			clocks = <&ccu CLK_BUS_MIPI_DSI>,
+				 <&ccu CLK_DSI_DPHY>;
+			clock-names = "bus", "mod";
+			resets = <&ccu RST_BUS_MIPI_DSI>;
+			status = "disabled";
+			#phy-cells = <0>;
+		};
+
 		hdmi: hdmi@1ee0000 {
 			compatible = "allwinner,sun50i-a64-dw-hdmi",
 				     "allwinner,sun8i-a83t-dw-hdmi";
-- 
2.18.0.321.gffc6fa0e3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[DO NOT MERGE] [PATCH v12 7/7] arm64: dts: allwinner: bananapi-m64: Enable Bananapi S070WV20-CT16 DSI panel

[Jagan Teki]

This patch add support for Bananapi S070WV20-CT16 DSI panel to
BPI-M64 board.

DSI panel connected via board DSI port with,
- DLDO1 as VCC-DSI supply
- DCDC1 as VDD supply
- PD7 gpio for lcd enable pin
- PD6 gpio for lcd reset pin
- PD5 gpio for backlight enable pin

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

 .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index 208373efee49..6beaecdd802a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -45,6 +45,7 @@
 #include "sun50i-a64.dtsi"
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>
 
 / {
 	model = "BananaPi-M64";
@@ -56,6 +57,14 @@
 		serial1 = &uart1;
 	};
 
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&r_pwm 0 50000 PWM_POLARITY_INVERTED>;
+		brightness-levels = <1 2 4 8 16 32 64 128 255>;
+		default-brightness-level = <2>;
+		enable-gpios = <&pio 3 5 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PD5 */
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
@@ -116,6 +125,24 @@
 	status = "okay";
 };
 
+&dphy {
+	status = "okay";
+};
+
+&dsi {
+	vcc-dsi-supply = <&reg_dldo1>;		/* VCC3V3-DSI */
+	status = "okay";
+
+	panel@0 {
+		compatible = "bananapi,s070wv20-ct16-icn6211";
+		reg = <0>;
+		enable-gpios = <&pio 3 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PD7 */
+		reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD6 */
+		vdd-supply = <&reg_dcdc1>;
+		backlight = <&backlight>;
+	};
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -206,6 +233,10 @@
 	status = "okay";
 };
 
+&r_pwm {
+	status = "okay";
+};
+
 &r_rsb {
 	status = "okay";
 
-- 
2.18.0.321.gffc6fa0e3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel