[PATCH 1/3 v4] drm/panel: Add generic DSI panel YAML bindings

[PATCH 1/3 v4] drm/panel: Add generic DSI panel YAML bindings

[Linus Walleij]

This adds a starting point for processing and defining generic
bindings used by DSI panels. We just define one single bool
property to force the panel into video mode for now.

Cc: devicetree@vger.kernel.org
Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v3->v4:
- Rename into display/dsi-controller.yaml
- Require a virtual channel number for the DSI panel, as
  DSI have this 2-bit virtual address field.
- Bring in some but not all properties from the existing MIPI
  DSI bindings. This schema can be used with simpler panels but
  not complex panels with multiple virtual channels for the
  moment. Let's handle it when we get there.
- Add an example.
ChangeLog v2->v3:
- Make a more complete DSI panel binding including the controller
  and its address-cells and size-cells and a pattern for the panel
  nodes. The panel is one per DSI master, the reg property is
  compulsory but should always be 0 (as far as I can tell) as
  only one panel can be connected. The bus doesn't really have
  any addresses for the panel, the address/reg notation seems
  to be cargo-culted from the port graphs and is not necessary
  to parse some device trees, it is used to tell whether the
  node is a panel or not rather than any addressing.
- I have no idea how many displays you can daisychain on a single
  DSI master, I just guess 15 will be enough. The MIPI-specs
  are memberwalled. Someone who knows can tell perhaps?
ChangeLog v1->v2:
- New patch after feedback.
---
 .../bindings/display/dsi-controller.yaml      | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/dsi-controller.yaml

diff --git a/Documentation/devicetree/bindings/display/dsi-controller.yaml b/Documentation/devicetree/bindings/display/dsi-controller.yaml
new file mode 100644
index 000000000000..2a6d872a40c5
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/dsi-controller.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/panel-dsi-common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Properties for DSI Display Panels
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+description: |
+  This document defines device tree properties common to DSI, Display
+  Serial Interface panels. It doesn't constitute a device tree binding
+  specification by itself but is meant to be referenced by device tree
+  bindings.
+
+  When referenced from panel device tree bindings the properties defined in
+  this document are defined as follows. The panel device tree bindings are
+  responsible for defining whether each property is required or optional.
+
+  Notice: this binding concerns DSI panels connected directly to a master
+  without any intermediate port graph to the panel. Each DSI master
+  can control exactly one panel. They should all just have a node "panel"
+  for their panel with their reg-property set to 0.
+
+properties:
+  $nodename:
+    pattern: "^dsi-controller(@.*)?$"
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^panel@[0-3]$":
+    type: object
+
+    properties:
+      reg:
+        minimum: 0
+        maximum: 3
+        description:
+          The virtual channel number of a DSI peripheral. Must be in the range
+          from 0 to 3, as DSI uses a 2-bit addressing scheme. Some DSI
+          peripherals respond to more than a single virtual channel. In that
+          case the reg property can take multiple entries, one for each virtual
+          channel that the peripheral responds to.
+
+      clock-master:
+        type: boolean
+        description:
+           Should be enabled if the host is being used in conjunction with
+           another DSI host to drive the same peripheral. Hardware supporting
+           such a configuration generally requires the data on both the busses
+           to be driven by the same clock. Only the DSI host instance
+           controlling this clock should contain this property.
+
+      enforce-video-mode:
+        type: boolean
+        description:
+          The best option is usually to run a panel in command mode, as this
+          gives better control over the panel hardware. However for different
+          reasons like broken hardware, missing features or testing, it may be
+          useful to be able to force a command mode-capable panel into video
+          mode.
+
+    required:
+      - reg
+
+examples:
+  - |
+    dsi-controller@55aa55aa {
+      compatible = "acme,foo";
+      reg = <0x55aa55aa>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      panel@0 {
+        compatible = "acme,bar";
+        reg = <0>;
+        vddi-supply = <&foo>;
+        reset-gpios = <&foo_gpio 0 GPIO_ACTIVE_LOW>;
+      };
+    };
+
+...
-- 
2.21.0

[PATCH 2/3 v4] drm/panel: Add DT bindings for Sony ACX424AKP

[Linus Walleij]

This adds device tree bindings for the Sony ACX424AKP panel.
Let's use YAML.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v3->v4:
- Adjust to adjusted DSI bindings.
ChangeLog v2->v3:
- Put the example inside a dsi-controller so we have a complete
  example that verifies to the DSI panel generic binding.
ChangeLog v1->v2:
- Suggest a stand-alone YAML bindings file for DSI panels in
  a separate patch, and use that to reference the
  boolean "enforce-video-mode" attribute for DSI panels
---
 .../display/panel/sony,acx424akp.yaml         | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
new file mode 100644
index 000000000000..a2f49b9a5958
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/sony,acx424akp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony ACX424AKP 4" 480x864 AMOLED panel
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+allOf:
+  - $ref: panel-common.yaml#
+  - $ref: ../dsi-controller.yaml#
+
+properties:
+  compatible:
+    const: sony,acx424akp
+  reg: true
+  port: true
+  reset-gpios: true
+  vddi-supply:
+     description: regulator that supplies the vddi voltage
+  enforce-video-mode: true
+
+required:
+  - compatible
+  - reg
+  - port
+  - reset-gpios
+  - power-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    dsi-controller@0 {
+        compatible = "foo";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "sony,acx424akp";
+            reg = <0>;
+            vddi-supply = <&foo>;
+            reset-gpios = <&foo_gpio 0 GPIO_ACTIVE_LOW>;
+        };
+    };
+
+...
\ No newline at end of file
-- 
2.21.0

Re: [PATCH 2/3 v4] drm/panel: Add DT bindings for Sony ACX424AKP

[Rob Herring]

On Thu, Oct 24, 2019 at 01:43:04PM +0200, Linus Walleij wrote:
> This adds device tree bindings for the Sony ACX424AKP panel.
> Let's use YAML.

Also broken. Run 'make dt_binding_check'.

> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v3->v4:
> - Adjust to adjusted DSI bindings.
> ChangeLog v2->v3:
> - Put the example inside a dsi-controller so we have a complete
>   example that verifies to the DSI panel generic binding.
> ChangeLog v1->v2:
> - Suggest a stand-alone YAML bindings file for DSI panels in
>   a separate patch, and use that to reference the
>   boolean "enforce-video-mode" attribute for DSI panels
> ---
>  .../display/panel/sony,acx424akp.yaml         | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
> new file mode 100644
> index 000000000000..a2f49b9a5958
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/sony,acx424akp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony ACX424AKP 4" 480x864 AMOLED panel
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +  - $ref: ../dsi-controller.yaml#

This is wrong now because it applies to the controller node, not the 
panel node. IOW, DSI controllers need to include it.

> +
> +properties:
> +  compatible:
> +    const: sony,acx424akp
> +  reg: true
> +  port: true
> +  reset-gpios: true
> +  vddi-supply:
> +     description: regulator that supplies the vddi voltage
> +  enforce-video-mode: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - port
> +  - reset-gpios
> +  - power-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    dsi-controller@0 {

unit-address without 'reg'...

> +        compatible = "foo";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        panel@0 {
> +            compatible = "sony,acx424akp";
> +            reg = <0>;
> +            vddi-supply = <&foo>;
> +            reset-gpios = <&foo_gpio 0 GPIO_ACTIVE_LOW>;
> +        };
> +    };
> +
> +...
> \ No newline at end of file

Should fix this...

> -- 
> 2.21.0
> 

Re: [PATCH 2/3 v4] drm/panel: Add DT bindings for Sony ACX424AKP

[Linus Walleij]

On Fri, Oct 25, 2019 at 9:25 PM Rob Herring <robh@kernel.org> wrote:
> On Thu, Oct 24, 2019 at 01:43:04PM +0200, Linus Walleij wrote:
> > This adds device tree bindings for the Sony ACX424AKP panel.
> > Let's use YAML.
>
> Also broken. Run 'make dt_binding_check'.

That is what I'm doing.

make -f Makefile -j5 -l4 ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf-
KBUILD_OUTPUT=/home/linus/linux-stericsson/build-ux500
dt_binding_check
  CHKDT   Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
  CHKDT   Documentation/devicetree/bindings/display/dsi-controller.yaml
  SCHEMA  Documentation/devicetree/bindings/processed-schema.yaml
(...)

I'm a bit unsure how this thing works. Are the several passes?
Because later on this breaks because of an unrelated error in
the bindings upstream:
/home/linus/linux-stericsson/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml:
properties:compatible:enum:0: {'const': 'regulator-fixed'} is not of
type 'string'
/home/linus/linux-stericsson/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml:
properties:compatible:enum:1: {'const': 'regulator-fixed-clock'} is
not of type 'string'
(...)

This is v5.4-rc1.

Is there any way I can selectively make dt_bindings_check just target
the files I wanna check as any brokenness upstream cause problems
like this? (And I assume that will keep happening.)

Yours,
Linus Walleij

Re: [PATCH 2/3 v4] drm/panel: Add DT bindings for Sony ACX424AKP

[Rob Herring]

On Wed, Oct 30, 2019 at 11:38 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Oct 25, 2019 at 9:25 PM Rob Herring <robh@kernel.org> wrote:
> > On Thu, Oct 24, 2019 at 01:43:04PM +0200, Linus Walleij wrote:
> > > This adds device tree bindings for the Sony ACX424AKP panel.
> > > Let's use YAML.
> >
> > Also broken. Run 'make dt_binding_check'.
>
> That is what I'm doing.
>
> make -f Makefile -j5 -l4 ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf-
> KBUILD_OUTPUT=/home/linus/linux-stericsson/build-ux500
> dt_binding_check
>   CHKDT   Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
>   CHKDT   Documentation/devicetree/bindings/display/dsi-controller.yaml
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.yaml
> (...)
>
> I'm a bit unsure how this thing works. Are the several passes?
> Because later on this breaks because of an unrelated error in
> the bindings upstream:
> /home/linus/linux-stericsson/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml:
> properties:compatible:enum:0: {'const': 'regulator-fixed'} is not of
> type 'string'
> /home/linus/linux-stericsson/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml:
> properties:compatible:enum:1: {'const': 'regulator-fixed-clock'} is
> not of type 'string'
> (...)
>
> This is v5.4-rc1.
>
> Is there any way I can selectively make dt_bindings_check just target
> the files I wanna check as any brokenness upstream cause problems
> like this? (And I assume that will keep happening.)

I pass '-k' to make so we don't stop. You can also set DT_SCHEMA_FILES
to the file you want to check. Linus' tree is fixed now (and next just
broke :( ).

Yes, it will keep happening as long as maintainers don't run checks
and/or take patches before I review them.

Rob

Re: [PATCH 1/3 v4] drm/panel: Add generic DSI panel YAML bindings

[Rob Herring]

On Thu, Oct 24, 2019 at 01:43:03PM +0200, Linus Walleij wrote:
> This adds a starting point for processing and defining generic
> bindings used by DSI panels. We just define one single bool
> property to force the panel into video mode for now.
> 
> Cc: devicetree@vger.kernel.org
> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v3->v4:
> - Rename into display/dsi-controller.yaml
> - Require a virtual channel number for the DSI panel, as
>   DSI have this 2-bit virtual address field.
> - Bring in some but not all properties from the existing MIPI
>   DSI bindings. This schema can be used with simpler panels but
>   not complex panels with multiple virtual channels for the
>   moment. Let's handle it when we get there.
> - Add an example.

A broken example...

> ChangeLog v2->v3:
> - Make a more complete DSI panel binding including the controller
>   and its address-cells and size-cells and a pattern for the panel
>   nodes. The panel is one per DSI master, the reg property is
>   compulsory but should always be 0 (as far as I can tell) as
>   only one panel can be connected. The bus doesn't really have
>   any addresses for the panel, the address/reg notation seems
>   to be cargo-culted from the port graphs and is not necessary
>   to parse some device trees, it is used to tell whether the
>   node is a panel or not rather than any addressing.
> - I have no idea how many displays you can daisychain on a single
>   DSI master, I just guess 15 will be enough. The MIPI-specs
>   are memberwalled. Someone who knows can tell perhaps?
> ChangeLog v1->v2:
> - New patch after feedback.
> ---
>  .../bindings/display/dsi-controller.yaml      | 88 +++++++++++++++++++
>  1 file changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/dsi-controller.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/dsi-controller.yaml b/Documentation/devicetree/bindings/display/dsi-controller.yaml
> new file mode 100644
> index 000000000000..2a6d872a40c5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/dsi-controller.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/panel-dsi-common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common Properties for DSI Display Panels
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +description: |
> +  This document defines device tree properties common to DSI, Display
> +  Serial Interface panels. It doesn't constitute a device tree binding
> +  specification by itself but is meant to be referenced by device tree
> +  bindings.
> +
> +  When referenced from panel device tree bindings the properties defined in
> +  this document are defined as follows. The panel device tree bindings are
> +  responsible for defining whether each property is required or optional.
> +
> +  Notice: this binding concerns DSI panels connected directly to a master
> +  without any intermediate port graph to the panel. Each DSI master
> +  can control exactly one panel. They should all just have a node "panel"
> +  for their panel with their reg-property set to 0.
> +
> +properties:
> +  $nodename:
> +    pattern: "^dsi-controller(@.*)?$"
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^panel@[0-3]$":
> +    type: object
> +
> +    properties:
> +      reg:
> +        minimum: 0
> +        maximum: 3
> +        description:
> +          The virtual channel number of a DSI peripheral. Must be in the range
> +          from 0 to 3, as DSI uses a 2-bit addressing scheme. Some DSI
> +          peripherals respond to more than a single virtual channel. In that
> +          case the reg property can take multiple entries, one for each virtual
> +          channel that the peripheral responds to.
> +
> +      clock-master:
> +        type: boolean
> +        description:
> +           Should be enabled if the host is being used in conjunction with
> +           another DSI host to drive the same peripheral. Hardware supporting
> +           such a configuration generally requires the data on both the busses
> +           to be driven by the same clock. Only the DSI host instance
> +           controlling this clock should contain this property.
> +
> +      enforce-video-mode:
> +        type: boolean
> +        description:
> +          The best option is usually to run a panel in command mode, as this
> +          gives better control over the panel hardware. However for different
> +          reasons like broken hardware, missing features or testing, it may be
> +          useful to be able to force a command mode-capable panel into video
> +          mode.
> +
> +    required:
> +      - reg
> +
> +examples:
> +  - |
> +    dsi-controller@55aa55aa {
> +      compatible = "acme,foo";

Eventually, I want to start reporting if compatible strings aren't 
documented (i.e. matched to a schema). This will break then...

> +      reg = <0x55aa55aa>;

This will fail because a size cell is expected.

I'd just drop both properties here. A 'dsi-controller' node name is 
enough to select this schema.

> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      panel@0 {
> +        compatible = "acme,bar";

Just pick a real one. (Or drop the example because the binding for the 
real panel will have the same example).

> +        reg = <0>;
> +        vddi-supply = <&foo>;
> +        reset-gpios = <&foo_gpio 0 GPIO_ACTIVE_LOW>;
> +      };
> +    };
> +
> +...
> -- 
> 2.21.0
> 

Re: [PATCH 1/3 v4] drm/panel: Add generic DSI panel YAML bindings

[Linus Walleij]

On Fri, Oct 25, 2019 at 9:20 PM Rob Herring <robh@kernel.org> wrote:

> [Me]
> > +    dsi-controller@55aa55aa {
> > +      compatible = "acme,foo";
>
> Eventually, I want to start reporting if compatible strings aren't
> documented (i.e. matched to a schema). This will break then...

OK I drop this.

> > +      reg = <0x55aa55aa>;
>
> This will fail because a size cell is expected.

Do you mean it will fail the day we actually check reg properties to
be properly formed under the circumstances?

Because:
  HOSTLD  scripts/dtc/dtc
  CHKDT   Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
  CHKDT   Documentation/devicetree/bindings/display/dsi-controller.yaml
  SCHEMA  Documentation/devicetree/bindings/processed-schema.yaml
(...)

It passes just fine.

I get a bit nervous since the schema is all about being strict about
stuff so I get the feeling that "should have failed".

Yours,
Linus Walleij

Re: [PATCH 1/3 v4] drm/panel: Add generic DSI panel YAML bindings

[Rob Herring]

On Wed, Oct 30, 2019 at 8:50 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Oct 25, 2019 at 9:20 PM Rob Herring <robh@kernel.org> wrote:
>
> > [Me]
> > > +    dsi-controller@55aa55aa {
> > > +      compatible = "acme,foo";
> >
> > Eventually, I want to start reporting if compatible strings aren't
> > documented (i.e. matched to a schema). This will break then...
>
> OK I drop this.
>
> > > +      reg = <0x55aa55aa>;
> >
> > This will fail because a size cell is expected.
>
> Do you mean it will fail the day we actually check reg properties to
> be properly formed under the circumstances?
>
> Because:
>   HOSTLD  scripts/dtc/dtc
>   CHKDT   Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
>   CHKDT   Documentation/devicetree/bindings/display/dsi-controller.yaml
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.yaml
> (...)
>
> It passes just fine.

Yes, but the examples are built and validated after this. With the
other errors currently, you're erroring out before this happens.

> I get a bit nervous since the schema is all about being strict about
> stuff so I get the feeling that "should have failed".

It's actually dtc that will complain here because it's aware of
#*-cells values. Can't do that with schema.

Rob