| Message ID | 20231220100853.20616-3-moudy.ho@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | introduce more MDP3 components in MT8195 | expand |
On 20/12/2023 11:08, Moudy Ho wrote: > To simplify maintenance and avoid branches, the identical component > should be merged and placed in the path belonging to the MDP > (from display/* to media/*). Combining bindings into one bigger meta-binding makes it usually more difficult to maintain and review. > > In addition, currently only MDP utilizes RDMA through CMDQ, and the > necessary properties for "mediatek,gce-events", and "mboxes" have been > set up for this purpose. > Within DISP, it directly receives component interrupt signals. > > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com> > --- > .../display/mediatek/mediatek,mdp-rdma.yaml | 92 ------------------- > .../bindings/media/mediatek,mdp3-rdma.yaml | 43 ++++++++- > 2 files changed, 40 insertions(+), 95 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml > deleted file mode 100644 > index 7570a0684967..000000000000 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml > +++ /dev/null > @@ -1,92 +0,0 @@ > -# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > -%YAML 1.2 > ---- > -$id: http://devicetree.org/schemas/display/mediatek/mediatek,mdp-rdma.yaml# > -$schema: http://devicetree.org/meta-schemas/core.yaml# > - > -title: MediaTek MDP RDMA > - > -maintainers: > - - Chun-Kuang Hu <chunkuang.hu@kernel.org> > - - Philipp Zabel <p.zabel@pengutronix.de> > - > -description: > - The MediaTek MDP RDMA stands for Read Direct Memory Access. > - It provides real time data to the back-end panel driver, such as DSI, > - DPI and DP_INTF. > - It contains one line buffer to store the sufficient pixel data. > - RDMA device node must be siblings to the central MMSYS_CONFIG node. > - For a description of the MMSYS_CONFIG binding, see > - Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml for details. > - > -properties: > - compatible: > - oneOf: > - - const: mediatek,mt8195-vdo1-rdma > - - items: > - - const: mediatek,mt8188-vdo1-rdma > - - const: mediatek,mt8195-vdo1-rdma > - > - reg: > - maxItems: 1 > - > - interrupts: > - maxItems: 1 > - > - power-domains: > - maxItems: 1 > - > - clocks: > - items: > - - description: RDMA Clock > - > - iommus: > - maxItems: 1 > - > - mediatek,gce-client-reg: > - description: > - The register of display function block to be set by gce. There are 4 arguments, > - such as gce node, subsys id, offset and register size. The subsys id that is > - mapping to the register of display function blocks is defined in the gce header > - include/dt-bindings/gce/<chip>-gce.h of each chips. > - $ref: /schemas/types.yaml#/definitions/phandle-array > - items: > - items: > - - description: phandle of GCE > - - description: GCE subsys id > - - description: register offset > - - description: register size > - maxItems: 1 > - > -required: > - - compatible > - - reg > - - power-domains > - - clocks > - - iommus > - - mediatek,gce-client-reg > - > -additionalProperties: false > - > -examples: > - - | > - #include <dt-bindings/interrupt-controller/arm-gic.h> > - #include <dt-bindings/clock/mt8195-clk.h> > - #include <dt-bindings/power/mt8195-power.h> > - #include <dt-bindings/gce/mt8195-gce.h> > - #include <dt-bindings/memory/mt8195-memory-port.h> > - > - soc { > - #address-cells = <2>; > - #size-cells = <2>; > - > - rdma@1c104000 { > - compatible = "mediatek,mt8195-vdo1-rdma"; > - reg = <0 0x1c104000 0 0x1000>; > - interrupts = <GIC_SPI 495 IRQ_TYPE_LEVEL_HIGH 0>; > - clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>; > - power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>; > - iommus = <&iommu_vdo M4U_PORT_L2_MDP_RDMA0>; > - mediatek,gce-client-reg = <&gce0 SUBSYS_1c10XXXX 0x4000 0x1000>; > - }; > - }; > diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml > index e1ffe7eb2cdf..e9955639ce19 100644 > --- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml > +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml > @@ -20,8 +20,12 @@ description: | > > properties: > compatible: > - items: > + oneOf: > - const: mediatek,mt8183-mdp3-rdma > + - const: mediatek,mt8195-vdo1-rdma > + - items: > + - const: mediatek,mt8188-vdo1-rdma > + - const: mediatek,mt8195-vdo1-rdma > > reg: > maxItems: 1 > @@ -60,6 +64,7 @@ properties: > items: > - description: RDMA clock > - description: RSZ clock > + minItems: 1 > > iommus: > maxItems: 1 > @@ -68,6 +73,10 @@ properties: > items: > - description: used for 1st data pipe from RDMA > - description: used for 2nd data pipe from RDMA > + minItems: 1 > + > + interrupts: > + maxItems: 1 Why existing devices now support interrupts? > > '#dma-cells': > const: 1 > @@ -76,13 +85,41 @@ required: > - compatible > - reg > - mediatek,gce-client-reg > - - mediatek,gce-events > - power-domains > - clocks > - iommus > - - mboxes > - '#dma-cells' I see little value in this commit, considering that next month you will want to split it because it will grow unmaintainable. Best regards, Krzysztof
On Thu, Dec 21, 2023 at 10:28:52AM +0100, Krzysztof Kozlowski wrote: > On 20/12/2023 11:08, Moudy Ho wrote: > > To simplify maintenance and avoid branches, the identical component > > should be merged and placed in the path belonging to the MDP > > (from display/* to media/*). > > Combining bindings into one bigger meta-binding makes it usually more > difficult to maintain and review. Yeah, but these mediatek blocks appear to be used for both display and video codec pipelines. So having different bindings was probably wrong to start with. Rob
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml deleted file mode 100644 index 7570a0684967..000000000000 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml +++ /dev/null @@ -1,92 +0,0 @@ -# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) -%YAML 1.2 ---- -$id: http://devicetree.org/schemas/display/mediatek/mediatek,mdp-rdma.yaml# -$schema: http://devicetree.org/meta-schemas/core.yaml# - -title: MediaTek MDP RDMA - -maintainers: - - Chun-Kuang Hu <chunkuang.hu@kernel.org> - - Philipp Zabel <p.zabel@pengutronix.de> - -description: - The MediaTek MDP RDMA stands for Read Direct Memory Access. - It provides real time data to the back-end panel driver, such as DSI, - DPI and DP_INTF. - It contains one line buffer to store the sufficient pixel data. - RDMA device node must be siblings to the central MMSYS_CONFIG node. - For a description of the MMSYS_CONFIG binding, see - Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml for details. - -properties: - compatible: - oneOf: - - const: mediatek,mt8195-vdo1-rdma - - items: - - const: mediatek,mt8188-vdo1-rdma - - const: mediatek,mt8195-vdo1-rdma - - reg: - maxItems: 1 - - interrupts: - maxItems: 1 - - power-domains: - maxItems: 1 - - clocks: - items: - - description: RDMA Clock - - iommus: - maxItems: 1 - - mediatek,gce-client-reg: - description: - The register of display function block to be set by gce. There are 4 arguments, - such as gce node, subsys id, offset and register size. The subsys id that is - mapping to the register of display function blocks is defined in the gce header - include/dt-bindings/gce/<chip>-gce.h of each chips. - $ref: /schemas/types.yaml#/definitions/phandle-array - items: - items: - - description: phandle of GCE - - description: GCE subsys id - - description: register offset - - description: register size - maxItems: 1 - -required: - - compatible - - reg - - power-domains - - clocks - - iommus - - mediatek,gce-client-reg - -additionalProperties: false - -examples: - - | - #include <dt-bindings/interrupt-controller/arm-gic.h> - #include <dt-bindings/clock/mt8195-clk.h> - #include <dt-bindings/power/mt8195-power.h> - #include <dt-bindings/gce/mt8195-gce.h> - #include <dt-bindings/memory/mt8195-memory-port.h> - - soc { - #address-cells = <2>; - #size-cells = <2>; - - rdma@1c104000 { - compatible = "mediatek,mt8195-vdo1-rdma"; - reg = <0 0x1c104000 0 0x1000>; - interrupts = <GIC_SPI 495 IRQ_TYPE_LEVEL_HIGH 0>; - clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>; - power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>; - iommus = <&iommu_vdo M4U_PORT_L2_MDP_RDMA0>; - mediatek,gce-client-reg = <&gce0 SUBSYS_1c10XXXX 0x4000 0x1000>; - }; - }; diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml index e1ffe7eb2cdf..e9955639ce19 100644 --- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml @@ -20,8 +20,12 @@ description: | properties: compatible: - items: + oneOf: - const: mediatek,mt8183-mdp3-rdma + - const: mediatek,mt8195-vdo1-rdma + - items: + - const: mediatek,mt8188-vdo1-rdma + - const: mediatek,mt8195-vdo1-rdma reg: maxItems: 1 @@ -60,6 +64,7 @@ properties: items: - description: RDMA clock - description: RSZ clock + minItems: 1 iommus: maxItems: 1 @@ -68,6 +73,10 @@ properties: items: - description: used for 1st data pipe from RDMA - description: used for 2nd data pipe from RDMA + minItems: 1 + + interrupts: + maxItems: 1 '#dma-cells': const: 1 @@ -76,13 +85,41 @@ required: - compatible - reg - mediatek,gce-client-reg - - mediatek,gce-events - power-domains - clocks - iommus - - mboxes - '#dma-cells' +allOf: + - if: + properties: + compatible: + contains: + const: mediatek,mt8183-mdp3-rdma + + then: + properties: + clocks: + minItems: 2 + + mboxes: + minItems: 2 + + required: + - mboxes + - mediatek,gce-events + + - if: + properties: + compatible: + contains: + const: mediatek,mt8195-vdo1-rdma + + then: + properties: + clocks: + maxItems: 1 + additionalProperties: false examples:
To simplify maintenance and avoid branches, the identical component should be merged and placed in the path belonging to the MDP (from display/* to media/*). In addition, currently only MDP utilizes RDMA through CMDQ, and the necessary properties for "mediatek,gce-events", and "mboxes" have been set up for this purpose. Within DISP, it directly receives component interrupt signals. Signed-off-by: Moudy Ho <moudy.ho@mediatek.com> --- .../display/mediatek/mediatek,mdp-rdma.yaml | 92 ------------------- .../bindings/media/mediatek,mdp3-rdma.yaml | 43 ++++++++- 2 files changed, 40 insertions(+), 95 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml