| Message ID | 20240521105817.3301-2-keith.zhao@starfivetech.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | drm/verisilicon : support DC8200 and inno hdmi | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
On 21/05/2024 12:58, keith wrote: > JH7110 SoC display pipeline includes a display controller and hdmi. > Dc controller IP : Vivante DC8200 Dual Display > HDMI IP : INNOSILICON HDMI2.0 > > As the INNO hdmi ip is also used by rockchip SoC in the driver code, > the innosilicon,inno-hdmi.yaml schema containing the common properties > for the INNO DesignWare HDMI TX controller isn't a full device > tree binding specification, but is meant to be referenced by > platform-specific bindings for the IP core. > > Signed-off-by: keith <keith.zhao@starfivetech.com> > --- > .../display/bridge/innosilicon,inno-hdmi.yaml | 49 +++++ > .../display/rockchip/rockchip,inno-hdmi.yaml | 27 +-- > .../starfive/starfive,dsi-encoder.yaml | 92 ++++++++++ > .../starfive/starfive,jh7110-dc8200.yaml | 169 ++++++++++++++++++ > .../starfive/starfive,jh7110-inno-hdmi.yaml | 75 ++++++++ > .../soc/starfive/starfive,jh7110-syscon.yaml | 1 + > MAINTAINERS | 8 + > 7 files changed, 396 insertions(+), 25 deletions(-) > create mode 100644 Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml > create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml > create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml > create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml > > diff --git a/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml > new file mode 100644 > index 000000000000..8540174dcaeb > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml > @@ -0,0 +1,49 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/innosilicon,inno-hdmi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Common Properties for Innosilicon HDMI TX IP Your patch is difficult to review. Split changing existing bindings (and defining common part) to a separate patch. > + > +maintainers: > + - Keith Zhao <keith.zhao@starfivetech.com> > + > +description: | > + This document defines device tree properties for the Innosilicon HDMI TX Nothing improved here. Don't say obvious that this documents says something. It cannot do anything else. "Innosilicon HDMI TX is a foo bar device present on zap SoC ...." > + controller (INNO HDMI) IP core. It doesn't constitute a full device tree > + binding specification by itself but is meant to be referenced by device tree > + bindings for the platform-specific integrations of the INNO HDMI. I don't understand this at all. I don't know what is "full device tree binding specification". > + > + When referenced from platform device tree bindings the properties defined in > + this document are defined as follows. The platform device tree bindings are > + responsible for defining whether each property is required or optional. Nothing improved - drop paragraph. > + > +properties: > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + Port node with one endpoint connected to a display controller node. > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + Port node with one endpoint connected to a hdmi-connector node. > + > + required: > + - port@0 > + - port@1 ... > diff --git a/Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml b/Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml > new file mode 100644 > index 000000000000..07aa147a9db1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml > @@ -0,0 +1,92 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/starfive/starfive,dsi-encoder.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: starfive jh7110 SoC Encoder > + > +maintainers: > + - Keith Zhao <keith.zhao@starfivetech.com> > + > +description: > + Device-Tree bindings for simple encoder. Again you ignored the comments. When you receive a comment, apply it everywhere, not in only one part of patch while keeping wrong code everywhere else. > + Simple encoder driver only has basic functionality. Not related to bindings, drop. This is about hardware, not your driver. > + the hardware of dc8200 has 2 output interface, and uses > + syscon to select which one to be used. Nothing improved. Read my previous comments: 1. Please make it a proper sentences, with proper wrapping. > + > +properties: > + compatible: > + const: starfive,dsi-encoder > + > + starfive,syscon: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + items: > + - items: > + - description: phandle to syscon that select crtc output. > + - description: Offset of crtc selection > + - description: Shift of crtc selection > + description: > + A phandle to syscon with two arguments that configure select output. > + The argument one is the offset of crtc selection, the > + argument two is the shift of crtc selection. Don't repeat constraints in free form text. Say something useful instead, like what is its purpose. No reg? Then why this is not just part of syscon? That's not a separate device, no. You received a feedback already about this: do not create fake bindings just to instantiate your drivers. > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + The first port should be the input coming from the associated dc channel. > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + The second port should be the output coming from the associated bridge. > + > + required: > + - port@0 > + - port@1 > + > +required: > + - compatible > + - ports > + > +additionalProperties: false > + > +examples: > + - | > + dssctrl: dssctrl@295b0000 { Drop node, not erlated. > + compatible = "starfive,jh7110-vout-syscon", "syscon"; > + reg = <0x295b0000 0x90>; > + }; > + > + dsi_encoder: dsi_encoder { Totally messed indentation. Use 4 spaces for example indentation. Also, drop label. Also, underscores are not allowed in node names. Also, Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation This is terrible code. > + compatible = "starfive,dsi-encoder"; > + starfive,syscon = <&dssctrl 0x8 0x12>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + /* input */ > + port@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0>; > + dsi_input0:endpoint@0 { > + reg = <0>; > + remote-endpoint = <&dc_out_dpi1>; > + }; > + }; > + /* output */ > + port@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + dsi_out:endpoint { > + remote-endpoint = <&mipi_in>; > + }; > + }; > + }; > + }; > diff --git a/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml > new file mode 100644 > index 000000000000..a28dfdd471b6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml > @@ -0,0 +1,169 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/starfive/starfive,jh7110-dc8200.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: STARFIVE JH7110 SoC display controller > + > +description: > + The STARFIVE JH7110 SoC uses the display controller based on Verisilicon IP(DC8200) > + to transfer the image data from a video memory buffer to an external LCD interface. > + > + pipe0 binds HDMI for primary display, > + pipe1 binds DSI for external display. > + > + +------------------------------+ > + | | > + | | > + +----+ | +-------------------+ | +-------+ +------+ +------+ > + | +----->+ dc controller 0 +--->----->+HDMICtl| ->+ PHY +-->+PANEL0+ > + |AXI | | +-------------------+ | +-------+ +------+ +------+ > + | | | | > + | | | | > + | | | | > + | | | | > + |APB | | +-------------------+ +---------+ +------+ +-------+ > + | +----->+ dc controller 1 +--->---->+ dsiTx +--->+DPHY +->+ PANEL1+ > + | | | +-------------------+ +---------+ +------+ +-------+ > + +----+ | | > + +------------------------------+ > + > +maintainers: > + - Keith Zhao <keith.zhao@starfivetech.com> > + > +properties: > + compatible: > + const: starfive,jh7110-dc8200 > + > + reg: > + items: > + - description: host interface address and length > + - description: display physical base address and length > + > + reg-names: > + items: > + - const: host > + - const: base > + > + clocks: > + items: > + - description: Clock for display system noc bus. > + - description: Core clock for display controller. > + - description: Clock for axi bus to access ddr. > + - description: Clock for ahb bus to R/W the phy regs. > + - description: Pixel clock for display channel 0. > + - description: Pixel clock for display channel 1. > + - description: Pixel clock from hdmi. > + - description: Pixel clock for soc . > + > + clock-names: > + items: > + - const: noc_bus > + - const: dc_core > + - const: axi_core > + - const: ahb > + - const: channel0 > + - const: channel1 > + - const: hdmi_tx > + - const: dc_parent > + > + resets: > + items: > + - description: Reset for axi bus. > + - description: Reset for ahb bus. > + - description: Core reset of display controller. > + > + reset-names: > + items: > + - const: axi > + - const: ahb > + - const: core > + > + interrupts: > + items: > + - description: The interrupt will be generated when DC finish one frame > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + channel 0 output > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + channel 1 output > + > + required: > + - port@0 > + - port@1 > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - ports > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/starfive,jh7110-crg.h> > + #include <dt-bindings/reset/starfive,jh7110-crg.h> > + dc8200: lcd-controller@29400000 { > + compatible = "starfive,jh7110-dc8200"; > + reg = <0x29400000 0x100>, > + <0x29400800 0x2000>; > + reg-names = "host", "base"; > + > + interrupts = <95>; > + clocks = <&syscrg JH7110_SYSCLK_NOC_BUS_DISP_AXI>, > + <&voutcrg JH7110_VOUTCLK_DC8200_CORE>, Align the lines. In other places as well. > + <&voutcrg JH7110_VOUTCLK_DC8200_AXI>, > + <&voutcrg JH7110_VOUTCLK_DC8200_AHB>, > + <&voutcrg JH7110_VOUTCLK_DC8200_PIX0>, > + <&voutcrg JH7110_VOUTCLK_DC8200_PIX1>, > + <&hdmitx0_pixelclk>, > + <&voutcrg JH7110_VOUTCLK_DC8200_PIX>; > + clock-names = "noc_bus", "dc_core", "axi_core", "ahb", > + "channel0", "channel1", "hdmi_tx", "dc_parent"; > + resets = <&voutcrg JH7110_VOUTRST_DC8200_AXI>, > + <&voutcrg JH7110_VOUTRST_DC8200_AHB>, > + <&voutcrg JH7110_VOUTRST_DC8200_CORE>; > + reset-names = "axi","ahb", "core"; > + > + crtc_out: ports { Totally messed indentation. > diff --git a/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml > new file mode 100644 > index 000000000000..bfd7dc41fc14 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml > @@ -0,0 +1,75 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/starfive/starfive,jh7110-inno-hdmi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Starfive JH7110 Innosilicon HDMI controller > + > +maintainers: > + - Keith Zhao <keith.zhao@starfivetech.com> > + > +allOf: > + - $ref: ../bridge/innosilicon,inno-hdmi.yaml# > + > +properties: > + compatible: > + const: "starfive,jh7110-inno-hdmi" You did not test it. Drop quotes. Untested bindings, so I will skip review for now. Best regards, Krzysztof
Hi Krzysztof: > -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: 2024年5月21日 15:30 > To: Keith Zhao <keith.zhao@starfivetech.com>; andrzej.hajda@intel.com; > neil.armstrong@linaro.org; rfoss@kernel.org; > Laurent.pinchart@ideasonboard.com; jonas@kwiboo.se; > jernej.skrabec@gmail.com; maarten.lankhorst@linux.intel.com; > mripard@kernel.org; tzimmermann@suse.de; airlied@gmail.com; > daniel@ffwll.ch; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; > hjc@rock-chips.com; heiko@sntech.de; andy.yan@rock-chips.com; Xingyu Wu > <xingyu.wu@starfivetech.com>; p.zabel@pengutronix.de; Jack Zhu > <jack.zhu@starfivetech.com>; Shengyang Chen > <shengyang.chen@starfivetech.com> > Cc: dri-devel@lists.freedesktop.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH v4 01/10] dt-bindings: display: Add YAML schema for JH7110 > display pipeline > > On 21/05/2024 12:58, keith wrote: > > JH7110 SoC display pipeline includes a display controller and hdmi. > > Dc controller IP : Vivante DC8200 Dual Display HDMI IP : INNOSILICON > > HDMI2.0 > > > > As the INNO hdmi ip is also used by rockchip SoC in the driver code, > > the innosilicon,inno-hdmi.yaml schema containing the common properties > > for the INNO DesignWare HDMI TX controller isn't a full device tree > > binding specification, but is meant to be referenced by > > platform-specific bindings for the IP core. > > > > Signed-off-by: keith <keith.zhao@starfivetech.com> > > --- > > .../display/bridge/innosilicon,inno-hdmi.yaml | 49 +++++ > > .../display/rockchip/rockchip,inno-hdmi.yaml | 27 +-- > > .../starfive/starfive,dsi-encoder.yaml | 92 ++++++++++ > > .../starfive/starfive,jh7110-dc8200.yaml | 169 ++++++++++++++++++ > > .../starfive/starfive,jh7110-inno-hdmi.yaml | 75 ++++++++ > > .../soc/starfive/starfive,jh7110-syscon.yaml | 1 + > > MAINTAINERS | 8 + > > 7 files changed, 396 insertions(+), 25 deletions(-) create mode > > 100644 > > Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi > > .yaml create mode 100644 > > Documentation/devicetree/bindings/display/starfive/starfive,dsi-encode > > r.yaml create mode 100644 > > Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8 > > 200.yaml create mode 100644 > > Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inn > > o-hdmi.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hd > > mi.yaml > > b/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hd > > mi.yaml > > new file mode 100644 > > index 000000000000..8540174dcaeb > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/bridge/innosilicon,inn > > +++ o-hdmi.yaml > > @@ -0,0 +1,49 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +http://devicetree.org/schemas/display/bridge/innosilicon,inno-hdmi.ya > > +ml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Common Properties for Innosilicon HDMI TX IP > > Your patch is difficult to review. Split changing existing bindings (and defining > common part) to a separate patch. The background is that because the existing inno hdmi driver on the main line has some interfaces that can be public, its yaml counterpart attempts to make a public reference file > > > + > > +maintainers: > > + - Keith Zhao <keith.zhao@starfivetech.com> > > + > > +description: | > > + This document defines device tree properties for the Innosilicon > > +HDMI TX > > Nothing improved here. Don't say obvious that this documents says something. > It cannot do anything else. > > "Innosilicon HDMI TX is a foo bar device present on zap SoC ...." > > > > + controller (INNO HDMI) IP core. It doesn't constitute a full device > > + tree binding specification by itself but is meant to be referenced > > + by device tree bindings for the platform-specific integrations of the INNO > HDMI. > > I don't understand this at all. I don't know what is "full device tree binding > specification". > > > + > > + When referenced from platform device tree bindings the properties > > + defined in this document are defined as follows. The platform > > + device tree bindings are responsible for defining whether each property is > required or optional. > > Nothing improved - drop paragraph. These descriptions are based this file, and because their roles are close, only some key elements have been changed of course, I will make corresponding modifications according to your remarks https://elixir.bootlin.com/linux/v6.9/source/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi.yaml > > > + > > +properties: > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > + > > + properties: > > + port@0: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: > > + Port node with one endpoint connected to a display controller > node. > > + > > + port@1: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: > > + Port node with one endpoint connected to a hdmi-connector > node. > > + > > + required: > > + - port@0 > > + - port@1 > > ... > > > diff --git > > a/Documentation/devicetree/bindings/display/starfive/starfive,dsi-enco > > der.yaml > > b/Documentation/devicetree/bindings/display/starfive/starfive,dsi-enco > > der.yaml > > new file mode 100644 > > index 000000000000..07aa147a9db1 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/starfive/starfive,dsi- > > +++ encoder.yaml > > @@ -0,0 +1,92 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +http://devicetree.org/schemas/display/starfive/starfive,dsi-encoder.y > > +aml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: starfive jh7110 SoC Encoder > > + > > +maintainers: > > + - Keith Zhao <keith.zhao@starfivetech.com> > > + > > +description: > > + Device-Tree bindings for simple encoder. > > Again you ignored the comments. > > When you receive a comment, apply it everywhere, not in only one part of patch > while keeping wrong code everywhere else. > > > + Simple encoder driver only has basic functionality. > > Not related to bindings, drop. This is about hardware, not your driver. > > > + the hardware of dc8200 has 2 output interface, and uses syscon to > > + select which one to be used. > > Nothing improved. > > Read my previous comments: > > 1. Please make it a proper sentences, with proper wrapping. > > > + > > +properties: > > + compatible: > > + const: starfive,dsi-encoder > > + > > + starfive,syscon: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + items: > > + - items: > > + - description: phandle to syscon that select crtc output. > > + - description: Offset of crtc selection > > + - description: Shift of crtc selection > > + description: > > + A phandle to syscon with two arguments that configure select > output. > > + The argument one is the offset of crtc selection, the > > + argument two is the shift of crtc selection. > > Don't repeat constraints in free form text. Say something useful instead, like > what is its purpose. > > No reg? Then why this is not just part of syscon? That's not a separate device, > no. > > You received a feedback already about this: do not create fake bindings just to > instantiate your drivers. > Yes , "do not create fake bindings just to instantiate your drivers." , I got this message , In lastest patchset, there was a display-subsystem dt-binding with no reg , I deleted it. reg != syscon reg , got it . I will restructure the code structure in the next version to meet this requirement, so that this dts node can not be added, same with yaml file. > > + > > + ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > + > > + properties: > > + port@0: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: > > + The first port should be the input coming from the associated dc > channel. > > + > > + port@1: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: > > + The second port should be the output coming from the > associated bridge. > > + > > + required: > > + - port@0 > > + - port@1 > > + > > +required: > > + - compatible > > + - ports > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + dssctrl: dssctrl@295b0000 { > > Drop node, not erlated. > > > + compatible = "starfive,jh7110-vout-syscon", "syscon"; > > + reg = <0x295b0000 0x90>; > > + }; > > + > > + dsi_encoder: dsi_encoder { > > Totally messed indentation. Use 4 spaces for example indentation. > > Also, drop label. > > Also, underscores are not allowed in node names. > > Also, Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree- > basics.html#generic-names-recommendation > > This is terrible code. > > > + compatible = "starfive,dsi-encoder"; > > + starfive,syscon = <&dssctrl 0x8 0x12>; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + /* input */ > > + port@0 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + reg = <0>; > > + dsi_input0:endpoint@0 { > > + reg = <0>; > > + remote-endpoint = <&dc_out_dpi1>; > > + }; > > + }; > > + /* output */ > > + port@1 { > > + reg = <1>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + dsi_out:endpoint { > > + remote-endpoint = <&mipi_in>; > > + }; > > + }; > > + }; > > + }; > > diff --git > > a/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-d > > c8200.yaml > > b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-d > > c8200.yaml > > new file mode 100644 > > index 000000000000..a28dfdd471b6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/starfive/starfive,jh71 > > +++ 10-dc8200.yaml > > @@ -0,0 +1,169 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +http://devicetree.org/schemas/display/starfive/starfive,jh7110-dc8200 > > +.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: STARFIVE JH7110 SoC display controller > > + > > +description: > > + The STARFIVE JH7110 SoC uses the display controller based on > > +Verisilicon IP(DC8200) > > + to transfer the image data from a video memory buffer to an external LCD > interface. > > + > > + pipe0 binds HDMI for primary display, > > + pipe1 binds DSI for external display. > > + > > + +------------------------------+ > > + | | > > + | | > > + +----+ | +-------------------+ | +-------+ +------+ +------+ > > + | +----->+ dc controller 0 +--->----->+HDMICtl| ->+ PHY > +-->+PANEL0+ > > + |AXI | | +-------------------+ | +-------+ +------+ +------+ > > + | | | | > > + | | | | > > + | | | | > > + | | | | > > + |APB | | +-------------------+ +---------+ +------+ +-------+ > > + | +----->+ dc controller 1 +--->---->+ dsiTx +--->+DPHY +->+ > PANEL1+ > > + | | | +-------------------+ +---------+ +------+ +-------+ > > + +----+ | | > > + +------------------------------+ > > + > > +maintainers: > > + - Keith Zhao <keith.zhao@starfivetech.com> > > + > > +properties: > > + compatible: > > + const: starfive,jh7110-dc8200 > > + > > + reg: > > + items: > > + - description: host interface address and length > > + - description: display physical base address and length > > + > > + reg-names: > > + items: > > + - const: host > > + - const: base > > + > > + clocks: > > + items: > > + - description: Clock for display system noc bus. > > + - description: Core clock for display controller. > > + - description: Clock for axi bus to access ddr. > > + - description: Clock for ahb bus to R/W the phy regs. > > + - description: Pixel clock for display channel 0. > > + - description: Pixel clock for display channel 1. > > + - description: Pixel clock from hdmi. > > + - description: Pixel clock for soc . > > + > > + clock-names: > > + items: > > + - const: noc_bus > > + - const: dc_core > > + - const: axi_core > > + - const: ahb > > + - const: channel0 > > + - const: channel1 > > + - const: hdmi_tx > > + - const: dc_parent > > + > > + resets: > > + items: > > + - description: Reset for axi bus. > > + - description: Reset for ahb bus. > > + - description: Core reset of display controller. > > + > > + reset-names: > > + items: > > + - const: axi > > + - const: ahb > > + - const: core > > + > > + interrupts: > > + items: > > + - description: The interrupt will be generated when DC finish > > + one frame > > + > > + ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > + > > + properties: > > + port@0: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: > > + channel 0 output > > + > > + port@1: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: > > + channel 1 output > > + > > + required: > > + - port@0 > > + - port@1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + - clock-names > > + - ports > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/starfive,jh7110-crg.h> > > + #include <dt-bindings/reset/starfive,jh7110-crg.h> > > + dc8200: lcd-controller@29400000 { > > + compatible = "starfive,jh7110-dc8200"; > > + reg = <0x29400000 0x100>, > > + <0x29400800 0x2000>; > > + reg-names = "host", "base"; > > + > > + interrupts = <95>; > > + clocks = <&syscrg JH7110_SYSCLK_NOC_BUS_DISP_AXI>, > > + <&voutcrg JH7110_VOUTCLK_DC8200_CORE>, > > Align the lines. In other places as well. > > > + <&voutcrg JH7110_VOUTCLK_DC8200_AXI>, > > + <&voutcrg JH7110_VOUTCLK_DC8200_AHB>, > > + <&voutcrg JH7110_VOUTCLK_DC8200_PIX0>, > > + <&voutcrg JH7110_VOUTCLK_DC8200_PIX1>, > > + <&hdmitx0_pixelclk>, > > + <&voutcrg JH7110_VOUTCLK_DC8200_PIX>; > > + clock-names = "noc_bus", "dc_core", "axi_core", "ahb", > > + "channel0", "channel1", "hdmi_tx", "dc_parent"; > > + resets = <&voutcrg JH7110_VOUTRST_DC8200_AXI>, > > + <&voutcrg JH7110_VOUTRST_DC8200_AHB>, > > + <&voutcrg JH7110_VOUTRST_DC8200_CORE>; > > + reset-names = "axi","ahb", "core"; > > + > > + crtc_out: ports { > > Totally messed indentation. > > > > diff --git > > a/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-i > > nno-hdmi.yaml > > b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-i > > nno-hdmi.yaml > > new file mode 100644 > > index 000000000000..bfd7dc41fc14 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/starfive/starfive,jh71 > > +++ 10-inno-hdmi.yaml > > @@ -0,0 +1,75 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +http://devicetree.org/schemas/display/starfive/starfive,jh7110-inno-h > > +dmi.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Starfive JH7110 Innosilicon HDMI controller > > + > > +maintainers: > > + - Keith Zhao <keith.zhao@starfivetech.com> > > + > > +allOf: > > + - $ref: ../bridge/innosilicon,inno-hdmi.yaml# > > + > > +properties: > > + compatible: > > + const: "starfive,jh7110-inno-hdmi" > > You did not test it. Drop quotes. > > Untested bindings, so I will skip review for now. In fact, I have used the check instruction, the result is pass with no warning, is there a difference in the command? make -j8 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/starfive/ ...... DTEX Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.example.dts DTEX Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.example.dts DTEX Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.example.dts DTC_CHK Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.example.dtb DTC_CHK Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.example.dtb DTC_CHK Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.example.dtb Thanks for your remarks , I hope it does not affect your happy life > > > > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml new file mode 100644 index 000000000000..8540174dcaeb --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml @@ -0,0 +1,49 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/innosilicon,inno-hdmi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Common Properties for Innosilicon HDMI TX IP + +maintainers: + - Keith Zhao <keith.zhao@starfivetech.com> + +description: | + This document defines device tree properties for the Innosilicon HDMI TX + controller (INNO HDMI) IP core. It doesn't constitute a full device tree + binding specification by itself but is meant to be referenced by device tree + bindings for the platform-specific integrations of the INNO HDMI. + + When referenced from platform device tree bindings the properties defined in + this document are defined as follows. The platform device tree bindings are + responsible for defining whether each property is required or optional. + +properties: + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: + Port node with one endpoint connected to a display controller node. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + Port node with one endpoint connected to a hdmi-connector node. + + required: + - port@0 + - port@1 + +additionalProperties: true + +... diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml index 5b87b0f1963e..589dedfcbee0 100644 --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml @@ -16,12 +16,6 @@ properties: - rockchip,rk3036-inno-hdmi - rockchip,rk3128-inno-hdmi - reg: - maxItems: 1 - - interrupts: - maxItems: 1 - clocks: minItems: 1 items: @@ -40,24 +34,6 @@ properties: "#sound-dai-cells": const: 0 - ports: - $ref: /schemas/graph.yaml#/properties/ports - - properties: - port@0: - $ref: /schemas/graph.yaml#/properties/port - description: - Port node with one endpoint connected to a vop node. - - port@1: - $ref: /schemas/graph.yaml#/properties/port - description: - Port node with one endpoint connected to a hdmi-connector node. - - required: - - port@0 - - port@1 - required: - compatible - reg @@ -69,6 +45,7 @@ required: - ports allOf: + - $ref: ../bridge/innosilicon,inno-hdmi.yaml# - $ref: /schemas/sound/dai-common.yaml# - if: properties: @@ -95,7 +72,7 @@ allOf: required: - power-domains -additionalProperties: false +unevaluatedProperties: false examples: - | diff --git a/Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml b/Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml new file mode 100644 index 000000000000..07aa147a9db1 --- /dev/null +++ b/Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml @@ -0,0 +1,92 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/starfive/starfive,dsi-encoder.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: starfive jh7110 SoC Encoder + +maintainers: + - Keith Zhao <keith.zhao@starfivetech.com> + +description: + Device-Tree bindings for simple encoder. + Simple encoder driver only has basic functionality. + the hardware of dc8200 has 2 output interface, and uses + syscon to select which one to be used. + +properties: + compatible: + const: starfive,dsi-encoder + + starfive,syscon: + $ref: /schemas/types.yaml#/definitions/phandle-array + items: + - items: + - description: phandle to syscon that select crtc output. + - description: Offset of crtc selection + - description: Shift of crtc selection + description: + A phandle to syscon with two arguments that configure select output. + The argument one is the offset of crtc selection, the + argument two is the shift of crtc selection. + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: + The first port should be the input coming from the associated dc channel. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + The second port should be the output coming from the associated bridge. + + required: + - port@0 + - port@1 + +required: + - compatible + - ports + +additionalProperties: false + +examples: + - | + dssctrl: dssctrl@295b0000 { + compatible = "starfive,jh7110-vout-syscon", "syscon"; + reg = <0x295b0000 0x90>; + }; + + dsi_encoder: dsi_encoder { + compatible = "starfive,dsi-encoder"; + starfive,syscon = <&dssctrl 0x8 0x12>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + /* input */ + port@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + dsi_input0:endpoint@0 { + reg = <0>; + remote-endpoint = <&dc_out_dpi1>; + }; + }; + /* output */ + port@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + dsi_out:endpoint { + remote-endpoint = <&mipi_in>; + }; + }; + }; + }; diff --git a/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml new file mode 100644 index 000000000000..a28dfdd471b6 --- /dev/null +++ b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml @@ -0,0 +1,169 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/starfive/starfive,jh7110-dc8200.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: STARFIVE JH7110 SoC display controller + +description: + The STARFIVE JH7110 SoC uses the display controller based on Verisilicon IP(DC8200) + to transfer the image data from a video memory buffer to an external LCD interface. + + pipe0 binds HDMI for primary display, + pipe1 binds DSI for external display. + + +------------------------------+ + | | + | | + +----+ | +-------------------+ | +-------+ +------+ +------+ + | +----->+ dc controller 0 +--->----->+HDMICtl| ->+ PHY +-->+PANEL0+ + |AXI | | +-------------------+ | +-------+ +------+ +------+ + | | | | + | | | | + | | | | + | | | | + |APB | | +-------------------+ +---------+ +------+ +-------+ + | +----->+ dc controller 1 +--->---->+ dsiTx +--->+DPHY +->+ PANEL1+ + | | | +-------------------+ +---------+ +------+ +-------+ + +----+ | | + +------------------------------+ + +maintainers: + - Keith Zhao <keith.zhao@starfivetech.com> + +properties: + compatible: + const: starfive,jh7110-dc8200 + + reg: + items: + - description: host interface address and length + - description: display physical base address and length + + reg-names: + items: + - const: host + - const: base + + clocks: + items: + - description: Clock for display system noc bus. + - description: Core clock for display controller. + - description: Clock for axi bus to access ddr. + - description: Clock for ahb bus to R/W the phy regs. + - description: Pixel clock for display channel 0. + - description: Pixel clock for display channel 1. + - description: Pixel clock from hdmi. + - description: Pixel clock for soc . + + clock-names: + items: + - const: noc_bus + - const: dc_core + - const: axi_core + - const: ahb + - const: channel0 + - const: channel1 + - const: hdmi_tx + - const: dc_parent + + resets: + items: + - description: Reset for axi bus. + - description: Reset for ahb bus. + - description: Core reset of display controller. + + reset-names: + items: + - const: axi + - const: ahb + - const: core + + interrupts: + items: + - description: The interrupt will be generated when DC finish one frame + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: + channel 0 output + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + channel 1 output + + required: + - port@0 + - port@1 + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + - ports + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/starfive,jh7110-crg.h> + #include <dt-bindings/reset/starfive,jh7110-crg.h> + dc8200: lcd-controller@29400000 { + compatible = "starfive,jh7110-dc8200"; + reg = <0x29400000 0x100>, + <0x29400800 0x2000>; + reg-names = "host", "base"; + + interrupts = <95>; + clocks = <&syscrg JH7110_SYSCLK_NOC_BUS_DISP_AXI>, + <&voutcrg JH7110_VOUTCLK_DC8200_CORE>, + <&voutcrg JH7110_VOUTCLK_DC8200_AXI>, + <&voutcrg JH7110_VOUTCLK_DC8200_AHB>, + <&voutcrg JH7110_VOUTCLK_DC8200_PIX0>, + <&voutcrg JH7110_VOUTCLK_DC8200_PIX1>, + <&hdmitx0_pixelclk>, + <&voutcrg JH7110_VOUTCLK_DC8200_PIX>; + clock-names = "noc_bus", "dc_core", "axi_core", "ahb", + "channel0", "channel1", "hdmi_tx", "dc_parent"; + resets = <&voutcrg JH7110_VOUTRST_DC8200_AXI>, + <&voutcrg JH7110_VOUTRST_DC8200_AHB>, + <&voutcrg JH7110_VOUTRST_DC8200_CORE>; + reset-names = "axi","ahb", "core"; + + crtc_out: ports { + #address-cells = <1>; + #size-cells = <0>; + + dc_out0: port@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + + dc_out_dpi0: endpoint@0 { + reg = <0>; + remote-endpoint = <&hdmi_enc>; + }; + + }; + + dc_out1: port@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + + dc_out_dpi1: endpoint@1 { + reg = <1>; + remote-endpoint = <&dsi_enc>; + }; + + }; + }; + }; diff --git a/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml new file mode 100644 index 000000000000..bfd7dc41fc14 --- /dev/null +++ b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/starfive/starfive,jh7110-inno-hdmi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Starfive JH7110 Innosilicon HDMI controller + +maintainers: + - Keith Zhao <keith.zhao@starfivetech.com> + +allOf: + - $ref: ../bridge/innosilicon,inno-hdmi.yaml# + +properties: + compatible: + const: "starfive,jh7110-inno-hdmi" + + clocks: + items: + - description: System clock of HDMI module. + - description: Mclk clock of HDMI audio. + - description: Bclk clock of HDMI audio. + + clock-names: + items: + - const: sysclk + - const: mclk + - const: bclk + + resets: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + - resets + - ports + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/pinctrl/starfive,jh7110-pinctrl.h> + #include <dt-bindings/gpio/gpio.h> + hdmi: hdmi@29590000 { + compatible = "starfive,jh7110-inno-hdmi"; + reg = <0x29590000 0x4000>; + interrupts = <99>; + clocks = <&voutcrg 17>, <&voutcrg 15>, <&voutcrg 16>; + clock-names = "sysclk", "mclk","bclk"; + resets = <&voutcrg 9>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + hdmi_in: port@0 { + reg = <0>; + hdmi_in_vop: endpoint { + remote-endpoint = <&dc_out_dpi0>; + }; + }; + + hdmi_out: port@1 { + reg = <1>; + hdmi_out_con: endpoint { + remote-endpoint = <&hdmi_con_in>; + }; + }; + }; + }; diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml index 0039319e91fe..cf9b657d0e8a 100644 --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml @@ -24,6 +24,7 @@ properties: - enum: - starfive,jh7110-aon-syscon - starfive,jh7110-stg-syscon + - starfive,jh7110-vout-syscon - const: syscon reg: diff --git a/MAINTAINERS b/MAINTAINERS index c675fc296b19..8881652bb8b6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7148,6 +7148,14 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git F: Documentation/devicetree/bindings/display/ste,mcde.yaml F: drivers/gpu/drm/mcde/ +DRM DRIVERS FOR STARFIVE +M: Keith Zhao <keith.zhao@starfivetech.com> +L: dri-devel@lists.freedesktop.org +S: Maintained +T: git https://gitlab.freedesktop.org/drm/misc/kernel.git +F: Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml +F: Documentation/devicetree/bindings/display/starfive/ + DRM DRIVER FOR SYNAPTICS R63353 PANELS M: Michael Trimarchi <michael@amarulasolutions.com> S: Maintained
JH7110 SoC display pipeline includes a display controller and hdmi. Dc controller IP : Vivante DC8200 Dual Display HDMI IP : INNOSILICON HDMI2.0 As the INNO hdmi ip is also used by rockchip SoC in the driver code, the innosilicon,inno-hdmi.yaml schema containing the common properties for the INNO DesignWare HDMI TX controller isn't a full device tree binding specification, but is meant to be referenced by platform-specific bindings for the IP core. Signed-off-by: keith <keith.zhao@starfivetech.com> --- .../display/bridge/innosilicon,inno-hdmi.yaml | 49 +++++ .../display/rockchip/rockchip,inno-hdmi.yaml | 27 +-- .../starfive/starfive,dsi-encoder.yaml | 92 ++++++++++ .../starfive/starfive,jh7110-dc8200.yaml | 169 ++++++++++++++++++ .../starfive/starfive,jh7110-inno-hdmi.yaml | 75 ++++++++ .../soc/starfive/starfive,jh7110-syscon.yaml | 1 + MAINTAINERS | 8 + 7 files changed, 396 insertions(+), 25 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,dsi-encoder.yaml create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml