Hello,
This patch series adds support to the mxsfb driver for bus width override. The need came from a hardware platform where a 18-bpp panel had the R[5:0], G[5:0] and B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] and LCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] and LCD_DATA[17:12]. The bus width, automatically configured to 18 by querying the panel, is incorrect in this case, and needs to be set to 24.
To solve this issue, a new bus-width DT property is added to the mxsfb DT binding. Patch 1/7 first converts the binding to YAML, with a fix for the compatible string values in 2/7. Patch 3/7 then adds the new property.
Patches 4/7 to 5/7 then fix the DT sources to match the LCDIF bindings, as I noticed during the conversion that the compatible strings were badly managed (see patch 2/7 for a longer explanation). Patch 6/7 drops an unused clock from DT sources.
Patch 7/7 finally adds support for the bus-width property to the mxsfb driver.
Changes compared to v1 are minor and are listed in individual patches.
Laurent Pinchart (7): dt-bindings: display: mxsfb: Convert binding to YAML dt-bindings: display: mxsfb: Add and fix compatible strings dt-bindings: display: mxsfb: Add a bus-width endpoint property ARM: dts: imx: Fix LCDIF compatible strings arm64: dts: imx8mq: Fix LCDIF compatible strings ARM: dts: imx: Remove unneeded LCDIF disp_axi clock drm: mxsfb: Add support for the bus-width DT property
.../bindings/display/fsl,lcdif.yaml | 136 ++++++++++++++++++ .../devicetree/bindings/display/mxsfb.txt | 87 ----------- MAINTAINERS | 2 +- arch/arm/boot/dts/imx6sl.dtsi | 7 +- arch/arm/boot/dts/imx6sll.dtsi | 7 +- arch/arm/boot/dts/imx6sx.dtsi | 4 +- arch/arm/boot/dts/imx6ul.dtsi | 7 +- arch/arm64/boot/dts/freescale/imx8mq.dtsi | 2 +- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 26 ++++ drivers/gpu/drm/mxsfb/mxsfb_drv.h | 2 + drivers/gpu/drm/mxsfb/mxsfb_kms.c | 8 +- 11 files changed, 183 insertions(+), 105 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/fsl,lcdif.yaml delete mode 100644 Documentation/devicetree/bindings/display/mxsfb.txt
Convert the mxsfb binding to YAML. The deprecated binding is dropped, as neither the DT sources nor the driver support it anymore. The converted binding is named fsl,lcdif.yaml to match the usual bindings naming scheme.
The compatible strings are messy, and DT sources use different kinds of combination of documented and undocumented values. Keep it simple for now, and update the example to make it valid. Aligning the binding with the existing DT sources will be performed separately.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Reviewed-by: Sam Ravnborg sam@ravnborg.org -- Changes since v1:
- Drop unneeded quotes in string - Replace minItems with maxItems in conditional check - Add blank line before ... - Squash the rename in this commit --- .../bindings/display/fsl,lcdif.yaml | 116 ++++++++++++++++++ .../devicetree/bindings/display/mxsfb.txt | 87 ------------- MAINTAINERS | 2 +- 3 files changed, 117 insertions(+), 88 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/fsl,lcdif.yaml delete mode 100644 Documentation/devicetree/bindings/display/mxsfb.txt
diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml new file mode 100644 index 000000000000..063bb8c58114 --- /dev/null +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -0,0 +1,116 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/fsl,lcdif.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Freescale/NXP i.MX LCD Interface (LCDIF) + +maintainers: + - Marek Vasut marex@denx.de + - Stefan Agner stefan@agner.ch + +description: | + (e)LCDIF display controller found in the Freescale/NXP i.MX SoCs. + +properties: + compatible: + enum: + - fsl,imx23-lcdif + - fsl,imx28-lcdif + - fsl,imx6sx-lcdif + - fsl,imx8mq-lcdif + + reg: + maxItems: 1 + + clocks: + items: + - description: Pixel clock + - description: Bus clock + - description: Display AXI clock + minItems: 1 + + clock-names: + items: + - const: pix + - const: axi + - const: disp_axi + minItems: 1 + + interrupts: + maxItems: 1 + + port: + description: The LCDIF output port + type: object + + properties: + endpoint: + type: object + + properties: + remote-endpoint: + $ref: /schemas/types.yaml#/definitions/phandle + + required: + - remote-endpoint + + additionalProperties: false + + additionalProperties: false + +required: + - compatible + - reg + - clocks + - interrupts + - port + +additionalProperties: false + +allOf: + - if: + properties: + compatible: + contains: + const: fsl,imx6sx-lcdif + then: + properties: + clocks: + minItems: 2 + maxItems: 3 + clock-names: + minItems: 2 + maxItems: 3 + required: + - clock-names + else: + properties: + clocks: + maxItems: 1 + clock-names: + maxItems: 1 + +examples: + - | + #include <dt-bindings/clock/imx6sx-clock.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + display-controller@2220000 { + compatible = "fsl,imx6sx-lcdif"; + reg = <0x02220000 0x4000>; + interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, + <&clks IMX6SX_CLK_LCDIF_APB>, + <&clks IMX6SX_CLK_DISPLAY_AXI>; + clock-names = "pix", "axi", "disp_axi"; + + port { + endpoint { + remote-endpoint = <&panel_in>; + }; + }; + }; + +... diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt deleted file mode 100644 index c985871c46b3..000000000000 --- a/Documentation/devicetree/bindings/display/mxsfb.txt +++ /dev/null @@ -1,87 +0,0 @@ -* Freescale MXS LCD Interface (LCDIF) - -New bindings: -============= -Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. - Should be "fsl,imx28-lcdif" for i.MX28. - Should be "fsl,imx6sx-lcdif" for i.MX6SX. - Should be "fsl,imx8mq-lcdif" for i.MX8MQ. -- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupt -- clocks: A list of phandle + clock-specifier pairs, one for each - entry in 'clock-names'. -- clock-names: A list of clock names. For MXSFB it should contain: - - "pix" for the LCDIF block clock - - (MX6SX-only) "axi", "disp_axi" for the bus interface clock - -Required sub-nodes: - - port: The connection to an encoder chip. - -Example: - - lcdif1: display-controller@2220000 { - compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; - reg = <0x02220000 0x4000>; - interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, - <&clks IMX6SX_CLK_LCDIF_APB>, - <&clks IMX6SX_CLK_DISPLAY_AXI>; - clock-names = "pix", "axi", "disp_axi"; - - port { - parallel_out: endpoint { - remote-endpoint = <&panel_in_parallel>; - }; - }; - }; - -Deprecated bindings: -==================== -Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. - Should be "fsl,imx28-lcdif" for i.MX28. -- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupts -- display: phandle to display node (see below for details) - -* display node - -Required properties: -- bits-per-pixel: <16> for RGB565, <32> for RGB888/666. -- bus-width: number of data lines. Could be <8>, <16>, <18> or <24>. - -Required sub-node: -- display-timings: Refer to binding doc display-timing.txt for details. - -Examples: - -lcdif@80030000 { - compatible = "fsl,imx28-lcdif"; - reg = <0x80030000 2000>; - interrupts = <38 86>; - - display: display { - bits-per-pixel = <32>; - bus-width = <24>; - - display-timings { - native-mode = <&timing0>; - timing0: timing0 { - clock-frequency = <33500000>; - hactive = <800>; - vactive = <480>; - hfront-porch = <164>; - hback-porch = <89>; - hsync-len = <10>; - vback-porch = <23>; - vfront-porch = <10>; - vsync-len = <10>; - hsync-active = <0>; - vsync-active = <0>; - de-active = <1>; - pixelclk-active = <0>; - }; - }; - }; -}; diff --git a/MAINTAINERS b/MAINTAINERS index f0dd1f01703a..87e20680c104 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11910,7 +11910,7 @@ M: Stefan Agner stefan@agner.ch L: dri-devel@lists.freedesktop.org S: Supported T: git git://anongit.freedesktop.org/drm/drm-misc -F: Documentation/devicetree/bindings/display/mxsfb.txt +F: Documentation/devicetree/bindings/display/fsl,lcdif.yaml F: drivers/gpu/drm/mxsfb/
MYLEX DAC960 PCI RAID Controller
On 10/7/20 3:24 AM, Laurent Pinchart wrote: [...]
+properties:
- compatible:
- enum:
- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- fsl,imx8mq-lcdif
There is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, so the DT must specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4).
Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or dropped from the YAML file or neither ?
[...]
On Mi, 2020-10-07 at 10:32 +0200, Marek Vasut wrote:
On 10/7/20 3:24 AM, Laurent Pinchart wrote: [...]
+properties:
- compatible:
- enum:
- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- fsl,imx8mq-lcdifThere is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, so the DT must specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4).
Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or dropped from the YAML file or neither ?
Neither. As far as we know the block is compatible, so the DT should claim that it's compatible to "fsl,imx28-lcdif". However we don't know if there are any surprises, so we add the SoC specific compatible to be able to change the driver matching later without changing the DT if the need arises. For the DT validation to pass the SoC specific compatible needs to be documented, even if it currently unused by the driver.
Regards, Lucas
On 10/7/20 10:43 AM, Lucas Stach wrote:
On Mi, 2020-10-07 at 10:32 +0200, Marek Vasut wrote:
On 10/7/20 3:24 AM, Laurent Pinchart wrote: [...]
+properties:
- compatible:
- enum:
- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- fsl,imx8mq-lcdifThere is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, so the DT must specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4).
Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or dropped from the YAML file or neither ?
Neither. As far as we know the block is compatible, so the DT should claim that it's compatible to "fsl,imx28-lcdif". However we don't know if there are any surprises, so we add the SoC specific compatible to be able to change the driver matching later without changing the DT if the need arises. For the DT validation to pass the SoC specific compatible needs to be documented, even if it currently unused by the driver.
What in that binding says you should specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif"; and not e.g. "fsl,imx8mq-lcdif", "fsl,imx23-lcdif" or simply "fsl,imx8mq-lcdif" ?
Hi Marek,
On Wed, Oct 07, 2020 at 10:55:24AM +0200, Marek Vasut wrote:
On 10/7/20 10:43 AM, Lucas Stach wrote:
On Mi, 2020-10-07 at 10:32 +0200, Marek Vasut wrote:
On 10/7/20 3:24 AM, Laurent Pinchart wrote: [...]
+properties:
- compatible:
- enum:
- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- fsl,imx8mq-lcdifThere is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, so the DT must specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4).
Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or dropped from the YAML file or neither ?
Neither. As far as we know the block is compatible, so the DT should claim that it's compatible to "fsl,imx28-lcdif". However we don't know if there are any surprises, so we add the SoC specific compatible to be able to change the driver matching later without changing the DT if the need arises. For the DT validation to pass the SoC specific compatible needs to be documented, even if it currently unused by the driver.
What in that binding says you should specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif"; and not e.g. "fsl,imx8mq-lcdif", "fsl,imx23-lcdif" or simply "fsl,imx8mq-lcdif" ?
Nothing, until the next patch :-) This patch only handles the YAML conversion but doesn't fix issues.
On 10/7/20 3:33 PM, Laurent Pinchart wrote:
Hi Marek,
On Wed, Oct 07, 2020 at 10:55:24AM +0200, Marek Vasut wrote:
On 10/7/20 10:43 AM, Lucas Stach wrote:
On Mi, 2020-10-07 at 10:32 +0200, Marek Vasut wrote:
On 10/7/20 3:24 AM, Laurent Pinchart wrote: [...]
+properties:
- compatible:
- enum:
- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- fsl,imx8mq-lcdifThere is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, so the DT must specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4).
Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or dropped from the YAML file or neither ?
Neither. As far as we know the block is compatible, so the DT should claim that it's compatible to "fsl,imx28-lcdif". However we don't know if there are any surprises, so we add the SoC specific compatible to be able to change the driver matching later without changing the DT if the need arises. For the DT validation to pass the SoC specific compatible needs to be documented, even if it currently unused by the driver.
What in that binding says you should specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif"; and not e.g. "fsl,imx8mq-lcdif", "fsl,imx23-lcdif" or simply "fsl,imx8mq-lcdif" ?
Nothing, until the next patch :-) This patch only handles the YAML conversion but doesn't fix issues.
Good, thanks !
On Wed, Oct 07, 2020 at 04:24:32AM +0300, Laurent Pinchart wrote:
Convert the mxsfb binding to YAML. The deprecated binding is dropped, as neither the DT sources nor the driver support it anymore. The converted binding is named fsl,lcdif.yaml to match the usual bindings naming scheme.
The compatible strings are messy, and DT sources use different kinds of combination of documented and undocumented values. Keep it simple for now, and update the example to make it valid. Aligning the binding with the existing DT sources will be performed separately.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Reviewed-by: Sam Ravnborg sam@ravnborg.org -- Changes since v1:
- Drop unneeded quotes in string
- Replace minItems with maxItems in conditional check
- Add blank line before ...
- Squash the rename in this commit
.../bindings/display/fsl,lcdif.yaml | 116 ++++++++++++++++++ .../devicetree/bindings/display/mxsfb.txt | 87 ------------- MAINTAINERS | 2 +- 3 files changed, 117 insertions(+), 88 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/fsl,lcdif.yaml delete mode 100644 Documentation/devicetree/bindings/display/mxsfb.txt
diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml new file mode 100644 index 000000000000..063bb8c58114 --- /dev/null +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -0,0 +1,116 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/fsl,lcdif.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Freescale/NXP i.MX LCD Interface (LCDIF)
+maintainers:
- Marek Vasut marex@denx.de
- Stefan Agner stefan@agner.ch
+description: |
- (e)LCDIF display controller found in the Freescale/NXP i.MX SoCs.
+properties:
- compatible:
- enum:
- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- fsl,imx8mq-lcdif- reg:
- maxItems: 1
- clocks:
- items:
- description: Pixel clock- description: Bus clock- description: Display AXI clock- minItems: 1
- clock-names:
- items:
- const: pix- const: axi- const: disp_axi- minItems: 1
- interrupts:
- maxItems: 1
- port:
- description: The LCDIF output port
- type: object
- properties:
endpoint:
What happened on the graph binding schema work? I started a meta-schema for it BTW.
You can drop all the endpoint parts. With that,
Reviewed-by: Rob Herring robh@kernel.org
type: objectproperties:remote-endpoint:$ref: /schemas/types.yaml#/definitions/phandlerequired:- remote-endpointadditionalProperties: false- additionalProperties: false
+required:
- compatible
- reg
- clocks
- interrupts
- port
+additionalProperties: false
+allOf:
- if:
properties:compatible:contains:const: fsl,imx6sx-lcdif- then:
properties:clocks:minItems: 2maxItems: 3clock-names:minItems: 2maxItems: 3required:- clock-names- else:
properties:clocks:maxItems: 1clock-names:maxItems: 1+examples:
- |
- #include <dt-bindings/clock/imx6sx-clock.h>
- #include <dt-bindings/interrupt-controller/arm-gic.h>
- display-controller@2220000 {
compatible = "fsl,imx6sx-lcdif";reg = <0x02220000 0x4000>;interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>,<&clks IMX6SX_CLK_LCDIF_APB>,<&clks IMX6SX_CLK_DISPLAY_AXI>;clock-names = "pix", "axi", "disp_axi";port {endpoint {remote-endpoint = <&panel_in>;};};- };
+... diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt deleted file mode 100644 index c985871c46b3..000000000000 --- a/Documentation/devicetree/bindings/display/mxsfb.txt +++ /dev/null @@ -1,87 +0,0 @@ -* Freescale MXS LCD Interface (LCDIF)
-New bindings:
-Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23.
Should be "fsl,imx28-lcdif" for i.MX28.Should be "fsl,imx6sx-lcdif" for i.MX6SX.Should be "fsl,imx8mq-lcdif" for i.MX8MQ.-- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupt -- clocks: A list of phandle + clock-specifier pairs, one for each
entry in 'clock-names'.-- clock-names: A list of clock names. For MXSFB it should contain:
- "pix" for the LCDIF block clock
- (MX6SX-only) "axi", "disp_axi" for the bus interface clock
-Required sub-nodes:
- port: The connection to an encoder chip.
-Example:
- lcdif1: display-controller@2220000 {
compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif";reg = <0x02220000 0x4000>;interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>,<&clks IMX6SX_CLK_LCDIF_APB>,<&clks IMX6SX_CLK_DISPLAY_AXI>;clock-names = "pix", "axi", "disp_axi";port {parallel_out: endpoint {remote-endpoint = <&panel_in_parallel>;};};- };
-Deprecated bindings:
-Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23.
Should be "fsl,imx28-lcdif" for i.MX28.-- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupts -- display: phandle to display node (see below for details)
-* display node
-Required properties: -- bits-per-pixel: <16> for RGB565, <32> for RGB888/666. -- bus-width: number of data lines. Could be <8>, <16>, <18> or <24>.
-Required sub-node: -- display-timings: Refer to binding doc display-timing.txt for details.
-Examples:
-lcdif@80030000 {
- compatible = "fsl,imx28-lcdif";
- reg = <0x80030000 2000>;
- interrupts = <38 86>;
- display: display {
bits-per-pixel = <32>;bus-width = <24>;display-timings {native-mode = <&timing0>;timing0: timing0 {clock-frequency = <33500000>;hactive = <800>;vactive = <480>;hfront-porch = <164>;hback-porch = <89>;hsync-len = <10>;vback-porch = <23>;vfront-porch = <10>;vsync-len = <10>;hsync-active = <0>;vsync-active = <0>;de-active = <1>;pixelclk-active = <0>;};};- };
-}; diff --git a/MAINTAINERS b/MAINTAINERS index f0dd1f01703a..87e20680c104 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11910,7 +11910,7 @@ M: Stefan Agner stefan@agner.ch L: dri-devel@lists.freedesktop.org S: Supported T: git git://anongit.freedesktop.org/drm/drm-misc -F: Documentation/devicetree/bindings/display/mxsfb.txt +F: Documentation/devicetree/bindings/display/fsl,lcdif.yaml F: drivers/gpu/drm/mxsfb/
MYLEX DAC960 PCI RAID Controller
Regards,
Laurent Pinchart
On Wed, Oct 07, 2020 at 11:00:20AM -0500, Rob Herring wrote:
On Wed, Oct 07, 2020 at 04:24:32AM +0300, Laurent Pinchart wrote:
Convert the mxsfb binding to YAML. The deprecated binding is dropped, as neither the DT sources nor the driver support it anymore. The converted binding is named fsl,lcdif.yaml to match the usual bindings naming scheme.
The compatible strings are messy, and DT sources use different kinds of combination of documented and undocumented values. Keep it simple for now, and update the example to make it valid. Aligning the binding with the existing DT sources will be performed separately.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Reviewed-by: Sam Ravnborg sam@ravnborg.org -- Changes since v1:
- Drop unneeded quotes in string
- Replace minItems with maxItems in conditional check
- Add blank line before ...
- Squash the rename in this commit
.../bindings/display/fsl,lcdif.yaml | 116 ++++++++++++++++++ .../devicetree/bindings/display/mxsfb.txt | 87 ------------- MAINTAINERS | 2 +- 3 files changed, 117 insertions(+), 88 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/fsl,lcdif.yaml delete mode 100644 Documentation/devicetree/bindings/display/mxsfb.txt
diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml new file mode 100644 index 000000000000..063bb8c58114 --- /dev/null +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -0,0 +1,116 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/fsl,lcdif.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Freescale/NXP i.MX LCD Interface (LCDIF)
+maintainers:
- Marek Vasut marex@denx.de
- Stefan Agner stefan@agner.ch
+description: |
- (e)LCDIF display controller found in the Freescale/NXP i.MX SoCs.
+properties:
- compatible:
- enum:
- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- fsl,imx8mq-lcdif- reg:
- maxItems: 1
- clocks:
- items:
- description: Pixel clock- description: Bus clock- description: Display AXI clock- minItems: 1
- clock-names:
- items:
- const: pix- const: axi- const: disp_axi- minItems: 1
- interrupts:
- maxItems: 1
- port:
- description: The LCDIF output port
- type: object
- properties:
endpoint:What happened on the graph binding schema work? I started a meta-schema for it BTW.
You can drop all the endpoint parts. With that,
NM, I see in patch 3 you need it.
Reviewed-by: Rob Herring robh@kernel.org
type: objectproperties:remote-endpoint:$ref: /schemas/types.yaml#/definitions/phandlerequired:- remote-endpointadditionalProperties: false- additionalProperties: false
+required:
- compatible
- reg
- clocks
- interrupts
- port
+additionalProperties: false
+allOf:
- if:
properties:compatible:contains:const: fsl,imx6sx-lcdif- then:
properties:clocks:minItems: 2maxItems: 3clock-names:minItems: 2maxItems: 3required:- clock-names- else:
properties:clocks:maxItems: 1clock-names:maxItems: 1+examples:
- |
- #include <dt-bindings/clock/imx6sx-clock.h>
- #include <dt-bindings/interrupt-controller/arm-gic.h>
- display-controller@2220000 {
compatible = "fsl,imx6sx-lcdif";reg = <0x02220000 0x4000>;interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>,<&clks IMX6SX_CLK_LCDIF_APB>,<&clks IMX6SX_CLK_DISPLAY_AXI>;clock-names = "pix", "axi", "disp_axi";port {endpoint {remote-endpoint = <&panel_in>;};};- };
+... diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt deleted file mode 100644 index c985871c46b3..000000000000 --- a/Documentation/devicetree/bindings/display/mxsfb.txt +++ /dev/null @@ -1,87 +0,0 @@ -* Freescale MXS LCD Interface (LCDIF)
-New bindings:
-Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23.
Should be "fsl,imx28-lcdif" for i.MX28.Should be "fsl,imx6sx-lcdif" for i.MX6SX.Should be "fsl,imx8mq-lcdif" for i.MX8MQ.-- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupt -- clocks: A list of phandle + clock-specifier pairs, one for each
entry in 'clock-names'.-- clock-names: A list of clock names. For MXSFB it should contain:
- "pix" for the LCDIF block clock
- (MX6SX-only) "axi", "disp_axi" for the bus interface clock
-Required sub-nodes:
- port: The connection to an encoder chip.
-Example:
- lcdif1: display-controller@2220000 {
compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif";reg = <0x02220000 0x4000>;interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>,<&clks IMX6SX_CLK_LCDIF_APB>,<&clks IMX6SX_CLK_DISPLAY_AXI>;clock-names = "pix", "axi", "disp_axi";port {parallel_out: endpoint {remote-endpoint = <&panel_in_parallel>;};};- };
-Deprecated bindings:
-Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23.
Should be "fsl,imx28-lcdif" for i.MX28.-- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupts -- display: phandle to display node (see below for details)
-* display node
-Required properties: -- bits-per-pixel: <16> for RGB565, <32> for RGB888/666. -- bus-width: number of data lines. Could be <8>, <16>, <18> or <24>.
-Required sub-node: -- display-timings: Refer to binding doc display-timing.txt for details.
-Examples:
-lcdif@80030000 {
- compatible = "fsl,imx28-lcdif";
- reg = <0x80030000 2000>;
- interrupts = <38 86>;
- display: display {
bits-per-pixel = <32>;bus-width = <24>;display-timings {native-mode = <&timing0>;timing0: timing0 {clock-frequency = <33500000>;hactive = <800>;vactive = <480>;hfront-porch = <164>;hback-porch = <89>;hsync-len = <10>;vback-porch = <23>;vfront-porch = <10>;vsync-len = <10>;hsync-active = <0>;vsync-active = <0>;de-active = <1>;pixelclk-active = <0>;};};- };
-}; diff --git a/MAINTAINERS b/MAINTAINERS index f0dd1f01703a..87e20680c104 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11910,7 +11910,7 @@ M: Stefan Agner stefan@agner.ch L: dri-devel@lists.freedesktop.org S: Supported T: git git://anongit.freedesktop.org/drm/drm-misc -F: Documentation/devicetree/bindings/display/mxsfb.txt +F: Documentation/devicetree/bindings/display/fsl,lcdif.yaml F: drivers/gpu/drm/mxsfb/
MYLEX DAC960 PCI RAID Controller
Regards,
Laurent Pinchart
Hi Rob,
On Wed, Oct 07, 2020 at 11:00:20AM -0500, Rob Herring wrote:
On Wed, Oct 07, 2020 at 04:24:32AM +0300, Laurent Pinchart wrote:
Convert the mxsfb binding to YAML. The deprecated binding is dropped, as neither the DT sources nor the driver support it anymore. The converted binding is named fsl,lcdif.yaml to match the usual bindings naming scheme.
The compatible strings are messy, and DT sources use different kinds of combination of documented and undocumented values. Keep it simple for now, and update the example to make it valid. Aligning the binding with the existing DT sources will be performed separately.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Reviewed-by: Sam Ravnborg sam@ravnborg.org -- Changes since v1:
- Drop unneeded quotes in string
- Replace minItems with maxItems in conditional check
- Add blank line before ...
- Squash the rename in this commit
.../bindings/display/fsl,lcdif.yaml | 116 ++++++++++++++++++ .../devicetree/bindings/display/mxsfb.txt | 87 ------------- MAINTAINERS | 2 +- 3 files changed, 117 insertions(+), 88 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/fsl,lcdif.yaml delete mode 100644 Documentation/devicetree/bindings/display/mxsfb.txt
diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml new file mode 100644 index 000000000000..063bb8c58114 --- /dev/null +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -0,0 +1,116 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/fsl,lcdif.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Freescale/NXP i.MX LCD Interface (LCDIF)
+maintainers:
- Marek Vasut marex@denx.de
- Stefan Agner stefan@agner.ch
+description: |
- (e)LCDIF display controller found in the Freescale/NXP i.MX SoCs.
+properties:
- compatible:
- enum:
- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- fsl,imx8mq-lcdif- reg:
- maxItems: 1
- clocks:
- items:
- description: Pixel clock- description: Bus clock- description: Display AXI clock- minItems: 1
- clock-names:
- items:
- const: pix- const: axi- const: disp_axi- minItems: 1
- interrupts:
- maxItems: 1
- port:
- description: The LCDIF output port
- type: object
- properties:
endpoint:What happened on the graph binding schema work?
Still on my todo list, I hope to switch back to that task in the not too distant future.
I started a meta-schema for it BTW.
Nice :-) Is it available in a public tree ?
You can drop all the endpoint parts. With that,
Reviewed-by: Rob Herring robh@kernel.org
type: objectproperties:remote-endpoint:$ref: /schemas/types.yaml#/definitions/phandlerequired:- remote-endpointadditionalProperties: false- additionalProperties: false
+required:
- compatible
- reg
- clocks
- interrupts
- port
+additionalProperties: false
+allOf:
- if:
properties:compatible:contains:const: fsl,imx6sx-lcdif- then:
properties:clocks:minItems: 2maxItems: 3clock-names:minItems: 2maxItems: 3required:- clock-names- else:
properties:clocks:maxItems: 1clock-names:maxItems: 1+examples:
- |
- #include <dt-bindings/clock/imx6sx-clock.h>
- #include <dt-bindings/interrupt-controller/arm-gic.h>
- display-controller@2220000 {
compatible = "fsl,imx6sx-lcdif";reg = <0x02220000 0x4000>;interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>,<&clks IMX6SX_CLK_LCDIF_APB>,<&clks IMX6SX_CLK_DISPLAY_AXI>;clock-names = "pix", "axi", "disp_axi";port {endpoint {remote-endpoint = <&panel_in>;};};- };
+... diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt deleted file mode 100644 index c985871c46b3..000000000000 --- a/Documentation/devicetree/bindings/display/mxsfb.txt +++ /dev/null @@ -1,87 +0,0 @@ -* Freescale MXS LCD Interface (LCDIF)
-New bindings:
-Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23.
Should be "fsl,imx28-lcdif" for i.MX28.Should be "fsl,imx6sx-lcdif" for i.MX6SX.Should be "fsl,imx8mq-lcdif" for i.MX8MQ.-- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupt -- clocks: A list of phandle + clock-specifier pairs, one for each
entry in 'clock-names'.-- clock-names: A list of clock names. For MXSFB it should contain:
- "pix" for the LCDIF block clock
- (MX6SX-only) "axi", "disp_axi" for the bus interface clock
-Required sub-nodes:
- port: The connection to an encoder chip.
-Example:
- lcdif1: display-controller@2220000 {
compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif";reg = <0x02220000 0x4000>;interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>,<&clks IMX6SX_CLK_LCDIF_APB>,<&clks IMX6SX_CLK_DISPLAY_AXI>;clock-names = "pix", "axi", "disp_axi";port {parallel_out: endpoint {remote-endpoint = <&panel_in_parallel>;};};- };
-Deprecated bindings:
-Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23.
Should be "fsl,imx28-lcdif" for i.MX28.-- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupts -- display: phandle to display node (see below for details)
-* display node
-Required properties: -- bits-per-pixel: <16> for RGB565, <32> for RGB888/666. -- bus-width: number of data lines. Could be <8>, <16>, <18> or <24>.
-Required sub-node: -- display-timings: Refer to binding doc display-timing.txt for details.
-Examples:
-lcdif@80030000 {
- compatible = "fsl,imx28-lcdif";
- reg = <0x80030000 2000>;
- interrupts = <38 86>;
- display: display {
bits-per-pixel = <32>;bus-width = <24>;display-timings {native-mode = <&timing0>;timing0: timing0 {clock-frequency = <33500000>;hactive = <800>;vactive = <480>;hfront-porch = <164>;hback-porch = <89>;hsync-len = <10>;vback-porch = <23>;vfront-porch = <10>;vsync-len = <10>;hsync-active = <0>;vsync-active = <0>;de-active = <1>;pixelclk-active = <0>;};};- };
-}; diff --git a/MAINTAINERS b/MAINTAINERS index f0dd1f01703a..87e20680c104 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11910,7 +11910,7 @@ M: Stefan Agner stefan@agner.ch L: dri-devel@lists.freedesktop.org S: Supported T: git git://anongit.freedesktop.org/drm/drm-misc -F: Documentation/devicetree/bindings/display/mxsfb.txt +F: Documentation/devicetree/bindings/display/fsl,lcdif.yaml F: drivers/gpu/drm/mxsfb/
MYLEX DAC960 PCI RAID Controller
hi Laurent,
Do you mind me adding a DT property in the old format now? If so, I'd appreciated an ack in this thread: https://lore.kernel.org/linux-arm-kernel/20201201134638.GA305734@bogon.m.sig...
If it causes extra work for you and want to resend your series soon, I'll gladly delay them for later.
thanks, martin
Hi Martin,
On Fri, Jan 15, 2021 at 08:59:18AM +0100, Martin Kepplinger wrote:
hi Laurent,
Do you mind me adding a DT property in the old format now? If so, I'd appreciated an ack in this thread: https://lore.kernel.org/linux-arm-kernel/20201201134638.GA305734@bogon.m.sig...
If it causes extra work for you and want to resend your series soon, I'll gladly delay them for later.
I think the conversion ot YAML is ready. I've split it from the rest of my series, and posted a v3, asking Rob to merge it. Would you mind rebasing the addition of the new properties on top ?
Am 15. Jänner 2021 23:25:14 MEZ schrieb Laurent Pinchart laurent.pinchart@ideasonboard.com:
Hi Martin,
On Fri, Jan 15, 2021 at 08:59:18AM +0100, Martin Kepplinger wrote:
hi Laurent,
Do you mind me adding a DT property in the old format now? If so, I'd appreciated an ack in this thread:
https://lore.kernel.org/linux-arm-kernel/20201201134638.GA305734@bogon.m.sig...
If it causes extra work for you and want to resend your series soon,
I'll
gladly delay them for later.
I think the conversion ot YAML is ready. I've split it from the rest of my series, and posted a v3, asking Rob to merge it. Would you mind rebasing the addition of the new properties on top ?
Hi Laurent,
thanks for the timely answer. sounds good; I'll rebase.
martin
Additional compatible strings have been added in DT source for the i.MX6SL, i.MX6SLL, i.MX6UL and i.MX7D without updating the bindings. Most of the upstream DT sources use the fsl,imx28-lcdif compatible string, which mostly predates the realization that the LCDIF in the i.MX6 and newer SoCs have extra features compared to the i.MX28.
Update the bindings to add the missing compatible strings, with the correct fallback values. This fails to validate some of the upstream DT sources. Instead of adding the incorrect compatible fallback to the binding, the sources should be updated separately.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Reviewed-by: Sam Ravnborg sam@ravnborg.org --- Changes since v1:
- Fix indentation under enum --- .../devicetree/bindings/display/fsl,lcdif.yaml | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml index 063bb8c58114..404bd516b7f5 100644 --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -15,11 +15,19 @@ description: |
properties: compatible: - enum: - - fsl,imx23-lcdif - - fsl,imx28-lcdif - - fsl,imx6sx-lcdif - - fsl,imx8mq-lcdif + oneOf: + - enum: + - fsl,imx23-lcdif + - fsl,imx28-lcdif + - fsl,imx6sx-lcdif + - items: + - enum: + - fsl,imx6sl-lcdif + - fsl,imx6sll-lcdif + - fsl,imx6ul-lcdif + - fsl,imx7d-lcdif + - fsl,imx8mq-lcdif + - const: fsl,imx6sx-lcdif
reg: maxItems: 1
On 10/7/20 3:24 AM, Laurent Pinchart wrote:
[...]
properties: compatible:
- enum:
- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- fsl,imx8mq-lcdif
- oneOf:
- enum:- fsl,imx23-lcdif- fsl,imx28-lcdif- fsl,imx6sx-lcdif- items:- enum:- fsl,imx6sl-lcdif- fsl,imx6sll-lcdif- fsl,imx6ul-lcdif- fsl,imx7d-lcdif- fsl,imx8mq-lcdif- const: fsl,imx6sx-lcdif
You can also add fsl,imx8mm-lcdif into the list if you want, i.MX8MM has LCDIF V4 too.
[...]
On Wed, 07 Oct 2020 04:24:33 +0300, Laurent Pinchart wrote:
Additional compatible strings have been added in DT source for the i.MX6SL, i.MX6SLL, i.MX6UL and i.MX7D without updating the bindings. Most of the upstream DT sources use the fsl,imx28-lcdif compatible string, which mostly predates the realization that the LCDIF in the i.MX6 and newer SoCs have extra features compared to the i.MX28.
Update the bindings to add the missing compatible strings, with the correct fallback values. This fails to validate some of the upstream DT sources. Instead of adding the incorrect compatible fallback to the binding, the sources should be updated separately.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Reviewed-by: Sam Ravnborg sam@ravnborg.org
Changes since v1:
- Fix indentation under enum
.../devicetree/bindings/display/fsl,lcdif.yaml | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
Reviewed-by: Rob Herring robh@kernel.org
When the PCB routes the display data signals in an unconventional way, the output bus width may differ from the bus width of the connected panel or encoder. For instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] and LCD_DATA[23:18], the output bus width is 24 instead of 18 when the signals are routed to LCD_DATA[5:0], LCD_DATA[11:6] and LCD_DATA[17:12].
Add a bus-width property to describe this data routing.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- Changes since v1:
- Fix property name in binding --- .../devicetree/bindings/display/fsl,lcdif.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml index 404bd516b7f5..14b6103a9bd1 100644 --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -58,6 +58,18 @@ properties: type: object
properties: + bus-width: + enum: [16, 18, 24] + description: | + The output bus width. This value overrides the configuration + derived from the connected device (encoder or panel). It should + only be specified when PCB routing of the data signals require a + different bus width on the LCDIF and the connected device. For + instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and + B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] and + LCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] and + LCD_DATA[17:12], bus-width should be set to 24. + remote-endpoint: $ref: /schemas/types.yaml#/definitions/phandle
On 10/7/20 3:24 AM, Laurent Pinchart wrote:
[...]
bus-width:enum: [16, 18, 24]description: |The output bus width. This value overrides the configurationderived from the connected device (encoder or panel). It shouldonly be specified when PCB routing of the data signals require adifferent bus width on the LCDIF and the connected device. Forinstance, when a 18-bit RGB panel has its R[5:0], G[5:0] andB[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] andLCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] andLCD_DATA[17:12], bus-width should be set to 24.
The iMX6 IPUv3 uses interface-pix-fmt which is a bit more flexible, but I'm not sure whether it's the right way to go about this, see: Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt
Hi Marek,
On Wed, Oct 07, 2020 at 10:40:26AM +0200, Marek Vasut wrote:
On 10/7/20 3:24 AM, Laurent Pinchart wrote:
[...]
bus-width:enum: [16, 18, 24]description: |The output bus width. This value overrides the configurationderived from the connected device (encoder or panel). It shouldonly be specified when PCB routing of the data signals require adifferent bus width on the LCDIF and the connected device. Forinstance, when a 18-bit RGB panel has its R[5:0], G[5:0] andB[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] andLCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] andLCD_DATA[17:12], bus-width should be set to 24.The iMX6 IPUv3 uses interface-pix-fmt which is a bit more flexible, but I'm not sure whether it's the right way to go about this, see: Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt
I think specifying the bus with is better. It's a standard property, but more than that, a given bus width can carry different formats. For instance, a 24-bus could carry RGB666 data (with dithering for the LSBs).
On 10/10/20 1:58 AM, Laurent Pinchart wrote:
Hi Marek,
Hi,
On Wed, Oct 07, 2020 at 10:40:26AM +0200, Marek Vasut wrote:
On 10/7/20 3:24 AM, Laurent Pinchart wrote:
[...]
bus-width:enum: [16, 18, 24]description: |The output bus width. This value overrides the configurationderived from the connected device (encoder or panel). It shouldonly be specified when PCB routing of the data signals require adifferent bus width on the LCDIF and the connected device. Forinstance, when a 18-bit RGB panel has its R[5:0], G[5:0] andB[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] andLCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] andLCD_DATA[17:12], bus-width should be set to 24.The iMX6 IPUv3 uses interface-pix-fmt which is a bit more flexible, but I'm not sure whether it's the right way to go about this, see: Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt
I think specifying the bus with is better. It's a standard property, but more than that, a given bus width can carry different formats. For instance, a 24-bus could carry RGB666 data (with dithering for the LSBs).
I think that's exactly what the interface-pix-fmt was trying to solve for the IPUv3, there you could have e.g. both RGB666 and LVDS666 , which were different.
Hi Marek,
On Sat, Oct 10, 2020 at 10:47:05AM +0200, Marek Vasut wrote:
On 10/10/20 1:58 AM, Laurent Pinchart wrote:
Hi Marek,
Hi,
On Wed, Oct 07, 2020 at 10:40:26AM +0200, Marek Vasut wrote:
On 10/7/20 3:24 AM, Laurent Pinchart wrote:
[...]
bus-width:enum: [16, 18, 24]description: |The output bus width. This value overrides the configurationderived from the connected device (encoder or panel). It shouldonly be specified when PCB routing of the data signals require adifferent bus width on the LCDIF and the connected device. Forinstance, when a 18-bit RGB panel has its R[5:0], G[5:0] andB[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] andLCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] andLCD_DATA[17:12], bus-width should be set to 24.The iMX6 IPUv3 uses interface-pix-fmt which is a bit more flexible, but I'm not sure whether it's the right way to go about this, see: Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt
I think specifying the bus with is better. It's a standard property, but more than that, a given bus width can carry different formats. For instance, a 24-bus could carry RGB666 data (with dithering for the LSBs).
I think that's exactly what the interface-pix-fmt was trying to solve for the IPUv3, there you could have e.g. both RGB666 and LVDS666 , which were different.
My point is that the driver should support multiple formats that can be carried over a given bus width, with the actual format to be used queried from the sink (usually a panel) instead of being hardcoded in DT.
On 10/13/20 4:06 AM, Laurent Pinchart wrote:
Hi Marek,
On Sat, Oct 10, 2020 at 10:47:05AM +0200, Marek Vasut wrote:
On 10/10/20 1:58 AM, Laurent Pinchart wrote:
Hi Marek,
Hi,
On Wed, Oct 07, 2020 at 10:40:26AM +0200, Marek Vasut wrote:
On 10/7/20 3:24 AM, Laurent Pinchart wrote:
[...]
bus-width:enum: [16, 18, 24]description: |The output bus width. This value overrides the configurationderived from the connected device (encoder or panel). It shouldonly be specified when PCB routing of the data signals require adifferent bus width on the LCDIF and the connected device. Forinstance, when a 18-bit RGB panel has its R[5:0], G[5:0] andB[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] andLCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] andLCD_DATA[17:12], bus-width should be set to 24.The iMX6 IPUv3 uses interface-pix-fmt which is a bit more flexible, but I'm not sure whether it's the right way to go about this, see: Documentation/devicetree/bindings/display/imx/fsl-imx-drm.txt
I think specifying the bus with is better. It's a standard property, but more than that, a given bus width can carry different formats. For instance, a 24-bus could carry RGB666 data (with dithering for the LSBs).
I think that's exactly what the interface-pix-fmt was trying to solve for the IPUv3, there you could have e.g. both RGB666 and LVDS666 , which were different.
My point is that the driver should support multiple formats that can be carried over a given bus width, with the actual format to be used queried from the sink (usually a panel) instead of being hardcoded in DT.
So, should the IPUv3 be fixed as well then ?
On Wed, 07 Oct 2020 04:24:34 +0300, Laurent Pinchart wrote:
When the PCB routes the display data signals in an unconventional way, the output bus width may differ from the bus width of the connected panel or encoder. For instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] and LCD_DATA[23:18], the output bus width is 24 instead of 18 when the signals are routed to LCD_DATA[5:0], LCD_DATA[11:6] and LCD_DATA[17:12].
Add a bus-width property to describe this data routing.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
Changes since v1:
- Fix property name in binding
.../devicetree/bindings/display/fsl,lcdif.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Reviewed-by: Rob Herring robh@kernel.org
The LCDIF in the i.MX6 SoCs has additional features compared to the i.MX28. Replace the fsl,imx28-lcdif fallback compatible string with fsl,imx6sx-lcdif to reflect that.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Acked-by: Sam Ravnborg sam@ravnborg.org --- arch/arm/boot/dts/imx6sl.dtsi | 2 +- arch/arm/boot/dts/imx6sll.dtsi | 2 +- arch/arm/boot/dts/imx6sx.dtsi | 4 ++-- arch/arm/boot/dts/imx6ul.dtsi | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi index 1c7180f28539..1e506ffbcecc 100644 --- a/arch/arm/boot/dts/imx6sl.dtsi +++ b/arch/arm/boot/dts/imx6sl.dtsi @@ -769,7 +769,7 @@ epdc: epdc@20f4000 { };
lcdif: lcdif@20f8000 { - compatible = "fsl,imx6sl-lcdif", "fsl,imx28-lcdif"; + compatible = "fsl,imx6sl-lcdif", "fsl,imx6sx-lcdif"; reg = <0x020f8000 0x4000>; interrupts = <0 39 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6SL_CLK_LCDIF_PIX>, diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi index fb5d3bc50c6b..81137ba427a1 100644 --- a/arch/arm/boot/dts/imx6sll.dtsi +++ b/arch/arm/boot/dts/imx6sll.dtsi @@ -644,7 +644,7 @@ pxp: pxp@20f0000 { };
lcdif: lcd-controller@20f8000 { - compatible = "fsl,imx6sll-lcdif", "fsl,imx28-lcdif"; + compatible = "fsl,imx6sll-lcdif", "fsl,imx6sx-lcdif"; reg = <0x020f8000 0x4000>; interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6SLL_CLK_LCDIF_PIX>, diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi index b480dfa9e251..1f2b1c56757b 100644 --- a/arch/arm/boot/dts/imx6sx.dtsi +++ b/arch/arm/boot/dts/imx6sx.dtsi @@ -1261,7 +1261,7 @@ csi2: csi@221c000 { };
lcdif1: lcdif@2220000 { - compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; + compatible = "fsl,imx6sx-lcdif"; reg = <0x02220000 0x4000>; interrupts = <GIC_SPI 5 IRQ_TYPE_EDGE_RISING>; clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, @@ -1273,7 +1273,7 @@ lcdif1: lcdif@2220000 { };
lcdif2: lcdif@2224000 { - compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; + compatible = "fsl,imx6sx-lcdif"; reg = <0x02224000 0x4000>; interrupts = <GIC_SPI 6 IRQ_TYPE_EDGE_RISING>; clocks = <&clks IMX6SX_CLK_LCDIF2_PIX>, diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index 2b088f210331..776493923474 100644 --- a/arch/arm/boot/dts/imx6ul.dtsi +++ b/arch/arm/boot/dts/imx6ul.dtsi @@ -1003,7 +1003,7 @@ csi: csi@21c4000 { };
lcdif: lcdif@21c8000 { - compatible = "fsl,imx6ul-lcdif", "fsl,imx28-lcdif"; + compatible = "fsl,imx6ul-lcdif", "fsl,imx6sx-lcdif"; reg = <0x021c8000 0x4000>; interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6UL_CLK_LCDIF_PIX>,
The LCDIF in the i.MX8 SoCs has additional features compared to the i.MX28. Replace the fsl,imx28-lcdif fallback compatible string with fsl,imx6sx-lcdif to reflect that.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Acked-by: Sam Ravnborg sam@ravnborg.org --- arch/arm64/boot/dts/freescale/imx8mq.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi index 561fa792fe5a..5c7f4c0de7ca 100644 --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi @@ -509,7 +509,7 @@ sdma2: sdma@302c0000 { };
lcdif: lcd-controller@30320000 { - compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif"; + compatible = "fsl,imx8mq-lcdif", "fsl,imx6sx-lcdif"; reg = <0x30320000 0x10000>; interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clk IMX8MQ_CLK_LCDIF_PIXEL>;
The LCDIF disp_axi clock is not mandatory in the DT binding and not required by the driver. Remove it when it points to a dummy clock.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Acked-by: Sam Ravnborg sam@ravnborg.org --- arch/arm/boot/dts/imx6sl.dtsi | 5 ++--- arch/arm/boot/dts/imx6sll.dtsi | 5 ++--- arch/arm/boot/dts/imx6ul.dtsi | 5 ++--- 3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi index 1e506ffbcecc..27d8061e06af 100644 --- a/arch/arm/boot/dts/imx6sl.dtsi +++ b/arch/arm/boot/dts/imx6sl.dtsi @@ -773,9 +773,8 @@ lcdif: lcdif@20f8000 { reg = <0x020f8000 0x4000>; interrupts = <0 39 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6SL_CLK_LCDIF_PIX>, - <&clks IMX6SL_CLK_LCDIF_AXI>, - <&clks IMX6SL_CLK_DUMMY>; - clock-names = "pix", "axi", "disp_axi"; + <&clks IMX6SL_CLK_LCDIF_AXI>; + clock-names = "pix", "axi"; status = "disabled"; power-domains = <&pd_disp>; }; diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi index 81137ba427a1..80c7ef5af435 100644 --- a/arch/arm/boot/dts/imx6sll.dtsi +++ b/arch/arm/boot/dts/imx6sll.dtsi @@ -648,9 +648,8 @@ lcdif: lcd-controller@20f8000 { reg = <0x020f8000 0x4000>; interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6SLL_CLK_LCDIF_PIX>, - <&clks IMX6SLL_CLK_LCDIF_APB>, - <&clks IMX6SLL_CLK_DUMMY>; - clock-names = "pix", "axi", "disp_axi"; + <&clks IMX6SLL_CLK_LCDIF_APB>; + clock-names = "pix", "axi"; status = "disabled"; };
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi index 776493923474..6ecdbf9f63c6 100644 --- a/arch/arm/boot/dts/imx6ul.dtsi +++ b/arch/arm/boot/dts/imx6ul.dtsi @@ -1007,9 +1007,8 @@ lcdif: lcdif@21c8000 { reg = <0x021c8000 0x4000>; interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6UL_CLK_LCDIF_PIX>, - <&clks IMX6UL_CLK_LCDIF_APB>, - <&clks IMX6UL_CLK_DUMMY>; - clock-names = "pix", "axi", "disp_axi"; + <&clks IMX6UL_CLK_LCDIF_APB>; + clock-names = "pix", "axi"; status = "disabled"; };
A new bus-width DT property has been introduced in the bindings to allow overriding the bus width. Support it.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 26 ++++++++++++++++++++++++++ drivers/gpu/drm/mxsfb/mxsfb_drv.h | 2 ++ drivers/gpu/drm/mxsfb/mxsfb_kms.c | 8 ++++++-- 3 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 35122aef037b..d52cf8a506a5 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -114,10 +114,36 @@ static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb) { struct drm_device *drm = mxsfb->drm; struct drm_connector_list_iter iter; + struct device_node *ep; struct drm_panel *panel; struct drm_bridge *bridge; + u32 bus_width = 0; int ret;
+ ep = of_graph_get_endpoint_by_regs(drm->dev->of_node, 0, 0); + if (!ep) + return -ENODEV; + + of_property_read_u32(ep, "bus-width", &bus_width); + of_node_put(ep); + + switch (bus_width) { + case 16: + mxsfb->bus_format = MEDIA_BUS_FMT_RGB565_1X16; + break; + case 18: + mxsfb->bus_format = MEDIA_BUS_FMT_RGB666_1X18; + break; + case 24: + mxsfb->bus_format = MEDIA_BUS_FMT_RGB888_1X24; + break; + case 0: + break; + default: + DRM_DEV_ERROR(drm->dev, "Invalid bus-width %u", bus_width); + return -ENODEV; + } + ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, &panel, &bridge); if (ret) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h index 399d23e91ed1..c4f7a8a0c891 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h @@ -32,6 +32,8 @@ struct mxsfb_drm_private { struct clk *clk_axi; struct clk *clk_disp_axi;
+ u32 bus_format; + struct drm_device *drm; struct { struct drm_plane primary; diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index b721b8b262ce..6d512f346918 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -50,11 +50,15 @@ static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb) { struct drm_device *drm = mxsfb->drm; const u32 format = mxsfb->crtc.primary->state->fb->format->format; - u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; + u32 bus_format; u32 ctrl, ctrl1;
- if (mxsfb->connector->display_info.num_bus_formats) + if (mxsfb->bus_format) + bus_format = mxsfb->bus_format; + else if (mxsfb->connector->display_info.num_bus_formats) bus_format = mxsfb->connector->display_info.bus_formats[0]; + else + bus_format = MEDIA_BUS_FMT_RGB888_1X24;
DRM_DEV_DEBUG_DRIVER(drm->dev, "Using bus_format: 0x%08X\n", bus_format);
From: Marek Vasut marex@denx.de
NXP's i.MX8MM has an LCDIF as well.
Signed-off-by: Marek Vasut marex@denx.de Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- Changes since v1:
- Rebased on top of the YAML conversion --- Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml index 404bd516b7f5..c44441690ae7 100644 --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -26,6 +26,7 @@ properties: - fsl,imx6sll-lcdif - fsl,imx6ul-lcdif - fsl,imx7d-lcdif + - fsl,imx8mm-lcdif - fsl,imx8mq-lcdif - const: fsl,imx6sx-lcdif
dri-devel@lists.freedesktop.org
-
Laurent Pinchart -
Lucas Stach -
Marek Vasut -
Martin Kepplinger -
Rob Herring