* [PATCH v4 0/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-18 15:11 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-18 15:11 UTC (permalink / raw) To: robh+dt, thierry.reding Cc: sam, maxime, dave.stevenson, david, devicetree, dri-devel, Noralf Trønnes Hi, This patchset adds a driver that will work with most MIPI DBI compatible SPI panels out there. Maxime gave[1] a good overview of the situation with these displays and proposed to make a driver that works with all MIPI DBI compatible controllers and use a firmware file to provide the controller setup for a particular panel. Changes since version 3: - There should only be two compatible (Maxime) - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible - Move driver to drm/tiny where the other drivers of its kind are located. The driver module will not be shared with a future DPI driver after all. See wiki[2] for script to make command firmware files. Noralf. [1] https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/ [2] https://github.com/notro/panel-mipi-dbi/wiki Noralf Trønnes (3): dt-bindings: display: add bindings for MIPI DBI compatible SPI panels drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev drm/tiny: Add MIPI DBI compatible SPI driver .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++ MAINTAINERS | 8 + drivers/gpu/drm/tiny/Kconfig | 13 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/panel-mipi-dbi.c | 413 ++++++++++++++++++ include/drm/drm_mipi_dbi.h | 8 + 6 files changed, 568 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml create mode 100644 drivers/gpu/drm/tiny/panel-mipi-dbi.c -- 2.33.0 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4 0/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-18 15:11 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-18 15:11 UTC (permalink / raw) To: robh+dt, thierry.reding Cc: devicetree, david, dave.stevenson, dri-devel, Noralf Trønnes, maxime, sam Hi, This patchset adds a driver that will work with most MIPI DBI compatible SPI panels out there. Maxime gave[1] a good overview of the situation with these displays and proposed to make a driver that works with all MIPI DBI compatible controllers and use a firmware file to provide the controller setup for a particular panel. Changes since version 3: - There should only be two compatible (Maxime) - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible - Move driver to drm/tiny where the other drivers of its kind are located. The driver module will not be shared with a future DPI driver after all. See wiki[2] for script to make command firmware files. Noralf. [1] https://lore.kernel.org/dri-devel/20211129093946.xhp22mvdut3m67sc@houat/ [2] https://github.com/notro/panel-mipi-dbi/wiki Noralf Trønnes (3): dt-bindings: display: add bindings for MIPI DBI compatible SPI panels drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev drm/tiny: Add MIPI DBI compatible SPI driver .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++ MAINTAINERS | 8 + drivers/gpu/drm/tiny/Kconfig | 13 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/panel-mipi-dbi.c | 413 ++++++++++++++++++ include/drm/drm_mipi_dbi.h | 8 + 6 files changed, 568 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml create mode 100644 drivers/gpu/drm/tiny/panel-mipi-dbi.c -- 2.33.0 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels 2022-02-18 15:11 ` Noralf Trønnes @ 2022-02-18 15:11 ` Noralf Trønnes -1 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-18 15:11 UTC (permalink / raw) To: robh+dt, thierry.reding Cc: sam, maxime, dave.stevenson, david, devicetree, dri-devel, Noralf Trønnes Add binding for MIPI DBI compatible SPI panels. v4: - There should only be two compatible (Maxime) - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible v3: - Move properties to Device Tree (Maxime) - Use contains for compatible (Maxime) - Add backlight property to example - Flesh out description v2: - Fix path for panel-common.yaml - Use unevaluatedProperties - Drop properties which are in the allOf section - Drop model property (Rob) Acked-by: Maxime Ripard <maxime@cerno.tech> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ 1 file changed, 125 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..748c09113168 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml @@ -0,0 +1,125 @@ +# 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 <noralf@tronnes.org> + +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><D7><D6><D5><D4><D3><D2><D1><D0>|<D/CX><D7><D6><D5><D4><D3><D2><D1><D0>| + + - Option 2: 16-bit mode and D/CX as a 9th bit + | Command or data | + |<X><X><X><X><X><X><X><D/CX><D7><D6><D5><D4><D3><D2><D1><D0>| + + - Option 3: 8-bit mode and D/CX as a separate interface line + | Command or data | + |<D7><D6><D5><D4><D3><D2><D1><D0>| + + 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: + items: + - {} # Panel Specific Compatible + - const: panel-mipi-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 <dt-bindings/gpio/gpio.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + display@0{ + compatible = "sainsmart18", "panel-mipi-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>; + }; + }; + }; + +... -- 2.33.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels @ 2022-02-18 15:11 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-18 15:11 UTC (permalink / raw) To: robh+dt, thierry.reding Cc: devicetree, david, dave.stevenson, dri-devel, Noralf Trønnes, maxime, sam Add binding for MIPI DBI compatible SPI panels. v4: - There should only be two compatible (Maxime) - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible v3: - Move properties to Device Tree (Maxime) - Use contains for compatible (Maxime) - Add backlight property to example - Flesh out description v2: - Fix path for panel-common.yaml - Use unevaluatedProperties - Drop properties which are in the allOf section - Drop model property (Rob) Acked-by: Maxime Ripard <maxime@cerno.tech> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ 1 file changed, 125 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..748c09113168 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml @@ -0,0 +1,125 @@ +# 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 <noralf@tronnes.org> + +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><D7><D6><D5><D4><D3><D2><D1><D0>|<D/CX><D7><D6><D5><D4><D3><D2><D1><D0>| + + - Option 2: 16-bit mode and D/CX as a 9th bit + | Command or data | + |<X><X><X><X><X><X><X><D/CX><D7><D6><D5><D4><D3><D2><D1><D0>| + + - Option 3: 8-bit mode and D/CX as a separate interface line + | Command or data | + |<D7><D6><D5><D4><D3><D2><D1><D0>| + + 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: + items: + - {} # Panel Specific Compatible + - const: panel-mipi-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 <dt-bindings/gpio/gpio.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + display@0{ + compatible = "sainsmart18", "panel-mipi-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>; + }; + }; + }; + +... -- 2.33.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels 2022-02-18 15:11 ` Noralf Trønnes @ 2022-02-19 15:25 ` Sam Ravnborg -1 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-19 15:25 UTC (permalink / raw) To: Noralf Trønnes Cc: robh+dt, thierry.reding, maxime, dave.stevenson, david, devicetree, dri-devel Hi Noralf, On Fri, Feb 18, 2022 at 04:11:08PM +0100, Noralf Trønnes wrote: > Add binding for MIPI DBI compatible SPI panels. > > v4: > - There should only be two compatible (Maxime) > - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible > > v3: > - Move properties to Device Tree (Maxime) > - Use contains for compatible (Maxime) > - Add backlight property to example > - Flesh out description > > v2: > - Fix path for panel-common.yaml > - Use unevaluatedProperties > - Drop properties which are in the allOf section > - Drop model property (Rob) > > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ > 1 file changed, 125 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..748c09113168 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > @@ -0,0 +1,125 @@ > +# 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 <noralf@tronnes.org> > + > +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><D7><D6><D5><D4><D3><D2><D1><D0>|<D/CX><D7><D6><D5><D4><D3><D2><D1><D0>| > + > + - Option 2: 16-bit mode and D/CX as a 9th bit > + | Command or data | > + |<X><X><X><X><X><X><X><D/CX><D7><D6><D5><D4><D3><D2><D1><D0>| > + > + - Option 3: 8-bit mode and D/CX as a separate interface line > + | Command or data | > + |<D7><D6><D5><D4><D3><D2><D1><D0>| > + > + 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). Very informative description - well done. > + > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + items: > + - {} # Panel Specific Compatible > + - const: panel-mipi-dbi-spi > + > + write-only: > + type: boolean > + description: > + Controller is not readable (ie. MISO is not wired up). It would be easier to understand if this comment refers to one of the pins on the display described above. So maybe something like (ie. Din (MSIO on the SPI interface) is not wired up). With the comment updated to include a reference to Din, Acked-by: Sam Ravnborg <sam@ravnborg.org> ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels @ 2022-02-19 15:25 ` Sam Ravnborg 0 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-19 15:25 UTC (permalink / raw) To: Noralf Trønnes Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Hi Noralf, On Fri, Feb 18, 2022 at 04:11:08PM +0100, Noralf Trønnes wrote: > Add binding for MIPI DBI compatible SPI panels. > > v4: > - There should only be two compatible (Maxime) > - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible > > v3: > - Move properties to Device Tree (Maxime) > - Use contains for compatible (Maxime) > - Add backlight property to example > - Flesh out description > > v2: > - Fix path for panel-common.yaml > - Use unevaluatedProperties > - Drop properties which are in the allOf section > - Drop model property (Rob) > > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ > 1 file changed, 125 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..748c09113168 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > @@ -0,0 +1,125 @@ > +# 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 <noralf@tronnes.org> > + > +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><D7><D6><D5><D4><D3><D2><D1><D0>|<D/CX><D7><D6><D5><D4><D3><D2><D1><D0>| > + > + - Option 2: 16-bit mode and D/CX as a 9th bit > + | Command or data | > + |<X><X><X><X><X><X><X><D/CX><D7><D6><D5><D4><D3><D2><D1><D0>| > + > + - Option 3: 8-bit mode and D/CX as a separate interface line > + | Command or data | > + |<D7><D6><D5><D4><D3><D2><D1><D0>| > + > + 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). Very informative description - well done. > + > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + items: > + - {} # Panel Specific Compatible > + - const: panel-mipi-dbi-spi > + > + write-only: > + type: boolean > + description: > + Controller is not readable (ie. MISO is not wired up). It would be easier to understand if this comment refers to one of the pins on the display described above. So maybe something like (ie. Din (MSIO on the SPI interface) is not wired up). With the comment updated to include a reference to Din, Acked-by: Sam Ravnborg <sam@ravnborg.org> ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels 2022-02-18 15:11 ` Noralf Trønnes @ 2022-02-21 2:36 ` Rob Herring -1 siblings, 0 replies; 36+ messages in thread From: Rob Herring @ 2022-02-21 2:36 UTC (permalink / raw) To: Noralf Trønnes Cc: dri-devel, thierry.reding, dave.stevenson, maxime, robh+dt, sam, david, devicetree [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 1642 bytes --] On Fri, 18 Feb 2022 16:11:08 +0100, Noralf Trønnes wrote: > Add binding for MIPI DBI compatible SPI panels. > > v4: > - There should only be two compatible (Maxime) > - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible > > v3: > - Move properties to Device Tree (Maxime) > - Use contains for compatible (Maxime) > - Add backlight property to example > - Flesh out description > > v2: > - Fix path for panel-common.yaml > - Use unevaluatedProperties > - Drop properties which are in the allOf section > - Drop model property (Rob) > > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ > 1 file changed, 125 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: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml:0:0: /example-0/spi/display@0: failed to match any schema with compatible: ['sainsmart18', 'panel-mipi-dbi-spi'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1594817 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. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels @ 2022-02-21 2:36 ` Rob Herring 0 siblings, 0 replies; 36+ messages in thread From: Rob Herring @ 2022-02-21 2:36 UTC (permalink / raw) To: Noralf Trønnes Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime, sam [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 1642 bytes --] On Fri, 18 Feb 2022 16:11:08 +0100, Noralf Trønnes wrote: > Add binding for MIPI DBI compatible SPI panels. > > v4: > - There should only be two compatible (Maxime) > - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible > > v3: > - Move properties to Device Tree (Maxime) > - Use contains for compatible (Maxime) > - Add backlight property to example > - Flesh out description > > v2: > - Fix path for panel-common.yaml > - Use unevaluatedProperties > - Drop properties which are in the allOf section > - Drop model property (Rob) > > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ > 1 file changed, 125 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: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml:0:0: /example-0/spi/display@0: failed to match any schema with compatible: ['sainsmart18', 'panel-mipi-dbi-spi'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1594817 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. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels 2022-02-18 15:11 ` Noralf Trønnes @ 2022-02-21 11:31 ` Noralf Trønnes -1 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-21 11:31 UTC (permalink / raw) To: robh+dt, maxime Cc: sam, dave.stevenson, david, devicetree, dri-devel, thierry.reding, Noralf Trønnes Den 18.02.2022 16.11, skrev Noralf Trønnes: > Add binding for MIPI DBI compatible SPI panels. > > v4: > - There should only be two compatible (Maxime) > - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible > > v3: > - Move properties to Device Tree (Maxime) > - Use contains for compatible (Maxime) > - Add backlight property to example > - Flesh out description > > v2: > - Fix path for panel-common.yaml > - Use unevaluatedProperties > - Drop properties which are in the allOf section > - Drop model property (Rob) > > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ > 1 file changed, 125 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..748c09113168 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + items: > + - {} # Panel Specific Compatible > + - const: panel-mipi-dbi-spi > + Rob's bot uses a -m flag I didn't use, and with that the compatible fails: $ make DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml CHKDT Documentation/devicetree/bindings/processed-schema-examples.json SCHEMA Documentation/devicetree/bindings/processed-schema-examples.json DTEX Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dts DTC Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml CHECK Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml:0:0: /example-0/spi/display@0: failed to match any schema with compatible: ['sainsmart18', 'panel-mipi-dbi-spi'] How can I write the compatible property to make the checker happy? Noralf. > + 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 <dt-bindings/gpio/gpio.h> > + > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + display@0{ > + compatible = "sainsmart18", "panel-mipi-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>; > + }; > + }; > + }; > + > +... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels @ 2022-02-21 11:31 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-21 11:31 UTC (permalink / raw) To: robh+dt, maxime Cc: devicetree, Noralf Trønnes, david, dave.stevenson, dri-devel, thierry.reding, sam Den 18.02.2022 16.11, skrev Noralf Trønnes: > Add binding for MIPI DBI compatible SPI panels. > > v4: > - There should only be two compatible (Maxime) > - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible > > v3: > - Move properties to Device Tree (Maxime) > - Use contains for compatible (Maxime) > - Add backlight property to example > - Flesh out description > > v2: > - Fix path for panel-common.yaml > - Use unevaluatedProperties > - Drop properties which are in the allOf section > - Drop model property (Rob) > > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ > 1 file changed, 125 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..748c09113168 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + items: > + - {} # Panel Specific Compatible > + - const: panel-mipi-dbi-spi > + Rob's bot uses a -m flag I didn't use, and with that the compatible fails: $ make DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml CHKDT Documentation/devicetree/bindings/processed-schema-examples.json SCHEMA Documentation/devicetree/bindings/processed-schema-examples.json DTEX Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dts DTC Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml CHECK Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml:0:0: /example-0/spi/display@0: failed to match any schema with compatible: ['sainsmart18', 'panel-mipi-dbi-spi'] How can I write the compatible property to make the checker happy? Noralf. > + 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 <dt-bindings/gpio/gpio.h> > + > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + display@0{ > + compatible = "sainsmart18", "panel-mipi-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>; > + }; > + }; > + }; > + > +... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels 2022-02-21 11:31 ` Noralf Trønnes @ 2022-02-22 17:26 ` Rob Herring -1 siblings, 0 replies; 36+ messages in thread From: Rob Herring @ 2022-02-22 17:26 UTC (permalink / raw) To: Noralf Trønnes Cc: maxime, sam, dave.stevenson, david, devicetree, dri-devel, thierry.reding On Mon, Feb 21, 2022 at 12:31:08PM +0100, Noralf Trønnes wrote: > > > Den 18.02.2022 16.11, skrev Noralf Trønnes: > > Add binding for MIPI DBI compatible SPI panels. > > > > v4: > > - There should only be two compatible (Maxime) > > - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible > > > > v3: > > - Move properties to Device Tree (Maxime) > > - Use contains for compatible (Maxime) > > - Add backlight property to example > > - Flesh out description > > > > v2: > > - Fix path for panel-common.yaml > > - Use unevaluatedProperties > > - Drop properties which are in the allOf section > > - Drop model property (Rob) > > > > Acked-by: Maxime Ripard <maxime@cerno.tech> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > --- > > .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ > > 1 file changed, 125 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..748c09113168 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > > > +allOf: > > + - $ref: panel-common.yaml# > > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > > + > > +properties: > > + compatible: > > + items: > > + - {} # Panel Specific Compatible > > + - const: panel-mipi-dbi-spi > > + > > Rob's bot uses a -m flag I didn't use, and with that the compatible fails: > > $ make DT_CHECKER_FLAGS=-m dt_binding_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > CHKDT Documentation/devicetree/bindings/processed-schema-examples.json > SCHEMA Documentation/devicetree/bindings/processed-schema-examples.json > DTEX > Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dts > DTC > Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml > CHECK > Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml > Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml:0:0: > /example-0/spi/display@0: failed to match any schema with compatible: > ['sainsmart18', 'panel-mipi-dbi-spi'] > > How can I write the compatible property to make the checker happy? You need to partition the schemas as I outlined before. Given the DBI spec does define power and reset, maybe you can get away with 1 schema just by changing the '- {}' entry above to an enum with a list of compatibles. But as soon as there is a panel with extra or different properties, this schema will have to be split. Rob ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels @ 2022-02-22 17:26 ` Rob Herring 0 siblings, 0 replies; 36+ messages in thread From: Rob Herring @ 2022-02-22 17:26 UTC (permalink / raw) To: Noralf Trønnes Cc: devicetree, david, dave.stevenson, dri-devel, thierry.reding, maxime, sam On Mon, Feb 21, 2022 at 12:31:08PM +0100, Noralf Trønnes wrote: > > > Den 18.02.2022 16.11, skrev Noralf Trønnes: > > Add binding for MIPI DBI compatible SPI panels. > > > > v4: > > - There should only be two compatible (Maxime) > > - s/panel-dbi-spi/panel-mipi-dbi-spi/in compatible > > > > v3: > > - Move properties to Device Tree (Maxime) > > - Use contains for compatible (Maxime) > > - Add backlight property to example > > - Flesh out description > > > > v2: > > - Fix path for panel-common.yaml > > - Use unevaluatedProperties > > - Drop properties which are in the allOf section > > - Drop model property (Rob) > > > > Acked-by: Maxime Ripard <maxime@cerno.tech> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > --- > > .../display/panel/panel-mipi-dbi-spi.yaml | 125 ++++++++++++++++++ > > 1 file changed, 125 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..748c09113168 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > > > +allOf: > > + - $ref: panel-common.yaml# > > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > > + > > +properties: > > + compatible: > > + items: > > + - {} # Panel Specific Compatible > > + - const: panel-mipi-dbi-spi > > + > > Rob's bot uses a -m flag I didn't use, and with that the compatible fails: > > $ make DT_CHECKER_FLAGS=-m dt_binding_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > CHKDT Documentation/devicetree/bindings/processed-schema-examples.json > SCHEMA Documentation/devicetree/bindings/processed-schema-examples.json > DTEX > Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dts > DTC > Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml > CHECK > Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml > Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.example.dt.yaml:0:0: > /example-0/spi/display@0: failed to match any schema with compatible: > ['sainsmart18', 'panel-mipi-dbi-spi'] > > How can I write the compatible property to make the checker happy? You need to partition the schemas as I outlined before. Given the DBI spec does define power and reset, maybe you can get away with 1 schema just by changing the '- {}' entry above to an enum with a list of compatibles. But as soon as there is a panel with extra or different properties, this schema will have to be split. Rob ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4 2/3] drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev 2022-02-18 15:11 ` Noralf Trønnes @ 2022-02-18 15:11 ` Noralf Trønnes -1 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-18 15:11 UTC (permalink / raw) To: robh+dt, thierry.reding Cc: sam, maxime, dave.stevenson, david, devicetree, dri-devel, Noralf Trønnes devm_drm_dev_alloc() can't allocate structures that embed a structure which then again embeds drm_device. Workaround this by adding a driver_private pointer to struct mipi_dbi_dev which the driver can use for its additional state. v3: - Add documentation Acked-by: Maxime Ripard <maxime@cerno.tech> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- include/drm/drm_mipi_dbi.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h index 6fe13cce2670..dad2f187b64b 100644 --- a/include/drm/drm_mipi_dbi.h +++ b/include/drm/drm_mipi_dbi.h @@ -130,6 +130,14 @@ struct mipi_dbi_dev { * @dbi: MIPI DBI interface */ struct mipi_dbi dbi; + + /** + * @driver_private: Driver private data. + * Necessary for drivers with private data since devm_drm_dev_alloc() + * can't allocate structures that embed a structure which then again + * embeds drm_device. + */ + void *driver_private; }; static inline struct mipi_dbi_dev *drm_to_mipi_dbi_dev(struct drm_device *drm) -- 2.33.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 2/3] drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev @ 2022-02-18 15:11 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-18 15:11 UTC (permalink / raw) To: robh+dt, thierry.reding Cc: devicetree, david, dave.stevenson, dri-devel, Noralf Trønnes, maxime, sam devm_drm_dev_alloc() can't allocate structures that embed a structure which then again embeds drm_device. Workaround this by adding a driver_private pointer to struct mipi_dbi_dev which the driver can use for its additional state. v3: - Add documentation Acked-by: Maxime Ripard <maxime@cerno.tech> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- include/drm/drm_mipi_dbi.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h index 6fe13cce2670..dad2f187b64b 100644 --- a/include/drm/drm_mipi_dbi.h +++ b/include/drm/drm_mipi_dbi.h @@ -130,6 +130,14 @@ struct mipi_dbi_dev { * @dbi: MIPI DBI interface */ struct mipi_dbi dbi; + + /** + * @driver_private: Driver private data. + * Necessary for drivers with private data since devm_drm_dev_alloc() + * can't allocate structures that embed a structure which then again + * embeds drm_device. + */ + void *driver_private; }; static inline struct mipi_dbi_dev *drm_to_mipi_dbi_dev(struct drm_device *drm) -- 2.33.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/3] drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev 2022-02-18 15:11 ` Noralf Trønnes @ 2022-02-19 15:25 ` Sam Ravnborg -1 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-19 15:25 UTC (permalink / raw) To: Noralf Trønnes Cc: robh+dt, thierry.reding, maxime, dave.stevenson, david, devicetree, dri-devel Hi Noralf, On Fri, Feb 18, 2022 at 04:11:09PM +0100, Noralf Trønnes wrote: > devm_drm_dev_alloc() can't allocate structures that embed a structure > which then again embeds drm_device. Workaround this by adding a > driver_private pointer to struct mipi_dbi_dev which the driver can use for > its additional state. > > v3: > - Add documentation > > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> Acked-by: Sam Ravnborg <sam@ravnborg.org> ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/3] drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev @ 2022-02-19 15:25 ` Sam Ravnborg 0 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-19 15:25 UTC (permalink / raw) To: Noralf Trønnes Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Hi Noralf, On Fri, Feb 18, 2022 at 04:11:09PM +0100, Noralf Trønnes wrote: > devm_drm_dev_alloc() can't allocate structures that embed a structure > which then again embeds drm_device. Workaround this by adding a > driver_private pointer to struct mipi_dbi_dev which the driver can use for > its additional state. > > v3: > - Add documentation > > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> Acked-by: Sam Ravnborg <sam@ravnborg.org> ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-18 15:11 ` Noralf Trønnes @ 2022-02-18 15:11 ` Noralf Trønnes -1 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-18 15:11 UTC (permalink / raw) To: robh+dt, thierry.reding Cc: sam, maxime, dave.stevenson, david, devicetree, dri-devel, Noralf Trønnes Add a driver that will work with most MIPI DBI compatible SPI panels. This avoids adding a driver for every new MIPI DBI compatible controller that is to be used by Linux. The 'compatible' Device Tree property with a '.bin' suffix will be used to load a firmware file that contains the controller configuration. Example (driver will load sainsmart18.bin): display@0 { compatible = "sainsmart18", "panel-mipi-dbi-spi"; ... }; v4: - Move driver to drm/tiny where the other drivers of its kind are located. The driver module will not be shared with a future DPI driver after all. v3: - Move properties to DT (Maxime) - The MIPI DPI spec has optional support for DPI where the controller is configured over DBI. Rework the command functions so they can be moved to drm_mipi_dbi and shared with a future panel-mipi-dpi-spi driver v2: - Drop model property and use compatible instead (Rob) - Add wiki entry in MAINTAINERS Acked-by: Maxime Ripard <maxime@cerno.tech> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- MAINTAINERS | 8 + drivers/gpu/drm/tiny/Kconfig | 13 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/panel-mipi-dbi.c | 413 ++++++++++++++++++++++++++ 4 files changed, 435 insertions(+) create mode 100644 drivers/gpu/drm/tiny/panel-mipi-dbi.c diff --git a/MAINTAINERS b/MAINTAINERS index 8e6e892f99f0..3a0e57f23ad0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6107,6 +6107,14 @@ T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt F: drivers/gpu/drm/tiny/mi0283qt.c +DRM DRIVER FOR MIPI DBI compatible panels +M: Noralf Trønnes <noralf@tronnes.org> +S: Maintained +W: https://github.com/notro/panel-mipi-dbi/wiki +T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml +F: drivers/gpu/drm/tiny/panel-mipi-dbi.c + DRM DRIVER FOR MSM ADRENO GPU M: Rob Clark <robdclark@gmail.com> M: Sean Paul <sean@poorly.run> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 712e0004e96e..d552e1618da7 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -51,6 +51,19 @@ config DRM_GM12U320 This is a KMS driver for projectors which use the GM12U320 chipset for video transfer over USB2/3, such as the Acer C120 mini projector. +config DRM_PANEL_MIPI_DBI + tristate "DRM support for MIPI DBI compatible panels" + depends on DRM && SPI + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + select DRM_MIPI_DBI + select BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for MIPI DBI compatible + panels. The controller command setup can be provided using a + firmware file. + To compile this driver as a module, choose M here. + config DRM_SIMPLEDRM tristate "Simple framebuffer driver" depends on DRM && MMU diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index 5d5505d40e7b..1d9d6227e7ab 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o obj-$(CONFIG_DRM_BOCHS) += bochs.o obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o obj-$(CONFIG_DRM_GM12U320) += gm12u320.o +obj-$(CONFIG_DRM_PANEL_MIPI_DBI) += panel-mipi-dbi.o obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm.o obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o obj-$(CONFIG_TINYDRM_ILI9163) += ili9163.o diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c new file mode 100644 index 000000000000..9240fdec38d6 --- /dev/null +++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c @@ -0,0 +1,413 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * DRM driver for MIPI DBI compatible display panels + * + * Copyright 2022 Noralf Trønnes + */ + +#include <linux/backlight.h> +#include <linux/delay.h> +#include <linux/firmware.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/property.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_drv.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_gem_atomic_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_managed.h> +#include <drm/drm_mipi_dbi.h> +#include <drm/drm_modeset_helper.h> + +#include <video/display_timing.h> +#include <video/mipi_display.h> +#include <video/of_display_timing.h> +#include <video/videomode.h> + +static const u8 panel_mipi_dbi_magic[15] = { 'M', 'I', 'P', 'I', ' ', 'D', 'B', 'I', + 0, 0, 0, 0, 0, 0, 0 }; + +/* + * The optional display controller configuration is stored in a firmware file. + * The Device Tree 'compatible' property value with a '.bin' suffix is passed + * to request_firmware() to fetch this file. + */ +struct panel_mipi_dbi_config { + /* Magic string: panel_mipi_dbi_magic */ + u8 magic[15]; + + /* Config file format version */ + u8 file_format_version; + + /* + * MIPI commands to execute when the display pipeline is enabled. + * This is used to configure the display controller. + * + * The commands are stored in a byte array with the format: + * command, num_parameters, [ parameter, ...], command, ... + * + * Some commands require a pause before the next command can be received. + * Inserting a delay in the command sequence is done by using the NOP command with one + * parameter: delay in miliseconds (the No Operation command is part of the MIPI Display + * Command Set where it has no parameters). + * + * Example: + * command 0x11 + * sleep 120ms + * command 0xb1 parameters 0x01, 0x2c, 0x2d + * command 0x29 + * + * Byte sequence: + * 0x11 0x00 + * 0x00 0x01 0x78 + * 0xb1 0x03 0x01 0x2c 0x2d + * 0x29 0x00 + */ + u8 commands[]; +}; + +struct panel_mipi_dbi_commands { + const u8 *buf; + size_t len; +}; + +static struct panel_mipi_dbi_commands * +panel_mipi_dbi_check_commands(struct device *dev, const struct firmware *fw) +{ + const struct panel_mipi_dbi_config *config = (struct panel_mipi_dbi_config *)fw->data; + struct panel_mipi_dbi_commands *commands; + size_t size = fw->size, commands_len; + unsigned int i = 0; + + if (size < sizeof(*config) + 2) { /* At least 1 command */ + dev_err(dev, "config: file size=%zu is too small\n", size); + return ERR_PTR(-EINVAL); + } + + if (memcmp(config->magic, panel_mipi_dbi_magic, sizeof(config->magic))) { + dev_err(dev, "config: Bad magic: %15ph\n", config->magic); + return ERR_PTR(-EINVAL); + } + + if (config->file_format_version != 1) { + dev_err(dev, "config: version=%u is not supported\n", config->file_format_version); + return ERR_PTR(-EINVAL); + } + + drm_dev_dbg(dev, DRM_UT_DRIVER, "size=%zu version=%u\n", size, config->file_format_version); + + commands_len = size - sizeof(*config); + + while ((i + 1) < commands_len) { + u8 command = config->commands[i++]; + u8 num_parameters = config->commands[i++]; + const u8 *parameters = &config->commands[i]; + + i += num_parameters; + if (i > commands_len) { + dev_err(dev, "config: command=0x%02x num_parameters=%u overflows\n", + command, num_parameters); + return ERR_PTR(-EINVAL); + } + + if (command == 0x00 && num_parameters == 1) + drm_dev_dbg(dev, DRM_UT_DRIVER, "sleep %ums\n", parameters[0]); + else + drm_dev_dbg(dev, DRM_UT_DRIVER, "command %02x %*ph\n", + command, num_parameters, parameters); + } + + if (i != commands_len) { + dev_err(dev, "config: malformed command array\n"); + return ERR_PTR(-EINVAL); + } + + commands = devm_kzalloc(dev, sizeof(*commands), GFP_KERNEL); + if (!commands) + return ERR_PTR(-ENOMEM); + + commands->len = commands_len; + commands->buf = devm_kmemdup(dev, config->commands, commands->len, GFP_KERNEL); + if (!commands->buf) + return ERR_PTR(-ENOMEM); + + return commands; +} + +static struct panel_mipi_dbi_commands *panel_mipi_dbi_commands_from_fw(struct device *dev) +{ + struct panel_mipi_dbi_commands *commands; + const struct firmware *fw; + const char *compatible; + struct property *prop; + char fw_name[40]; + int ret; + + of_property_for_each_string(dev->of_node, "compatible", prop, compatible) { + snprintf(fw_name, sizeof(fw_name), "%s.bin", compatible); + ret = firmware_request_nowarn(&fw, fw_name, dev); + if (ret) { + drm_dev_dbg(dev, DRM_UT_DRIVER, + "No config file found for compatible: '%s' (error=%d)\n", + compatible, ret); + continue; + } + + commands = panel_mipi_dbi_check_commands(dev, fw); + release_firmware(fw); + return commands; + } + + return NULL; +} + +static void panel_mipi_dbi_commands_execute(struct mipi_dbi *dbi, + struct panel_mipi_dbi_commands *commands) +{ + unsigned int i = 0; + + if (!commands) + return; + + while (i < commands->len) { + u8 command = commands->buf[i++]; + u8 num_parameters = commands->buf[i++]; + const u8 *parameters = &commands->buf[i]; + + if (command == 0x00 && num_parameters == 1) + msleep(parameters[0]); + else if (num_parameters) + mipi_dbi_command_stackbuf(dbi, command, parameters, num_parameters); + else + mipi_dbi_command(dbi, command); + + i += num_parameters; + } +} + +static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) +{ + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev); + struct mipi_dbi *dbi = &dbidev->dbi; + int ret, idx; + + if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return; + + drm_dbg(pipe->crtc.dev, "\n"); + + ret = mipi_dbi_poweron_conditional_reset(dbidev); + if (ret < 0) + goto out_exit; + if (!ret) + panel_mipi_dbi_commands_execute(dbi, dbidev->driver_private); + + mipi_dbi_enable_flush(dbidev, crtc_state, plane_state); +out_exit: + drm_dev_exit(idx); +} + +static const struct drm_simple_display_pipe_funcs panel_mipi_dbi_pipe_funcs = { + .enable = panel_mipi_dbi_enable, + .disable = mipi_dbi_pipe_disable, + .update = mipi_dbi_pipe_update, +}; + +DEFINE_DRM_GEM_CMA_FOPS(panel_mipi_dbi_fops); + +static const struct drm_driver panel_mipi_dbi_driver = { + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, + .fops = &panel_mipi_dbi_fops, + DRM_GEM_CMA_DRIVER_OPS_VMAP, + .debugfs_init = mipi_dbi_debugfs_init, + .name = "panel-mipi-dbi", + .desc = "MIPI DBI compatible display panel", + .date = "20220103", + .major = 1, + .minor = 0, +}; + +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) +{ + struct device *dev = dbidev->drm.dev; + u32 width_mm = 0, height_mm = 0; + struct display_timing timing; + struct videomode vm; + int ret; + + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); + if (ret) { + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); + return ret; + } + + videomode_from_timing(&timing, &vm); + + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || + (vm.hback_porch + vm.hactive) > 0xffff || + !vm.vactive || vm.vfront_porch || vm.vsync_len || + (vm.vback_porch + vm.vactive) > 0xffff || + vm.flags) { + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); + return -EINVAL; + } + + /* The driver doesn't use the pixel clock but it is mandatory so fake one if not set */ + if (!vm.pixelclock) + vm.pixelclock = (vm.hback_porch + vm.hactive) * (vm.vback_porch + vm.vactive) * 60; + + dbidev->top_offset = vm.vback_porch; + dbidev->left_offset = vm.hback_porch; + + memset(mode, 0, sizeof(*mode)); + drm_display_mode_from_videomode(&vm, mode); + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; + + ret = device_property_read_u32(dev, "width-mm", &width_mm); + if (ret && ret != -EINVAL) + return ret; + + ret = device_property_read_u32(dev, "height-mm", &height_mm); + if (ret && ret != -EINVAL) + return ret; + + mode->width_mm = width_mm; + mode->height_mm = height_mm; + + drm_mode_debug_printmodeline(mode); + + return 0; +} + +static int panel_mipi_dbi_spi_probe(struct spi_device *spi) +{ + struct device *dev = &spi->dev; + struct drm_display_mode mode; + struct mipi_dbi_dev *dbidev; + struct drm_device *drm; + struct mipi_dbi *dbi; + struct gpio_desc *dc; + int ret; + + dbidev = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver, struct mipi_dbi_dev, drm); + if (IS_ERR(dbidev)) + return PTR_ERR(dbidev); + + dbi = &dbidev->dbi; + drm = &dbidev->drm; + + ret = panel_mipi_dbi_get_mode(dbidev, &mode); + if (ret) + return ret; + + dbidev->regulator = devm_regulator_get(dev, "power"); + if (IS_ERR(dbidev->regulator)) + return dev_err_probe(dev, PTR_ERR(dbidev->regulator), + "Failed to get regulator 'power'\n"); + + dbidev->backlight = devm_of_find_backlight(dev); + if (IS_ERR(dbidev->backlight)) + return dev_err_probe(dev, PTR_ERR(dbidev->backlight), "Failed to get backlight\n"); + + dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(dbi->reset)) + return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n"); + + dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); + if (IS_ERR(dc)) + return dev_err_probe(dev, PTR_ERR(dc), "Failed to get GPIO 'dc'\n"); + + ret = mipi_dbi_spi_init(spi, dbi, dc); + if (ret) + return ret; + + if (device_property_present(dev, "write-only")) + dbi->read_commands = NULL; + + dbidev->driver_private = panel_mipi_dbi_commands_from_fw(dev); + if (IS_ERR(dbidev->driver_private)) + return PTR_ERR(dbidev->driver_private); + + ret = mipi_dbi_dev_init(dbidev, &panel_mipi_dbi_pipe_funcs, &mode, 0); + if (ret) + return ret; + + drm_mode_config_reset(drm); + + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + spi_set_drvdata(spi, drm); + + drm_fbdev_generic_setup(drm, 0); + + return 0; +} + +static int panel_mipi_dbi_spi_remove(struct spi_device *spi) +{ + struct drm_device *drm = spi_get_drvdata(spi); + + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + + return 0; +} + +static void panel_mipi_dbi_spi_shutdown(struct spi_device *spi) +{ + drm_atomic_helper_shutdown(spi_get_drvdata(spi)); +} + +static int __maybe_unused panel_mipi_dbi_pm_suspend(struct device *dev) +{ + return drm_mode_config_helper_suspend(dev_get_drvdata(dev)); +} + +static int __maybe_unused panel_mipi_dbi_pm_resume(struct device *dev) +{ + drm_mode_config_helper_resume(dev_get_drvdata(dev)); + + return 0; +} + +static const struct dev_pm_ops panel_mipi_dbi_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(panel_mipi_dbi_pm_suspend, panel_mipi_dbi_pm_resume) +}; + +static const struct of_device_id panel_mipi_dbi_spi_of_match[] = { + { .compatible = "panel-mipi-dbi-spi" }, + {}, +}; +MODULE_DEVICE_TABLE(of, panel_mipi_dbi_spi_of_match); + +static const struct spi_device_id panel_mipi_dbi_spi_id[] = { + { "panel-mipi-dbi-spi", 0 }, + { }, +}; +MODULE_DEVICE_TABLE(spi, panel_mipi_dbi_spi_id); + +static struct spi_driver panel_mipi_dbi_spi_driver = { + .driver = { + .name = "panel-mipi-dbi-spi", + .owner = THIS_MODULE, + .of_match_table = panel_mipi_dbi_spi_of_match, + .pm = &panel_mipi_dbi_pm_ops, + }, + .id_table = panel_mipi_dbi_spi_id, + .probe = panel_mipi_dbi_spi_probe, + .remove = panel_mipi_dbi_spi_remove, + .shutdown = panel_mipi_dbi_spi_shutdown, +}; +module_spi_driver(panel_mipi_dbi_spi_driver); + +MODULE_DESCRIPTION("MIPI DBI compatible display panel driver"); +MODULE_AUTHOR("Noralf Trønnes"); +MODULE_LICENSE("GPL"); -- 2.33.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-18 15:11 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-18 15:11 UTC (permalink / raw) To: robh+dt, thierry.reding Cc: devicetree, david, dave.stevenson, dri-devel, Noralf Trønnes, maxime, sam Add a driver that will work with most MIPI DBI compatible SPI panels. This avoids adding a driver for every new MIPI DBI compatible controller that is to be used by Linux. The 'compatible' Device Tree property with a '.bin' suffix will be used to load a firmware file that contains the controller configuration. Example (driver will load sainsmart18.bin): display@0 { compatible = "sainsmart18", "panel-mipi-dbi-spi"; ... }; v4: - Move driver to drm/tiny where the other drivers of its kind are located. The driver module will not be shared with a future DPI driver after all. v3: - Move properties to DT (Maxime) - The MIPI DPI spec has optional support for DPI where the controller is configured over DBI. Rework the command functions so they can be moved to drm_mipi_dbi and shared with a future panel-mipi-dpi-spi driver v2: - Drop model property and use compatible instead (Rob) - Add wiki entry in MAINTAINERS Acked-by: Maxime Ripard <maxime@cerno.tech> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- MAINTAINERS | 8 + drivers/gpu/drm/tiny/Kconfig | 13 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/panel-mipi-dbi.c | 413 ++++++++++++++++++++++++++ 4 files changed, 435 insertions(+) create mode 100644 drivers/gpu/drm/tiny/panel-mipi-dbi.c diff --git a/MAINTAINERS b/MAINTAINERS index 8e6e892f99f0..3a0e57f23ad0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6107,6 +6107,14 @@ T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt F: drivers/gpu/drm/tiny/mi0283qt.c +DRM DRIVER FOR MIPI DBI compatible panels +M: Noralf Trønnes <noralf@tronnes.org> +S: Maintained +W: https://github.com/notro/panel-mipi-dbi/wiki +T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml +F: drivers/gpu/drm/tiny/panel-mipi-dbi.c + DRM DRIVER FOR MSM ADRENO GPU M: Rob Clark <robdclark@gmail.com> M: Sean Paul <sean@poorly.run> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 712e0004e96e..d552e1618da7 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -51,6 +51,19 @@ config DRM_GM12U320 This is a KMS driver for projectors which use the GM12U320 chipset for video transfer over USB2/3, such as the Acer C120 mini projector. +config DRM_PANEL_MIPI_DBI + tristate "DRM support for MIPI DBI compatible panels" + depends on DRM && SPI + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + select DRM_MIPI_DBI + select BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for MIPI DBI compatible + panels. The controller command setup can be provided using a + firmware file. + To compile this driver as a module, choose M here. + config DRM_SIMPLEDRM tristate "Simple framebuffer driver" depends on DRM && MMU diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index 5d5505d40e7b..1d9d6227e7ab 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o obj-$(CONFIG_DRM_BOCHS) += bochs.o obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o obj-$(CONFIG_DRM_GM12U320) += gm12u320.o +obj-$(CONFIG_DRM_PANEL_MIPI_DBI) += panel-mipi-dbi.o obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm.o obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o obj-$(CONFIG_TINYDRM_ILI9163) += ili9163.o diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c new file mode 100644 index 000000000000..9240fdec38d6 --- /dev/null +++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c @@ -0,0 +1,413 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * DRM driver for MIPI DBI compatible display panels + * + * Copyright 2022 Noralf Trønnes + */ + +#include <linux/backlight.h> +#include <linux/delay.h> +#include <linux/firmware.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/property.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_drv.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_gem_atomic_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_managed.h> +#include <drm/drm_mipi_dbi.h> +#include <drm/drm_modeset_helper.h> + +#include <video/display_timing.h> +#include <video/mipi_display.h> +#include <video/of_display_timing.h> +#include <video/videomode.h> + +static const u8 panel_mipi_dbi_magic[15] = { 'M', 'I', 'P', 'I', ' ', 'D', 'B', 'I', + 0, 0, 0, 0, 0, 0, 0 }; + +/* + * The optional display controller configuration is stored in a firmware file. + * The Device Tree 'compatible' property value with a '.bin' suffix is passed + * to request_firmware() to fetch this file. + */ +struct panel_mipi_dbi_config { + /* Magic string: panel_mipi_dbi_magic */ + u8 magic[15]; + + /* Config file format version */ + u8 file_format_version; + + /* + * MIPI commands to execute when the display pipeline is enabled. + * This is used to configure the display controller. + * + * The commands are stored in a byte array with the format: + * command, num_parameters, [ parameter, ...], command, ... + * + * Some commands require a pause before the next command can be received. + * Inserting a delay in the command sequence is done by using the NOP command with one + * parameter: delay in miliseconds (the No Operation command is part of the MIPI Display + * Command Set where it has no parameters). + * + * Example: + * command 0x11 + * sleep 120ms + * command 0xb1 parameters 0x01, 0x2c, 0x2d + * command 0x29 + * + * Byte sequence: + * 0x11 0x00 + * 0x00 0x01 0x78 + * 0xb1 0x03 0x01 0x2c 0x2d + * 0x29 0x00 + */ + u8 commands[]; +}; + +struct panel_mipi_dbi_commands { + const u8 *buf; + size_t len; +}; + +static struct panel_mipi_dbi_commands * +panel_mipi_dbi_check_commands(struct device *dev, const struct firmware *fw) +{ + const struct panel_mipi_dbi_config *config = (struct panel_mipi_dbi_config *)fw->data; + struct panel_mipi_dbi_commands *commands; + size_t size = fw->size, commands_len; + unsigned int i = 0; + + if (size < sizeof(*config) + 2) { /* At least 1 command */ + dev_err(dev, "config: file size=%zu is too small\n", size); + return ERR_PTR(-EINVAL); + } + + if (memcmp(config->magic, panel_mipi_dbi_magic, sizeof(config->magic))) { + dev_err(dev, "config: Bad magic: %15ph\n", config->magic); + return ERR_PTR(-EINVAL); + } + + if (config->file_format_version != 1) { + dev_err(dev, "config: version=%u is not supported\n", config->file_format_version); + return ERR_PTR(-EINVAL); + } + + drm_dev_dbg(dev, DRM_UT_DRIVER, "size=%zu version=%u\n", size, config->file_format_version); + + commands_len = size - sizeof(*config); + + while ((i + 1) < commands_len) { + u8 command = config->commands[i++]; + u8 num_parameters = config->commands[i++]; + const u8 *parameters = &config->commands[i]; + + i += num_parameters; + if (i > commands_len) { + dev_err(dev, "config: command=0x%02x num_parameters=%u overflows\n", + command, num_parameters); + return ERR_PTR(-EINVAL); + } + + if (command == 0x00 && num_parameters == 1) + drm_dev_dbg(dev, DRM_UT_DRIVER, "sleep %ums\n", parameters[0]); + else + drm_dev_dbg(dev, DRM_UT_DRIVER, "command %02x %*ph\n", + command, num_parameters, parameters); + } + + if (i != commands_len) { + dev_err(dev, "config: malformed command array\n"); + return ERR_PTR(-EINVAL); + } + + commands = devm_kzalloc(dev, sizeof(*commands), GFP_KERNEL); + if (!commands) + return ERR_PTR(-ENOMEM); + + commands->len = commands_len; + commands->buf = devm_kmemdup(dev, config->commands, commands->len, GFP_KERNEL); + if (!commands->buf) + return ERR_PTR(-ENOMEM); + + return commands; +} + +static struct panel_mipi_dbi_commands *panel_mipi_dbi_commands_from_fw(struct device *dev) +{ + struct panel_mipi_dbi_commands *commands; + const struct firmware *fw; + const char *compatible; + struct property *prop; + char fw_name[40]; + int ret; + + of_property_for_each_string(dev->of_node, "compatible", prop, compatible) { + snprintf(fw_name, sizeof(fw_name), "%s.bin", compatible); + ret = firmware_request_nowarn(&fw, fw_name, dev); + if (ret) { + drm_dev_dbg(dev, DRM_UT_DRIVER, + "No config file found for compatible: '%s' (error=%d)\n", + compatible, ret); + continue; + } + + commands = panel_mipi_dbi_check_commands(dev, fw); + release_firmware(fw); + return commands; + } + + return NULL; +} + +static void panel_mipi_dbi_commands_execute(struct mipi_dbi *dbi, + struct panel_mipi_dbi_commands *commands) +{ + unsigned int i = 0; + + if (!commands) + return; + + while (i < commands->len) { + u8 command = commands->buf[i++]; + u8 num_parameters = commands->buf[i++]; + const u8 *parameters = &commands->buf[i]; + + if (command == 0x00 && num_parameters == 1) + msleep(parameters[0]); + else if (num_parameters) + mipi_dbi_command_stackbuf(dbi, command, parameters, num_parameters); + else + mipi_dbi_command(dbi, command); + + i += num_parameters; + } +} + +static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) +{ + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev); + struct mipi_dbi *dbi = &dbidev->dbi; + int ret, idx; + + if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return; + + drm_dbg(pipe->crtc.dev, "\n"); + + ret = mipi_dbi_poweron_conditional_reset(dbidev); + if (ret < 0) + goto out_exit; + if (!ret) + panel_mipi_dbi_commands_execute(dbi, dbidev->driver_private); + + mipi_dbi_enable_flush(dbidev, crtc_state, plane_state); +out_exit: + drm_dev_exit(idx); +} + +static const struct drm_simple_display_pipe_funcs panel_mipi_dbi_pipe_funcs = { + .enable = panel_mipi_dbi_enable, + .disable = mipi_dbi_pipe_disable, + .update = mipi_dbi_pipe_update, +}; + +DEFINE_DRM_GEM_CMA_FOPS(panel_mipi_dbi_fops); + +static const struct drm_driver panel_mipi_dbi_driver = { + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, + .fops = &panel_mipi_dbi_fops, + DRM_GEM_CMA_DRIVER_OPS_VMAP, + .debugfs_init = mipi_dbi_debugfs_init, + .name = "panel-mipi-dbi", + .desc = "MIPI DBI compatible display panel", + .date = "20220103", + .major = 1, + .minor = 0, +}; + +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) +{ + struct device *dev = dbidev->drm.dev; + u32 width_mm = 0, height_mm = 0; + struct display_timing timing; + struct videomode vm; + int ret; + + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); + if (ret) { + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); + return ret; + } + + videomode_from_timing(&timing, &vm); + + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || + (vm.hback_porch + vm.hactive) > 0xffff || + !vm.vactive || vm.vfront_porch || vm.vsync_len || + (vm.vback_porch + vm.vactive) > 0xffff || + vm.flags) { + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); + return -EINVAL; + } + + /* The driver doesn't use the pixel clock but it is mandatory so fake one if not set */ + if (!vm.pixelclock) + vm.pixelclock = (vm.hback_porch + vm.hactive) * (vm.vback_porch + vm.vactive) * 60; + + dbidev->top_offset = vm.vback_porch; + dbidev->left_offset = vm.hback_porch; + + memset(mode, 0, sizeof(*mode)); + drm_display_mode_from_videomode(&vm, mode); + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; + + ret = device_property_read_u32(dev, "width-mm", &width_mm); + if (ret && ret != -EINVAL) + return ret; + + ret = device_property_read_u32(dev, "height-mm", &height_mm); + if (ret && ret != -EINVAL) + return ret; + + mode->width_mm = width_mm; + mode->height_mm = height_mm; + + drm_mode_debug_printmodeline(mode); + + return 0; +} + +static int panel_mipi_dbi_spi_probe(struct spi_device *spi) +{ + struct device *dev = &spi->dev; + struct drm_display_mode mode; + struct mipi_dbi_dev *dbidev; + struct drm_device *drm; + struct mipi_dbi *dbi; + struct gpio_desc *dc; + int ret; + + dbidev = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver, struct mipi_dbi_dev, drm); + if (IS_ERR(dbidev)) + return PTR_ERR(dbidev); + + dbi = &dbidev->dbi; + drm = &dbidev->drm; + + ret = panel_mipi_dbi_get_mode(dbidev, &mode); + if (ret) + return ret; + + dbidev->regulator = devm_regulator_get(dev, "power"); + if (IS_ERR(dbidev->regulator)) + return dev_err_probe(dev, PTR_ERR(dbidev->regulator), + "Failed to get regulator 'power'\n"); + + dbidev->backlight = devm_of_find_backlight(dev); + if (IS_ERR(dbidev->backlight)) + return dev_err_probe(dev, PTR_ERR(dbidev->backlight), "Failed to get backlight\n"); + + dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(dbi->reset)) + return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n"); + + dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); + if (IS_ERR(dc)) + return dev_err_probe(dev, PTR_ERR(dc), "Failed to get GPIO 'dc'\n"); + + ret = mipi_dbi_spi_init(spi, dbi, dc); + if (ret) + return ret; + + if (device_property_present(dev, "write-only")) + dbi->read_commands = NULL; + + dbidev->driver_private = panel_mipi_dbi_commands_from_fw(dev); + if (IS_ERR(dbidev->driver_private)) + return PTR_ERR(dbidev->driver_private); + + ret = mipi_dbi_dev_init(dbidev, &panel_mipi_dbi_pipe_funcs, &mode, 0); + if (ret) + return ret; + + drm_mode_config_reset(drm); + + ret = drm_dev_register(drm, 0); + if (ret) + return ret; + + spi_set_drvdata(spi, drm); + + drm_fbdev_generic_setup(drm, 0); + + return 0; +} + +static int panel_mipi_dbi_spi_remove(struct spi_device *spi) +{ + struct drm_device *drm = spi_get_drvdata(spi); + + drm_dev_unplug(drm); + drm_atomic_helper_shutdown(drm); + + return 0; +} + +static void panel_mipi_dbi_spi_shutdown(struct spi_device *spi) +{ + drm_atomic_helper_shutdown(spi_get_drvdata(spi)); +} + +static int __maybe_unused panel_mipi_dbi_pm_suspend(struct device *dev) +{ + return drm_mode_config_helper_suspend(dev_get_drvdata(dev)); +} + +static int __maybe_unused panel_mipi_dbi_pm_resume(struct device *dev) +{ + drm_mode_config_helper_resume(dev_get_drvdata(dev)); + + return 0; +} + +static const struct dev_pm_ops panel_mipi_dbi_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(panel_mipi_dbi_pm_suspend, panel_mipi_dbi_pm_resume) +}; + +static const struct of_device_id panel_mipi_dbi_spi_of_match[] = { + { .compatible = "panel-mipi-dbi-spi" }, + {}, +}; +MODULE_DEVICE_TABLE(of, panel_mipi_dbi_spi_of_match); + +static const struct spi_device_id panel_mipi_dbi_spi_id[] = { + { "panel-mipi-dbi-spi", 0 }, + { }, +}; +MODULE_DEVICE_TABLE(spi, panel_mipi_dbi_spi_id); + +static struct spi_driver panel_mipi_dbi_spi_driver = { + .driver = { + .name = "panel-mipi-dbi-spi", + .owner = THIS_MODULE, + .of_match_table = panel_mipi_dbi_spi_of_match, + .pm = &panel_mipi_dbi_pm_ops, + }, + .id_table = panel_mipi_dbi_spi_id, + .probe = panel_mipi_dbi_spi_probe, + .remove = panel_mipi_dbi_spi_remove, + .shutdown = panel_mipi_dbi_spi_shutdown, +}; +module_spi_driver(panel_mipi_dbi_spi_driver); + +MODULE_DESCRIPTION("MIPI DBI compatible display panel driver"); +MODULE_AUTHOR("Noralf Trønnes"); +MODULE_LICENSE("GPL"); -- 2.33.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-18 15:11 ` Noralf Trønnes @ 2022-02-19 22:10 ` Sam Ravnborg -1 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-19 22:10 UTC (permalink / raw) To: Noralf Trønnes Cc: robh+dt, thierry.reding, maxime, dave.stevenson, david, devicetree, dri-devel Hi Noralf, On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote: > Add a driver that will work with most MIPI DBI compatible SPI panels. > This avoids adding a driver for every new MIPI DBI compatible controller > that is to be used by Linux. The 'compatible' Device Tree property with > a '.bin' suffix will be used to load a firmware file that contains the > controller configuration. A solution where we have the command sequences as part of the DT-overlay is IMO a much better way to solve this: - The users needs to deal only with a single file, so there is less that goes wrong - The user need not reading special instructions how to handle a .bin file, if the overlay is present everything is fine - The file that contains the command sequences can be properly annotated with comments - The people that creates the command sequences has no need for a special script to create the file for a display - this is all readable ascii. - Using a DT-overlay the parsing of the DT-overlay can be done by well-tested functions, no need to invent something new to parse the file The idea with a common mipi DBI SPI driver is super, it is the detail with the .bin file that I am against. With the above said, a few comments to the current implementation below. As we know it from you - a very well-written driver. Sam > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > MAINTAINERS | 8 + > drivers/gpu/drm/tiny/Kconfig | 13 + > drivers/gpu/drm/tiny/Makefile | 1 + > drivers/gpu/drm/tiny/panel-mipi-dbi.c | 413 ++++++++++++++++++++++++++ > 4 files changed, 435 insertions(+) > create mode 100644 drivers/gpu/drm/tiny/panel-mipi-dbi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 8e6e892f99f0..3a0e57f23ad0 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6107,6 +6107,14 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > F: Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt > F: drivers/gpu/drm/tiny/mi0283qt.c > > +DRM DRIVER FOR MIPI DBI compatible panels > +M: Noralf Trønnes <noralf@tronnes.org> > +S: Maintained > +W: https://github.com/notro/panel-mipi-dbi/wiki Nice with a wiki for this, I can see this will grow over time and be a place to find how to support more panels. > +T: git git://anongit.freedesktop.org/drm/drm-misc > +F: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > +F: drivers/gpu/drm/tiny/panel-mipi-dbi.c > + > DRM DRIVER FOR MSM ADRENO GPU > M: Rob Clark <robdclark@gmail.com> > M: Sean Paul <sean@poorly.run> > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig > index 712e0004e96e..d552e1618da7 100644 > --- a/drivers/gpu/drm/tiny/Kconfig > +++ b/drivers/gpu/drm/tiny/Kconfig > @@ -51,6 +51,19 @@ config DRM_GM12U320 > This is a KMS driver for projectors which use the GM12U320 chipset > for video transfer over USB2/3, such as the Acer C120 mini projector. > > +config DRM_PANEL_MIPI_DBI > + tristate "DRM support for MIPI DBI compatible panels" > + depends on DRM && SPI > + select DRM_KMS_HELPER > + select DRM_KMS_CMA_HELPER This symbol is not present in my drm-misc-next tree (which is a few weeks old, so it may be newer). > + select DRM_MIPI_DBI > + select BACKLIGHT_CLASS_DEVICE > + help > + Say Y here if you want to enable support for MIPI DBI compatible > + panels. The controller command setup can be provided using a > + firmware file. Consider adding a link to the wiki here - this may make it easier for the user to find it. > + To compile this driver as a module, choose M here. > + > config DRM_SIMPLEDRM > tristate "Simple framebuffer driver" > depends on DRM && MMU > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile > index 5d5505d40e7b..1d9d6227e7ab 100644 > --- a/drivers/gpu/drm/tiny/Makefile > +++ b/drivers/gpu/drm/tiny/Makefile > @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o > obj-$(CONFIG_DRM_BOCHS) += bochs.o > obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o > obj-$(CONFIG_DRM_GM12U320) += gm12u320.o > +obj-$(CONFIG_DRM_PANEL_MIPI_DBI) += panel-mipi-dbi.o > obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm.o > obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o > obj-$(CONFIG_TINYDRM_ILI9163) += ili9163.o > diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c > new file mode 100644 > index 000000000000..9240fdec38d6 > --- /dev/null > +++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c > @@ -0,0 +1,413 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * DRM driver for MIPI DBI compatible display panels > + * > + * Copyright 2022 Noralf Trønnes > + */ > + > +#include <linux/backlight.h> > +#include <linux/delay.h> > +#include <linux/firmware.h> > +#include <linux/gpio/consumer.h> > +#include <linux/module.h> > +#include <linux/property.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > + > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_drv.h> > +#include <drm/drm_fb_helper.h> > +#include <drm/drm_gem_atomic_helper.h> > +#include <drm/drm_gem_cma_helper.h> > +#include <drm/drm_managed.h> > +#include <drm/drm_mipi_dbi.h> > +#include <drm/drm_modeset_helper.h> > + > +#include <video/display_timing.h> > +#include <video/mipi_display.h> > +#include <video/of_display_timing.h> > +#include <video/videomode.h> videomode should not be used in new drivers, it is an fbdev artifact. But that said - we are still missing a direct display_timing => display_mode - so maybe we need it here. If it is needed Kconfig needs to be extended with: select VIDEOMODE_HELPERS > + > +static const u8 panel_mipi_dbi_magic[15] = { 'M', 'I', 'P', 'I', ' ', 'D', 'B', 'I', > + 0, 0, 0, 0, 0, 0, 0 }; > + > +/* > + * The optional display controller configuration is stored in a firmware file. > + * The Device Tree 'compatible' property value with a '.bin' suffix is passed > + * to request_firmware() to fetch this file. > + */ > +struct panel_mipi_dbi_config { > + /* Magic string: panel_mipi_dbi_magic */ > + u8 magic[15]; > + > + /* Config file format version */ > + u8 file_format_version; > + > + /* > + * MIPI commands to execute when the display pipeline is enabled. > + * This is used to configure the display controller. > + * > + * The commands are stored in a byte array with the format: > + * command, num_parameters, [ parameter, ...], command, ... > + * > + * Some commands require a pause before the next command can be received. > + * Inserting a delay in the command sequence is done by using the NOP command with one > + * parameter: delay in miliseconds (the No Operation command is part of the MIPI Display > + * Command Set where it has no parameters). > + * > + * Example: > + * command 0x11 > + * sleep 120ms > + * command 0xb1 parameters 0x01, 0x2c, 0x2d > + * command 0x29 > + * > + * Byte sequence: > + * 0x11 0x00 > + * 0x00 0x01 0x78 > + * 0xb1 0x03 0x01 0x2c 0x2d > + * 0x29 0x00 > + */ > + u8 commands[]; > +}; > + > +struct panel_mipi_dbi_commands { > + const u8 *buf; > + size_t len; > +}; > + > +static struct panel_mipi_dbi_commands * > +panel_mipi_dbi_check_commands(struct device *dev, const struct firmware *fw) > +{ > + const struct panel_mipi_dbi_config *config = (struct panel_mipi_dbi_config *)fw->data; > + struct panel_mipi_dbi_commands *commands; > + size_t size = fw->size, commands_len; > + unsigned int i = 0; > + > + if (size < sizeof(*config) + 2) { /* At least 1 command */ > + dev_err(dev, "config: file size=%zu is too small\n", size); > + return ERR_PTR(-EINVAL); > + } > + > + if (memcmp(config->magic, panel_mipi_dbi_magic, sizeof(config->magic))) { > + dev_err(dev, "config: Bad magic: %15ph\n", config->magic); > + return ERR_PTR(-EINVAL); > + } > + > + if (config->file_format_version != 1) { > + dev_err(dev, "config: version=%u is not supported\n", config->file_format_version); > + return ERR_PTR(-EINVAL); > + } > + > + drm_dev_dbg(dev, DRM_UT_DRIVER, "size=%zu version=%u\n", size, config->file_format_version); > + > + commands_len = size - sizeof(*config); > + > + while ((i + 1) < commands_len) { > + u8 command = config->commands[i++]; > + u8 num_parameters = config->commands[i++]; > + const u8 *parameters = &config->commands[i]; > + > + i += num_parameters; > + if (i > commands_len) { > + dev_err(dev, "config: command=0x%02x num_parameters=%u overflows\n", > + command, num_parameters); > + return ERR_PTR(-EINVAL); > + } > + > + if (command == 0x00 && num_parameters == 1) > + drm_dev_dbg(dev, DRM_UT_DRIVER, "sleep %ums\n", parameters[0]); > + else > + drm_dev_dbg(dev, DRM_UT_DRIVER, "command %02x %*ph\n", > + command, num_parameters, parameters); > + } > + > + if (i != commands_len) { > + dev_err(dev, "config: malformed command array\n"); > + return ERR_PTR(-EINVAL); > + } > + > + commands = devm_kzalloc(dev, sizeof(*commands), GFP_KERNEL); > + if (!commands) > + return ERR_PTR(-ENOMEM); > + > + commands->len = commands_len; > + commands->buf = devm_kmemdup(dev, config->commands, commands->len, GFP_KERNEL); > + if (!commands->buf) > + return ERR_PTR(-ENOMEM); > + > + return commands; > +} > + > +static struct panel_mipi_dbi_commands *panel_mipi_dbi_commands_from_fw(struct device *dev) > +{ > + struct panel_mipi_dbi_commands *commands; > + const struct firmware *fw; > + const char *compatible; > + struct property *prop; > + char fw_name[40]; > + int ret; > + > + of_property_for_each_string(dev->of_node, "compatible", prop, compatible) { > + snprintf(fw_name, sizeof(fw_name), "%s.bin", compatible); > + ret = firmware_request_nowarn(&fw, fw_name, dev); > + if (ret) { > + drm_dev_dbg(dev, DRM_UT_DRIVER, > + "No config file found for compatible: '%s' (error=%d)\n", > + compatible, ret); It would be more helpful to spell out that we failed to find a file named compatible.bin here as the user may not be aware that the .bin file is needed. > + continue; > + } > + > + commands = panel_mipi_dbi_check_commands(dev, fw); > + release_firmware(fw); > + return commands; > + } > + > + return NULL; > +} > + > +static void panel_mipi_dbi_commands_execute(struct mipi_dbi *dbi, > + struct panel_mipi_dbi_commands *commands) > +{ > + unsigned int i = 0; > + > + if (!commands) > + return; > + > + while (i < commands->len) { > + u8 command = commands->buf[i++]; > + u8 num_parameters = commands->buf[i++]; > + const u8 *parameters = &commands->buf[i]; > + > + if (command == 0x00 && num_parameters == 1) > + msleep(parameters[0]); > + else if (num_parameters) > + mipi_dbi_command_stackbuf(dbi, command, parameters, num_parameters); > + else > + mipi_dbi_command(dbi, command); > + > + i += num_parameters; > + } > +} > + > +static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe, > + struct drm_crtc_state *crtc_state, > + struct drm_plane_state *plane_state) > +{ > + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev); > + struct mipi_dbi *dbi = &dbidev->dbi; > + int ret, idx; > + > + if (!drm_dev_enter(pipe->crtc.dev, &idx)) > + return; > + > + drm_dbg(pipe->crtc.dev, "\n"); > + > + ret = mipi_dbi_poweron_conditional_reset(dbidev); > + if (ret < 0) > + goto out_exit; > + if (!ret) > + panel_mipi_dbi_commands_execute(dbi, dbidev->driver_private); > + > + mipi_dbi_enable_flush(dbidev, crtc_state, plane_state); > +out_exit: > + drm_dev_exit(idx); > +} > + > +static const struct drm_simple_display_pipe_funcs panel_mipi_dbi_pipe_funcs = { > + .enable = panel_mipi_dbi_enable, > + .disable = mipi_dbi_pipe_disable, > + .update = mipi_dbi_pipe_update, > +}; > + > +DEFINE_DRM_GEM_CMA_FOPS(panel_mipi_dbi_fops); > + > +static const struct drm_driver panel_mipi_dbi_driver = { > + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > + .fops = &panel_mipi_dbi_fops, > + DRM_GEM_CMA_DRIVER_OPS_VMAP, > + .debugfs_init = mipi_dbi_debugfs_init, > + .name = "panel-mipi-dbi", > + .desc = "MIPI DBI compatible display panel", > + .date = "20220103", > + .major = 1, > + .minor = 0, > +}; > + > +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) > +{ > + struct device *dev = dbidev->drm.dev; > + u32 width_mm = 0, height_mm = 0; > + struct display_timing timing; > + struct videomode vm; > + int ret; > + > + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); > + if (ret) { > + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); > + return ret; > + } > + > + videomode_from_timing(&timing, &vm); > + > + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || > + (vm.hback_porch + vm.hactive) > 0xffff || > + !vm.vactive || vm.vfront_porch || vm.vsync_len || > + (vm.vback_porch + vm.vactive) > 0xffff || > + vm.flags) { > + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > + return -EINVAL; > + } We should have a helper that implements this. Maybe the display_timing => display_mode helper could do it. > + > + /* The driver doesn't use the pixel clock but it is mandatory so fake one if not set */ > + if (!vm.pixelclock) > + vm.pixelclock = (vm.hback_porch + vm.hactive) * (vm.vback_porch + vm.vactive) * 60; > + > + dbidev->top_offset = vm.vback_porch; > + dbidev->left_offset = vm.hback_porch; > + > + memset(mode, 0, sizeof(*mode)); > + drm_display_mode_from_videomode(&vm, mode); > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > + > + ret = device_property_read_u32(dev, "width-mm", &width_mm); > + if (ret && ret != -EINVAL) > + return ret; > + > + ret = device_property_read_u32(dev, "height-mm", &height_mm); > + if (ret && ret != -EINVAL) > + return ret; > + > + mode->width_mm = width_mm; > + mode->height_mm = height_mm; > + > + drm_mode_debug_printmodeline(mode); > + > + return 0; > +} > + > +static int panel_mipi_dbi_spi_probe(struct spi_device *spi) > +{ > + struct device *dev = &spi->dev; > + struct drm_display_mode mode; > + struct mipi_dbi_dev *dbidev; > + struct drm_device *drm; > + struct mipi_dbi *dbi; > + struct gpio_desc *dc; > + int ret; > + > + dbidev = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver, struct mipi_dbi_dev, drm); > + if (IS_ERR(dbidev)) > + return PTR_ERR(dbidev); > + > + dbi = &dbidev->dbi; > + drm = &dbidev->drm; > + > + ret = panel_mipi_dbi_get_mode(dbidev, &mode); > + if (ret) > + return ret; > + > + dbidev->regulator = devm_regulator_get(dev, "power"); > + if (IS_ERR(dbidev->regulator)) > + return dev_err_probe(dev, PTR_ERR(dbidev->regulator), > + "Failed to get regulator 'power'\n"); > + > + dbidev->backlight = devm_of_find_backlight(dev); > + if (IS_ERR(dbidev->backlight)) > + return dev_err_probe(dev, PTR_ERR(dbidev->backlight), "Failed to get backlight\n"); > + > + dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(dbi->reset)) > + return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n"); > + > + dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); > + if (IS_ERR(dc)) > + return dev_err_probe(dev, PTR_ERR(dc), "Failed to get GPIO 'dc'\n"); > + > + ret = mipi_dbi_spi_init(spi, dbi, dc); > + if (ret) > + return ret; > + > + if (device_property_present(dev, "write-only")) > + dbi->read_commands = NULL; read_commands are unused - so the write-only property is in practice ignored. > + > + dbidev->driver_private = panel_mipi_dbi_commands_from_fw(dev); > + if (IS_ERR(dbidev->driver_private)) > + return PTR_ERR(dbidev->driver_private); > + > + ret = mipi_dbi_dev_init(dbidev, &panel_mipi_dbi_pipe_funcs, &mode, 0); > + if (ret) > + return ret; > + > + drm_mode_config_reset(drm); > + > + ret = drm_dev_register(drm, 0); > + if (ret) > + return ret; > + > + spi_set_drvdata(spi, drm); > + > + drm_fbdev_generic_setup(drm, 0); > + > + return 0; > +} > + > +static int panel_mipi_dbi_spi_remove(struct spi_device *spi) > +{ > + struct drm_device *drm = spi_get_drvdata(spi); > + > + drm_dev_unplug(drm); > + drm_atomic_helper_shutdown(drm); > + > + return 0; > +} > + > +static void panel_mipi_dbi_spi_shutdown(struct spi_device *spi) > +{ > + drm_atomic_helper_shutdown(spi_get_drvdata(spi)); > +} > + > +static int __maybe_unused panel_mipi_dbi_pm_suspend(struct device *dev) > +{ > + return drm_mode_config_helper_suspend(dev_get_drvdata(dev)); > +} > + > +static int __maybe_unused panel_mipi_dbi_pm_resume(struct device *dev) > +{ > + drm_mode_config_helper_resume(dev_get_drvdata(dev)); > + > + return 0; > +} > + > +static const struct dev_pm_ops panel_mipi_dbi_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(panel_mipi_dbi_pm_suspend, panel_mipi_dbi_pm_resume) > +}; > + > +static const struct of_device_id panel_mipi_dbi_spi_of_match[] = { > + { .compatible = "panel-mipi-dbi-spi" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, panel_mipi_dbi_spi_of_match); > + > +static const struct spi_device_id panel_mipi_dbi_spi_id[] = { > + { "panel-mipi-dbi-spi", 0 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(spi, panel_mipi_dbi_spi_id); > + > +static struct spi_driver panel_mipi_dbi_spi_driver = { > + .driver = { > + .name = "panel-mipi-dbi-spi", > + .owner = THIS_MODULE, > + .of_match_table = panel_mipi_dbi_spi_of_match, > + .pm = &panel_mipi_dbi_pm_ops, > + }, > + .id_table = panel_mipi_dbi_spi_id, > + .probe = panel_mipi_dbi_spi_probe, > + .remove = panel_mipi_dbi_spi_remove, > + .shutdown = panel_mipi_dbi_spi_shutdown, > +}; > +module_spi_driver(panel_mipi_dbi_spi_driver); > + > +MODULE_DESCRIPTION("MIPI DBI compatible display panel driver"); > +MODULE_AUTHOR("Noralf Trønnes"); > +MODULE_LICENSE("GPL"); > -- > 2.33.0 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-19 22:10 ` Sam Ravnborg 0 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-19 22:10 UTC (permalink / raw) To: Noralf Trønnes Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Hi Noralf, On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote: > Add a driver that will work with most MIPI DBI compatible SPI panels. > This avoids adding a driver for every new MIPI DBI compatible controller > that is to be used by Linux. The 'compatible' Device Tree property with > a '.bin' suffix will be used to load a firmware file that contains the > controller configuration. A solution where we have the command sequences as part of the DT-overlay is IMO a much better way to solve this: - The users needs to deal only with a single file, so there is less that goes wrong - The user need not reading special instructions how to handle a .bin file, if the overlay is present everything is fine - The file that contains the command sequences can be properly annotated with comments - The people that creates the command sequences has no need for a special script to create the file for a display - this is all readable ascii. - Using a DT-overlay the parsing of the DT-overlay can be done by well-tested functions, no need to invent something new to parse the file The idea with a common mipi DBI SPI driver is super, it is the detail with the .bin file that I am against. With the above said, a few comments to the current implementation below. As we know it from you - a very well-written driver. Sam > Acked-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > MAINTAINERS | 8 + > drivers/gpu/drm/tiny/Kconfig | 13 + > drivers/gpu/drm/tiny/Makefile | 1 + > drivers/gpu/drm/tiny/panel-mipi-dbi.c | 413 ++++++++++++++++++++++++++ > 4 files changed, 435 insertions(+) > create mode 100644 drivers/gpu/drm/tiny/panel-mipi-dbi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 8e6e892f99f0..3a0e57f23ad0 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6107,6 +6107,14 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > F: Documentation/devicetree/bindings/display/multi-inno,mi0283qt.txt > F: drivers/gpu/drm/tiny/mi0283qt.c > > +DRM DRIVER FOR MIPI DBI compatible panels > +M: Noralf Trønnes <noralf@tronnes.org> > +S: Maintained > +W: https://github.com/notro/panel-mipi-dbi/wiki Nice with a wiki for this, I can see this will grow over time and be a place to find how to support more panels. > +T: git git://anongit.freedesktop.org/drm/drm-misc > +F: Documentation/devicetree/bindings/display/panel/panel-mipi-dbi-spi.yaml > +F: drivers/gpu/drm/tiny/panel-mipi-dbi.c > + > DRM DRIVER FOR MSM ADRENO GPU > M: Rob Clark <robdclark@gmail.com> > M: Sean Paul <sean@poorly.run> > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig > index 712e0004e96e..d552e1618da7 100644 > --- a/drivers/gpu/drm/tiny/Kconfig > +++ b/drivers/gpu/drm/tiny/Kconfig > @@ -51,6 +51,19 @@ config DRM_GM12U320 > This is a KMS driver for projectors which use the GM12U320 chipset > for video transfer over USB2/3, such as the Acer C120 mini projector. > > +config DRM_PANEL_MIPI_DBI > + tristate "DRM support for MIPI DBI compatible panels" > + depends on DRM && SPI > + select DRM_KMS_HELPER > + select DRM_KMS_CMA_HELPER This symbol is not present in my drm-misc-next tree (which is a few weeks old, so it may be newer). > + select DRM_MIPI_DBI > + select BACKLIGHT_CLASS_DEVICE > + help > + Say Y here if you want to enable support for MIPI DBI compatible > + panels. The controller command setup can be provided using a > + firmware file. Consider adding a link to the wiki here - this may make it easier for the user to find it. > + To compile this driver as a module, choose M here. > + > config DRM_SIMPLEDRM > tristate "Simple framebuffer driver" > depends on DRM && MMU > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile > index 5d5505d40e7b..1d9d6227e7ab 100644 > --- a/drivers/gpu/drm/tiny/Makefile > +++ b/drivers/gpu/drm/tiny/Makefile > @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o > obj-$(CONFIG_DRM_BOCHS) += bochs.o > obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o > obj-$(CONFIG_DRM_GM12U320) += gm12u320.o > +obj-$(CONFIG_DRM_PANEL_MIPI_DBI) += panel-mipi-dbi.o > obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm.o > obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o > obj-$(CONFIG_TINYDRM_ILI9163) += ili9163.o > diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c > new file mode 100644 > index 000000000000..9240fdec38d6 > --- /dev/null > +++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c > @@ -0,0 +1,413 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * DRM driver for MIPI DBI compatible display panels > + * > + * Copyright 2022 Noralf Trønnes > + */ > + > +#include <linux/backlight.h> > +#include <linux/delay.h> > +#include <linux/firmware.h> > +#include <linux/gpio/consumer.h> > +#include <linux/module.h> > +#include <linux/property.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > + > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_drv.h> > +#include <drm/drm_fb_helper.h> > +#include <drm/drm_gem_atomic_helper.h> > +#include <drm/drm_gem_cma_helper.h> > +#include <drm/drm_managed.h> > +#include <drm/drm_mipi_dbi.h> > +#include <drm/drm_modeset_helper.h> > + > +#include <video/display_timing.h> > +#include <video/mipi_display.h> > +#include <video/of_display_timing.h> > +#include <video/videomode.h> videomode should not be used in new drivers, it is an fbdev artifact. But that said - we are still missing a direct display_timing => display_mode - so maybe we need it here. If it is needed Kconfig needs to be extended with: select VIDEOMODE_HELPERS > + > +static const u8 panel_mipi_dbi_magic[15] = { 'M', 'I', 'P', 'I', ' ', 'D', 'B', 'I', > + 0, 0, 0, 0, 0, 0, 0 }; > + > +/* > + * The optional display controller configuration is stored in a firmware file. > + * The Device Tree 'compatible' property value with a '.bin' suffix is passed > + * to request_firmware() to fetch this file. > + */ > +struct panel_mipi_dbi_config { > + /* Magic string: panel_mipi_dbi_magic */ > + u8 magic[15]; > + > + /* Config file format version */ > + u8 file_format_version; > + > + /* > + * MIPI commands to execute when the display pipeline is enabled. > + * This is used to configure the display controller. > + * > + * The commands are stored in a byte array with the format: > + * command, num_parameters, [ parameter, ...], command, ... > + * > + * Some commands require a pause before the next command can be received. > + * Inserting a delay in the command sequence is done by using the NOP command with one > + * parameter: delay in miliseconds (the No Operation command is part of the MIPI Display > + * Command Set where it has no parameters). > + * > + * Example: > + * command 0x11 > + * sleep 120ms > + * command 0xb1 parameters 0x01, 0x2c, 0x2d > + * command 0x29 > + * > + * Byte sequence: > + * 0x11 0x00 > + * 0x00 0x01 0x78 > + * 0xb1 0x03 0x01 0x2c 0x2d > + * 0x29 0x00 > + */ > + u8 commands[]; > +}; > + > +struct panel_mipi_dbi_commands { > + const u8 *buf; > + size_t len; > +}; > + > +static struct panel_mipi_dbi_commands * > +panel_mipi_dbi_check_commands(struct device *dev, const struct firmware *fw) > +{ > + const struct panel_mipi_dbi_config *config = (struct panel_mipi_dbi_config *)fw->data; > + struct panel_mipi_dbi_commands *commands; > + size_t size = fw->size, commands_len; > + unsigned int i = 0; > + > + if (size < sizeof(*config) + 2) { /* At least 1 command */ > + dev_err(dev, "config: file size=%zu is too small\n", size); > + return ERR_PTR(-EINVAL); > + } > + > + if (memcmp(config->magic, panel_mipi_dbi_magic, sizeof(config->magic))) { > + dev_err(dev, "config: Bad magic: %15ph\n", config->magic); > + return ERR_PTR(-EINVAL); > + } > + > + if (config->file_format_version != 1) { > + dev_err(dev, "config: version=%u is not supported\n", config->file_format_version); > + return ERR_PTR(-EINVAL); > + } > + > + drm_dev_dbg(dev, DRM_UT_DRIVER, "size=%zu version=%u\n", size, config->file_format_version); > + > + commands_len = size - sizeof(*config); > + > + while ((i + 1) < commands_len) { > + u8 command = config->commands[i++]; > + u8 num_parameters = config->commands[i++]; > + const u8 *parameters = &config->commands[i]; > + > + i += num_parameters; > + if (i > commands_len) { > + dev_err(dev, "config: command=0x%02x num_parameters=%u overflows\n", > + command, num_parameters); > + return ERR_PTR(-EINVAL); > + } > + > + if (command == 0x00 && num_parameters == 1) > + drm_dev_dbg(dev, DRM_UT_DRIVER, "sleep %ums\n", parameters[0]); > + else > + drm_dev_dbg(dev, DRM_UT_DRIVER, "command %02x %*ph\n", > + command, num_parameters, parameters); > + } > + > + if (i != commands_len) { > + dev_err(dev, "config: malformed command array\n"); > + return ERR_PTR(-EINVAL); > + } > + > + commands = devm_kzalloc(dev, sizeof(*commands), GFP_KERNEL); > + if (!commands) > + return ERR_PTR(-ENOMEM); > + > + commands->len = commands_len; > + commands->buf = devm_kmemdup(dev, config->commands, commands->len, GFP_KERNEL); > + if (!commands->buf) > + return ERR_PTR(-ENOMEM); > + > + return commands; > +} > + > +static struct panel_mipi_dbi_commands *panel_mipi_dbi_commands_from_fw(struct device *dev) > +{ > + struct panel_mipi_dbi_commands *commands; > + const struct firmware *fw; > + const char *compatible; > + struct property *prop; > + char fw_name[40]; > + int ret; > + > + of_property_for_each_string(dev->of_node, "compatible", prop, compatible) { > + snprintf(fw_name, sizeof(fw_name), "%s.bin", compatible); > + ret = firmware_request_nowarn(&fw, fw_name, dev); > + if (ret) { > + drm_dev_dbg(dev, DRM_UT_DRIVER, > + "No config file found for compatible: '%s' (error=%d)\n", > + compatible, ret); It would be more helpful to spell out that we failed to find a file named compatible.bin here as the user may not be aware that the .bin file is needed. > + continue; > + } > + > + commands = panel_mipi_dbi_check_commands(dev, fw); > + release_firmware(fw); > + return commands; > + } > + > + return NULL; > +} > + > +static void panel_mipi_dbi_commands_execute(struct mipi_dbi *dbi, > + struct panel_mipi_dbi_commands *commands) > +{ > + unsigned int i = 0; > + > + if (!commands) > + return; > + > + while (i < commands->len) { > + u8 command = commands->buf[i++]; > + u8 num_parameters = commands->buf[i++]; > + const u8 *parameters = &commands->buf[i]; > + > + if (command == 0x00 && num_parameters == 1) > + msleep(parameters[0]); > + else if (num_parameters) > + mipi_dbi_command_stackbuf(dbi, command, parameters, num_parameters); > + else > + mipi_dbi_command(dbi, command); > + > + i += num_parameters; > + } > +} > + > +static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe, > + struct drm_crtc_state *crtc_state, > + struct drm_plane_state *plane_state) > +{ > + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev); > + struct mipi_dbi *dbi = &dbidev->dbi; > + int ret, idx; > + > + if (!drm_dev_enter(pipe->crtc.dev, &idx)) > + return; > + > + drm_dbg(pipe->crtc.dev, "\n"); > + > + ret = mipi_dbi_poweron_conditional_reset(dbidev); > + if (ret < 0) > + goto out_exit; > + if (!ret) > + panel_mipi_dbi_commands_execute(dbi, dbidev->driver_private); > + > + mipi_dbi_enable_flush(dbidev, crtc_state, plane_state); > +out_exit: > + drm_dev_exit(idx); > +} > + > +static const struct drm_simple_display_pipe_funcs panel_mipi_dbi_pipe_funcs = { > + .enable = panel_mipi_dbi_enable, > + .disable = mipi_dbi_pipe_disable, > + .update = mipi_dbi_pipe_update, > +}; > + > +DEFINE_DRM_GEM_CMA_FOPS(panel_mipi_dbi_fops); > + > +static const struct drm_driver panel_mipi_dbi_driver = { > + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > + .fops = &panel_mipi_dbi_fops, > + DRM_GEM_CMA_DRIVER_OPS_VMAP, > + .debugfs_init = mipi_dbi_debugfs_init, > + .name = "panel-mipi-dbi", > + .desc = "MIPI DBI compatible display panel", > + .date = "20220103", > + .major = 1, > + .minor = 0, > +}; > + > +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) > +{ > + struct device *dev = dbidev->drm.dev; > + u32 width_mm = 0, height_mm = 0; > + struct display_timing timing; > + struct videomode vm; > + int ret; > + > + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); > + if (ret) { > + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); > + return ret; > + } > + > + videomode_from_timing(&timing, &vm); > + > + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || > + (vm.hback_porch + vm.hactive) > 0xffff || > + !vm.vactive || vm.vfront_porch || vm.vsync_len || > + (vm.vback_porch + vm.vactive) > 0xffff || > + vm.flags) { > + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > + return -EINVAL; > + } We should have a helper that implements this. Maybe the display_timing => display_mode helper could do it. > + > + /* The driver doesn't use the pixel clock but it is mandatory so fake one if not set */ > + if (!vm.pixelclock) > + vm.pixelclock = (vm.hback_porch + vm.hactive) * (vm.vback_porch + vm.vactive) * 60; > + > + dbidev->top_offset = vm.vback_porch; > + dbidev->left_offset = vm.hback_porch; > + > + memset(mode, 0, sizeof(*mode)); > + drm_display_mode_from_videomode(&vm, mode); > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > + > + ret = device_property_read_u32(dev, "width-mm", &width_mm); > + if (ret && ret != -EINVAL) > + return ret; > + > + ret = device_property_read_u32(dev, "height-mm", &height_mm); > + if (ret && ret != -EINVAL) > + return ret; > + > + mode->width_mm = width_mm; > + mode->height_mm = height_mm; > + > + drm_mode_debug_printmodeline(mode); > + > + return 0; > +} > + > +static int panel_mipi_dbi_spi_probe(struct spi_device *spi) > +{ > + struct device *dev = &spi->dev; > + struct drm_display_mode mode; > + struct mipi_dbi_dev *dbidev; > + struct drm_device *drm; > + struct mipi_dbi *dbi; > + struct gpio_desc *dc; > + int ret; > + > + dbidev = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver, struct mipi_dbi_dev, drm); > + if (IS_ERR(dbidev)) > + return PTR_ERR(dbidev); > + > + dbi = &dbidev->dbi; > + drm = &dbidev->drm; > + > + ret = panel_mipi_dbi_get_mode(dbidev, &mode); > + if (ret) > + return ret; > + > + dbidev->regulator = devm_regulator_get(dev, "power"); > + if (IS_ERR(dbidev->regulator)) > + return dev_err_probe(dev, PTR_ERR(dbidev->regulator), > + "Failed to get regulator 'power'\n"); > + > + dbidev->backlight = devm_of_find_backlight(dev); > + if (IS_ERR(dbidev->backlight)) > + return dev_err_probe(dev, PTR_ERR(dbidev->backlight), "Failed to get backlight\n"); > + > + dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(dbi->reset)) > + return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n"); > + > + dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); > + if (IS_ERR(dc)) > + return dev_err_probe(dev, PTR_ERR(dc), "Failed to get GPIO 'dc'\n"); > + > + ret = mipi_dbi_spi_init(spi, dbi, dc); > + if (ret) > + return ret; > + > + if (device_property_present(dev, "write-only")) > + dbi->read_commands = NULL; read_commands are unused - so the write-only property is in practice ignored. > + > + dbidev->driver_private = panel_mipi_dbi_commands_from_fw(dev); > + if (IS_ERR(dbidev->driver_private)) > + return PTR_ERR(dbidev->driver_private); > + > + ret = mipi_dbi_dev_init(dbidev, &panel_mipi_dbi_pipe_funcs, &mode, 0); > + if (ret) > + return ret; > + > + drm_mode_config_reset(drm); > + > + ret = drm_dev_register(drm, 0); > + if (ret) > + return ret; > + > + spi_set_drvdata(spi, drm); > + > + drm_fbdev_generic_setup(drm, 0); > + > + return 0; > +} > + > +static int panel_mipi_dbi_spi_remove(struct spi_device *spi) > +{ > + struct drm_device *drm = spi_get_drvdata(spi); > + > + drm_dev_unplug(drm); > + drm_atomic_helper_shutdown(drm); > + > + return 0; > +} > + > +static void panel_mipi_dbi_spi_shutdown(struct spi_device *spi) > +{ > + drm_atomic_helper_shutdown(spi_get_drvdata(spi)); > +} > + > +static int __maybe_unused panel_mipi_dbi_pm_suspend(struct device *dev) > +{ > + return drm_mode_config_helper_suspend(dev_get_drvdata(dev)); > +} > + > +static int __maybe_unused panel_mipi_dbi_pm_resume(struct device *dev) > +{ > + drm_mode_config_helper_resume(dev_get_drvdata(dev)); > + > + return 0; > +} > + > +static const struct dev_pm_ops panel_mipi_dbi_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(panel_mipi_dbi_pm_suspend, panel_mipi_dbi_pm_resume) > +}; > + > +static const struct of_device_id panel_mipi_dbi_spi_of_match[] = { > + { .compatible = "panel-mipi-dbi-spi" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, panel_mipi_dbi_spi_of_match); > + > +static const struct spi_device_id panel_mipi_dbi_spi_id[] = { > + { "panel-mipi-dbi-spi", 0 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(spi, panel_mipi_dbi_spi_id); > + > +static struct spi_driver panel_mipi_dbi_spi_driver = { > + .driver = { > + .name = "panel-mipi-dbi-spi", > + .owner = THIS_MODULE, > + .of_match_table = panel_mipi_dbi_spi_of_match, > + .pm = &panel_mipi_dbi_pm_ops, > + }, > + .id_table = panel_mipi_dbi_spi_id, > + .probe = panel_mipi_dbi_spi_probe, > + .remove = panel_mipi_dbi_spi_remove, > + .shutdown = panel_mipi_dbi_spi_shutdown, > +}; > +module_spi_driver(panel_mipi_dbi_spi_driver); > + > +MODULE_DESCRIPTION("MIPI DBI compatible display panel driver"); > +MODULE_AUTHOR("Noralf Trønnes"); > +MODULE_LICENSE("GPL"); > -- > 2.33.0 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-19 22:10 ` Sam Ravnborg (?) @ 2022-02-20 10:04 ` Sam Ravnborg 2022-02-20 14:19 ` Noralf Trønnes -1 siblings, 1 reply; 36+ messages in thread From: Sam Ravnborg @ 2022-02-20 10:04 UTC (permalink / raw) To: Noralf Trønnes Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Hi Noralf, > > +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) > > +{ > > + struct device *dev = dbidev->drm.dev; > > + u32 width_mm = 0, height_mm = 0; > > + struct display_timing timing; > > + struct videomode vm; > > + int ret; > > + > > + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); > > + if (ret) { > > + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); > > + return ret; > > + } > > + > > + videomode_from_timing(&timing, &vm); > > + > > + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || > > + (vm.hback_porch + vm.hactive) > 0xffff || > > + !vm.vactive || vm.vfront_porch || vm.vsync_len || > > + (vm.vback_porch + vm.vactive) > 0xffff || > > + vm.flags) { > > + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > > + return -EINVAL; > > + } > We should have a helper that implements this. Maybe the display_timing > => display_mode helper could do it. It would be nice with a drm_display_timing_to_mode() but that can come later - the comment above should not be understood that I consider it mandatory for this driver. Sam ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-20 10:04 ` Sam Ravnborg @ 2022-02-20 14:19 ` Noralf Trønnes 2022-02-20 18:11 ` Noralf Trønnes 0 siblings, 1 reply; 36+ messages in thread From: Noralf Trønnes @ 2022-02-20 14:19 UTC (permalink / raw) To: Sam Ravnborg Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Den 20.02.2022 11.04, skrev Sam Ravnborg: > Hi Noralf, > >>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) >>> +{ >>> + struct device *dev = dbidev->drm.dev; >>> + u32 width_mm = 0, height_mm = 0; >>> + struct display_timing timing; >>> + struct videomode vm; >>> + int ret; >>> + >>> + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); >>> + if (ret) { >>> + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); >>> + return ret; >>> + } >>> + >>> + videomode_from_timing(&timing, &vm); >>> + >>> + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || >>> + (vm.hback_porch + vm.hactive) > 0xffff || >>> + !vm.vactive || vm.vfront_porch || vm.vsync_len || >>> + (vm.vback_porch + vm.vactive) > 0xffff || >>> + vm.flags) { >>> + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); >>> + return -EINVAL; >>> + } >> We should have a helper that implements this. Maybe the display_timing >> => display_mode helper could do it. > > It would be nice with a drm_display_timing_to_mode() but that can come > later - the comment above should not be understood that I consider it > mandatory for this driver. > I did consider adding an of_get_drm_panel_mode() fashioned after of_get_drm_display_mode() but I didn't find any other driver that would actually be able to use it and I would have to do some substraction to get back the {h,v}front_porch values that I need and the optional pixel clock calculation becomes more complex acting from a drm_display_mode so I decided against it. Looking at it now, what I could do is add a function like what of_get_videomode() does for "display-timings": /** * of_get_panel_videomode - get the panel-timing videomode from devicetree * @np: devicenode containing the panel-timing subnode * @vm: returns the videomode * * Returns: * Zero on success, negative error code on failure. **/ int of_get_panel_videomode(struct device_node *np, struct videomode *vm) { struct display_timing timing; int ret; ret = of_get_display_timing(np, "panel-timing", &timing); if (ret) return ret; videomode_from_timing(&timing, vm); return 0; } This could also be used by panel-lvds and 2 fbdev drivers, the other panel-timing users need/use the display_timing itself, some for bounds checking. Noralf. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-20 14:19 ` Noralf Trønnes @ 2022-02-20 18:11 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-20 18:11 UTC (permalink / raw) To: noralf Cc: dave.stevenson, david, devicetree, dri-devel, maxime, robh+dt, sam, thierry.reding > Den 20.02.2022 11.04, skrev Sam Ravnborg: > > Hi Noralf, > > > >>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) > >>> +{ > >>> + struct device *dev = dbidev->drm.dev; > >>> + u32 width_mm = 0, height_mm = 0; > >>> + struct display_timing timing; > >>> + struct videomode vm; > >>> + int ret; > >>> + > >>> + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); > >>> + if (ret) { > >>> + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); > >>> + return ret; > >>> + } > >>> + > >>> + videomode_from_timing(&timing, &vm); > >>> + > >>> + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || > >>> + (vm.hback_porch + vm.hactive) > 0xffff || > >>> + !vm.vactive || vm.vfront_porch || vm.vsync_len || > >>> + (vm.vback_porch + vm.vactive) > 0xffff || > >>> + vm.flags) { > >>> + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > >>> + return -EINVAL; > >>> + } > >> We should have a helper that implements this. Maybe the display_timing > >> => display_mode helper could do it. > > > > It would be nice with a drm_display_timing_to_mode() but that can come > > later - the comment above should not be understood that I consider it > > mandatory for this driver. > > > > I did consider adding an of_get_drm_panel_mode() fashioned after > of_get_drm_display_mode() but I didn't find any other driver that would > actually be able to use it and I would have to do some substraction to > get back the {h,v}front_porch values that I need and the optional pixel > clock calculation becomes more complex acting from a drm_display_mode so > I decided against it. > > Looking at it now, what I could do is add a function like what > of_get_videomode() does for "display-timings": > > /** > * of_get_panel_videomode - get the panel-timing videomode from devicetree > * @np: devicenode containing the panel-timing subnode > * @vm: returns the videomode > * > * Returns: > * Zero on success, negative error code on failure. > **/ > int of_get_panel_videomode(struct device_node *np, struct videomode *vm) > { > struct display_timing timing; > int ret; > > ret = of_get_display_timing(np, "panel-timing", &timing); > if (ret) > return ret; > > videomode_from_timing(&timing, vm); > > return 0; > } > > This could also be used by panel-lvds and 2 fbdev drivers, the other > panel-timing users need/use the display_timing itself, some for bounds > checking. Scratch that, since videomode is to be avoided I tried adding a drm_display_mode function and it didn't complicate matter as I though it would so I'll do that instead: static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) { struct device *dev = dbidev->drm.dev; u32 width_mm = 0, height_mm = 0; u16 hback_porch, vback_porch; struct videomode vm; int ret; ret = of_get_drm_panel_display_mode(dev->of_node, mode, NULL); if (ret) { dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); return ret; } mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; hback_porch = mode->htotal - mode->hsync_end; vback_porch = mode->vtotal - mode->vsync_end; if (mode->hsync_end > mode->hdisplay || (hback_porch + mode->hdisplay) > 0xffff || mode->vsync_end > mode->vdisplay || (vback_porch + mode->vdisplay) > 0xffff || mode->flags) { dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); return -EINVAL; } /* The driver doesn't use the pixel clock but it is mandatory so fake one if not set */ if (!mode->pixelclock) mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; dbidev->top_offset = vback_porch; dbidev->left_offset = hback_porch; return 0; } int of_get_drm_panel_display_mode(struct device_node *np, struct drm_display_mode *dmode, u32 *bus_flags) { u32 width_mm = 0, height_mm = 0; struct display_timing timing; struct videomode vm; int ret; ret = of_get_display_timing(np, "panel-timing", &timing); if (ret) return ret; videomode_from_timing(&timing, vm); memset(dmode, 0, sizeof(*dmode)); drm_display_mode_from_videomode(&vm, dmode); if (bus_flags) drm_bus_flags_from_videomode(&vm, bus_flags); ret = of_property_read_u32(np, "width-mm", &width_mm); if (ret && ret != -EINVAL) return ret; ret = of_property_read_u32(np, "height-mm", &height_mm); if (ret && ret != -EINVAL) return ret; mode->width_mm = width_mm; mode->height_mm = height_mm; drm_mode_debug_printmodeline(dmode); return 0; } EXPORT_SYMBOL_GPL(of_get_drm_display_mode); Noralf. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-20 18:11 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-20 18:11 UTC (permalink / raw) To: noralf Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime, sam > Den 20.02.2022 11.04, skrev Sam Ravnborg: > > Hi Noralf, > > > >>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) > >>> +{ > >>> + struct device *dev = dbidev->drm.dev; > >>> + u32 width_mm = 0, height_mm = 0; > >>> + struct display_timing timing; > >>> + struct videomode vm; > >>> + int ret; > >>> + > >>> + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); > >>> + if (ret) { > >>> + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); > >>> + return ret; > >>> + } > >>> + > >>> + videomode_from_timing(&timing, &vm); > >>> + > >>> + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || > >>> + (vm.hback_porch + vm.hactive) > 0xffff || > >>> + !vm.vactive || vm.vfront_porch || vm.vsync_len || > >>> + (vm.vback_porch + vm.vactive) > 0xffff || > >>> + vm.flags) { > >>> + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > >>> + return -EINVAL; > >>> + } > >> We should have a helper that implements this. Maybe the display_timing > >> => display_mode helper could do it. > > > > It would be nice with a drm_display_timing_to_mode() but that can come > > later - the comment above should not be understood that I consider it > > mandatory for this driver. > > > > I did consider adding an of_get_drm_panel_mode() fashioned after > of_get_drm_display_mode() but I didn't find any other driver that would > actually be able to use it and I would have to do some substraction to > get back the {h,v}front_porch values that I need and the optional pixel > clock calculation becomes more complex acting from a drm_display_mode so > I decided against it. > > Looking at it now, what I could do is add a function like what > of_get_videomode() does for "display-timings": > > /** > * of_get_panel_videomode - get the panel-timing videomode from devicetree > * @np: devicenode containing the panel-timing subnode > * @vm: returns the videomode > * > * Returns: > * Zero on success, negative error code on failure. > **/ > int of_get_panel_videomode(struct device_node *np, struct videomode *vm) > { > struct display_timing timing; > int ret; > > ret = of_get_display_timing(np, "panel-timing", &timing); > if (ret) > return ret; > > videomode_from_timing(&timing, vm); > > return 0; > } > > This could also be used by panel-lvds and 2 fbdev drivers, the other > panel-timing users need/use the display_timing itself, some for bounds > checking. Scratch that, since videomode is to be avoided I tried adding a drm_display_mode function and it didn't complicate matter as I though it would so I'll do that instead: static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode) { struct device *dev = dbidev->drm.dev; u32 width_mm = 0, height_mm = 0; u16 hback_porch, vback_porch; struct videomode vm; int ret; ret = of_get_drm_panel_display_mode(dev->of_node, mode, NULL); if (ret) { dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret); return ret; } mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; hback_porch = mode->htotal - mode->hsync_end; vback_porch = mode->vtotal - mode->vsync_end; if (mode->hsync_end > mode->hdisplay || (hback_porch + mode->hdisplay) > 0xffff || mode->vsync_end > mode->vdisplay || (vback_porch + mode->vdisplay) > 0xffff || mode->flags) { dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); return -EINVAL; } /* The driver doesn't use the pixel clock but it is mandatory so fake one if not set */ if (!mode->pixelclock) mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; dbidev->top_offset = vback_porch; dbidev->left_offset = hback_porch; return 0; } int of_get_drm_panel_display_mode(struct device_node *np, struct drm_display_mode *dmode, u32 *bus_flags) { u32 width_mm = 0, height_mm = 0; struct display_timing timing; struct videomode vm; int ret; ret = of_get_display_timing(np, "panel-timing", &timing); if (ret) return ret; videomode_from_timing(&timing, vm); memset(dmode, 0, sizeof(*dmode)); drm_display_mode_from_videomode(&vm, dmode); if (bus_flags) drm_bus_flags_from_videomode(&vm, bus_flags); ret = of_property_read_u32(np, "width-mm", &width_mm); if (ret && ret != -EINVAL) return ret; ret = of_property_read_u32(np, "height-mm", &height_mm); if (ret && ret != -EINVAL) return ret; mode->width_mm = width_mm; mode->height_mm = height_mm; drm_mode_debug_printmodeline(dmode); return 0; } EXPORT_SYMBOL_GPL(of_get_drm_display_mode); Noralf. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-20 18:11 ` Noralf Trønnes @ 2022-02-20 19:57 ` Sam Ravnborg -1 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-20 19:57 UTC (permalink / raw) To: Noralf Trønnes Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Hi Noralf. On Sun, Feb 20, 2022 at 07:11:14PM +0100, Noralf Trønnes wrote: > > Den 20.02.2022 11.04, skrev Sam Ravnborg: > > > Hi Noralf, > > > > > >>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, > struct drm_display_mode *mode) > > >>> +{ > > >>> + struct device *dev = dbidev->drm.dev; > > >>> + u32 width_mm = 0, height_mm = 0; > > >>> + struct display_timing timing; > > >>> + struct videomode vm; > > >>> + int ret; > > >>> + > > >>> + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); > > >>> + if (ret) { > > >>> + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", > dev->of_node, ret); > > >>> + return ret; > > >>> + } > > >>> + > > >>> + videomode_from_timing(&timing, &vm); > > >>> + > > >>> + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || > > >>> + (vm.hback_porch + vm.hactive) > 0xffff || > > >>> + !vm.vactive || vm.vfront_porch || vm.vsync_len || > > >>> + (vm.vback_porch + vm.vactive) > 0xffff || > > >>> + vm.flags) { > > >>> + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > > >>> + return -EINVAL; > > >>> + } > > >> We should have a helper that implements this. Maybe the display_timing > > >> => display_mode helper could do it. > > > > > > It would be nice with a drm_display_timing_to_mode() but that can come > > > later - the comment above should not be understood that I consider it > > > mandatory for this driver. > > > > > > > I did consider adding an of_get_drm_panel_mode() fashioned after > > of_get_drm_display_mode() but I didn't find any other driver that would > > actually be able to use it and I would have to do some substraction to > > get back the {h,v}front_porch values that I need and the optional pixel > > clock calculation becomes more complex acting from a drm_display_mode so > > I decided against it. > > > > Looking at it now, what I could do is add a function like what > > of_get_videomode() does for "display-timings": > > > > /** > > * of_get_panel_videomode - get the panel-timing videomode from devicetree > > * @np: devicenode containing the panel-timing subnode > > * @vm: returns the videomode > > * > > * Returns: > > * Zero on success, negative error code on failure. > > **/ > > int of_get_panel_videomode(struct device_node *np, struct videomode *vm) > > { > > struct display_timing timing; > > int ret; > > > > ret = of_get_display_timing(np, "panel-timing", &timing); > > if (ret) > > return ret; > > > > videomode_from_timing(&timing, vm); > > > > return 0; > > } > > > > This could also be used by panel-lvds and 2 fbdev drivers, the other > > panel-timing users need/use the display_timing itself, some for bounds > > checking. > > Scratch that, since videomode is to be avoided I tried adding a > drm_display_mode function and it didn't complicate matter as I though it > would so I'll do that instead: I like, but would like to get rid of video_mode entirely. > > static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct > drm_display_mode *mode) > { > struct device *dev = dbidev->drm.dev; > u32 width_mm = 0, height_mm = 0; > u16 hback_porch, vback_porch; > struct videomode vm; > int ret; > > ret = of_get_drm_panel_display_mode(dev->of_node, mode, NULL); > if (ret) { > dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", > dev->of_node, ret); > return ret; > } > > mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > > hback_porch = mode->htotal - mode->hsync_end; > vback_porch = mode->vtotal - mode->vsync_end; The accesss functions I posed below could be used here - so we avoid typing these (trivial) calculations in many places. > > if (mode->hsync_end > mode->hdisplay || (hback_porch + mode->hdisplay) > > 0xffff || > mode->vsync_end > mode->vdisplay || (vback_porch + mode->vdisplay) > > 0xffff || > mode->flags) { > dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > return -EINVAL; > } With the display_timing => drm_display_mode I think the above is no longer required. > > /* The driver doesn't use the pixel clock but it is mandatory so fake > one if not set */ > if (!mode->pixelclock) > mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; > > dbidev->top_offset = vback_porch; > dbidev->left_offset = hback_porch; > > return 0; > } > > > int of_get_drm_panel_display_mode(struct device_node *np, > struct drm_display_mode *dmode, u32 *bus_flags) > { Not sure about the bus_flags argument here - seems misplaced. > u32 width_mm = 0, height_mm = 0; > struct display_timing timing; > struct videomode vm; > int ret; > > ret = of_get_display_timing(np, "panel-timing", &timing); > if (ret) > return ret; > > videomode_from_timing(&timing, vm); > > memset(dmode, 0, sizeof(*dmode)); > drm_display_mode_from_videomode(&vm, dmode); > if (bus_flags) > drm_bus_flags_from_videomode(&vm, bus_flags); This does a: display_timing => video_mode => drm_display_display_mode We could do a: display_timing => drm_display_mode. Sample implementation could look like this: void drm_mode_from_display_timing(struct drm_display_mode *mode, const struct display_timing *dt) { mode->hdisplay = dt->hactive.typ; mode->hsync_start = dt->hactive.typ + dt->hfront_porch.typ; mode->hsync_end = dt->hactive.typ + dt->hfront_porch.typ + dt->hsync_len.typ; mode->htotal = dt->hactive.typ + dt->hfront_porch.typ + dt->hsync_len.typ + dt->hback_porch.typ; mode->vdisplay = dt->vactive.typ; mode->vsync_start = dt->vactive.typ + dt->vfront_porch.typ; mode->vsync_end = dt->vactive.typ + dt->vfront_porch.typ + dt->vsync_len.typ; mode->vtotal = dt->vactive.typ + dt->vfront_porch.typ + dt->vsync_len.typ + dt->vback_porch.typ; mode->clock = dt->pixelclock.typ / 1000; mode->flags = 0; if (dt->flags & DISPLAY_FLAGS_HSYNC_HIGH) mode->flags |= DRM_MODE_FLAG_PHSYNC; else if (dt->flags & DISPLAY_FLAGS_HSYNC_LOW) mode->flags |= DRM_MODE_FLAG_NHSYNC; if (dt->flags & DISPLAY_FLAGS_VSYNC_HIGH) mode->flags |= DRM_MODE_FLAG_PVSYNC; else if (dt->flags & DISPLAY_FLAGS_VSYNC_LOW) mode->flags |= DRM_MODE_FLAG_NVSYNC; if (dt->flags & DISPLAY_FLAGS_INTERLACED) mode->flags |= DRM_MODE_FLAG_INTERLACE; if (dt->flags & DISPLAY_FLAGS_DOUBLESCAN) mode->flags |= DRM_MODE_FLAG_DBLSCAN; if (dt->flags & DISPLAY_FLAGS_DOUBLECLK) mode->flags |= DRM_MODE_FLAG_DBLCLK; drm_mode_set_name(mode); } EXPORT_SYMBOL_GPL(drm_mode_from_display_timing); If we then on top of this would like easy access to the names we know we could add a few access functions: static inline u32 drm_mode_hactive(const struct drm_display_mode *mode) { mode->hdisplay; } static inline u32 drm_mode_hfront_porch(const struct drm_display_mode *mode) { mode->hsync_start - mode->hdisplay; } static inline u32 drm_mode_hback_porch(const struct drm_display_mode *mode) { mode->htotal - mode->hsync_start; } static inline u32 drm_mode_hsync_len(const struct drm_display_mode *mode) { return mode->hsync_end - mode->hsync_start; } static inline u32 drm_mode_vactive(const struct drm_display_mode *mode) { return mode->vdisplay; } static inline u32 drm_mode_vfront_porch(const struct drm_display_mode *mode) { return mode->vsync_start - mode->vdisplay; } static inline u32 drm_mode_vsync_len(const struct drm_display_mode *mode) { return mode->vsync_end - mode->vsync_start; } static inline u32 drm_mode_vback_porch(const struct drm_display_mode *mode) { return mode->vtotal - mode->vsync_end; } The above was something I just quickly typed. This was the easy part. Writing kernel-.doc and fix it so it works is the time consuming part.. Sam ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-20 19:57 ` Sam Ravnborg 0 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-20 19:57 UTC (permalink / raw) To: Noralf Trønnes Cc: dave.stevenson, david, devicetree, dri-devel, maxime, robh+dt, thierry.reding Hi Noralf. On Sun, Feb 20, 2022 at 07:11:14PM +0100, Noralf Trønnes wrote: > > Den 20.02.2022 11.04, skrev Sam Ravnborg: > > > Hi Noralf, > > > > > >>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, > struct drm_display_mode *mode) > > >>> +{ > > >>> + struct device *dev = dbidev->drm.dev; > > >>> + u32 width_mm = 0, height_mm = 0; > > >>> + struct display_timing timing; > > >>> + struct videomode vm; > > >>> + int ret; > > >>> + > > >>> + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); > > >>> + if (ret) { > > >>> + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", > dev->of_node, ret); > > >>> + return ret; > > >>> + } > > >>> + > > >>> + videomode_from_timing(&timing, &vm); > > >>> + > > >>> + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || > > >>> + (vm.hback_porch + vm.hactive) > 0xffff || > > >>> + !vm.vactive || vm.vfront_porch || vm.vsync_len || > > >>> + (vm.vback_porch + vm.vactive) > 0xffff || > > >>> + vm.flags) { > > >>> + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > > >>> + return -EINVAL; > > >>> + } > > >> We should have a helper that implements this. Maybe the display_timing > > >> => display_mode helper could do it. > > > > > > It would be nice with a drm_display_timing_to_mode() but that can come > > > later - the comment above should not be understood that I consider it > > > mandatory for this driver. > > > > > > > I did consider adding an of_get_drm_panel_mode() fashioned after > > of_get_drm_display_mode() but I didn't find any other driver that would > > actually be able to use it and I would have to do some substraction to > > get back the {h,v}front_porch values that I need and the optional pixel > > clock calculation becomes more complex acting from a drm_display_mode so > > I decided against it. > > > > Looking at it now, what I could do is add a function like what > > of_get_videomode() does for "display-timings": > > > > /** > > * of_get_panel_videomode - get the panel-timing videomode from devicetree > > * @np: devicenode containing the panel-timing subnode > > * @vm: returns the videomode > > * > > * Returns: > > * Zero on success, negative error code on failure. > > **/ > > int of_get_panel_videomode(struct device_node *np, struct videomode *vm) > > { > > struct display_timing timing; > > int ret; > > > > ret = of_get_display_timing(np, "panel-timing", &timing); > > if (ret) > > return ret; > > > > videomode_from_timing(&timing, vm); > > > > return 0; > > } > > > > This could also be used by panel-lvds and 2 fbdev drivers, the other > > panel-timing users need/use the display_timing itself, some for bounds > > checking. > > Scratch that, since videomode is to be avoided I tried adding a > drm_display_mode function and it didn't complicate matter as I though it > would so I'll do that instead: I like, but would like to get rid of video_mode entirely. > > static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct > drm_display_mode *mode) > { > struct device *dev = dbidev->drm.dev; > u32 width_mm = 0, height_mm = 0; > u16 hback_porch, vback_porch; > struct videomode vm; > int ret; > > ret = of_get_drm_panel_display_mode(dev->of_node, mode, NULL); > if (ret) { > dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", > dev->of_node, ret); > return ret; > } > > mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > > hback_porch = mode->htotal - mode->hsync_end; > vback_porch = mode->vtotal - mode->vsync_end; The accesss functions I posed below could be used here - so we avoid typing these (trivial) calculations in many places. > > if (mode->hsync_end > mode->hdisplay || (hback_porch + mode->hdisplay) > > 0xffff || > mode->vsync_end > mode->vdisplay || (vback_porch + mode->vdisplay) > > 0xffff || > mode->flags) { > dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > return -EINVAL; > } With the display_timing => drm_display_mode I think the above is no longer required. > > /* The driver doesn't use the pixel clock but it is mandatory so fake > one if not set */ > if (!mode->pixelclock) > mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; > > dbidev->top_offset = vback_porch; > dbidev->left_offset = hback_porch; > > return 0; > } > > > int of_get_drm_panel_display_mode(struct device_node *np, > struct drm_display_mode *dmode, u32 *bus_flags) > { Not sure about the bus_flags argument here - seems misplaced. > u32 width_mm = 0, height_mm = 0; > struct display_timing timing; > struct videomode vm; > int ret; > > ret = of_get_display_timing(np, "panel-timing", &timing); > if (ret) > return ret; > > videomode_from_timing(&timing, vm); > > memset(dmode, 0, sizeof(*dmode)); > drm_display_mode_from_videomode(&vm, dmode); > if (bus_flags) > drm_bus_flags_from_videomode(&vm, bus_flags); This does a: display_timing => video_mode => drm_display_display_mode We could do a: display_timing => drm_display_mode. Sample implementation could look like this: void drm_mode_from_display_timing(struct drm_display_mode *mode, const struct display_timing *dt) { mode->hdisplay = dt->hactive.typ; mode->hsync_start = dt->hactive.typ + dt->hfront_porch.typ; mode->hsync_end = dt->hactive.typ + dt->hfront_porch.typ + dt->hsync_len.typ; mode->htotal = dt->hactive.typ + dt->hfront_porch.typ + dt->hsync_len.typ + dt->hback_porch.typ; mode->vdisplay = dt->vactive.typ; mode->vsync_start = dt->vactive.typ + dt->vfront_porch.typ; mode->vsync_end = dt->vactive.typ + dt->vfront_porch.typ + dt->vsync_len.typ; mode->vtotal = dt->vactive.typ + dt->vfront_porch.typ + dt->vsync_len.typ + dt->vback_porch.typ; mode->clock = dt->pixelclock.typ / 1000; mode->flags = 0; if (dt->flags & DISPLAY_FLAGS_HSYNC_HIGH) mode->flags |= DRM_MODE_FLAG_PHSYNC; else if (dt->flags & DISPLAY_FLAGS_HSYNC_LOW) mode->flags |= DRM_MODE_FLAG_NHSYNC; if (dt->flags & DISPLAY_FLAGS_VSYNC_HIGH) mode->flags |= DRM_MODE_FLAG_PVSYNC; else if (dt->flags & DISPLAY_FLAGS_VSYNC_LOW) mode->flags |= DRM_MODE_FLAG_NVSYNC; if (dt->flags & DISPLAY_FLAGS_INTERLACED) mode->flags |= DRM_MODE_FLAG_INTERLACE; if (dt->flags & DISPLAY_FLAGS_DOUBLESCAN) mode->flags |= DRM_MODE_FLAG_DBLSCAN; if (dt->flags & DISPLAY_FLAGS_DOUBLECLK) mode->flags |= DRM_MODE_FLAG_DBLCLK; drm_mode_set_name(mode); } EXPORT_SYMBOL_GPL(drm_mode_from_display_timing); If we then on top of this would like easy access to the names we know we could add a few access functions: static inline u32 drm_mode_hactive(const struct drm_display_mode *mode) { mode->hdisplay; } static inline u32 drm_mode_hfront_porch(const struct drm_display_mode *mode) { mode->hsync_start - mode->hdisplay; } static inline u32 drm_mode_hback_porch(const struct drm_display_mode *mode) { mode->htotal - mode->hsync_start; } static inline u32 drm_mode_hsync_len(const struct drm_display_mode *mode) { return mode->hsync_end - mode->hsync_start; } static inline u32 drm_mode_vactive(const struct drm_display_mode *mode) { return mode->vdisplay; } static inline u32 drm_mode_vfront_porch(const struct drm_display_mode *mode) { return mode->vsync_start - mode->vdisplay; } static inline u32 drm_mode_vsync_len(const struct drm_display_mode *mode) { return mode->vsync_end - mode->vsync_start; } static inline u32 drm_mode_vback_porch(const struct drm_display_mode *mode) { return mode->vtotal - mode->vsync_end; } The above was something I just quickly typed. This was the easy part. Writing kernel-.doc and fix it so it works is the time consuming part.. Sam ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-20 19:57 ` Sam Ravnborg @ 2022-02-20 20:34 ` Noralf Trønnes -1 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-20 20:34 UTC (permalink / raw) To: Sam Ravnborg Cc: dave.stevenson, david, devicetree, dri-devel, maxime, robh+dt, thierry.reding Den 20.02.2022 20.57, skrev Sam Ravnborg: > Hi Noralf. > > On Sun, Feb 20, 2022 at 07:11:14PM +0100, Noralf Trønnes wrote: >>> Den 20.02.2022 11.04, skrev Sam Ravnborg: >>>> Hi Noralf, >>>> >>>>>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, >> struct drm_display_mode *mode) >>>>>> +{ >>>>>> + struct device *dev = dbidev->drm.dev; >>>>>> + u32 width_mm = 0, height_mm = 0; >>>>>> + struct display_timing timing; >>>>>> + struct videomode vm; >>>>>> + int ret; >>>>>> + >>>>>> + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); >>>>>> + if (ret) { >>>>>> + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", >> dev->of_node, ret); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> + videomode_from_timing(&timing, &vm); >>>>>> + >>>>>> + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || >>>>>> + (vm.hback_porch + vm.hactive) > 0xffff || >>>>>> + !vm.vactive || vm.vfront_porch || vm.vsync_len || >>>>>> + (vm.vback_porch + vm.vactive) > 0xffff || >>>>>> + vm.flags) { >>>>>> + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); >>>>>> + return -EINVAL; >>>>>> + } >>>>> We should have a helper that implements this. Maybe the display_timing >>>>> => display_mode helper could do it. >>>> >>>> It would be nice with a drm_display_timing_to_mode() but that can come >>>> later - the comment above should not be understood that I consider it >>>> mandatory for this driver. >>>> >>> >>> I did consider adding an of_get_drm_panel_mode() fashioned after >>> of_get_drm_display_mode() but I didn't find any other driver that would >>> actually be able to use it and I would have to do some substraction to >>> get back the {h,v}front_porch values that I need and the optional pixel >>> clock calculation becomes more complex acting from a drm_display_mode so >>> I decided against it. >>> >>> Looking at it now, what I could do is add a function like what >>> of_get_videomode() does for "display-timings": >>> >>> /** >>> * of_get_panel_videomode - get the panel-timing videomode from devicetree >>> * @np: devicenode containing the panel-timing subnode >>> * @vm: returns the videomode >>> * >>> * Returns: >>> * Zero on success, negative error code on failure. >>> **/ >>> int of_get_panel_videomode(struct device_node *np, struct videomode *vm) >>> { >>> struct display_timing timing; >>> int ret; >>> >>> ret = of_get_display_timing(np, "panel-timing", &timing); >>> if (ret) >>> return ret; >>> >>> videomode_from_timing(&timing, vm); >>> >>> return 0; >>> } >>> >>> This could also be used by panel-lvds and 2 fbdev drivers, the other >>> panel-timing users need/use the display_timing itself, some for bounds >>> checking. >> >> Scratch that, since videomode is to be avoided I tried adding a >> drm_display_mode function and it didn't complicate matter as I though it >> would so I'll do that instead: > > I like, but would like to get rid of video_mode entirely. > >> >> static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct >> drm_display_mode *mode) >> { >> struct device *dev = dbidev->drm.dev; >> u32 width_mm = 0, height_mm = 0; >> u16 hback_porch, vback_porch; >> struct videomode vm; >> int ret; >> >> ret = of_get_drm_panel_display_mode(dev->of_node, mode, NULL); >> if (ret) { >> dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", >> dev->of_node, ret); >> return ret; >> } >> >> mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; >> >> hback_porch = mode->htotal - mode->hsync_end; >> vback_porch = mode->vtotal - mode->vsync_end; > The accesss functions I posed below could be used here - so we avoid > typing these (trivial) calculations in many places. > >> >> if (mode->hsync_end > mode->hdisplay || (hback_porch + mode->hdisplay) >>> 0xffff || >> mode->vsync_end > mode->vdisplay || (vback_porch + mode->vdisplay) >>> 0xffff || >> mode->flags) { >> dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); >> return -EINVAL; >> } > With the display_timing => drm_display_mode I think the above is no > longer required. > I still need to verify the values to ensure that front_porch and sync_len are zero. Maybe I need a comment now to tell what I'm checking since I'm further away from the DT values: /* * Make sure width and height are set and that only back porch and * pixelclock are set in the other timing values. Also check that * width and height don't exceed the 16-bit value specified by MIPI DCS. */ >> >> /* The driver doesn't use the pixel clock but it is mandatory so fake >> one if not set */ >> if (!mode->pixelclock) >> mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; >> >> dbidev->top_offset = vback_porch; >> dbidev->left_offset = hback_porch; >> >> return 0; >> } >> >> >> int of_get_drm_panel_display_mode(struct device_node *np, >> struct drm_display_mode *dmode, u32 *bus_flags) >> { > Not sure about the bus_flags argument here - seems misplaced. > I did the same as of_get_drm_display_mode(), don't panel drivers need the bus flags? >> u32 width_mm = 0, height_mm = 0; >> struct display_timing timing; >> struct videomode vm; >> int ret; >> >> ret = of_get_display_timing(np, "panel-timing", &timing); >> if (ret) >> return ret; >> >> videomode_from_timing(&timing, vm); >> >> memset(dmode, 0, sizeof(*dmode)); >> drm_display_mode_from_videomode(&vm, dmode); >> if (bus_flags) >> drm_bus_flags_from_videomode(&vm, bus_flags); > > This does a: > display_timing => video_mode => drm_display_display_mode > > We could do a: > display_timing => drm_display_mode. > I'll leave this to others to sort out. I want the function to look the same as of_get_drm_display_mode() and it uses videomode. If videomode goes away both can be fixed at the same time. > Sample implementation could look like this: > void drm_mode_from_display_timing(struct drm_display_mode *mode, > const struct display_timing *dt) > { > mode->hdisplay = dt->hactive.typ; > mode->hsync_start = dt->hactive.typ + dt->hfront_porch.typ; > mode->hsync_end = dt->hactive.typ + dt->hfront_porch.typ + dt->hsync_len.typ; > mode->htotal = dt->hactive.typ + dt->hfront_porch.typ + dt->hsync_len.typ + dt->hback_porch.typ; > > mode->vdisplay = dt->vactive.typ; > mode->vsync_start = dt->vactive.typ + dt->vfront_porch.typ; > mode->vsync_end = dt->vactive.typ + dt->vfront_porch.typ + dt->vsync_len.typ; > mode->vtotal = dt->vactive.typ + dt->vfront_porch.typ + dt->vsync_len.typ + dt->vback_porch.typ; > > mode->clock = dt->pixelclock.typ / 1000; > > mode->flags = 0; > if (dt->flags & DISPLAY_FLAGS_HSYNC_HIGH) > mode->flags |= DRM_MODE_FLAG_PHSYNC; > else if (dt->flags & DISPLAY_FLAGS_HSYNC_LOW) > mode->flags |= DRM_MODE_FLAG_NHSYNC; > if (dt->flags & DISPLAY_FLAGS_VSYNC_HIGH) > mode->flags |= DRM_MODE_FLAG_PVSYNC; > else if (dt->flags & DISPLAY_FLAGS_VSYNC_LOW) > mode->flags |= DRM_MODE_FLAG_NVSYNC; > if (dt->flags & DISPLAY_FLAGS_INTERLACED) > mode->flags |= DRM_MODE_FLAG_INTERLACE; > if (dt->flags & DISPLAY_FLAGS_DOUBLESCAN) > mode->flags |= DRM_MODE_FLAG_DBLSCAN; > if (dt->flags & DISPLAY_FLAGS_DOUBLECLK) > mode->flags |= DRM_MODE_FLAG_DBLCLK; > drm_mode_set_name(mode); > } > EXPORT_SYMBOL_GPL(drm_mode_from_display_timing); > > If we then on top of this would like easy access to the names we know we > could add a few access functions: > I don't think I'll do these either, it's more work than I can invest in this. Noralf. > static inline u32 drm_mode_hactive(const struct drm_display_mode *mode) > { > mode->hdisplay; > } > > static inline u32 drm_mode_hfront_porch(const struct drm_display_mode *mode) > { > mode->hsync_start - mode->hdisplay; > } > > static inline u32 drm_mode_hback_porch(const struct drm_display_mode *mode) > { > mode->htotal - mode->hsync_start; > } > > static inline u32 drm_mode_hsync_len(const struct drm_display_mode *mode) > { > return mode->hsync_end - mode->hsync_start; > } > > static inline u32 drm_mode_vactive(const struct drm_display_mode *mode) > { > return mode->vdisplay; > } > > static inline u32 drm_mode_vfront_porch(const struct drm_display_mode *mode) > { > return mode->vsync_start - mode->vdisplay; > } > > static inline u32 drm_mode_vsync_len(const struct drm_display_mode *mode) > { > return mode->vsync_end - mode->vsync_start; > } > > static inline u32 drm_mode_vback_porch(const struct drm_display_mode *mode) > { > return mode->vtotal - mode->vsync_end; > } > > > The above was something I just quickly typed. This was the easy part. > Writing kernel-.doc and fix it so it works is the time consuming part.. > > Sam ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-20 20:34 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-20 20:34 UTC (permalink / raw) To: Sam Ravnborg Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Den 20.02.2022 20.57, skrev Sam Ravnborg: > Hi Noralf. > > On Sun, Feb 20, 2022 at 07:11:14PM +0100, Noralf Trønnes wrote: >>> Den 20.02.2022 11.04, skrev Sam Ravnborg: >>>> Hi Noralf, >>>> >>>>>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, >> struct drm_display_mode *mode) >>>>>> +{ >>>>>> + struct device *dev = dbidev->drm.dev; >>>>>> + u32 width_mm = 0, height_mm = 0; >>>>>> + struct display_timing timing; >>>>>> + struct videomode vm; >>>>>> + int ret; >>>>>> + >>>>>> + ret = of_get_display_timing(dev->of_node, "panel-timing", &timing); >>>>>> + if (ret) { >>>>>> + dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", >> dev->of_node, ret); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> + videomode_from_timing(&timing, &vm); >>>>>> + >>>>>> + if (!vm.hactive || vm.hfront_porch || vm.hsync_len || >>>>>> + (vm.hback_porch + vm.hactive) > 0xffff || >>>>>> + !vm.vactive || vm.vfront_porch || vm.vsync_len || >>>>>> + (vm.vback_porch + vm.vactive) > 0xffff || >>>>>> + vm.flags) { >>>>>> + dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); >>>>>> + return -EINVAL; >>>>>> + } >>>>> We should have a helper that implements this. Maybe the display_timing >>>>> => display_mode helper could do it. >>>> >>>> It would be nice with a drm_display_timing_to_mode() but that can come >>>> later - the comment above should not be understood that I consider it >>>> mandatory for this driver. >>>> >>> >>> I did consider adding an of_get_drm_panel_mode() fashioned after >>> of_get_drm_display_mode() but I didn't find any other driver that would >>> actually be able to use it and I would have to do some substraction to >>> get back the {h,v}front_porch values that I need and the optional pixel >>> clock calculation becomes more complex acting from a drm_display_mode so >>> I decided against it. >>> >>> Looking at it now, what I could do is add a function like what >>> of_get_videomode() does for "display-timings": >>> >>> /** >>> * of_get_panel_videomode - get the panel-timing videomode from devicetree >>> * @np: devicenode containing the panel-timing subnode >>> * @vm: returns the videomode >>> * >>> * Returns: >>> * Zero on success, negative error code on failure. >>> **/ >>> int of_get_panel_videomode(struct device_node *np, struct videomode *vm) >>> { >>> struct display_timing timing; >>> int ret; >>> >>> ret = of_get_display_timing(np, "panel-timing", &timing); >>> if (ret) >>> return ret; >>> >>> videomode_from_timing(&timing, vm); >>> >>> return 0; >>> } >>> >>> This could also be used by panel-lvds and 2 fbdev drivers, the other >>> panel-timing users need/use the display_timing itself, some for bounds >>> checking. >> >> Scratch that, since videomode is to be avoided I tried adding a >> drm_display_mode function and it didn't complicate matter as I though it >> would so I'll do that instead: > > I like, but would like to get rid of video_mode entirely. > >> >> static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct >> drm_display_mode *mode) >> { >> struct device *dev = dbidev->drm.dev; >> u32 width_mm = 0, height_mm = 0; >> u16 hback_porch, vback_porch; >> struct videomode vm; >> int ret; >> >> ret = of_get_drm_panel_display_mode(dev->of_node, mode, NULL); >> if (ret) { >> dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", >> dev->of_node, ret); >> return ret; >> } >> >> mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; >> >> hback_porch = mode->htotal - mode->hsync_end; >> vback_porch = mode->vtotal - mode->vsync_end; > The accesss functions I posed below could be used here - so we avoid > typing these (trivial) calculations in many places. > >> >> if (mode->hsync_end > mode->hdisplay || (hback_porch + mode->hdisplay) >>> 0xffff || >> mode->vsync_end > mode->vdisplay || (vback_porch + mode->vdisplay) >>> 0xffff || >> mode->flags) { >> dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); >> return -EINVAL; >> } > With the display_timing => drm_display_mode I think the above is no > longer required. > I still need to verify the values to ensure that front_porch and sync_len are zero. Maybe I need a comment now to tell what I'm checking since I'm further away from the DT values: /* * Make sure width and height are set and that only back porch and * pixelclock are set in the other timing values. Also check that * width and height don't exceed the 16-bit value specified by MIPI DCS. */ >> >> /* The driver doesn't use the pixel clock but it is mandatory so fake >> one if not set */ >> if (!mode->pixelclock) >> mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; >> >> dbidev->top_offset = vback_porch; >> dbidev->left_offset = hback_porch; >> >> return 0; >> } >> >> >> int of_get_drm_panel_display_mode(struct device_node *np, >> struct drm_display_mode *dmode, u32 *bus_flags) >> { > Not sure about the bus_flags argument here - seems misplaced. > I did the same as of_get_drm_display_mode(), don't panel drivers need the bus flags? >> u32 width_mm = 0, height_mm = 0; >> struct display_timing timing; >> struct videomode vm; >> int ret; >> >> ret = of_get_display_timing(np, "panel-timing", &timing); >> if (ret) >> return ret; >> >> videomode_from_timing(&timing, vm); >> >> memset(dmode, 0, sizeof(*dmode)); >> drm_display_mode_from_videomode(&vm, dmode); >> if (bus_flags) >> drm_bus_flags_from_videomode(&vm, bus_flags); > > This does a: > display_timing => video_mode => drm_display_display_mode > > We could do a: > display_timing => drm_display_mode. > I'll leave this to others to sort out. I want the function to look the same as of_get_drm_display_mode() and it uses videomode. If videomode goes away both can be fixed at the same time. > Sample implementation could look like this: > void drm_mode_from_display_timing(struct drm_display_mode *mode, > const struct display_timing *dt) > { > mode->hdisplay = dt->hactive.typ; > mode->hsync_start = dt->hactive.typ + dt->hfront_porch.typ; > mode->hsync_end = dt->hactive.typ + dt->hfront_porch.typ + dt->hsync_len.typ; > mode->htotal = dt->hactive.typ + dt->hfront_porch.typ + dt->hsync_len.typ + dt->hback_porch.typ; > > mode->vdisplay = dt->vactive.typ; > mode->vsync_start = dt->vactive.typ + dt->vfront_porch.typ; > mode->vsync_end = dt->vactive.typ + dt->vfront_porch.typ + dt->vsync_len.typ; > mode->vtotal = dt->vactive.typ + dt->vfront_porch.typ + dt->vsync_len.typ + dt->vback_porch.typ; > > mode->clock = dt->pixelclock.typ / 1000; > > mode->flags = 0; > if (dt->flags & DISPLAY_FLAGS_HSYNC_HIGH) > mode->flags |= DRM_MODE_FLAG_PHSYNC; > else if (dt->flags & DISPLAY_FLAGS_HSYNC_LOW) > mode->flags |= DRM_MODE_FLAG_NHSYNC; > if (dt->flags & DISPLAY_FLAGS_VSYNC_HIGH) > mode->flags |= DRM_MODE_FLAG_PVSYNC; > else if (dt->flags & DISPLAY_FLAGS_VSYNC_LOW) > mode->flags |= DRM_MODE_FLAG_NVSYNC; > if (dt->flags & DISPLAY_FLAGS_INTERLACED) > mode->flags |= DRM_MODE_FLAG_INTERLACE; > if (dt->flags & DISPLAY_FLAGS_DOUBLESCAN) > mode->flags |= DRM_MODE_FLAG_DBLSCAN; > if (dt->flags & DISPLAY_FLAGS_DOUBLECLK) > mode->flags |= DRM_MODE_FLAG_DBLCLK; > drm_mode_set_name(mode); > } > EXPORT_SYMBOL_GPL(drm_mode_from_display_timing); > > If we then on top of this would like easy access to the names we know we > could add a few access functions: > I don't think I'll do these either, it's more work than I can invest in this. Noralf. > static inline u32 drm_mode_hactive(const struct drm_display_mode *mode) > { > mode->hdisplay; > } > > static inline u32 drm_mode_hfront_porch(const struct drm_display_mode *mode) > { > mode->hsync_start - mode->hdisplay; > } > > static inline u32 drm_mode_hback_porch(const struct drm_display_mode *mode) > { > mode->htotal - mode->hsync_start; > } > > static inline u32 drm_mode_hsync_len(const struct drm_display_mode *mode) > { > return mode->hsync_end - mode->hsync_start; > } > > static inline u32 drm_mode_vactive(const struct drm_display_mode *mode) > { > return mode->vdisplay; > } > > static inline u32 drm_mode_vfront_porch(const struct drm_display_mode *mode) > { > return mode->vsync_start - mode->vdisplay; > } > > static inline u32 drm_mode_vsync_len(const struct drm_display_mode *mode) > { > return mode->vsync_end - mode->vsync_start; > } > > static inline u32 drm_mode_vback_porch(const struct drm_display_mode *mode) > { > return mode->vtotal - mode->vsync_end; > } > > > The above was something I just quickly typed. This was the easy part. > Writing kernel-.doc and fix it so it works is the time consuming part.. > > Sam ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-20 20:34 ` Noralf Trønnes @ 2022-02-20 21:30 ` Sam Ravnborg -1 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-20 21:30 UTC (permalink / raw) To: Noralf Trønnes Cc: dave.stevenson, david, devicetree, dri-devel, maxime, robh+dt, thierry.reding Hi Noralf, > >> mode->flags) { > >> dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > >> return -EINVAL; > >> } > > With the display_timing => drm_display_mode I think the above is no > > longer required. > > > > I still need to verify the values to ensure that front_porch and > sync_len are zero. Maybe I need a comment now to tell what I'm checking > since I'm further away from the DT values: > > /* > * Make sure width and height are set and that only back porch and > * pixelclock are set in the other timing values. Also check that > * width and height don't exceed the 16-bit value specified by MIPI DCS. > */ Yes, that would be nice. > > >> > >> /* The driver doesn't use the pixel clock but it is mandatory so fake > >> one if not set */ > >> if (!mode->pixelclock) > >> mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; > >> > >> dbidev->top_offset = vback_porch; > >> dbidev->left_offset = hback_porch; > >> > >> return 0; > >> } > >> > >> > >> int of_get_drm_panel_display_mode(struct device_node *np, > >> struct drm_display_mode *dmode, u32 *bus_flags) > >> { > > Not sure about the bus_flags argument here - seems misplaced. > > > > I did the same as of_get_drm_display_mode(), don't panel drivers need > the bus flags? In my haste I missed the display_timing combines flags for the bus and the mode - so yes it is needed. > > >> u32 width_mm = 0, height_mm = 0; > >> struct display_timing timing; > >> struct videomode vm; > >> int ret; > >> > >> ret = of_get_display_timing(np, "panel-timing", &timing); > >> if (ret) > >> return ret; > >> > >> videomode_from_timing(&timing, vm); > >> > >> memset(dmode, 0, sizeof(*dmode)); > >> drm_display_mode_from_videomode(&vm, dmode); > >> if (bus_flags) > >> drm_bus_flags_from_videomode(&vm, bus_flags); > > > > This does a: > > display_timing => video_mode => drm_display_display_mode > > > > We could do a: > > display_timing => drm_display_mode. > > > > I'll leave this to others to sort out. I want the function to look the > same as of_get_drm_display_mode() and it uses videomode. If videomode > goes away both can be fixed at the same time. When I have dig myself out of the bridge hole I am in I may take a look at this. Sam ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-20 21:30 ` Sam Ravnborg 0 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-20 21:30 UTC (permalink / raw) To: Noralf Trønnes Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Hi Noralf, > >> mode->flags) { > >> dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node); > >> return -EINVAL; > >> } > > With the display_timing => drm_display_mode I think the above is no > > longer required. > > > > I still need to verify the values to ensure that front_porch and > sync_len are zero. Maybe I need a comment now to tell what I'm checking > since I'm further away from the DT values: > > /* > * Make sure width and height are set and that only back porch and > * pixelclock are set in the other timing values. Also check that > * width and height don't exceed the 16-bit value specified by MIPI DCS. > */ Yes, that would be nice. > > >> > >> /* The driver doesn't use the pixel clock but it is mandatory so fake > >> one if not set */ > >> if (!mode->pixelclock) > >> mode->pixelclock = mode->htotal * mode->vtotal * 60 / 1000; > >> > >> dbidev->top_offset = vback_porch; > >> dbidev->left_offset = hback_porch; > >> > >> return 0; > >> } > >> > >> > >> int of_get_drm_panel_display_mode(struct device_node *np, > >> struct drm_display_mode *dmode, u32 *bus_flags) > >> { > > Not sure about the bus_flags argument here - seems misplaced. > > > > I did the same as of_get_drm_display_mode(), don't panel drivers need > the bus flags? In my haste I missed the display_timing combines flags for the bus and the mode - so yes it is needed. > > >> u32 width_mm = 0, height_mm = 0; > >> struct display_timing timing; > >> struct videomode vm; > >> int ret; > >> > >> ret = of_get_display_timing(np, "panel-timing", &timing); > >> if (ret) > >> return ret; > >> > >> videomode_from_timing(&timing, vm); > >> > >> memset(dmode, 0, sizeof(*dmode)); > >> drm_display_mode_from_videomode(&vm, dmode); > >> if (bus_flags) > >> drm_bus_flags_from_videomode(&vm, bus_flags); > > > > This does a: > > display_timing => video_mode => drm_display_display_mode > > > > We could do a: > > display_timing => drm_display_mode. > > > > I'll leave this to others to sort out. I want the function to look the > same as of_get_drm_display_mode() and it uses videomode. If videomode > goes away both can be fixed at the same time. When I have dig myself out of the bridge hole I am in I may take a look at this. Sam ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-19 22:10 ` Sam Ravnborg @ 2022-02-20 15:59 ` Noralf Trønnes -1 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-20 15:59 UTC (permalink / raw) To: Sam Ravnborg Cc: robh+dt, thierry.reding, maxime, dave.stevenson, david, devicetree, dri-devel Den 19.02.2022 23.10, skrev Sam Ravnborg: > Hi Noralf, > On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote: >> Add a driver that will work with most MIPI DBI compatible SPI panels. >> This avoids adding a driver for every new MIPI DBI compatible controller >> that is to be used by Linux. The 'compatible' Device Tree property with >> a '.bin' suffix will be used to load a firmware file that contains the >> controller configuration. > A solution where we have the command sequences as part of the DT-overlay > is IMO a much better way to solve this: > - The users needs to deal only with a single file, so there is less that > goes wrong > - The user need not reading special instructions how to handle a .bin > file, if the overlay is present everything is fine > - The file that contains the command sequences can be properly annotated > with comments > - The people that creates the command sequences has no need for a special > script to create the file for a display - this is all readable ascii. > - Using a DT-overlay the parsing of the DT-overlay can be done by > well-tested functions, no need to invent something new to parse the > file > > > The idea with a common mipi DBI SPI driver is super, it is the detail > with the .bin file that I am against. > The fbtft drivers has an init= DT property that contains the command sequence. Example for the MZ61581 display: init = <0x10000b0 00 0x1000011 0x20000ff 0x10000b3 0x02 0x00 0x00 0x00 0x10000c0 0x13 0x3b 0x00 0x02 0x00 0x01 0x00 0x43 0x10000c1 0x08 0x16 0x08 0x08 0x10000c4 0x11 0x07 0x03 0x03 0x10000c6 0x00 0x10000c8 0x03 0x03 0x13 0x5c 0x03 0x07 0x14 0x08 0x00 0x21 0x08 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00 0x1000035 0x00 0x1000036 0xa0 0x100003a 0x55 0x1000044 0x00 0x01 0x10000d0 0x07 0x07 0x1d 0x03 0x10000d1 0x03 0x30 0x10 0x10000d2 0x03 0x14 0x04 0x1000029 0x100002c>; Parsed here: https://elixir.bootlin.com/linux/latest/C/ident/fbtft_init_display_from_property Before fbdev was closed for new drivers I looked at fixing up fbtft for proper inclusion and asked on the Device Tree mailinglist about the init property and how to handle the controller configuration generically. I was politely told that this should be done in the driver, so when I made my first DRM driver I made a driver specifically for a panel (mi0283qt.c). Later I found another post that in clear words stated that setting random registers from DT was not acceptable. So I think Maxime has provided a clever way of dealing with this problem to let us have a generic driver: The OS is in charge of how to configure the controller and in this case does it using a firmware file. > With the above said, a few comments to the current implementation below. > As we know it from you - a very well-written driver. > > Sam > >> Acked-by: Maxime Ripard <maxime@cerno.tech> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> --- >> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig >> index 712e0004e96e..d552e1618da7 100644 >> --- a/drivers/gpu/drm/tiny/Kconfig >> +++ b/drivers/gpu/drm/tiny/Kconfig >> @@ -51,6 +51,19 @@ config DRM_GM12U320 >> This is a KMS driver for projectors which use the GM12U320 chipset >> for video transfer over USB2/3, such as the Acer C120 mini projector. >> >> +config DRM_PANEL_MIPI_DBI >> + tristate "DRM support for MIPI DBI compatible panels" >> + depends on DRM && SPI >> + select DRM_KMS_HELPER >> + select DRM_KMS_CMA_HELPER > This symbol is not present in my drm-misc-next tree (which is a few > weeks old, so it may be newer). > Turns out this was removed in 09717af7d13d. This should be DRM_GEM_CMA_HELPER now. >> + select DRM_MIPI_DBI >> + select BACKLIGHT_CLASS_DEVICE >> + help >> + Say Y here if you want to enable support for MIPI DBI compatible >> + panels. The controller command setup can be provided using a >> + firmware file. > Consider adding a link to the wiki here - this may make it easier for > the user to find it. > >> + To compile this driver as a module, choose M here. >> + >> config DRM_SIMPLEDRM >> tristate "Simple framebuffer driver" >> depends on DRM && MMU >> diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c >> new file mode 100644 >> index 000000000000..9240fdec38d6 >> --- /dev/null >> +++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c >> @@ -0,0 +1,413 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * DRM driver for MIPI DBI compatible display panels >> + * >> + * Copyright 2022 Noralf Trønnes >> + */ >> + >> +#include <linux/backlight.h> >> +#include <linux/delay.h> >> +#include <linux/firmware.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/module.h> >> +#include <linux/property.h> >> +#include <linux/regulator/consumer.h> >> +#include <linux/spi/spi.h> >> + >> +#include <drm/drm_atomic_helper.h> >> +#include <drm/drm_drv.h> >> +#include <drm/drm_fb_helper.h> >> +#include <drm/drm_gem_atomic_helper.h> >> +#include <drm/drm_gem_cma_helper.h> >> +#include <drm/drm_managed.h> >> +#include <drm/drm_mipi_dbi.h> >> +#include <drm/drm_modeset_helper.h> >> + >> +#include <video/display_timing.h> >> +#include <video/mipi_display.h> >> +#include <video/of_display_timing.h> >> +#include <video/videomode.h> > videomode should not be used in new drivers, it is an fbdev artifact. > But that said - we are still missing a direct display_timing => > display_mode - so maybe we need it here. > > If it is needed Kconfig needs to be extended with: > select VIDEOMODE_HELPERS > Good catch! >> +static int panel_mipi_dbi_spi_probe(struct spi_device *spi) >> +{ >> + struct device *dev = &spi->dev; >> + struct drm_display_mode mode; >> + struct mipi_dbi_dev *dbidev; >> + struct drm_device *drm; >> + struct mipi_dbi *dbi; >> + struct gpio_desc *dc; >> + int ret; >> + >> + dbidev = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver, struct mipi_dbi_dev, drm); >> + if (IS_ERR(dbidev)) >> + return PTR_ERR(dbidev); >> + >> + dbi = &dbidev->dbi; >> + drm = &dbidev->drm; >> + >> + ret = panel_mipi_dbi_get_mode(dbidev, &mode); >> + if (ret) >> + return ret; >> + >> + dbidev->regulator = devm_regulator_get(dev, "power"); >> + if (IS_ERR(dbidev->regulator)) >> + return dev_err_probe(dev, PTR_ERR(dbidev->regulator), >> + "Failed to get regulator 'power'\n"); >> + >> + dbidev->backlight = devm_of_find_backlight(dev); >> + if (IS_ERR(dbidev->backlight)) >> + return dev_err_probe(dev, PTR_ERR(dbidev->backlight), "Failed to get backlight\n"); >> + >> + dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); >> + if (IS_ERR(dbi->reset)) >> + return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n"); >> + >> + dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); >> + if (IS_ERR(dc)) >> + return dev_err_probe(dev, PTR_ERR(dc), "Failed to get GPIO 'dc'\n"); >> + >> + ret = mipi_dbi_spi_init(spi, dbi, dc); >> + if (ret) >> + return ret; >> + >> + if (device_property_present(dev, "write-only")) >> + dbi->read_commands = NULL; > read_commands are unused - so the write-only property is in practice > ignored. > >read_commands is set to a default in mipi_dbi_spi_init() and we clear it when write-only. Noralf. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-20 15:59 ` Noralf Trønnes 0 siblings, 0 replies; 36+ messages in thread From: Noralf Trønnes @ 2022-02-20 15:59 UTC (permalink / raw) To: Sam Ravnborg Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Den 19.02.2022 23.10, skrev Sam Ravnborg: > Hi Noralf, > On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote: >> Add a driver that will work with most MIPI DBI compatible SPI panels. >> This avoids adding a driver for every new MIPI DBI compatible controller >> that is to be used by Linux. The 'compatible' Device Tree property with >> a '.bin' suffix will be used to load a firmware file that contains the >> controller configuration. > A solution where we have the command sequences as part of the DT-overlay > is IMO a much better way to solve this: > - The users needs to deal only with a single file, so there is less that > goes wrong > - The user need not reading special instructions how to handle a .bin > file, if the overlay is present everything is fine > - The file that contains the command sequences can be properly annotated > with comments > - The people that creates the command sequences has no need for a special > script to create the file for a display - this is all readable ascii. > - Using a DT-overlay the parsing of the DT-overlay can be done by > well-tested functions, no need to invent something new to parse the > file > > > The idea with a common mipi DBI SPI driver is super, it is the detail > with the .bin file that I am against. > The fbtft drivers has an init= DT property that contains the command sequence. Example for the MZ61581 display: init = <0x10000b0 00 0x1000011 0x20000ff 0x10000b3 0x02 0x00 0x00 0x00 0x10000c0 0x13 0x3b 0x00 0x02 0x00 0x01 0x00 0x43 0x10000c1 0x08 0x16 0x08 0x08 0x10000c4 0x11 0x07 0x03 0x03 0x10000c6 0x00 0x10000c8 0x03 0x03 0x13 0x5c 0x03 0x07 0x14 0x08 0x00 0x21 0x08 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00 0x1000035 0x00 0x1000036 0xa0 0x100003a 0x55 0x1000044 0x00 0x01 0x10000d0 0x07 0x07 0x1d 0x03 0x10000d1 0x03 0x30 0x10 0x10000d2 0x03 0x14 0x04 0x1000029 0x100002c>; Parsed here: https://elixir.bootlin.com/linux/latest/C/ident/fbtft_init_display_from_property Before fbdev was closed for new drivers I looked at fixing up fbtft for proper inclusion and asked on the Device Tree mailinglist about the init property and how to handle the controller configuration generically. I was politely told that this should be done in the driver, so when I made my first DRM driver I made a driver specifically for a panel (mi0283qt.c). Later I found another post that in clear words stated that setting random registers from DT was not acceptable. So I think Maxime has provided a clever way of dealing with this problem to let us have a generic driver: The OS is in charge of how to configure the controller and in this case does it using a firmware file. > With the above said, a few comments to the current implementation below. > As we know it from you - a very well-written driver. > > Sam > >> Acked-by: Maxime Ripard <maxime@cerno.tech> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> --- >> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig >> index 712e0004e96e..d552e1618da7 100644 >> --- a/drivers/gpu/drm/tiny/Kconfig >> +++ b/drivers/gpu/drm/tiny/Kconfig >> @@ -51,6 +51,19 @@ config DRM_GM12U320 >> This is a KMS driver for projectors which use the GM12U320 chipset >> for video transfer over USB2/3, such as the Acer C120 mini projector. >> >> +config DRM_PANEL_MIPI_DBI >> + tristate "DRM support for MIPI DBI compatible panels" >> + depends on DRM && SPI >> + select DRM_KMS_HELPER >> + select DRM_KMS_CMA_HELPER > This symbol is not present in my drm-misc-next tree (which is a few > weeks old, so it may be newer). > Turns out this was removed in 09717af7d13d. This should be DRM_GEM_CMA_HELPER now. >> + select DRM_MIPI_DBI >> + select BACKLIGHT_CLASS_DEVICE >> + help >> + Say Y here if you want to enable support for MIPI DBI compatible >> + panels. The controller command setup can be provided using a >> + firmware file. > Consider adding a link to the wiki here - this may make it easier for > the user to find it. > >> + To compile this driver as a module, choose M here. >> + >> config DRM_SIMPLEDRM >> tristate "Simple framebuffer driver" >> depends on DRM && MMU >> diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c >> new file mode 100644 >> index 000000000000..9240fdec38d6 >> --- /dev/null >> +++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c >> @@ -0,0 +1,413 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * DRM driver for MIPI DBI compatible display panels >> + * >> + * Copyright 2022 Noralf Trønnes >> + */ >> + >> +#include <linux/backlight.h> >> +#include <linux/delay.h> >> +#include <linux/firmware.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/module.h> >> +#include <linux/property.h> >> +#include <linux/regulator/consumer.h> >> +#include <linux/spi/spi.h> >> + >> +#include <drm/drm_atomic_helper.h> >> +#include <drm/drm_drv.h> >> +#include <drm/drm_fb_helper.h> >> +#include <drm/drm_gem_atomic_helper.h> >> +#include <drm/drm_gem_cma_helper.h> >> +#include <drm/drm_managed.h> >> +#include <drm/drm_mipi_dbi.h> >> +#include <drm/drm_modeset_helper.h> >> + >> +#include <video/display_timing.h> >> +#include <video/mipi_display.h> >> +#include <video/of_display_timing.h> >> +#include <video/videomode.h> > videomode should not be used in new drivers, it is an fbdev artifact. > But that said - we are still missing a direct display_timing => > display_mode - so maybe we need it here. > > If it is needed Kconfig needs to be extended with: > select VIDEOMODE_HELPERS > Good catch! >> +static int panel_mipi_dbi_spi_probe(struct spi_device *spi) >> +{ >> + struct device *dev = &spi->dev; >> + struct drm_display_mode mode; >> + struct mipi_dbi_dev *dbidev; >> + struct drm_device *drm; >> + struct mipi_dbi *dbi; >> + struct gpio_desc *dc; >> + int ret; >> + >> + dbidev = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver, struct mipi_dbi_dev, drm); >> + if (IS_ERR(dbidev)) >> + return PTR_ERR(dbidev); >> + >> + dbi = &dbidev->dbi; >> + drm = &dbidev->drm; >> + >> + ret = panel_mipi_dbi_get_mode(dbidev, &mode); >> + if (ret) >> + return ret; >> + >> + dbidev->regulator = devm_regulator_get(dev, "power"); >> + if (IS_ERR(dbidev->regulator)) >> + return dev_err_probe(dev, PTR_ERR(dbidev->regulator), >> + "Failed to get regulator 'power'\n"); >> + >> + dbidev->backlight = devm_of_find_backlight(dev); >> + if (IS_ERR(dbidev->backlight)) >> + return dev_err_probe(dev, PTR_ERR(dbidev->backlight), "Failed to get backlight\n"); >> + >> + dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); >> + if (IS_ERR(dbi->reset)) >> + return dev_err_probe(dev, PTR_ERR(dbi->reset), "Failed to get GPIO 'reset'\n"); >> + >> + dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); >> + if (IS_ERR(dc)) >> + return dev_err_probe(dev, PTR_ERR(dc), "Failed to get GPIO 'dc'\n"); >> + >> + ret = mipi_dbi_spi_init(spi, dbi, dc); >> + if (ret) >> + return ret; >> + >> + if (device_property_present(dev, "write-only")) >> + dbi->read_commands = NULL; > read_commands are unused - so the write-only property is in practice > ignored. > >read_commands is set to a default in mipi_dbi_spi_init() and we clear it when write-only. Noralf. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-20 15:59 ` Noralf Trønnes @ 2022-02-20 21:32 ` Sam Ravnborg -1 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-20 21:32 UTC (permalink / raw) To: Noralf Trønnes Cc: robh+dt, thierry.reding, maxime, dave.stevenson, david, devicetree, dri-devel Hi Noralf, On Sun, Feb 20, 2022 at 04:59:34PM +0100, Noralf Trønnes wrote: > > > Den 19.02.2022 23.10, skrev Sam Ravnborg: > > Hi Noralf, > > On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote: > >> Add a driver that will work with most MIPI DBI compatible SPI panels. > >> This avoids adding a driver for every new MIPI DBI compatible controller > >> that is to be used by Linux. The 'compatible' Device Tree property with > >> a '.bin' suffix will be used to load a firmware file that contains the > >> controller configuration. > > A solution where we have the command sequences as part of the DT-overlay > > is IMO a much better way to solve this: > > - The users needs to deal only with a single file, so there is less that > > goes wrong > > - The user need not reading special instructions how to handle a .bin > > file, if the overlay is present everything is fine > > - The file that contains the command sequences can be properly annotated > > with comments > > - The people that creates the command sequences has no need for a special > > script to create the file for a display - this is all readable ascii. > > - Using a DT-overlay the parsing of the DT-overlay can be done by > > well-tested functions, no need to invent something new to parse the > > file > > > > > > The idea with a common mipi DBI SPI driver is super, it is the detail > > with the .bin file that I am against. > > > > The fbtft drivers has an init= DT property that contains the command > sequence. Example for the MZ61581 display: > > init = <0x10000b0 00 > 0x1000011 > 0x20000ff > 0x10000b3 0x02 0x00 0x00 0x00 > 0x10000c0 0x13 0x3b 0x00 0x02 0x00 0x01 0x00 0x43 > 0x10000c1 0x08 0x16 0x08 0x08 > 0x10000c4 0x11 0x07 0x03 0x03 > 0x10000c6 0x00 > 0x10000c8 0x03 0x03 0x13 0x5c 0x03 0x07 0x14 0x08 0x00 0x21 0x08 > 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00 > 0x1000035 0x00 > 0x1000036 0xa0 > 0x100003a 0x55 > 0x1000044 0x00 0x01 > 0x10000d0 0x07 0x07 0x1d 0x03 > 0x10000d1 0x03 0x30 0x10 > 0x10000d2 0x03 0x14 0x04 > 0x1000029 > 0x100002c>; > > Parsed here: > https://elixir.bootlin.com/linux/latest/C/ident/fbtft_init_display_from_property > > Before fbdev was closed for new drivers I looked at fixing up fbtft for > proper inclusion and asked on the Device Tree mailinglist about the init > property and how to handle the controller configuration generically. > I was politely told that this should be done in the driver, so when I > made my first DRM driver I made a driver specifically for a panel > (mi0283qt.c). > > Later I found another post that in clear words stated that setting > random registers from DT was not acceptable. Understood and thanks for the explanation. It is a shame that the users loose here :-( Sam ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-20 21:32 ` Sam Ravnborg 0 siblings, 0 replies; 36+ messages in thread From: Sam Ravnborg @ 2022-02-20 21:32 UTC (permalink / raw) To: Noralf Trønnes Cc: devicetree, david, dave.stevenson, dri-devel, robh+dt, thierry.reding, maxime Hi Noralf, On Sun, Feb 20, 2022 at 04:59:34PM +0100, Noralf Trønnes wrote: > > > Den 19.02.2022 23.10, skrev Sam Ravnborg: > > Hi Noralf, > > On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote: > >> Add a driver that will work with most MIPI DBI compatible SPI panels. > >> This avoids adding a driver for every new MIPI DBI compatible controller > >> that is to be used by Linux. The 'compatible' Device Tree property with > >> a '.bin' suffix will be used to load a firmware file that contains the > >> controller configuration. > > A solution where we have the command sequences as part of the DT-overlay > > is IMO a much better way to solve this: > > - The users needs to deal only with a single file, so there is less that > > goes wrong > > - The user need not reading special instructions how to handle a .bin > > file, if the overlay is present everything is fine > > - The file that contains the command sequences can be properly annotated > > with comments > > - The people that creates the command sequences has no need for a special > > script to create the file for a display - this is all readable ascii. > > - Using a DT-overlay the parsing of the DT-overlay can be done by > > well-tested functions, no need to invent something new to parse the > > file > > > > > > The idea with a common mipi DBI SPI driver is super, it is the detail > > with the .bin file that I am against. > > > > The fbtft drivers has an init= DT property that contains the command > sequence. Example for the MZ61581 display: > > init = <0x10000b0 00 > 0x1000011 > 0x20000ff > 0x10000b3 0x02 0x00 0x00 0x00 > 0x10000c0 0x13 0x3b 0x00 0x02 0x00 0x01 0x00 0x43 > 0x10000c1 0x08 0x16 0x08 0x08 > 0x10000c4 0x11 0x07 0x03 0x03 > 0x10000c6 0x00 > 0x10000c8 0x03 0x03 0x13 0x5c 0x03 0x07 0x14 0x08 0x00 0x21 0x08 > 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00 > 0x1000035 0x00 > 0x1000036 0xa0 > 0x100003a 0x55 > 0x1000044 0x00 0x01 > 0x10000d0 0x07 0x07 0x1d 0x03 > 0x10000d1 0x03 0x30 0x10 > 0x10000d2 0x03 0x14 0x04 > 0x1000029 > 0x100002c>; > > Parsed here: > https://elixir.bootlin.com/linux/latest/C/ident/fbtft_init_display_from_property > > Before fbdev was closed for new drivers I looked at fixing up fbtft for > proper inclusion and asked on the Device Tree mailinglist about the init > property and how to handle the controller configuration generically. > I was politely told that this should be done in the driver, so when I > made my first DRM driver I made a driver specifically for a panel > (mi0283qt.c). > > Later I found another post that in clear words stated that setting > random registers from DT was not acceptable. Understood and thanks for the explanation. It is a shame that the users loose here :-( Sam ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver 2022-02-20 21:32 ` Sam Ravnborg @ 2022-02-21 9:05 ` Maxime Ripard -1 siblings, 0 replies; 36+ messages in thread From: Maxime Ripard @ 2022-02-21 9:05 UTC (permalink / raw) To: Sam Ravnborg Cc: devicetree, david, dave.stevenson, robh+dt, Noralf Trønnes, thierry.reding, dri-devel [-- Attachment #1: Type: text/plain, Size: 3568 bytes --] On Sun, Feb 20, 2022 at 10:32:25PM +0100, Sam Ravnborg wrote: > Hi Noralf, > > On Sun, Feb 20, 2022 at 04:59:34PM +0100, Noralf Trønnes wrote: > > > > > > Den 19.02.2022 23.10, skrev Sam Ravnborg: > > > Hi Noralf, > > > On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote: > > >> Add a driver that will work with most MIPI DBI compatible SPI panels. > > >> This avoids adding a driver for every new MIPI DBI compatible controller > > >> that is to be used by Linux. The 'compatible' Device Tree property with > > >> a '.bin' suffix will be used to load a firmware file that contains the > > >> controller configuration. > > > A solution where we have the command sequences as part of the DT-overlay > > > is IMO a much better way to solve this: > > > - The users needs to deal only with a single file, so there is less that > > > goes wrong > > > - The user need not reading special instructions how to handle a .bin > > > file, if the overlay is present everything is fine > > > - The file that contains the command sequences can be properly annotated > > > with comments > > > - The people that creates the command sequences has no need for a special > > > script to create the file for a display - this is all readable ascii. > > > - Using a DT-overlay the parsing of the DT-overlay can be done by > > > well-tested functions, no need to invent something new to parse the > > > file > > > > > > > > > The idea with a common mipi DBI SPI driver is super, it is the detail > > > with the .bin file that I am against. > > > > > > > The fbtft drivers has an init= DT property that contains the command > > sequence. Example for the MZ61581 display: > > > > init = <0x10000b0 00 > > 0x1000011 > > 0x20000ff > > 0x10000b3 0x02 0x00 0x00 0x00 > > 0x10000c0 0x13 0x3b 0x00 0x02 0x00 0x01 0x00 0x43 > > 0x10000c1 0x08 0x16 0x08 0x08 > > 0x10000c4 0x11 0x07 0x03 0x03 > > 0x10000c6 0x00 > > 0x10000c8 0x03 0x03 0x13 0x5c 0x03 0x07 0x14 0x08 0x00 0x21 0x08 > > 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00 > > 0x1000035 0x00 > > 0x1000036 0xa0 > > 0x100003a 0x55 > > 0x1000044 0x00 0x01 > > 0x10000d0 0x07 0x07 0x1d 0x03 > > 0x10000d1 0x03 0x30 0x10 > > 0x10000d2 0x03 0x14 0x04 > > 0x1000029 > > 0x100002c>; > > > > Parsed here: > > https://elixir.bootlin.com/linux/latest/C/ident/fbtft_init_display_from_property > > > > Before fbdev was closed for new drivers I looked at fixing up fbtft for > > proper inclusion and asked on the Device Tree mailinglist about the init > > property and how to handle the controller configuration generically. > > I was politely told that this should be done in the driver, so when I > > made my first DRM driver I made a driver specifically for a panel > > (mi0283qt.c). > > > > Later I found another post that in clear words stated that setting > > random registers from DT was not acceptable. > > Understood and thanks for the explanation. > It is a shame that the users loose here :-( From a user point-of-view, nothing prevents the overlays and the firmware to be in the same package, provided by whatever distro or build-system they would use. The only case where it's a bit less efficient is for a panel that isn't supported already, but it's just a documentation and tooling issue, and Noralf has an awesome track record for both. And the DT syntax throw so much people off that I'm not sure it's such a benefit :) Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver @ 2022-02-21 9:05 ` Maxime Ripard 0 siblings, 0 replies; 36+ messages in thread From: Maxime Ripard @ 2022-02-21 9:05 UTC (permalink / raw) To: Sam Ravnborg Cc: Noralf Trønnes, robh+dt, thierry.reding, dave.stevenson, david, devicetree, dri-devel [-- Attachment #1: Type: text/plain, Size: 3568 bytes --] On Sun, Feb 20, 2022 at 10:32:25PM +0100, Sam Ravnborg wrote: > Hi Noralf, > > On Sun, Feb 20, 2022 at 04:59:34PM +0100, Noralf Trønnes wrote: > > > > > > Den 19.02.2022 23.10, skrev Sam Ravnborg: > > > Hi Noralf, > > > On Fri, Feb 18, 2022 at 04:11:10PM +0100, Noralf Trønnes wrote: > > >> Add a driver that will work with most MIPI DBI compatible SPI panels. > > >> This avoids adding a driver for every new MIPI DBI compatible controller > > >> that is to be used by Linux. The 'compatible' Device Tree property with > > >> a '.bin' suffix will be used to load a firmware file that contains the > > >> controller configuration. > > > A solution where we have the command sequences as part of the DT-overlay > > > is IMO a much better way to solve this: > > > - The users needs to deal only with a single file, so there is less that > > > goes wrong > > > - The user need not reading special instructions how to handle a .bin > > > file, if the overlay is present everything is fine > > > - The file that contains the command sequences can be properly annotated > > > with comments > > > - The people that creates the command sequences has no need for a special > > > script to create the file for a display - this is all readable ascii. > > > - Using a DT-overlay the parsing of the DT-overlay can be done by > > > well-tested functions, no need to invent something new to parse the > > > file > > > > > > > > > The idea with a common mipi DBI SPI driver is super, it is the detail > > > with the .bin file that I am against. > > > > > > > The fbtft drivers has an init= DT property that contains the command > > sequence. Example for the MZ61581 display: > > > > init = <0x10000b0 00 > > 0x1000011 > > 0x20000ff > > 0x10000b3 0x02 0x00 0x00 0x00 > > 0x10000c0 0x13 0x3b 0x00 0x02 0x00 0x01 0x00 0x43 > > 0x10000c1 0x08 0x16 0x08 0x08 > > 0x10000c4 0x11 0x07 0x03 0x03 > > 0x10000c6 0x00 > > 0x10000c8 0x03 0x03 0x13 0x5c 0x03 0x07 0x14 0x08 0x00 0x21 0x08 > > 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00 > > 0x1000035 0x00 > > 0x1000036 0xa0 > > 0x100003a 0x55 > > 0x1000044 0x00 0x01 > > 0x10000d0 0x07 0x07 0x1d 0x03 > > 0x10000d1 0x03 0x30 0x10 > > 0x10000d2 0x03 0x14 0x04 > > 0x1000029 > > 0x100002c>; > > > > Parsed here: > > https://elixir.bootlin.com/linux/latest/C/ident/fbtft_init_display_from_property > > > > Before fbdev was closed for new drivers I looked at fixing up fbtft for > > proper inclusion and asked on the Device Tree mailinglist about the init > > property and how to handle the controller configuration generically. > > I was politely told that this should be done in the driver, so when I > > made my first DRM driver I made a driver specifically for a panel > > (mi0283qt.c). > > > > Later I found another post that in clear words stated that setting > > random registers from DT was not acceptable. > > Understood and thanks for the explanation. > It is a shame that the users loose here :-( From a user point-of-view, nothing prevents the overlays and the firmware to be in the same package, provided by whatever distro or build-system they would use. The only case where it's a bit less efficient is for a panel that isn't supported already, but it's just a documentation and tooling issue, and Noralf has an awesome track record for both. And the DT syntax throw so much people off that I'm not sure it's such a benefit :) Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2022-02-22 17:26 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-18 15:11 [PATCH v4 0/3] drm/tiny: Add MIPI DBI compatible SPI driver Noralf Trønnes 2022-02-18 15:11 ` Noralf Trønnes 2022-02-18 15:11 ` [PATCH v4 1/3] dt-bindings: display: add bindings for MIPI DBI compatible SPI panels Noralf Trønnes 2022-02-18 15:11 ` Noralf Trønnes 2022-02-19 15:25 ` Sam Ravnborg 2022-02-19 15:25 ` Sam Ravnborg 2022-02-21 2:36 ` Rob Herring 2022-02-21 2:36 ` Rob Herring 2022-02-21 11:31 ` Noralf Trønnes 2022-02-21 11:31 ` Noralf Trønnes 2022-02-22 17:26 ` Rob Herring 2022-02-22 17:26 ` Rob Herring 2022-02-18 15:11 ` [PATCH v4 2/3] drm/mipi-dbi: Add driver_private member to struct mipi_dbi_dev Noralf Trønnes 2022-02-18 15:11 ` Noralf Trønnes 2022-02-19 15:25 ` Sam Ravnborg 2022-02-19 15:25 ` Sam Ravnborg 2022-02-18 15:11 ` [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver Noralf Trønnes 2022-02-18 15:11 ` Noralf Trønnes 2022-02-19 22:10 ` Sam Ravnborg 2022-02-19 22:10 ` Sam Ravnborg 2022-02-20 10:04 ` Sam Ravnborg 2022-02-20 14:19 ` Noralf Trønnes 2022-02-20 18:11 ` Noralf Trønnes 2022-02-20 18:11 ` Noralf Trønnes 2022-02-20 19:57 ` Sam Ravnborg 2022-02-20 19:57 ` Sam Ravnborg 2022-02-20 20:34 ` Noralf Trønnes 2022-02-20 20:34 ` Noralf Trønnes 2022-02-20 21:30 ` Sam Ravnborg 2022-02-20 21:30 ` Sam Ravnborg 2022-02-20 15:59 ` Noralf Trønnes 2022-02-20 15:59 ` Noralf Trønnes 2022-02-20 21:32 ` Sam Ravnborg 2022-02-20 21:32 ` Sam Ravnborg 2022-02-21 9:05 ` Maxime Ripard 2022-02-21 9:05 ` Maxime Ripard
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.