diff mbox series

[1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels

Message ID 20220123172520.48741-2-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show
Series drm/panel: Add MIPI DBI compatible SPI driver | expand

Commit Message

Noralf Trønnes Jan. 23, 2022, 5:25 p.m. UTC
Add binding for MIPI DBI compatible SPI panels.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml

Comments

Rob Herring (Arm) Jan. 24, 2022, 4:01 p.m. UTC | #1
On Sun, 23 Jan 2022 18:25:18 +0100, Noralf Trønnes wrote:
> Add binding for MIPI DBI compatible SPI panels.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
schemas/display/panel/panel/panel-common.yaml: ignoring, error parsing file
make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml'
schemas/display/panel/panel/panel-common.yaml: ignoring, error parsing file
Traceback (most recent call last):
  File "/usr/local/bin/dt-validate", line 170, in <module>
    sg.check_trees(filename, testtree)
  File "/usr/local/bin/dt-validate", line 119, in check_trees
    self.check_subtree(dt, subtree, False, "/", "/", filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 110, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 105, in check_subtree
    self.check_node(tree, subtree, disabled, nodename, fullname, filename)
  File "/usr/local/bin/dt-validate", line 49, in check_node
    errors = sorted(dtschema.DTValidator(schema).iter_errors(node), key=lambda e: e.linecol)
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 229, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 362, in allOf
    yield from validator.descend(instance, subschema, schema_path=index)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 245, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 229, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/_validators.py", line 298, in ref
    yield from validator.descend(instance, resolved)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 245, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.8/dist-packages/dtschema/lib.py", line 771, in iter_errors
    for error in super().iter_errors(instance, _schema):
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 219, in iter_errors
    scope = id_of(_schema)
  File "/usr/local/lib/python3.8/dist-packages/jsonschema/validators.py", line 96, in _id_of
    return schema.get("$id", "")
AttributeError: 'NoneType' object has no attribute 'get'
make[1]: *** [scripts/Makefile.lib:378: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1398: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1583159

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Rob Herring Jan. 24, 2022, 4:08 p.m. UTC | #2
On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>
> Add binding for MIPI DBI compatible SPI panels.

I'm sure we already have MIPI DBI panels. What's this for?

>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> new file mode 100644
> index 000000000000..d6c8accb045c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MIPI DBI SPI Panels Device Tree Bindings
> +
> +maintainers:
> +  - Noralf Trønnes <noralf@tronnes.org>
> +
> +description:
> +  This binding is for display panels using a MIPI DBI controller
> +  in SPI mode.
> +
> +allOf:
> +  - $ref: panel/panel-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: panel-mipi-dbi-spi

Does the MIPI spec define how to power on a DBI panel with regulators,
enable/reset lines, etc. and all the timing constraints between those?
If not, then this compatible on its own is useless. It's fine as a
fallback if it's presence means the panel uses only standard DBI
commands and no vendor specific commands.

> +
> +  model:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: The name of the display panel.
> +
> +  write-only:
> +    type: boolean
> +    description:
> +      Controller is not readable (ie. MISO is not wired up).
> +
> +  dc-gpios:
> +    maxItems: 1
> +    description: |
> +      Controller data/command selection (D/CX) in 4-line SPI mode.
> +      If not set, the controller is in 3-line SPI mode.
> +
> +  backlight: true
> +  reg: true
> +  reset-gpios: true
> +
> +required:
> +  - compatible
> +  - model
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            display@0{
> +                    compatible = "panel-mipi-dbi-spi";
> +                    model = "sainsmart18";
> +                    reg = <0>;
> +                    spi-max-frequency = <40000000>;
> +                    dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> +                    reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
> +                    write-only;
> +            };
> +    };
> +
> +...
> --
> 2.33.0
>
Noralf Trønnes Jan. 24, 2022, 4:28 p.m. UTC | #3
Den 24.01.2022 17.08, skrev Rob Herring:
> On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>> Add binding for MIPI DBI compatible SPI panels.
> 
> I'm sure we already have MIPI DBI panels. What's this for?
> 

It aims to use one driver to cover all MIPI DBI panels where the
controller setup is loaded from userspace in a firmware file. The cover
letter points to the discussion where Maxime proposed this:

https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/

Noralf.

>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>  .../display/panel/panel-mipi-dbi-spi.yaml     | 69 +++++++++++++++++++
>>  1 file changed, 69 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>> new file mode 100644
>> index 000000000000..d6c8accb045c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
>> @@ -0,0 +1,69 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MIPI DBI SPI Panels Device Tree Bindings
>> +
>> +maintainers:
>> +  - Noralf Trønnes <noralf@tronnes.org>
>> +
>> +description:
>> +  This binding is for display panels using a MIPI DBI controller
>> +  in SPI mode.
>> +
>> +allOf:
>> +  - $ref: panel/panel-common.yaml#
>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: panel-mipi-dbi-spi
> 
> Does the MIPI spec define how to power on a DBI panel with regulators,
> enable/reset lines, etc. and all the timing constraints between those?
> If not, then this compatible on its own is useless. It's fine as a
> fallback if it's presence means the panel uses only standard DBI
> commands and no vendor specific commands.
> 
>> +
>> +  model:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description: The name of the display panel.
>> +
>> +  write-only:
>> +    type: boolean
>> +    description:
>> +      Controller is not readable (ie. MISO is not wired up).
>> +
>> +  dc-gpios:
>> +    maxItems: 1
>> +    description: |
>> +      Controller data/command selection (D/CX) in 4-line SPI mode.
>> +      If not set, the controller is in 3-line SPI mode.
>> +
>> +  backlight: true
>> +  reg: true
>> +  reset-gpios: true
>> +
>> +required:
>> +  - compatible
>> +  - model
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    spi {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            display@0{
>> +                    compatible = "panel-mipi-dbi-spi";
>> +                    model = "sainsmart18";
>> +                    reg = <0>;
>> +                    spi-max-frequency = <40000000>;
>> +                    dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
>> +                    reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
>> +                    write-only;
>> +            };
>> +    };
>> +
>> +...
>> --
>> 2.33.0
>>
Rob Herring Jan. 24, 2022, 4:42 p.m. UTC | #4
On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>
>
>
> Den 24.01.2022 17.08, skrev Rob Herring:
> > On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> >>
> >> Add binding for MIPI DBI compatible SPI panels.
> >
> > I'm sure we already have MIPI DBI panels. What's this for?
> >
>
> It aims to use one driver to cover all MIPI DBI panels where the
> controller setup is loaded from userspace in a firmware file.

What's the solution when the user wants a splash screen in the
bootloader and also wants multiple panels supported?

Also, 1 driver doesn't dictate 1 compatible. A one to many
relationship is fine and makes the decision entirely the OS's.

> The cover
> letter points to the discussion where Maxime proposed this:
>
> https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/

The proposal there is:

> compatible = "panel-spi";
> model = "panel-from-random-place-42";

The same thing can be accomplished with this:

compatible = "panel-from-random-place-42", "panel-spi";

What's the advantage of hijacking 'model'?

Rob
Noralf Trønnes Jan. 24, 2022, 5:04 p.m. UTC | #5
Den 24.01.2022 17.42, skrev Rob Herring:
> On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>>
>>
>> Den 24.01.2022 17.08, skrev Rob Herring:
>>> On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>>>
>>>> Add binding for MIPI DBI compatible SPI panels.
>>>
>>> I'm sure we already have MIPI DBI panels. What's this for?
>>>
>>
>> It aims to use one driver to cover all MIPI DBI panels where the
>> controller setup is loaded from userspace in a firmware file.
> 
> What's the solution when the user wants a splash screen in the
> bootloader and also wants multiple panels supported?
> 

In that case the bootloader needs builtin support for the screen. The OS
driver can then detect that the controller is configured and just skip
the configure stage. I have yet to see someone implement suppport for
such a SPI panel in a bootloader though.

> Also, 1 driver doesn't dictate 1 compatible. A one to many
> relationship is fine and makes the decision entirely the OS's.
> 
>> The cover
>> letter points to the discussion where Maxime proposed this:
>>
>> https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
> 
> The proposal there is:
> 
>> compatible = "panel-spi";
>> model = "panel-from-random-place-42";
> 
> The same thing can be accomplished with this:
> 
> compatible = "panel-from-random-place-42", "panel-spi";
> 
> What's the advantage of hijacking 'model'?
> 

That's what Maxime proposed, but having it in the compatible like this
is even better. I suppose it's ok for the driver to use the compatible
to load the firmware file?

Noralf.
Maxime Ripard Jan. 27, 2022, 9:36 a.m. UTC | #6
Hi Rob,

On Mon, Jan 24, 2022 at 10:42:37AM -0600, Rob Herring wrote:
> On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> >
> >
> >
> > Den 24.01.2022 17.08, skrev Rob Herring:
> > > On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> > >>
> > >> Add binding for MIPI DBI compatible SPI panels.
> > >
> > > I'm sure we already have MIPI DBI panels. What's this for?
> > >
> >
> > It aims to use one driver to cover all MIPI DBI panels where the
> > controller setup is loaded from userspace in a firmware file.
> 
> What's the solution when the user wants a splash screen in the
> bootloader and also wants multiple panels supported?
> 
> Also, 1 driver doesn't dictate 1 compatible. A one to many
> relationship is fine and makes the decision entirely the OS's.
> 
> > The cover
> > letter points to the discussion where Maxime proposed this:
> >
> > https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
> 
> The proposal there is:
> 
> > compatible = "panel-spi";
> > model = "panel-from-random-place-42";
> 
> The same thing can be accomplished with this:
> 
> compatible = "panel-from-random-place-42", "panel-spi";
> 
> What's the advantage of hijacking 'model'?

So, the main issue is that a panel is essentially two things: a
controller and the actual panel.

The controller has an initialization sequence of its own, and part of it
is parameters to match the panel.

So you can have identical controllers that won't have the same
initialization sequence because they don't have the same panel.

I was assuming that a compatible would be more about the controller, so
we needed something else, thus "model"

Maxime
Rob Herring Jan. 27, 2022, 8:18 p.m. UTC | #7
On Thu, Jan 27, 2022 at 3:36 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Rob,
>
> On Mon, Jan 24, 2022 at 10:42:37AM -0600, Rob Herring wrote:
> > On Mon, Jan 24, 2022 at 10:28 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> > >
> > >
> > >
> > > Den 24.01.2022 17.08, skrev Rob Herring:
> > > > On Sun, Jan 23, 2022 at 11:25 AM Noralf Trønnes <noralf@tronnes.org> wrote:
> > > >>
> > > >> Add binding for MIPI DBI compatible SPI panels.
> > > >
> > > > I'm sure we already have MIPI DBI panels. What's this for?
> > > >
> > >
> > > It aims to use one driver to cover all MIPI DBI panels where the
> > > controller setup is loaded from userspace in a firmware file.
> >
> > What's the solution when the user wants a splash screen in the
> > bootloader and also wants multiple panels supported?
> >
> > Also, 1 driver doesn't dictate 1 compatible. A one to many
> > relationship is fine and makes the decision entirely the OS's.
> >
> > > The cover
> > > letter points to the discussion where Maxime proposed this:
> > >
> > > https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/
> >
> > The proposal there is:
> >
> > > compatible = "panel-spi";
> > > model = "panel-from-random-place-42";
> >
> > The same thing can be accomplished with this:
> >
> > compatible = "panel-from-random-place-42", "panel-spi";
> >
> > What's the advantage of hijacking 'model'?
>
> So, the main issue is that a panel is essentially two things: a
> controller and the actual panel.
>
> The controller has an initialization sequence of its own, and part of it
> is parameters to match the panel.
>
> So you can have identical controllers that won't have the same
> initialization sequence because they don't have the same panel.
>
> I was assuming that a compatible would be more about the controller, so
> we needed something else, thus "model"

We already have cases like that which have "foo,some-panel", "bar,a-ctrlr-ic".

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
new file mode 100644
index 000000000000..d6c8accb045c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml
@@ -0,0 +1,69 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/panel-mipi-dbi-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MIPI DBI SPI Panels Device Tree Bindings
+
+maintainers:
+  - Noralf Trønnes <noralf@tronnes.org>
+
+description:
+  This binding is for display panels using a MIPI DBI controller
+  in SPI mode.
+
+allOf:
+  - $ref: panel/panel-common.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    const: panel-mipi-dbi-spi
+
+  model:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: The name of the display panel.
+
+  write-only:
+    type: boolean
+    description:
+      Controller is not readable (ie. MISO is not wired up).
+
+  dc-gpios:
+    maxItems: 1
+    description: |
+      Controller data/command selection (D/CX) in 4-line SPI mode.
+      If not set, the controller is in 3-line SPI mode.
+
+  backlight: true
+  reg: true
+  reset-gpios: true
+
+required:
+  - compatible
+  - model
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            display@0{
+                    compatible = "panel-mipi-dbi-spi";
+                    model = "sainsmart18";
+                    reg = <0>;
+                    spi-max-frequency = <40000000>;
+                    dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
+                    reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>;
+                    write-only;
+            };
+    };
+
+...