| Message ID | 20220523104758.29531-3-granquet@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/mediatek: Add mt8195 DisplayPort driver | expand |
On Mon, 2022-05-23 at 12:47 +0200, Guillaume Ranquet wrote: > From: Markus Schneider-Pargmann <msp@baylibre.com> > > This controller is present on several mediatek hardware. Currently > mt8195 and mt8395 have this controller without a functional > difference, > so only one compatible field is added. > > The controller can have two forms, as a normal display port and as an > embedded display port. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > --- > .../display/mediatek/mediatek,dp.yaml | 99 > +++++++++++++++++++ > 1 file changed, 99 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > new file mode 100644 > index 000000000000..36ae0a6df299 > --- /dev/null > +++ > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > @@ -0,0 +1,99 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: > http://devicetree.org/schemas/display/mediatek/mediatek,dp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek Display Port Controller > + > +maintainers: > + - CK Hu <ck.hu@mediatek.com> > + - Jitao shi <jitao.shi@mediatek.com> > + > +description: | > + Device tree bindings for the MediaTek (embedded) Display Port > controller > + present on some MediaTek SoCs. > + > +properties: > + compatible: > + enum: > + - mediatek,mt8195-dp-tx > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: faxi clock > + > + clock-names: > + items: > + - const: faxi > + > + power-domains: > + maxItems: 1 > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: Input endpoint of the controller, usually > dp_intf > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: Output endpoint of the controller > + > + max-lanes: > + maxItems: 1 > + description: maximum number of lanes supported by the hardware > + > + max-linkrate: > + maxItems: 1 > + description: maximum link rate supported by the hardware > + > +required: > + - compatible > + - reg > + - interrupts > + - ports > + - max-lanes > + - max-linkrate > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/power/mt8195-power.h> > + edp_tx: edp_tx@1c500000 { 'edp_tx: ' can be removed > + compatible = "mediatek,mt8195-dp-tx"; > + reg = <0 0x1c500000 0 0x8000>; reg = <0x1c500000 0x8000>; #address-cells, #size-cells are both 1 by default > + interrupts = <GIC_SPI 676 IRQ_TYPE_LEVEL_HIGH 0>; > + power-domains = <&spm MT8195_POWER_DOMAIN_EPD_TX>; > + pinctrl-names = "default"; > + pinctrl-0 = <&edp_pin>; > + max-lanes = /bits/ 8 <4>; > + max-linkrate = /bits/ 8 <0x1e>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + edp_in: endpoint { > + remote-endpoint = <&dp_intf0_out>; > + }; > + }; > + port@1 { > + reg = <1>; > + edp_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > + };
Il 23/05/22 12:47, Guillaume Ranquet ha scritto: > From: Markus Schneider-Pargmann <msp@baylibre.com> > > This controller is present on several mediatek hardware. Currently > mt8195 and mt8395 have this controller without a functional difference, > so only one compatible field is added. > > The controller can have two forms, as a normal display port and as an > embedded display port. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > --- > .../display/mediatek/mediatek,dp.yaml | 99 +++++++++++++++++++ > 1 file changed, 99 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > new file mode 100644 > index 000000000000..36ae0a6df299 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > @@ -0,0 +1,99 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/mediatek/mediatek,dp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek Display Port Controller > + > +maintainers: > + - CK Hu <ck.hu@mediatek.com> > + - Jitao shi <jitao.shi@mediatek.com> > + > +description: | > + Device tree bindings for the MediaTek (embedded) Display Port controller > + present on some MediaTek SoCs. > + > +properties: > + compatible: > + enum: > + - mediatek,mt8195-dp-tx > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: faxi clock > + > + clock-names: > + items: > + - const: faxi > + > + power-domains: > + maxItems: 1 > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: Input endpoint of the controller, usually dp_intf > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: Output endpoint of the controller > + You should add port@0 (and port@1, probably) as required... with what you've done here, you're saying that "ports" is required, but you're allowing it to be empty.. ports: $ref: /schemas/graph.yaml#/properties/ports properties: port@0: $ref: /schemas/graph.yaml#/properties/port description: Input endpoint of the controller, usually dp_intf port@1: $ref: /schemas/graph.yaml#/properties/port description: Output endpoint of the controller required: - port@0 - port@1 ^^^ that's how it should look. > + max-lanes: > + maxItems: 1 > + description: maximum number of lanes supported by the hardware > + > + max-linkrate: > + maxItems: 1 > + description: maximum link rate supported by the hardware As you've put it (in the example below), the max-linkrate property wants a value that corresponds to what you find in the HW registers... this is wrong. Devicetree bindings should be generic and devicetrees shouldn't have hardware specific bits inside, hence, please change this property to accept a link rate specified in Mbps and also specify that in the description. Thanks, Angelo > + > +required: > + - compatible > + - reg > + - interrupts > + - ports > + - max-lanes > + - max-linkrate > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/power/mt8195-power.h> > + edp_tx: edp_tx@1c500000 { > + compatible = "mediatek,mt8195-dp-tx"; > + reg = <0 0x1c500000 0 0x8000>; > + interrupts = <GIC_SPI 676 IRQ_TYPE_LEVEL_HIGH 0>; > + power-domains = <&spm MT8195_POWER_DOMAIN_EPD_TX>; > + pinctrl-names = "default"; > + pinctrl-0 = <&edp_pin>; > + max-lanes = /bits/ 8 <4>; > + max-linkrate = /bits/ 8 <0x1e>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + edp_in: endpoint { > + remote-endpoint = <&dp_intf0_out>; > + }; > + }; > + port@1 { > + reg = <1>; > + edp_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > + };
On Mon, May 23, 2022 at 12:47:35PM +0200, Guillaume Ranquet wrote: > From: Markus Schneider-Pargmann <msp@baylibre.com> > > This controller is present on several mediatek hardware. Currently > mt8195 and mt8395 have this controller without a functional difference, > so only one compatible field is added. > > The controller can have two forms, as a normal display port and as an > embedded display port. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > --- > .../display/mediatek/mediatek,dp.yaml | 99 +++++++++++++++++++ > 1 file changed, 99 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml The example has warnings. Run 'make dt_binding_check' before submitting.
On Tue, 2022-05-24 at 11:35 +0800, Chunfeng Yun wrote: > On Mon, 2022-05-23 at 12:47 +0200, Guillaume Ranquet wrote: > > From: Markus Schneider-Pargmann <msp@baylibre.com> > > > > This controller is present on several mediatek hardware. Currently > > mt8195 and mt8395 have this controller without a functional > > difference, > > so only one compatible field is added. > > > > The controller can have two forms, as a normal display port and as > > an > > embedded display port. > > > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > > --- > > .../display/mediatek/mediatek,dp.yaml | 99 > > +++++++++++++++++++ > > 1 file changed, 99 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.ya > > ml > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.ya > > ml > > new file mode 100644 > > index 000000000000..36ae0a6df299 > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.ya > > ml > > @@ -0,0 +1,99 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > > http://devicetree.org/schemas/display/mediatek/mediatek,dp.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: MediaTek Display Port Controller > > + > > +maintainers: > > + - CK Hu <ck.hu@mediatek.com> > > + - Jitao shi <jitao.shi@mediatek.com> > > + > > +description: | > > + Device tree bindings for the MediaTek (embedded) Display Port > > controller > > + present on some MediaTek SoCs. > > + > > +properties: > > + compatible: > > + enum: > > + - mediatek,mt8195-dp-tx > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: faxi clock > > + > > + clock-names: > > + items: > > + - const: faxi > > + > > + power-domains: > > + maxItems: 1 > > + > > + ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > + properties: > > + port@0: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: Input endpoint of the controller, usually > > dp_intf > > + > > + port@1: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: Output endpoint of the controller > > + > > + max-lanes: > > + maxItems: 1 > > + description: maximum number of lanes supported by the hardware > > + > > + max-linkrate: > > + maxItems: 1 > > + description: maximum link rate supported by the hardware > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - ports > > + - max-lanes > > + - max-linkrate > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/power/mt8195-power.h> > > + edp_tx: edp_tx@1c500000 { > > 'edp_tx: ' can be removed > Hello Chunfeng, ok, I will drop it. > > + compatible = "mediatek,mt8195-dp-tx"; > > + reg = <0 0x1c500000 0 0x8000>; > > reg = <0x1c500000 0x8000>; > #address-cells, #size-cells are both 1 by default > I will use "eg = <0x1c500000 0x8000>;" in binding example. BRs, Bo-Chen > > + interrupts = <GIC_SPI 676 IRQ_TYPE_LEVEL_HIGH 0>; > > + power-domains = <&spm MT8195_POWER_DOMAIN_EPD_TX>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&edp_pin>; > > + max-lanes = /bits/ 8 <4>; > > + max-linkrate = /bits/ 8 <0x1e>; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + edp_in: endpoint { > > + remote-endpoint = <&dp_intf0_out>; > > + }; > > + }; > > + port@1 { > > + reg = <1>; > > + edp_out: endpoint { > > + remote-endpoint = <&panel_in>; > > + }; > > + }; > > + }; > > + };
On Wed, 2022-05-25 at 17:30 +0200, AngeloGioacchino Del Regno wrote: > Il 23/05/22 12:47, Guillaume Ranquet ha scritto: > > From: Markus Schneider-Pargmann <msp@baylibre.com> > > > > This controller is present on several mediatek hardware. Currently > > mt8195 and mt8395 have this controller without a functional > > difference, > > so only one compatible field is added. > > > > The controller can have two forms, as a normal display port and as > > an > > embedded display port. > > > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > > --- > > .../display/mediatek/mediatek,dp.yaml | 99 > > +++++++++++++++++++ > > 1 file changed, 99 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.ya > > ml > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.ya > > ml > > new file mode 100644 > > index 000000000000..36ae0a6df299 > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.ya > > ml > > @@ -0,0 +1,99 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > > http://devicetree.org/schemas/display/mediatek/mediatek,dp.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: MediaTek Display Port Controller > > + > > +maintainers: > > + - CK Hu <ck.hu@mediatek.com> > > + - Jitao shi <jitao.shi@mediatek.com> > > + > > +description: | > > + Device tree bindings for the MediaTek (embedded) Display Port > > controller > > + present on some MediaTek SoCs. > > + > > +properties: > > + compatible: > > + enum: > > + - mediatek,mt8195-dp-tx > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: faxi clock > > + > > + clock-names: > > + items: > > + - const: faxi > > + > > + power-domains: > > + maxItems: 1 > > + > > + ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > + properties: > > + port@0: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: Input endpoint of the controller, usually > > dp_intf > > + > > + port@1: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: Output endpoint of the controller > > + > > You should add port@0 (and port@1, probably) as required... with what > you've done > here, you're saying that "ports" is required, but you're allowing it > to be empty.. > > ports: > $ref: /schemas/graph.yaml#/properties/ports > properties: > port@0: > $ref: /schemas/graph.yaml#/properties/port > description: Input endpoint of the controller, usually > dp_intf > > port@1: > $ref: /schemas/graph.yaml#/properties/port > description: Output endpoint of the controller > > required: > - port@0 > - port@1 > > ^^^ that's how it should look. > Hello Angelo, ok, I will add this. > > + max-lanes: > > + maxItems: 1 > > + description: maximum number of lanes supported by the hardware > > + > > + max-linkrate: > > + maxItems: 1 > > + description: maximum link rate supported by the hardware > > As you've put it (in the example below), the max-linkrate property > wants a value > that corresponds to what you find in the HW registers... this is > wrong. > > Devicetree bindings should be generic and devicetrees shouldn't have > hardware > specific bits inside, hence, please change this property to accept a > link rate > specified in Mbps and also specify that in the description. > > Thanks, > Angelo > ok, I will change it to real linkrate value. BRs, Bo-Chen > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - ports > > + - max-lanes > > + - max-linkrate > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/power/mt8195-power.h> > > + edp_tx: edp_tx@1c500000 { > > + compatible = "mediatek,mt8195-dp-tx"; > > + reg = <0 0x1c500000 0 0x8000>; > > + interrupts = <GIC_SPI 676 IRQ_TYPE_LEVEL_HIGH 0>; > > + power-domains = <&spm MT8195_POWER_DOMAIN_EPD_TX>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&edp_pin>; > > + max-lanes = /bits/ 8 <4>; > > + max-linkrate = /bits/ 8 <0x1e>; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + edp_in: endpoint { > > + remote-endpoint = <&dp_intf0_out>; > > + }; > > + }; > > + port@1 { > > + reg = <1>; > > + edp_out: endpoint { > > + remote-endpoint = <&panel_in>; > > + }; > > + }; > > + }; > > + }; > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml new file mode 100644 index 000000000000..36ae0a6df299 --- /dev/null +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/mediatek/mediatek,dp.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek Display Port Controller + +maintainers: + - CK Hu <ck.hu@mediatek.com> + - Jitao shi <jitao.shi@mediatek.com> + +description: | + Device tree bindings for the MediaTek (embedded) Display Port controller + present on some MediaTek SoCs. + +properties: + compatible: + enum: + - mediatek,mt8195-dp-tx + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + items: + - description: faxi clock + + clock-names: + items: + - const: faxi + + power-domains: + maxItems: 1 + + ports: + $ref: /schemas/graph.yaml#/properties/ports + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: Input endpoint of the controller, usually dp_intf + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: Output endpoint of the controller + + max-lanes: + maxItems: 1 + description: maximum number of lanes supported by the hardware + + max-linkrate: + maxItems: 1 + description: maximum link rate supported by the hardware + +required: + - compatible + - reg + - interrupts + - ports + - max-lanes + - max-linkrate + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/power/mt8195-power.h> + edp_tx: edp_tx@1c500000 { + compatible = "mediatek,mt8195-dp-tx"; + reg = <0 0x1c500000 0 0x8000>; + interrupts = <GIC_SPI 676 IRQ_TYPE_LEVEL_HIGH 0>; + power-domains = <&spm MT8195_POWER_DOMAIN_EPD_TX>; + pinctrl-names = "default"; + pinctrl-0 = <&edp_pin>; + max-lanes = /bits/ 8 <4>; + max-linkrate = /bits/ 8 <0x1e>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + edp_in: endpoint { + remote-endpoint = <&dp_intf0_out>; + }; + }; + port@1 { + reg = <1>; + edp_out: endpoint { + remote-endpoint = <&panel_in>; + }; + }; + }; + };