Re: [RFC][PATCH 1/2] dt-bindings: drm/mediatek: Add Mediatek DRM dts binding
Hi,
overall it looks to me like this binding is modeled after the Linux DRM
abstractions. Instead, the device nodes should be modeled after the
hardware.
Describing each function block as a separate device node is probably not
the best choice, as would be combining all devices in the mmsys range
into a single device node. So a somewhat arbitrary decision has to be
made what to group together. See my comments below:
Am Mittwoch, den 13.05.2015, 23:23 +0800 schrieb CK Hu:
> This patch includes
> 1. Mediatek DRM Device binding
> 2. Mediatek DSI Device binding
> 3. Mediatek CRTC Main Device binding
> 4. Mediatek DDP Device binding
>
> Signed-off-by: CK Hu
> ---
> .../bindings/drm/mediatek/mediatek,crtc-main.txt | 38
> ++
> .../bindings/drm/mediatek/mediatek,ddp.txt | 22 +
> .../bindings/drm/mediatek/mediatek,drm.txt | 27 +++
> .../bindings/drm/mediatek/mediatek,dsi.txt | 20
> 4 files changed, 107 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
> create mode 100644
> Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
> create mode 100644
> Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
> create mode 100644
> Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt
>
> diff --git
> a/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
> b/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
> new file mode 100644
> index 000..5c6c420
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
> @@ -0,0 +1,38 @@
> +Mediatek CRTC Main Device
> +
> +
> +The Mediatek CRTC Main device is a crtc device of DRM system.
"crtc" does not belong in the device tree. There is no crtc hardware.
What this node describes is a useful, but fixed configuration of a part
of the DISP subsystem function blocks.
In my opinion, it would be better to not describe the separate display
pipes in the device tree at all if they are dynamically configurable.
What for example if you model two separate fixed pipelines in the device
tree and then in the future you want to support the MERGE or SPLIT
blocks (I'm not sure what MERGE does, SPLIT seems to be needed for
8-lane DSI)?
As far as I currently understand, there are five source function blocks
that can read from memory (OVL0, OVL1, RDMA0, RDMA1, RDMA2) and three
sink function blocks (DSI0, DSI1, DPI0) that can be connected to panels,
or encoder bridges. How these map to the crtcs doesn't necessarily have
to be described in the device tree.
How about a single node that contains all of the DISP functional blocks
that don't need their own node (like DSI, which has to be connectable to
bridges or panels):
disp: disp@0x1400c000 {
compatible = "mediatek,mt8173-disp";
interrupts = , /* OVL0 */
; /* OVL1 */
interrupt-names = "ovl0", "ovl1";
reg = <0 0x1400c000 0 0x1000>, /* OVL0 */
<0 0x1400d000 0 0x1000>, /* OVL1 */
<0 0x1400e000 0 0x1000>, /* RDMA0 */
<0 0x1400f000 0 0x1000>, /* RDMA1 */
<0 0x1401 0 0x1000>, /* RDMA2 */
<0 0x14013000 0 0x1000>, /* COLOR0 */
<0 0x14014000 0 0x1000>, /* COLOR1 */
<0 0x14015000 0 0x1000>, /* AAL */
<0 0x14016000 0 0x1000>, /* GAMMA */
<0 0x14017000 0 0x1000>, /* MERGE */
<0 0x14018000 0 0x1000>, /* SPLIT0 */
<0 0x14019000 0 0x1000>, /* SPLIT1 */
<0 0x1401a000 0 0x1000>, /* UFOE */
<0 0x1402 0 0x1000>; /* MUTEX */
<0 0x14023000 0 0x1000>; /* OD */
reg-names = "ovl0", "ovl1", "rdma0", "rdma1", "rdma2",
"color0", "color1", "aal", "gamma", "merge",
"split0", "split1", "ufoe", "mutex", "od";
clocks = < CLK_MM_DISP_OVL0>,
< CLK_MM_DISP_OVL1>,
< CLK_MM_DISP_RDMA0>,
< CLK_MM_DISP_RDMA1>,
< CLK_MM_DISP_RDMA2>,
< CLK_MM_DISP_COLOR0>,
< CLK_MM_DISP_COLOR1>,
< CLK_MM_DISP_AAL>,
< CLK_MM_DISP_GAMMA>,
< CLK_MM_DISP_MERGE>,
< CLK_MM_DISP_SPLIT0>,
< CLK_MM_DISP_SPLIT1>,
< CLK_MM_DISP_UFOE>,
< CLK_MM_MUTEX_32K>,
< CLK_MM_DISP_OD>;
clock-names = "ovl0", "ovl1", "rdma0", "rdma1", "rdma2",
"color0", "color1", "aal", "gamma", "merge",
"split0", "split1", "ufoe", "mutex", "od";
power-domains = < MT8173_POWER_DOMAIN_DIS>;
config = <>; /* syscon */
};
How the muxes in the config area are set up to connect those blocks
could be determined by the driver.
> +Required properties:
> +-
Re: [RFC][PATCH 1/2] dt-bindings: drm/mediatek: Add Mediatek DRM dts binding
Hi,
overall it looks to me like this binding is modeled after the Linux DRM
abstractions. Instead, the device nodes should be modeled after the
hardware.
Describing each function block as a separate device node is probably not
the best choice, as would be combining all devices in the mmsys range
into a single device node. So a somewhat arbitrary decision has to be
made what to group together. See my comments below:
Am Mittwoch, den 13.05.2015, 23:23 +0800 schrieb CK Hu:
This patch includes
1. Mediatek DRM Device binding
2. Mediatek DSI Device binding
3. Mediatek CRTC Main Device binding
4. Mediatek DDP Device binding
Signed-off-by: CK Hu ck...@mediatek.com
---
.../bindings/drm/mediatek/mediatek,crtc-main.txt | 38
++
.../bindings/drm/mediatek/mediatek,ddp.txt | 22 +
.../bindings/drm/mediatek/mediatek,drm.txt | 27 +++
.../bindings/drm/mediatek/mediatek,dsi.txt | 20
4 files changed, 107 insertions(+)
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt
diff --git
a/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
b/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
new file mode 100644
index 000..5c6c420
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
@@ -0,0 +1,38 @@
+Mediatek CRTC Main Device
+
+
+The Mediatek CRTC Main device is a crtc device of DRM system.
crtc does not belong in the device tree. There is no crtc hardware.
What this node describes is a useful, but fixed configuration of a part
of the DISP subsystem function blocks.
In my opinion, it would be better to not describe the separate display
pipes in the device tree at all if they are dynamically configurable.
What for example if you model two separate fixed pipelines in the device
tree and then in the future you want to support the MERGE or SPLIT
blocks (I'm not sure what MERGE does, SPLIT seems to be needed for
8-lane DSI)?
As far as I currently understand, there are five source function blocks
that can read from memory (OVL0, OVL1, RDMA0, RDMA1, RDMA2) and three
sink function blocks (DSI0, DSI1, DPI0) that can be connected to panels,
or encoder bridges. How these map to the crtcs doesn't necessarily have
to be described in the device tree.
How about a single node that contains all of the DISP functional blocks
that don't need their own node (like DSI, which has to be connectable to
bridges or panels):
disp: disp@0x1400c000 {
compatible = mediatek,mt8173-disp;
interrupts = GIC_SPI 180 IRQ_TYPE_LEVEL_LOW, /* OVL0 */
GIC_SPI 181 IRQ_TYPE_LEVEL_LOW; /* OVL1 */
interrupt-names = ovl0, ovl1;
reg = 0 0x1400c000 0 0x1000, /* OVL0 */
0 0x1400d000 0 0x1000, /* OVL1 */
0 0x1400e000 0 0x1000, /* RDMA0 */
0 0x1400f000 0 0x1000, /* RDMA1 */
0 0x1401 0 0x1000, /* RDMA2 */
0 0x14013000 0 0x1000, /* COLOR0 */
0 0x14014000 0 0x1000, /* COLOR1 */
0 0x14015000 0 0x1000, /* AAL */
0 0x14016000 0 0x1000, /* GAMMA */
0 0x14017000 0 0x1000, /* MERGE */
0 0x14018000 0 0x1000, /* SPLIT0 */
0 0x14019000 0 0x1000, /* SPLIT1 */
0 0x1401a000 0 0x1000, /* UFOE */
0 0x1402 0 0x1000; /* MUTEX */
0 0x14023000 0 0x1000; /* OD */
reg-names = ovl0, ovl1, rdma0, rdma1, rdma2,
color0, color1, aal, gamma, merge,
split0, split1, ufoe, mutex, od;
clocks = mmsys CLK_MM_DISP_OVL0,
mmsys CLK_MM_DISP_OVL1,
mmsys CLK_MM_DISP_RDMA0,
mmsys CLK_MM_DISP_RDMA1,
mmsys CLK_MM_DISP_RDMA2,
mmsys CLK_MM_DISP_COLOR0,
mmsys CLK_MM_DISP_COLOR1,
mmsys CLK_MM_DISP_AAL,
mmsys CLK_MM_DISP_GAMMA,
mmsys CLK_MM_DISP_MERGE,
mmsys CLK_MM_DISP_SPLIT0,
mmsys CLK_MM_DISP_SPLIT1,
mmsys CLK_MM_DISP_UFOE,
mmsys CLK_MM_MUTEX_32K,
mmsys CLK_MM_DISP_OD;
clock-names = ovl0, ovl1, rdma0, rdma1, rdma2,
color0, color1, aal, gamma, merge,
split0, split1, ufoe, mutex, od;
power-domains = scpsys MT8173_POWER_DOMAIN_DIS;
config = mmsys; /* syscon */
};
How the muxes in the config area are set up to connect those blocks
could be determined by the driver.
+Required properties:
+-
[RFC][PATCH 1/2] dt-bindings: drm/mediatek: Add Mediatek DRM dts binding
This patch includes
1. Mediatek DRM Device binding
2. Mediatek DSI Device binding
3. Mediatek CRTC Main Device binding
4. Mediatek DDP Device binding
Signed-off-by: CK Hu
---
.../bindings/drm/mediatek/mediatek,crtc-main.txt | 38 ++
.../bindings/drm/mediatek/mediatek,ddp.txt | 22 +
.../bindings/drm/mediatek/mediatek,drm.txt | 27 +++
.../bindings/drm/mediatek/mediatek,dsi.txt | 20
4 files changed, 107 insertions(+)
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt
diff --git
a/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
b/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
new file mode 100644
index 000..5c6c420
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
@@ -0,0 +1,38 @@
+Mediatek CRTC Main Device
+
+
+The Mediatek CRTC Main device is a crtc device of DRM system.
+
+Required properties:
+- compatible: "mediatek,-crtc-main"
+- interrupts: The interrupt signal from the CRTC Main block.
+- reg: Physical base address and length of the controller's registers
+- clocks: device clocks
+ See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
+- ddp: phandle of ddp device which control display data path.
+
+Example:
+
+crtc_main: crtc@1400c000 {
+ compatible = "mediatek,mt8173-crtc-main";
+ interrupts = ;
+ reg = <0 0x1400c000 0 0x1000>, /* OVL0 */
+ <0 0x1400e000 0 0x1000>, /* RDMA0 */
+ <0 0x14013000 0 0x1000>, /* COLOR0 */
+ <0 0x14015000 0 0x1000>, /* AAL */
+ <0 0x1401a000 0 0x1000>, /* UFOE */
+ <0 0x14023000 0 0x1000>; /* OD */
+ clocks = < MM_DISP_OVL0>,
+< MM_DISP_RDMA0>,
+< MM_DISP_COLOR0>,
+< MM_DISP_AAL>,
+< MM_DISP_UFOE>,
+< MM_DISP_OD>;
+ clock-names = "ovl0_disp",
+ "rdma0_disp",
+ "color0_disp",
+ "aal_disp",
+ "ufoe_disp",
+ "od_disp";
+ ddp = <>;
+};
\ No newline at end of file
diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
b/Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
new file mode 100644
index 000..77cf630
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
@@ -0,0 +1,22 @@
+Mediatek DDP Device
+
+
+The Mediatek DDP device control the display data path.
+
+Required properties:
+- compatible: "mediatek,-ddp"
+- reg: Physical base address and length of the controller's registers
+- power-domains: a phandle to DDP power domain node.
+- clocks: device clocks
+ See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
+
+Example:
+
+ddp: ddp@1400 {
+ compatible = "mediatek,mt8173-ddp";
+ reg = <0 0x1400 0 0x100>, /* CONFIG */
+ <0 0x1402 0 0x1000>; /* MUTEX */
+ power-domains = < MT8173_POWER_DOMAIN_DIS>;
+ clocks = < MM_MUTEX_32K>;
+ clock-names = "mutex_disp";
+};
\ No newline at end of file
diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
b/Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
new file mode 100644
index 000..c4a5702
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
@@ -0,0 +1,27 @@
+Mediatek DRM Device
+
+
+The Mediatek DRM device is a device needed to list all
+display component nodes that comprise the display subsystem.
+And it list the memory-related interface.
+
+Required properties:
+- compatible: "mediatek,-drm"
+- larb: Should contain a list of phandles pointing to larb device.
+ larb definitions as defined in
+ Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt
+- iommus: required a iommu node
+- connectors: Should contain a list of phandles pointing to connector device.
+ connector device should be one component of this master.
+- crtcs: Should contain a list of phandles pointing to crtc device.
+ crtc device should be one component of this master.
+
+Example:
+
+drm0: drm {
+ compatible = "mediatek,mt8173-drm";
+ larb = <>;
+ iommus = < M4U_PORT_DISP_OVL0>;
+ connectors = <>;
+ crtcs = <_main>;
+};
\ No newline at end of file
diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt
b/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt
new file mode 100644
index 000..16e3eb3
---
[RFC][PATCH 1/2] dt-bindings: drm/mediatek: Add Mediatek DRM dts binding
This patch includes
1. Mediatek DRM Device binding
2. Mediatek DSI Device binding
3. Mediatek CRTC Main Device binding
4. Mediatek DDP Device binding
Signed-off-by: CK Hu ck...@mediatek.com
---
.../bindings/drm/mediatek/mediatek,crtc-main.txt | 38 ++
.../bindings/drm/mediatek/mediatek,ddp.txt | 22 +
.../bindings/drm/mediatek/mediatek,drm.txt | 27 +++
.../bindings/drm/mediatek/mediatek,dsi.txt | 20
4 files changed, 107 insertions(+)
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
create mode 100644
Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt
diff --git
a/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
b/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
new file mode 100644
index 000..5c6c420
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,crtc-main.txt
@@ -0,0 +1,38 @@
+Mediatek CRTC Main Device
+
+
+The Mediatek CRTC Main device is a crtc device of DRM system.
+
+Required properties:
+- compatible: mediatek,chip-crtc-main
+- interrupts: The interrupt signal from the CRTC Main block.
+- reg: Physical base address and length of the controller's registers
+- clocks: device clocks
+ See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
+- ddp: phandle of ddp device which control display data path.
+
+Example:
+
+crtc_main: crtc@1400c000 {
+ compatible = mediatek,mt8173-crtc-main;
+ interrupts = GIC_SPI 196 IRQ_TYPE_LEVEL_LOW;
+ reg = 0 0x1400c000 0 0x1000, /* OVL0 */
+ 0 0x1400e000 0 0x1000, /* RDMA0 */
+ 0 0x14013000 0 0x1000, /* COLOR0 */
+ 0 0x14015000 0 0x1000, /* AAL */
+ 0 0x1401a000 0 0x1000, /* UFOE */
+ 0 0x14023000 0 0x1000; /* OD */
+ clocks = mmsys MM_DISP_OVL0,
+mmsys MM_DISP_RDMA0,
+mmsys MM_DISP_COLOR0,
+mmsys MM_DISP_AAL,
+mmsys MM_DISP_UFOE,
+mmsys MM_DISP_OD;
+ clock-names = ovl0_disp,
+ rdma0_disp,
+ color0_disp,
+ aal_disp,
+ ufoe_disp,
+ od_disp;
+ ddp = ddp;
+};
\ No newline at end of file
diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
b/Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
new file mode 100644
index 000..77cf630
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,ddp.txt
@@ -0,0 +1,22 @@
+Mediatek DDP Device
+
+
+The Mediatek DDP device control the display data path.
+
+Required properties:
+- compatible: mediatek,chip-ddp
+- reg: Physical base address and length of the controller's registers
+- power-domains: a phandle to DDP power domain node.
+- clocks: device clocks
+ See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
+
+Example:
+
+ddp: ddp@1400 {
+ compatible = mediatek,mt8173-ddp;
+ reg = 0 0x1400 0 0x100, /* CONFIG */
+ 0 0x1402 0 0x1000; /* MUTEX */
+ power-domains = scpsys MT8173_POWER_DOMAIN_DIS;
+ clocks = mmsys MM_MUTEX_32K;
+ clock-names = mutex_disp;
+};
\ No newline at end of file
diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
b/Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
new file mode 100644
index 000..c4a5702
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,drm.txt
@@ -0,0 +1,27 @@
+Mediatek DRM Device
+
+
+The Mediatek DRM device is a device needed to list all
+display component nodes that comprise the display subsystem.
+And it list the memory-related interface.
+
+Required properties:
+- compatible: mediatek,chip-drm
+- larb: Should contain a list of phandles pointing to larb device.
+ larb definitions as defined in
+ Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt
+- iommus: required a iommu node
+- connectors: Should contain a list of phandles pointing to connector device.
+ connector device should be one component of this master.
+- crtcs: Should contain a list of phandles pointing to crtc device.
+ crtc device should be one component of this master.
+
+Example:
+
+drm0: drm {
+ compatible = mediatek,mt8173-drm;
+ larb = larb0;
+ iommus = iommu M4U_PORT_DISP_OVL0;
+ connectors = dsi;
+ crtcs = crtc_main;
+};
\ No newline at end of file
diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt
b/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt
