2017-04-13 07:33:17

by Minghsiu Tsai

[permalink] [raw]
Subject: [PATCH 0/3] Fix mdp device tree

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems. This also simplifies the
device tree.

Daniel Kurtz (2):
arm64: dts: mt8173: Fix mdp device tree
media: mtk-mdp: Fix mdp device tree

Minghsiu Tsai (1):
dt-bindings: mt8173: Fix mdp device tree

.../devicetree/bindings/media/mediatek-mdp.txt | 12 +-
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 126 ++++++++++-----------
drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 2 +-
3 files changed, 64 insertions(+), 76 deletions(-)

--
1.9.1


2017-04-13 07:33:20

by Minghsiu Tsai

[permalink] [raw]
Subject: [PATCH 3/3] media: mtk-mdp: Fix mdp device tree

From: Daniel Kurtz <[email protected]>

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems. This also simplifies the
device tree.

Signed-off-by: Daniel Kurtz <[email protected]>
Signed-off-by: Minghsiu Tsai <[email protected]>

---
drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index 9e4eb7d..a5ad586 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -118,7 +118,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
mutex_init(&mdp->vpulock);

/* Iterate over sibling MDP function blocks */
- for_each_child_of_node(dev->of_node, node) {
+ for_each_child_of_node(dev->of_node->parent, node) {
const struct of_device_id *of_id;
enum mtk_mdp_comp_type comp_type;
int comp_id;
--
1.9.1

2017-04-13 07:33:38

by Minghsiu Tsai

[permalink] [raw]
Subject: [PATCH 2/3] arm64: dts: mt8173: Fix mdp device tree

From: Daniel Kurtz <[email protected]>

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems. This also simplifies the
device tree.

Signed-off-by: Daniel Kurtz <[email protected]>
Signed-off-by: Minghsiu Tsai <[email protected]>

---
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 126 +++++++++++++++----------------
1 file changed, 60 insertions(+), 66 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 6922252..d28a363 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -792,80 +792,74 @@
#clock-cells = <1>;
};

- mdp {
- compatible = "mediatek,mt8173-mdp";
- #address-cells = <2>;
- #size-cells = <2>;
- ranges;
+ mdp_rdma0: rdma@14001000 {
+ compatible = "mediatek,mt8173-mdp-rdma",
+ "mediatek,mt8173-mdp";
+ reg = <0 0x14001000 0 0x1000>;
+ clocks = <&mmsys CLK_MM_MDP_RDMA0>,
+ <&mmsys CLK_MM_MUTEX_32K>;
+ power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+ iommus = <&iommu M4U_PORT_MDP_RDMA0>;
+ mediatek,larb = <&larb0>;
mediatek,vpu = <&vpu>;
+ };

- mdp_rdma0: rdma@14001000 {
- compatible = "mediatek,mt8173-mdp-rdma";
- reg = <0 0x14001000 0 0x1000>;
- clocks = <&mmsys CLK_MM_MDP_RDMA0>,
- <&mmsys CLK_MM_MUTEX_32K>;
- power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
- iommus = <&iommu M4U_PORT_MDP_RDMA0>;
- mediatek,larb = <&larb0>;
- };
-
- mdp_rdma1: rdma@14002000 {
- compatible = "mediatek,mt8173-mdp-rdma";
- reg = <0 0x14002000 0 0x1000>;
- clocks = <&mmsys CLK_MM_MDP_RDMA1>,
- <&mmsys CLK_MM_MUTEX_32K>;
- power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
- iommus = <&iommu M4U_PORT_MDP_RDMA1>;
- mediatek,larb = <&larb4>;
- };
+ mdp_rdma1: rdma@14002000 {
+ compatible = "mediatek,mt8173-mdp-rdma";
+ reg = <0 0x14002000 0 0x1000>;
+ clocks = <&mmsys CLK_MM_MDP_RDMA1>,
+ <&mmsys CLK_MM_MUTEX_32K>;
+ power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+ iommus = <&iommu M4U_PORT_MDP_RDMA1>;
+ mediatek,larb = <&larb4>;
+ };

- mdp_rsz0: rsz@14003000 {
- compatible = "mediatek,mt8173-mdp-rsz";
- reg = <0 0x14003000 0 0x1000>;
- clocks = <&mmsys CLK_MM_MDP_RSZ0>;
- power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
- };
+ mdp_rsz0: rsz@14003000 {
+ compatible = "mediatek,mt8173-mdp-rsz";
+ reg = <0 0x14003000 0 0x1000>;
+ clocks = <&mmsys CLK_MM_MDP_RSZ0>;
+ power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+ };

- mdp_rsz1: rsz@14004000 {
- compatible = "mediatek,mt8173-mdp-rsz";
- reg = <0 0x14004000 0 0x1000>;
- clocks = <&mmsys CLK_MM_MDP_RSZ1>;
- power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
- };
+ mdp_rsz1: rsz@14004000 {
+ compatible = "mediatek,mt8173-mdp-rsz";
+ reg = <0 0x14004000 0 0x1000>;
+ clocks = <&mmsys CLK_MM_MDP_RSZ1>;
+ power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+ };

- mdp_rsz2: rsz@14005000 {
- compatible = "mediatek,mt8173-mdp-rsz";
- reg = <0 0x14005000 0 0x1000>;
- clocks = <&mmsys CLK_MM_MDP_RSZ2>;
- power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
- };
+ mdp_rsz2: rsz@14005000 {
+ compatible = "mediatek,mt8173-mdp-rsz";
+ reg = <0 0x14005000 0 0x1000>;
+ clocks = <&mmsys CLK_MM_MDP_RSZ2>;
+ power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+ };

- mdp_wdma0: wdma@14006000 {
- compatible = "mediatek,mt8173-mdp-wdma";
- reg = <0 0x14006000 0 0x1000>;
- clocks = <&mmsys CLK_MM_MDP_WDMA>;
- power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
- iommus = <&iommu M4U_PORT_MDP_WDMA>;
- mediatek,larb = <&larb0>;
- };
+ mdp_wdma0: wdma@14006000 {
+ compatible = "mediatek,mt8173-mdp-wdma";
+ reg = <0 0x14006000 0 0x1000>;
+ clocks = <&mmsys CLK_MM_MDP_WDMA>;
+ power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+ iommus = <&iommu M4U_PORT_MDP_WDMA>;
+ mediatek,larb = <&larb0>;
+ };

- mdp_wrot0: wrot@14007000 {
- compatible = "mediatek,mt8173-mdp-wrot";
- reg = <0 0x14007000 0 0x1000>;
- clocks = <&mmsys CLK_MM_MDP_WROT0>;
- power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
- iommus = <&iommu M4U_PORT_MDP_WROT0>;
- mediatek,larb = <&larb0>;
- };
+ mdp_wrot0: wrot@14007000 {
+ compatible = "mediatek,mt8173-mdp-wrot";
+ reg = <0 0x14007000 0 0x1000>;
+ clocks = <&mmsys CLK_MM_MDP_WROT0>;
+ power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+ iommus = <&iommu M4U_PORT_MDP_WROT0>;
+ mediatek,larb = <&larb0>;
+ };

- mdp_wrot1: wrot@14008000 {
- compatible = "mediatek,mt8173-mdp-wrot";
- reg = <0 0x14008000 0 0x1000>;
- clocks = <&mmsys CLK_MM_MDP_WROT1>;
- power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
- iommus = <&iommu M4U_PORT_MDP_WROT1>;
- mediatek,larb = <&larb4>;
- };
+ mdp_wrot1: wrot@14008000 {
+ compatible = "mediatek,mt8173-mdp-wrot";
+ reg = <0 0x14008000 0 0x1000>;
+ clocks = <&mmsys CLK_MM_MDP_WROT1>;
+ power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+ iommus = <&iommu M4U_PORT_MDP_WROT1>;
+ mediatek,larb = <&larb4>;
};

ovl0: ovl@1400c000 {
--
1.9.1

2017-04-13 07:35:44

by Minghsiu Tsai

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: mt8173: Fix mdp device tree

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems. This also simplifies the
device tree.

Signed-off-by: Daniel Kurtz <[email protected]>
Signed-off-by: Minghsiu Tsai <[email protected]>

---
Documentation/devicetree/bindings/media/mediatek-mdp.txt | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek-mdp.txt b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
index 4182063..0d03e3a 100644
--- a/Documentation/devicetree/bindings/media/mediatek-mdp.txt
+++ b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
@@ -2,7 +2,7 @@

Media Data Path is used for scaling and color space conversion.

-Required properties (controller (parent) node):
+Required properties (controller node):
- compatible: "mediatek,mt8173-mdp"
- mediatek,vpu: the node of video processor unit, see
Documentation/devicetree/bindings/media/mediatek-vpu.txt for details.
@@ -32,21 +32,16 @@ Required properties (DMA function blocks, child node):
for details.

Example:
-mdp {
- compatible = "mediatek,mt8173-mdp";
- #address-cells = <2>;
- #size-cells = <2>;
- ranges;
- mediatek,vpu = <&vpu>;
-
mdp_rdma0: rdma@14001000 {
compatible = "mediatek,mt8173-mdp-rdma";
+ "mediatek,mt8173-mdp";
reg = <0 0x14001000 0 0x1000>;
clocks = <&mmsys CLK_MM_MDP_RDMA0>,
<&mmsys CLK_MM_MUTEX_32K>;
power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
iommus = <&iommu M4U_PORT_MDP_RDMA0>;
mediatek,larb = <&larb0>;
+ mediatek,vpu = <&vpu>;
};

mdp_rdma1: rdma@14002000 {
@@ -106,4 +101,3 @@ mdp {
iommus = <&iommu M4U_PORT_MDP_WROT1>;
mediatek,larb = <&larb4>;
};
-};
--
1.9.1

2017-04-19 21:35:47

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: mt8173: Fix mdp device tree

On Thu, Apr 13, 2017 at 03:33:05PM +0800, Minghsiu Tsai wrote:
> If the mdp_* nodes are under an mdp sub-node, their corresponding
> platform device does not automatically get its iommu assigned properly.
>
> Fix this by moving the mdp component nodes up a level such that they are
> siblings of mdp and all other SoC subsystems. This also simplifies the
> device tree.

It may simplify the DT, but it also breaks compatibility with old DT.
Not sure if that's a problem on Mediatek platforms, but please be
explicit here that you are breaking compatibility and why that is okay.

>
> Signed-off-by: Daniel Kurtz <[email protected]>
> Signed-off-by: Minghsiu Tsai <[email protected]>

Should this have Daniel as the author?

>
> ---
> Documentation/devicetree/bindings/media/mediatek-mdp.txt | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/mediatek-mdp.txt b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
> index 4182063..0d03e3a 100644
> --- a/Documentation/devicetree/bindings/media/mediatek-mdp.txt
> +++ b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
> @@ -2,7 +2,7 @@
>
> Media Data Path is used for scaling and color space conversion.
>
> -Required properties (controller (parent) node):
> +Required properties (controller node):
> - compatible: "mediatek,mt8173-mdp"
> - mediatek,vpu: the node of video processor unit, see
> Documentation/devicetree/bindings/media/mediatek-vpu.txt for details.
> @@ -32,21 +32,16 @@ Required properties (DMA function blocks, child node):
> for details.
>
> Example:
> -mdp {
> - compatible = "mediatek,mt8173-mdp";
> - #address-cells = <2>;
> - #size-cells = <2>;
> - ranges;
> - mediatek,vpu = <&vpu>;
> -
> mdp_rdma0: rdma@14001000 {
> compatible = "mediatek,mt8173-mdp-rdma";
> + "mediatek,mt8173-mdp";
> reg = <0 0x14001000 0 0x1000>;
> clocks = <&mmsys CLK_MM_MDP_RDMA0>,
> <&mmsys CLK_MM_MUTEX_32K>;
> power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> iommus = <&iommu M4U_PORT_MDP_RDMA0>;
> mediatek,larb = <&larb0>;
> + mediatek,vpu = <&vpu>;
> };
>
> mdp_rdma1: rdma@14002000 {
> @@ -106,4 +101,3 @@ mdp {
> iommus = <&iommu M4U_PORT_MDP_WROT1>;
> mediatek,larb = <&larb4>;
> };
> -};
> --
> 1.9.1
>

2017-04-21 04:06:20

by Minghsiu Tsai

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: mt8173: Fix mdp device tree

On Wed, 2017-04-19 at 16:35 -0500, Rob Herring wrote:
> On Thu, Apr 13, 2017 at 03:33:05PM +0800, Minghsiu Tsai wrote:
> > If the mdp_* nodes are under an mdp sub-node, their corresponding
> > platform device does not automatically get its iommu assigned properly.
> >
> > Fix this by moving the mdp component nodes up a level such that they are
> > siblings of mdp and all other SoC subsystems. This also simplifies the
> > device tree.
>
> It may simplify the DT, but it also breaks compatibility with old DT.
> Not sure if that's a problem on Mediatek platforms, but please be
> explicit here that you are breaking compatibility and why that is okay.
>

I will add the following description for more information.
"
Although it fixes iommu assignment issue, it also break compatibility
with old device tree, so driver patch[1] is needed to iterate over
sibling mdp device nodes, not child ones, to keep driver work properly.
In mtk_mdp_probe()
Old: for_each_child_of_node(dev->of_node, node)
New: for_each_child_of_node(dev->of_node->parent, node)

[1]https://patchwork.kernel.org/patch/9678833/
"

> >
> > Signed-off-by: Daniel Kurtz <[email protected]>
> > Signed-off-by: Minghsiu Tsai <[email protected]>
>
> Should this have Daniel as the author?

This patch is provided by Daniel, so I keep he is the author.
I just split his patch into two parts. One is dts only, and the other is
for driver. I also provide another patch to modify dts bindings
according to this patch.


>
> >
> > ---
> > Documentation/devicetree/bindings/media/mediatek-mdp.txt | 12 +++---------
> > 1 file changed, 3 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/mediatek-mdp.txt b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
> > index 4182063..0d03e3a 100644
> > --- a/Documentation/devicetree/bindings/media/mediatek-mdp.txt
> > +++ b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
> > @@ -2,7 +2,7 @@
> >
> > Media Data Path is used for scaling and color space conversion.
> >
> > -Required properties (controller (parent) node):
> > +Required properties (controller node):
> > - compatible: "mediatek,mt8173-mdp"
> > - mediatek,vpu: the node of video processor unit, see
> > Documentation/devicetree/bindings/media/mediatek-vpu.txt for details.
> > @@ -32,21 +32,16 @@ Required properties (DMA function blocks, child node):
> > for details.
> >
> > Example:
> > -mdp {
> > - compatible = "mediatek,mt8173-mdp";
> > - #address-cells = <2>;
> > - #size-cells = <2>;
> > - ranges;
> > - mediatek,vpu = <&vpu>;
> > -
> > mdp_rdma0: rdma@14001000 {
> > compatible = "mediatek,mt8173-mdp-rdma";
> > + "mediatek,mt8173-mdp";
> > reg = <0 0x14001000 0 0x1000>;
> > clocks = <&mmsys CLK_MM_MDP_RDMA0>,
> > <&mmsys CLK_MM_MUTEX_32K>;
> > power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> > iommus = <&iommu M4U_PORT_MDP_RDMA0>;
> > mediatek,larb = <&larb0>;
> > + mediatek,vpu = <&vpu>;
> > };
> >
> > mdp_rdma1: rdma@14002000 {
> > @@ -106,4 +101,3 @@ mdp {
> > iommus = <&iommu M4U_PORT_MDP_WROT1>;
> > mediatek,larb = <&larb4>;
> > };
> > -};
> > --
> > 1.9.1
> >