Hi Noralf, On Tue, Feb 08, 2022 at 01:16:44PM +0100, Noralf Trønnes wrote: > Den 08.02.2022 00.20, skrev Rob Herring: > > On Thu, Jan 27, 2022 at 10:37:22AM +0100, Maxime Ripard wrote: > >> Hi, > >> > >> On Tue, Jan 25, 2022 at 06:56:58PM +0100, Noralf Trønnes wrote: > >>> Add binding for MIPI DBI compatible SPI panels. > >>> > >>> v2: > >>> - Fix path for panel-common.yaml > >>> - Use unevaluatedProperties > >>> - Drop properties which are in the allOf section > >>> - Drop model property (Rob) > >>> > >>> Signed-off-by: Noralf Trønnes > >>> --- > >>> .../display/panel/panel-mipi-dbi-spi.yaml | 59 +++++++++++++++++++ > >>> 1 file changed, 59 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..b7cbeea0f8aa > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > >>> @@ -0,0 +1,59 @@ > >>> +# 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 > >>> + > >>> +description: > >>> + This binding is for display panels using a MIPI DBI controller > >>> + in SPI mode. > >>> + > >>> +allOf: > >>> + - $ref: panel-common.yaml# > >>> + - $ref: /schemas/spi/spi-peripheral-props.yaml# > >>> + > >>> +properties: > >>> + compatible: > >>> + const: panel-mipi-dbi-spi > >> > >> You need contains here, otherwise it will error out if you have two compatibles. > > > > Shouldn't it always have 2? > > > > Either way, this has to be split up between a common, shareable schema > > and specific, complete schema(s). Like this: > > > > - A schema for everything common (that allows additional properties) > > > > - A schema for 'panel-mipi-dbi-spi' referencing the common schema plus > > 'unevaluatedProperties: false' > > > > - Schemas for panels with their own additional properties (regulators, > > GPIOs, etc.) > > > > LVDS was restructured like this IIRC. > > The whole point of this exercise is to avoid the need for controller > specific bindings. I'm not sure to follow you here, nothing that either Rob or I discussed would require a controller specific binding. It would require a controller compatible, but the binding itself can just mandate a controller compatible in addition to the panel-mipi-dbi-spi compatible, without enforcing anything wrt the compatible itself. And the driver will just match panel-mipi-dbi-spi so there won't be any driver change either? In essence, it would be similar to the bindings of panel-lvds or the AT24 EEPROM binding: you have two compatibles, but the driver is generic and will just infer its behaviour based on the DT properties (and in our case will load a firmware based on the specific compatible). Wouldn't that work? > This binding will cover all specifics about these > controllers except for one thing and that is the controller > configuration. Each controller has its own configuration commands. These > commands will be loaded as a firmware file based on the compatible and > applied by the driver. > > So this binding, the panel-common and spi-peripheral-props covers > everything except for the controller configuration. > > Here's a copy of the DBI spec: https://www.docin.com/p-219732497.html > > This is my current version of the binding: > > # SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) > %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 Panel > > maintainers: > - Noralf Trønnes > > description: | > This binding is for display panels using a MIPI DBI compatible controller > in SPI mode. > > The MIPI Alliance Standard for Display Bus Interface defines the > electrical > and logical interfaces for display controllers historically used in mobile > phones. The standard defines 4 display architecture types and this > binding is > for type 1 which has full frame memory. There are 3 interface types in the > standard and type C is the serial interface. > > The standard defines the following interface signals for type C: > - Power: > - Vdd: Power supply for display module > - Vddi: Logic level supply for interface signals > Combined into one in this binding called: power-supply > - Interface: > - CSx: Chip select > - SCL: Serial clock > - Dout: Serial out > - Din: Serial in > - SDA: Bidrectional in/out > - D/CX: Data/command selection, high=data, low=command > Called dc-gpios in this binding. > - RESX: Reset when low > Called reset-gpios in this binding. > > The type C interface has 3 options: > > - Option 1: 9-bit mode and D/CX as the 9th bit > | Command | the next command or > following data | > > |<0>|| > > - Option 2: 16-bit mode and D/CX as a 9th bit > | Command or data | > || > > - Option 3: 8-bit mode and D/CX as a separate interface line > | Command or data | > || > > The panel resolution is specified using the panel-timing node properties > hactive (width) and vactive (height). The other mandatory panel-timing > properties should be set to zero except clock-frequency which can be > optionally set to inform about the actual pixel clock frequency. > > If the panel is wired to the controller at an offset specify this using > hback-porch (x-offset) and vback-porch (y-offset). > > allOf: > - $ref: panel-common.yaml# > - $ref: /schemas/spi/spi-peripheral-props.yaml# > > properties: > compatible: > contains: > const: panel-dbi-spi > > 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. > > required: > - compatible > - reg > - panel-timing > > unevaluatedProperties: false > > examples: > - | > #include > > spi { > #address-cells = <1>; > #size-cells = <0>; > > display@0{ > compatible = "sainsmart18", "panel-dbi-spi"; > reg = <0>; > spi-max-frequency = <40000000>; > > dc-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>; > reset-gpios = <&gpio 25 GPIO_ACTIVE_HIGH>; > write-only; > > backlight = <&backlight>; > > width-mm = <35>; > height-mm = <28>; > > panel-timing { > hactive = <160>; > vactive = <128>; > hback-porch = <0>; > vback-porch = <0>; > > clock-frequency = <0>; > hfront-porch = <0>; > hsync-len = <0>; > vfront-porch = <0>; > vsync-len = <0>; > }; > }; > }; > > ... Yep, this looks good to me Maxime