2020-08-04 11:15:50

by Frank Wunderlich

[permalink] [raw]
Subject: [PATCH v3 0/5] make hdmi work on bananapi-r2

From: Frank Wunderlich <[email protected]>

This Patch-Series adds missing Patches/Bugfixes to get hdmi working
on BPI-R2

v2->v3:
- use own mmsys-routing for mt7623 instead of code getting different
routing from dts
- remove ddp routing bls -> dpi from bpir2/rfb dts
- updated some commit-Messages as suggested by CK Hu
v1->v2:
- using get_possible_crtc API instead of hardcoded
- drop unused dts-nodes
- refine commit-messages as far as i can :)
"config component output by device node port" is needed to fix a WARN_ON()
"fix boot up for 720 and 480 but 1080" fixes flickering,
which may cause also some resolutions not working on some TFT (had some problems on my smaller TFT)

Frank Wunderlich (1):
drm: mediatek: add ddp routing for mt7623

Jitao Shi (1):
drm/mediatek: dpi/dsi: change the getting possible_crtc way

Ryder Lee (1):
arm: dts: mt7623: add display subsystem related device nodes

Stu Hsieh (1):
drm: Add get_possible_crtc API for dpi, dsi

chunhui dai (1):
drm/mediatek: disable tmds on mt2701

arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++
arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 72 +++++++
arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 72 +++++++
drivers/gpu/drm/mediatek/mtk_dpi.c | 3 +-
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 42 +++++
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 2 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 25 ++-
drivers/gpu/drm/mediatek/mtk_dsi.c | 3 +-
drivers/gpu/drm/mediatek/mtk_hdmi_phy.c | 3 +
drivers/gpu/drm/mediatek/mtk_hdmi_phy.h | 1 +
.../gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c | 1 +
11 files changed, 398 insertions(+), 3 deletions(-)

--
2.25.1


2020-08-04 11:17:08

by Frank Wunderlich

[permalink] [raw]
Subject: [PATCH v3 2/5] drm/mediatek: disable tmds on mt2701

From: chunhui dai <[email protected]>

disable tmds on phy on mt2701 to support other resolutions like 1280x1024

Signed-off-by: chunhui dai <[email protected]>
Signed-off-by: Frank Wunderlich <[email protected]>
Tested-by: Frank Wunderlich <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_hdmi_phy.c | 3 +++
drivers/gpu/drm/mediatek/mtk_hdmi_phy.h | 1 +
drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c | 1 +
3 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c
index 5223498502c4..edadb7a700f1 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.c
@@ -184,6 +184,9 @@ static int mtk_hdmi_phy_probe(struct platform_device *pdev)
return PTR_ERR(phy_provider);
}

+ if (hdmi_phy->conf->pll_default_off)
+ hdmi_phy->conf->hdmi_phy_disable_tmds(hdmi_phy);
+
return of_clk_add_provider(dev->of_node, of_clk_src_simple_get,
hdmi_phy->pll);
}
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h
index 2d8b3182470d..f472fdeb63dc 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi_phy.h
@@ -22,6 +22,7 @@ struct mtk_hdmi_phy;
struct mtk_hdmi_phy_conf {
bool tz_disabled;
unsigned long flags;
+ bool pll_default_off;
const struct clk_ops *hdmi_phy_clk_ops;
void (*hdmi_phy_enable_tmds)(struct mtk_hdmi_phy *hdmi_phy);
void (*hdmi_phy_disable_tmds)(struct mtk_hdmi_phy *hdmi_phy);
diff --git a/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c b/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
index d3cc4022e988..6fbedacfc1e8 100644
--- a/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
+++ b/drivers/gpu/drm/mediatek/mtk_mt2701_hdmi_phy.c
@@ -239,6 +239,7 @@ static void mtk_hdmi_phy_disable_tmds(struct mtk_hdmi_phy *hdmi_phy)
struct mtk_hdmi_phy_conf mtk_hdmi_phy_2701_conf = {
.tz_disabled = true,
.flags = CLK_SET_RATE_GATE,
+ .pll_default_off = true,
.hdmi_phy_clk_ops = &mtk_hdmi_phy_pll_ops,
.hdmi_phy_enable_tmds = mtk_hdmi_phy_enable_tmds,
.hdmi_phy_disable_tmds = mtk_hdmi_phy_disable_tmds,
--
2.25.1

2020-08-04 11:23:26

by Frank Wunderlich

[permalink] [raw]
Subject: [PATCH v3 5/5] arm: dts: mt7623: add display subsystem related device nodes

From: Ryder Lee <[email protected]>

Add display subsystem related device nodes for MT7623.

Cc: CK Hu <[email protected]>
Signed-off-by: chunhui dai <[email protected]>
Signed-off-by: Bibby Hsieh <[email protected]>
Signed-off-by: Ryder Lee <[email protected]>
Signed-off-by: Frank Wunderlich <[email protected]>
Tested-by: Frank Wunderlich <[email protected]>
---
changed v2->v3:
drop bls to dpi routing
---
arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++
arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 72 +++++++
arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 72 +++++++
3 files changed, 321 insertions(+)

diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
index a106c0d90a52..d584a3d678ba 100644
--- a/arch/arm/boot/dts/mt7623.dtsi
+++ b/arch/arm/boot/dts/mt7623.dtsi
@@ -24,6 +24,11 @@ / {
#address-cells = <2>;
#size-cells = <2>;

+ aliases {
+ rdma0 = &rdma0;
+ rdma1 = &rdma1;
+ };
+
cpu_opp_table: opp-table {
compatible = "operating-points-v2";
opp-shared;
@@ -321,6 +326,25 @@ pwrap: pwrap@1000d000 {
clock-names = "spi", "wrap";
};

+ mipi_tx0: mipi-dphy@10010000 {
+ compatible = "mediatek,mt7623-mipi-tx",
+ "mediatek,mt2701-mipi-tx";
+ reg = <0 0x10010000 0 0x90>;
+ clocks = <&clk26m>;
+ clock-output-names = "mipi_tx0_pll";
+ #clock-cells = <0>;
+ #phy-cells = <0>;
+ };
+
+ cec: cec@10012000 {
+ compatible = "mediatek,mt7623-cec",
+ "mediatek,mt8173-cec";
+ reg = <0 0x10012000 0 0xbc>;
+ interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&infracfg CLK_INFRA_CEC>;
+ status = "disabled";
+ };
+
cir: cir@10013000 {
compatible = "mediatek,mt7623-cir";
reg = <0 0x10013000 0 0x1000>;
@@ -369,6 +393,18 @@ apmixedsys: syscon@10209000 {
#clock-cells = <1>;
};

+ hdmi_phy: phy@10209100 {
+ compatible = "mediatek,mt7623-hdmi-phy",
+ "mediatek,mt2701-hdmi-phy";
+ reg = <0 0x10209100 0 0x24>;
+ clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
+ clock-names = "pll_ref";
+ clock-output-names = "hdmitx_dig_cts";
+ #clock-cells = <0>;
+ #phy-cells = <0>;
+ status = "disabled";
+ };
+
rng: rng@1020f000 {
compatible = "mediatek,mt7623-rng";
reg = <0 0x1020f000 0 0x1000>;
@@ -568,6 +604,16 @@ bch: ecc@1100e000 {
status = "disabled";
};

+ hdmiddc0: i2c@11013000 {
+ compatible = "mediatek,mt7623-hdmi-ddc",
+ "mediatek,mt8173-hdmi-ddc";
+ interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
+ reg = <0 0x11013000 0 0x1C>;
+ clocks = <&pericfg CLK_PERI_I2C3>;
+ clock-names = "ddc-i2c";
+ status = "disabled";
+ };
+
nor_flash: spi@11014000 {
compatible = "mediatek,mt7623-nor",
"mediatek,mt8173-nor";
@@ -766,6 +812,84 @@ mmsys: syscon@14000000 {
#clock-cells = <1>;
};

+ display_components: dispsys@14000000 {
+ compatible = "mediatek,mt7623-mmsys",
+ "mediatek,mt2701-mmsys";
+ reg = <0 0x14000000 0 0x1000>;
+ power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
+ };
+
+ ovl@14007000 {
+ compatible = "mediatek,mt7623-disp-ovl",
+ "mediatek,mt2701-disp-ovl";
+ reg = <0 0x14007000 0 0x1000>;
+ interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_OVL>;
+ iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
+ mediatek,larb = <&larb0>;
+ };
+
+ rdma0: rdma@14008000 {
+ compatible = "mediatek,mt7623-disp-rdma",
+ "mediatek,mt2701-disp-rdma";
+ reg = <0 0x14008000 0 0x1000>;
+ interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_RDMA>;
+ iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
+ mediatek,larb = <&larb0>;
+ };
+
+ wdma@14009000 {
+ compatible = "mediatek,mt7623-disp-wdma",
+ "mediatek,mt2701-disp-wdma";
+ reg = <0 0x14009000 0 0x1000>;
+ interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_WDMA>;
+ iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
+ mediatek,larb = <&larb0>;
+ };
+
+ bls: pwm@1400a000 {
+ compatible = "mediatek,mt7623-disp-pwm",
+ "mediatek,mt2701-disp-pwm";
+ reg = <0 0x1400a000 0 0x1000>;
+ #pwm-cells = <2>;
+ clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
+ <&mmsys CLK_MM_DISP_BLS>;
+ clock-names = "main", "mm";
+ status = "disabled";
+ };
+
+ color@1400b000 {
+ compatible = "mediatek,mt7623-disp-color",
+ "mediatek,mt2701-disp-color";
+ reg = <0 0x1400b000 0 0x1000>;
+ interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_COLOR>;
+ };
+
+ dsi: dsi@1400c000 {
+ compatible = "mediatek,mt7623-dsi",
+ "mediatek,mt2701-dsi";
+ reg = <0 0x1400c000 0 0x1000>;
+ interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DSI_ENGINE>,
+ <&mmsys CLK_MM_DSI_DIG>,
+ <&mipi_tx0>;
+ clock-names = "engine", "digital", "hs";
+ phys = <&mipi_tx0>;
+ phy-names = "dphy";
+ status = "disabled";
+ };
+
+ mutex: mutex@1400e000 {
+ compatible = "mediatek,mt7623-disp-mutex",
+ "mediatek,mt2701-disp-mutex";
+ reg = <0 0x1400e000 0 0x1000>;
+ interrupts = <GIC_SPI 161 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_MUTEX_32K>;
+ };
+
larb0: larb@14010000 {
compatible = "mediatek,mt7623-smi-larb",
"mediatek,mt2701-smi-larb";
@@ -778,6 +902,44 @@ larb0: larb@14010000 {
power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
};

+ rdma1: rdma@14012000 {
+ compatible = "mediatek,mt7623-disp-rdma",
+ "mediatek,mt2701-disp-rdma";
+ reg = <0 0x14012000 0 0x1000>;
+ interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_RDMA1>;
+ iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA1>;
+ mediatek,larb = <&larb0>;
+ };
+
+ dpi0: dpi@14014000 {
+ compatible = "mediatek,mt7623-dpi",
+ "mediatek,mt2701-dpi";
+ reg = <0 0x14014000 0 0x1000>;
+ interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DPI1_DIGL>,
+ <&mmsys CLK_MM_DPI1_ENGINE>,
+ <&apmixedsys CLK_APMIXED_TVDPLL>;
+ clock-names = "pixel", "engine", "pll";
+ status = "disabled";
+ };
+
+ hdmi0: hdmi@14015000 {
+ compatible = "mediatek,mt7623-hdmi",
+ "mediatek,mt8173-hdmi";
+ reg = <0 0x14015000 0 0x400>;
+ clocks = <&mmsys CLK_MM_HDMI_PIXEL>,
+ <&mmsys CLK_MM_HDMI_PLL>,
+ <&mmsys CLK_MM_HDMI_AUDIO>,
+ <&mmsys CLK_MM_HDMI_SPDIF>;
+ clock-names = "pixel", "pll", "bclk", "spdif";
+ phys = <&hdmi_phy>;
+ phy-names = "hdmi";
+ mediatek,syscon-hdmi = <&mmsys 0x900>;
+ cec = <&cec>;
+ status = "disabled";
+ };
+
imgsys: syscon@15000000 {
compatible = "mediatek,mt7623-imgsys",
"mediatek,mt2701-imgsys",
@@ -1102,6 +1264,21 @@ pins-cir {
};
};

+ hdmi_pins_a: hdmi-default {
+ pins-hdmi {
+ pinmux = <MT7623_PIN_123_HTPLG_FUNC_HTPLG>;
+ input-enable;
+ bias-pull-down;
+ };
+ };
+
+ hdmi_ddc_pins_a: hdmi_ddc-default {
+ pins-hdmi-ddc {
+ pinmux = <MT7623_PIN_124_GPIO124_FUNC_HDMISCK>,
+ <MT7623_PIN_125_GPIO125_FUNC_HDMISD>;
+ };
+ };
+
i2c0_pins_a: i2c0-default {
pins-i2c0 {
pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
index 2b760f90f38c..b1c83e919015 100644
--- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
+++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
@@ -21,6 +21,19 @@ chosen {
stdout-path = "serial2:115200n8";
};

+ connector {
+ compatible = "hdmi-connector";
+ label = "hdmi";
+ type = "d";
+ ddc-i2c-bus = <&hdmiddc0>;
+
+ port {
+ hdmi_connector_in: endpoint {
+ remote-endpoint = <&hdmi0_out>;
+ };
+ };
+ };
+
cpus {
cpu@0 {
proc-supply = <&mt6323_vproc_reg>;
@@ -114,10 +127,18 @@ memory@80000000 {
};
};

+&bls {
+ status = "okay";
+};
+
&btif {
status = "okay";
};

+&cec {
+ status = "okay";
+};
+
&cir {
pinctrl-names = "default";
pinctrl-0 = <&cir_pins_a>;
@@ -128,6 +149,21 @@ &crypto {
status = "okay";
};

+&dpi0 {
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ dpi0_out: endpoint {
+ remote-endpoint = <&hdmi0_in>;
+ };
+ };
+ };
+};
+
&eth {
status = "okay";

@@ -199,6 +235,42 @@ fixed-link {
};
};

+&hdmi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmi_pins_a>;
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ hdmi0_in: endpoint {
+ remote-endpoint = <&dpi0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ hdmi0_out: endpoint {
+ remote-endpoint = <&hdmi_connector_in>;
+ };
+ };
+ };
+};
+
+&hdmiddc0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmi_ddc_pins_a>;
+ status = "okay";
+};
+
+&hdmi_phy {
+ mediatek,ibias = <0xa>;
+ mediatek,ibias_up = <0x1c>;
+ status = "okay";
+};
+
&i2c0 {
pinctrl-names = "default";
pinctrl-0 = <&i2c0_pins_a>;
diff --git a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
index 0447748f9fa0..7e3ded7d9e7d 100644
--- a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
+++ b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
@@ -24,6 +24,19 @@ chosen {
stdout-path = "serial2:115200n8";
};

+ connector {
+ compatible = "hdmi-connector";
+ label = "hdmi";
+ type = "d";
+ ddc-i2c-bus = <&hdmiddc0>;
+
+ port {
+ hdmi_connector_in: endpoint {
+ remote-endpoint = <&hdmi0_out>;
+ };
+ };
+ };
+
cpus {
cpu@0 {
proc-supply = <&mt6323_vproc_reg>;
@@ -106,10 +119,18 @@ sound {
};
};

+&bls {
+ status = "okay";
+};
+
&btif {
status = "okay";
};

+&cec {
+ status = "okay";
+};
+
&cir {
pinctrl-names = "default";
pinctrl-0 = <&cir_pins_a>;
@@ -120,6 +141,21 @@ &crypto {
status = "okay";
};

+&dpi0 {
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ dpi0_out: endpoint {
+ remote-endpoint = <&hdmi0_in>;
+ };
+ };
+ };
+};
+
&eth {
status = "okay";

@@ -203,6 +239,42 @@ fixed-link {
};
};

+&hdmi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmi_pins_a>;
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ hdmi0_in: endpoint {
+ remote-endpoint = <&dpi0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ hdmi0_out: endpoint {
+ remote-endpoint = <&hdmi_connector_in>;
+ };
+ };
+ };
+};
+
+&hdmiddc0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmi_ddc_pins_a>;
+ status = "okay";
+};
+
+&hdmi_phy {
+ mediatek,ibias = <0xa>;
+ mediatek,ibias_up = <0x1c>;
+ status = "okay";
+};
+
&i2c0 {
pinctrl-names = "default";
pinctrl-0 = <&i2c0_pins_a>;
--
2.25.1

2020-08-04 11:23:56

by Frank Wunderlich

[permalink] [raw]
Subject: [PATCH v3 1/5] drm: mediatek: add ddp routing for mt7623

From: Frank Wunderlich <[email protected]>

on BPi-R2/mt7623 main-path have to be routed to DPI0 (hdmi) instead of DSI0
using compatible "mt7623-mmsys" already defined in dts

Signed-off-by: Frank Wunderlich <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 6bd369434d9d..9e418b83be48 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -74,7 +74,20 @@ static const enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = {
DDP_COMPONENT_DPI0,
};

-static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
+static const enum mtk_ddp_comp_id mt7623_mtk_ddp_main[] = {
+ DDP_COMPONENT_OVL0,
+ DDP_COMPONENT_RDMA0,
+ DDP_COMPONENT_COLOR0,
+ DDP_COMPONENT_BLS,
+ DDP_COMPONENT_DPI0,
+};
+
+static const enum mtk_ddp_comp_id mt7623_mtk_ddp_ext[] = {
+ DDP_COMPONENT_RDMA1,
+ DDP_COMPONENT_DSI0,
+};
+
+static enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
DDP_COMPONENT_OVL0,
DDP_COMPONENT_COLOR0,
DDP_COMPONENT_AAL0,
@@ -127,6 +140,14 @@ static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
.shadow_register = true,
};

+static const struct mtk_mmsys_driver_data mt7623_mmsys_driver_data = {
+ .main_path = mt7623_mtk_ddp_main,
+ .main_len = ARRAY_SIZE(mt7623_mtk_ddp_main),
+ .ext_path = mt7623_mtk_ddp_ext,
+ .ext_len = ARRAY_SIZE(mt7623_mtk_ddp_ext),
+ .shadow_register = true,
+};
+
static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
.main_path = mt2712_mtk_ddp_main,
.main_len = ARRAY_SIZE(mt2712_mtk_ddp_main),
@@ -422,6 +443,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
static const struct of_device_id mtk_drm_of_ids[] = {
{ .compatible = "mediatek,mt2701-mmsys",
.data = &mt2701_mmsys_driver_data},
+ { .compatible = "mediatek,mt7623-mmsys",
+ .data = &mt7623_mmsys_driver_data},
{ .compatible = "mediatek,mt2712-mmsys",
.data = &mt2712_mmsys_driver_data},
{ .compatible = "mediatek,mt8173-mmsys",
--
2.25.1

2020-08-04 14:39:15

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] drm: mediatek: add ddp routing for mt7623

Hi, Frank:

Frank Wunderlich <[email protected]> 於 2020年8月4日 週二 下午7:00寫道:
>
> From: Frank Wunderlich <[email protected]>
>
> on BPi-R2/mt7623 main-path have to be routed to DPI0 (hdmi) instead of DSI0
> using compatible "mt7623-mmsys" already defined in dts
>
> Signed-off-by: Frank Wunderlich <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 6bd369434d9d..9e418b83be48 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -74,7 +74,20 @@ static const enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = {
> DDP_COMPONENT_DPI0,
> };
>
> -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
> +static const enum mtk_ddp_comp_id mt7623_mtk_ddp_main[] = {
> + DDP_COMPONENT_OVL0,
> + DDP_COMPONENT_RDMA0,
> + DDP_COMPONENT_COLOR0,
> + DDP_COMPONENT_BLS,
> + DDP_COMPONENT_DPI0,
> +};
> +
> +static const enum mtk_ddp_comp_id mt7623_mtk_ddp_ext[] = {
> + DDP_COMPONENT_RDMA1,
> + DDP_COMPONENT_DSI0,
> +};
> +
> +static enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {

Why do you remove 'const'?

> DDP_COMPONENT_OVL0,
> DDP_COMPONENT_COLOR0,
> DDP_COMPONENT_AAL0,
> @@ -127,6 +140,14 @@ static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
> .shadow_register = true,
> };
>
> +static const struct mtk_mmsys_driver_data mt7623_mmsys_driver_data = {
> + .main_path = mt7623_mtk_ddp_main,
> + .main_len = ARRAY_SIZE(mt7623_mtk_ddp_main),
> + .ext_path = mt7623_mtk_ddp_ext,
> + .ext_len = ARRAY_SIZE(mt7623_mtk_ddp_ext),
> + .shadow_register = true,
> +};
> +
> static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
> .main_path = mt2712_mtk_ddp_main,
> .main_len = ARRAY_SIZE(mt2712_mtk_ddp_main),
> @@ -422,6 +443,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
> static const struct of_device_id mtk_drm_of_ids[] = {
> { .compatible = "mediatek,mt2701-mmsys",
> .data = &mt2701_mmsys_driver_data},
> + { .compatible = "mediatek,mt7623-mmsys",
> + .data = &mt7623_mmsys_driver_data},
> { .compatible = "mediatek,mt2712-mmsys",
> .data = &mt2712_mmsys_driver_data},
> { .compatible = "mediatek,mt8173-mmsys",
> --
> 2.25.1
>
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

2020-08-04 14:45:37

by Frank Wunderlich

[permalink] [raw]
Subject: Aw: Re: [PATCH v3 1/5] drm: mediatek: add ddp routing for mt7623

Hi,

> Gesendet: Dienstag, 04. August 2020 um 16:34 Uhr
> Von: "Chun-Kuang Hu" <[email protected]>

> > -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
> > +static enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
>
> Why do you remove 'const'?

was removed by previous patch and not re-added (revert failed), sorry. will fix in next version, if all other is ok.

regards Frank

2020-08-04 15:02:31

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] arm: dts: mt7623: add display subsystem related device nodes

Hi, Frank:

Frank Wunderlich <[email protected]> 於 2020年8月4日 週二 下午7:00寫道:
>
> From: Ryder Lee <[email protected]>
>
> Add display subsystem related device nodes for MT7623.
>
> Cc: CK Hu <[email protected]>
> Signed-off-by: chunhui dai <[email protected]>
> Signed-off-by: Bibby Hsieh <[email protected]>
> Signed-off-by: Ryder Lee <[email protected]>
> Signed-off-by: Frank Wunderlich <[email protected]>
> Tested-by: Frank Wunderlich <[email protected]>
> ---
> changed v2->v3:
> drop bls to dpi routing
> ---
> arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++
> arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 72 +++++++
> arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 72 +++++++
> 3 files changed, 321 insertions(+)
>
> diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> index a106c0d90a52..d584a3d678ba 100644
> --- a/arch/arm/boot/dts/mt7623.dtsi
> +++ b/arch/arm/boot/dts/mt7623.dtsi
> @@ -24,6 +24,11 @@ / {
> #address-cells = <2>;
> #size-cells = <2>;
>
> + aliases {
> + rdma0 = &rdma0;
> + rdma1 = &rdma1;
> + };
> +
> cpu_opp_table: opp-table {
> compatible = "operating-points-v2";
> opp-shared;
> @@ -321,6 +326,25 @@ pwrap: pwrap@1000d000 {
> clock-names = "spi", "wrap";
> };
>
> + mipi_tx0: mipi-dphy@10010000 {
> + compatible = "mediatek,mt7623-mipi-tx",

In mediatek,dsi.txt [1], "mediatek,mt7623-mipi-tx" is undefined.

[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdsi.txt

> + "mediatek,mt2701-mipi-tx";
> + reg = <0 0x10010000 0 0x90>;
> + clocks = <&clk26m>;
> + clock-output-names = "mipi_tx0_pll";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> + };
> +
> + cec: cec@10012000 {
> + compatible = "mediatek,mt7623-cec",

Please explicitly define "mediatek,mt7623-cec" in mediatek,hdmi.txt [2].

[2] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Chdmi.txt

> + "mediatek,mt8173-cec";
> + reg = <0 0x10012000 0 0xbc>;
> + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&infracfg CLK_INFRA_CEC>;
> + status = "disabled";
> + };
> +
> cir: cir@10013000 {
> compatible = "mediatek,mt7623-cir";
> reg = <0 0x10013000 0 0x1000>;
> @@ -369,6 +393,18 @@ apmixedsys: syscon@10209000 {
> #clock-cells = <1>;
> };
>
> + hdmi_phy: phy@10209100 {
> + compatible = "mediatek,mt7623-hdmi-phy",

Ditto.

> + "mediatek,mt2701-hdmi-phy";
> + reg = <0 0x10209100 0 0x24>;
> + clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
> + clock-names = "pll_ref";
> + clock-output-names = "hdmitx_dig_cts";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> + status = "disabled";
> + };
> +
> rng: rng@1020f000 {
> compatible = "mediatek,mt7623-rng";
> reg = <0 0x1020f000 0 0x1000>;
> @@ -568,6 +604,16 @@ bch: ecc@1100e000 {
> status = "disabled";
> };
>
> + hdmiddc0: i2c@11013000 {
> + compatible = "mediatek,mt7623-hdmi-ddc",

Ditto.

> + "mediatek,mt8173-hdmi-ddc";
> + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> + reg = <0 0x11013000 0 0x1C>;
> + clocks = <&pericfg CLK_PERI_I2C3>;
> + clock-names = "ddc-i2c";
> + status = "disabled";
> + };
> +
> nor_flash: spi@11014000 {
> compatible = "mediatek,mt7623-nor",
> "mediatek,mt8173-nor";
> @@ -766,6 +812,84 @@ mmsys: syscon@14000000 {
> #clock-cells = <1>;
> };
>
> + display_components: dispsys@14000000 {
> + compatible = "mediatek,mt7623-mmsys",
> + "mediatek,mt2701-mmsys";

In mediatek,mmsys.txt [3], this should be:

mmsys: syscon@14000000 {
compatible = "mediatek,mt7623-mmsys", "mediatek,mt2701-mmsys", "syscon"

[3] https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/mediatek/mediatek%2Cmmsys.txt

> + reg = <0 0x14000000 0 0x1000>;
> + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> + };
> +
> + ovl@14007000 {
> + compatible = "mediatek,mt7623-disp-ovl",

This is not defined in mediatek,disp.txt [4].

[4] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdisp.txt

> + "mediatek,mt2701-disp-ovl";
> + reg = <0 0x14007000 0 0x1000>;
> + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_OVL>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + rdma0: rdma@14008000 {
> + compatible = "mediatek,mt7623-disp-rdma",
> + "mediatek,mt2701-disp-rdma";
> + reg = <0 0x14008000 0 0x1000>;
> + interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_RDMA>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + wdma@14009000 {
> + compatible = "mediatek,mt7623-disp-wdma",
> + "mediatek,mt2701-disp-wdma";
> + reg = <0 0x14009000 0 0x1000>;
> + interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_WDMA>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + bls: pwm@1400a000 {
> + compatible = "mediatek,mt7623-disp-pwm",
> + "mediatek,mt2701-disp-pwm";
> + reg = <0 0x1400a000 0 0x1000>;
> + #pwm-cells = <2>;
> + clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
> + <&mmsys CLK_MM_DISP_BLS>;
> + clock-names = "main", "mm";
> + status = "disabled";
> + };
> +
> + color@1400b000 {
> + compatible = "mediatek,mt7623-disp-color",
> + "mediatek,mt2701-disp-color";
> + reg = <0 0x1400b000 0 0x1000>;
> + interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_COLOR>;
> + };
> +
> + dsi: dsi@1400c000 {
> + compatible = "mediatek,mt7623-dsi",

This is not defined in mediatek,dsi.txt [1].

> + "mediatek,mt2701-dsi";
> + reg = <0 0x1400c000 0 0x1000>;
> + interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DSI_ENGINE>,
> + <&mmsys CLK_MM_DSI_DIG>,
> + <&mipi_tx0>;
> + clock-names = "engine", "digital", "hs";
> + phys = <&mipi_tx0>;
> + phy-names = "dphy";
> + status = "disabled";
> + };
> +
> + mutex: mutex@1400e000 {
> + compatible = "mediatek,mt7623-disp-mutex",
> + "mediatek,mt2701-disp-mutex";
> + reg = <0 0x1400e000 0 0x1000>;
> + interrupts = <GIC_SPI 161 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_MUTEX_32K>;
> + };
> +
> larb0: larb@14010000 {
> compatible = "mediatek,mt7623-smi-larb",
> "mediatek,mt2701-smi-larb";
> @@ -778,6 +902,44 @@ larb0: larb@14010000 {
> power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> };
>
> + rdma1: rdma@14012000 {
> + compatible = "mediatek,mt7623-disp-rdma",
> + "mediatek,mt2701-disp-rdma";
> + reg = <0 0x14012000 0 0x1000>;
> + interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_RDMA1>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA1>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + dpi0: dpi@14014000 {
> + compatible = "mediatek,mt7623-dpi",

This is not defined in mediatek,dpi.txt [5].

[5] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdpi.txt

Regards,
Chun-Kuang.

> + "mediatek,mt2701-dpi";
> + reg = <0 0x14014000 0 0x1000>;
> + interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DPI1_DIGL>,
> + <&mmsys CLK_MM_DPI1_ENGINE>,
> + <&apmixedsys CLK_APMIXED_TVDPLL>;
> + clock-names = "pixel", "engine", "pll";
> + status = "disabled";
> + };
> +
> + hdmi0: hdmi@14015000 {
> + compatible = "mediatek,mt7623-hdmi",
> + "mediatek,mt8173-hdmi";
> + reg = <0 0x14015000 0 0x400>;
> + clocks = <&mmsys CLK_MM_HDMI_PIXEL>,
> + <&mmsys CLK_MM_HDMI_PLL>,
> + <&mmsys CLK_MM_HDMI_AUDIO>,
> + <&mmsys CLK_MM_HDMI_SPDIF>;
> + clock-names = "pixel", "pll", "bclk", "spdif";
> + phys = <&hdmi_phy>;
> + phy-names = "hdmi";
> + mediatek,syscon-hdmi = <&mmsys 0x900>;
> + cec = <&cec>;
> + status = "disabled";
> + };
> +

2020-08-04 15:03:43

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: Re: [PATCH v3 1/5] drm: mediatek: add ddp routing for mt7623

Frank Wunderlich <[email protected]> 於 2020年8月4日 週二 下午10:44寫道:
>
> Hi,
>
> > Gesendet: Dienstag, 04. August 2020 um 16:34 Uhr
> > Von: "Chun-Kuang Hu" <[email protected]>
>
> > > -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
> > > +static enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
> >
> > Why do you remove 'const'?
>
> was removed by previous patch and not re-added (revert failed), sorry. will fix in next version, if all other is ok.

all other is ok.

Regards,
Chun-Kuang.

>
> regards Frank

2020-08-04 15:26:53

by Frank Wunderlich

[permalink] [raw]
Subject: Aw: Re: [PATCH v3 5/5] arm: dts: mt7623: add display subsystem related device nodes

Hi

Except mmsys (i added in Patch #1) all mt7623-compatibles are not defined in code and fallback (mt2701-x/mt8173-x) is used. If i add it in dt-binding, it should be added in code too, right? or should i remove mt7623 compatibles and only add documentation for new mmsys?

regards Frank


> Gesendet: Dienstag, 04. August 2020 um 17:00 Uhr
> Von: "Chun-Kuang Hu" <[email protected]>
> An: "Frank Wunderlich" <[email protected]>
> > + mipi_tx0: mipi-dphy@10010000 {
> > + compatible = "mediatek,mt7623-mipi-tx",
>
> In mediatek,dsi.txt [1], "mediatek,mt7623-mipi-tx" is undefined.
>
> [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdsi.txt

in drivers/gpu/drm/mediatek/mtk_mipi_tx.c only the compatible for mt2701 is defined (which is fallback in dts). So should i remove mt7623 compatible here and in the other occurences?

>
> > + "mediatek,mt2701-mipi-tx";

> > +
> > + cec: cec@10012000 {
> > + compatible = "mediatek,mt7623-cec",
>
> Please explicitly define "mediatek,mt7623-cec" in mediatek,hdmi.txt [2].
>
> [2] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Chdmi.txt
>
> > + "mediatek,mt8173-cec";

same here...only mt8173-cec is defined in drivers/gpu/drm/mediatek/mtk_cec.c

> > cir: cir@10013000 {
> > compatible = "mediatek,mt7623-cir";
> > reg = <0 0x10013000 0 0x1000>;
> > @@ -369,6 +393,18 @@ apmixedsys: syscon@10209000 {
> > #clock-cells = <1>;
> > };
> >
> > + hdmi_phy: phy@10209100 {
> > + compatible = "mediatek,mt7623-hdmi-phy",
>
> Ditto.
>
> > + "mediatek,mt2701-hdmi-phy";
same as above (./drivers/gpu/drm/mediatek/mtk_hdmi_phy.c)

> > + hdmiddc0: i2c@11013000 {
> > + compatible = "mediatek,mt7623-hdmi-ddc",
>
> Ditto.
>
> > + "mediatek,mt8173-hdmi-ddc";
> > + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> > + reg = <0 0x11013000 0 0x1C>;
> > + clocks = <&pericfg CLK_PERI_I2C3>;
> > + clock-names = "ddc-i2c";
> > + status = "disabled";
> > + };
> > +
> > nor_flash: spi@11014000 {
> > compatible = "mediatek,mt7623-nor",
> > "mediatek,mt8173-nor";
> > @@ -766,6 +812,84 @@ mmsys: syscon@14000000 {
> > #clock-cells = <1>;
> > };
> >
> > + display_components: dispsys@14000000 {
> > + compatible = "mediatek,mt7623-mmsys",
> > + "mediatek,mt2701-mmsys";
>
> In mediatek,mmsys.txt [3], this should be:
>
> mmsys: syscon@14000000 {
> compatible = "mediatek,mt7623-mmsys", "mediatek,mt2701-mmsys", "syscon"
>
> [3] https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/mediatek/mediatek%2Cmmsys.txt

as i added this in driver, i add this to documentation (and maybe remove the fallback because it results in wrong routing?)

>
> > + reg = <0 0x14000000 0 0x1000>;
> > + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> > + };
> > +
> > + ovl@14007000 {
> > + compatible = "mediatek,mt7623-disp-ovl",
>
> This is not defined in mediatek,disp.txt [4].
>
> [4] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdisp.txt
also not defined in drivers/gpu/drm/mediatek/mtk_drm_drv.c so again fallback used

>
> > + "mediatek,mt2701-disp-ovl";

> > + reg = <0 0x14007000 0 0x1000>;
> > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_OVL>;
> > + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> > + mediatek,larb = <&larb0>;
> > + };
> > +
> > + rdma0: rdma@14008000 {
> > + compatible = "mediatek,mt7623-disp-rdma",
> > + "mediatek,mt2701-disp-rdma";
> > + reg = <0 0x14008000 0 0x1000>;
> > + interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_RDMA>;
> > + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
> > + mediatek,larb = <&larb0>;
> > + };
> > +
> > + wdma@14009000 {
> > + compatible = "mediatek,mt7623-disp-wdma",
> > + "mediatek,mt2701-disp-wdma";
> > + reg = <0 0x14009000 0 0x1000>;
> > + interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_WDMA>;
> > + iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
> > + mediatek,larb = <&larb0>;
> > + };
> > +
> > + bls: pwm@1400a000 {
> > + compatible = "mediatek,mt7623-disp-pwm",
> > + "mediatek,mt2701-disp-pwm";
> > + reg = <0 0x1400a000 0 0x1000>;
> > + #pwm-cells = <2>;
> > + clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
> > + <&mmsys CLK_MM_DISP_BLS>;
> > + clock-names = "main", "mm";
> > + status = "disabled";
> > + };
> > +
> > + color@1400b000 {
> > + compatible = "mediatek,mt7623-disp-color",
> > + "mediatek,mt2701-disp-color";
> > + reg = <0 0x1400b000 0 0x1000>;
> > + interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_COLOR>;
> > + };
> > +
> > + dsi: dsi@1400c000 {
> > + compatible = "mediatek,mt7623-dsi",
>
> This is not defined in mediatek,dsi.txt [1].
>
> > + "mediatek,mt2701-dsi";
also fallback used (drivers/gpu/drm/mediatek/mtk_drm_drv.c)


> > + dpi0: dpi@14014000 {
> > + compatible = "mediatek,mt7623-dpi",
>
> This is not defined in mediatek,dpi.txt [5].
>
> [5] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdpi.txt
>
> > + "mediatek,mt2701-dpi";

2020-08-04 15:43:45

by Frank Wunderlich

[permalink] [raw]
Subject: Aw: Re: [PATCH v3 5/5] arm: dts: mt7623: add display subsystem related device nodes

> Gesendet: Dienstag, 04. August 2020 um 17:00 Uhr
> Von: "Chun-Kuang Hu" <[email protected]>

> > + display_components: dispsys@14000000 {
> > + compatible = "mediatek,mt7623-mmsys",
> > + "mediatek,mt2701-mmsys";
>
> In mediatek,mmsys.txt [3], this should be:
>
> mmsys: syscon@14000000 {
> compatible = "mediatek,mt7623-mmsys", "mediatek,mt2701-mmsys", "syscon"
>
> [3] https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/mediatek/mediatek%2Cmmsys.txt

oh i see i can drop this because there is already a mmsys-node with the compatibles from documentation.
only powerdomain is missing, but this seems not be needed (hdmi works without this "display_components" node)...

mmsys: syscon@14000000 {
compatible = "mediatek,mt7623-mmsys",
"mediatek,mt2701-mmsys",
"syscon";
reg = <0 0x14000000 0 0x1000>;
#clock-cells = <1>;
};

display_components: dispsys@14000000 {
compatible = "mediatek,mt7623-mmsys",
"mediatek,mt2701-mmsys";
reg = <0 0x14000000 0 0x1000>;
power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
};

2020-08-04 16:02:22

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: Re: [PATCH v3 5/5] arm: dts: mt7623: add display subsystem related device nodes

Hi, Frank:

Frank Wunderlich <[email protected]> 於 2020年8月4日 週二 下午11:23寫道:
>
> Hi
>
> Except mmsys (i added in Patch #1) all mt7623-compatibles are not defined in code and fallback (mt2701-x/mt8173-x) is used. If i add it in dt-binding, it should be added in code too, right? or should i remove mt7623 compatibles and only add documentation for new mmsys?

You should add mt7623 compatibles in binding document and device node
(like mmsys node). The driver is optional to have it. If driver does
not implement mt7623, it would fallback to mt2701.

Regards,
Chun-Kuang.

>
> regards Frank
>
>
> > Gesendet: Dienstag, 04. August 2020 um 17:00 Uhr
> > Von: "Chun-Kuang Hu" <[email protected]>
> > An: "Frank Wunderlich" <[email protected]>
> > > + mipi_tx0: mipi-dphy@10010000 {
> > > + compatible = "mediatek,mt7623-mipi-tx",
> >
> > In mediatek,dsi.txt [1], "mediatek,mt7623-mipi-tx" is undefined.
> >
> > [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdsi.txt
>
> in drivers/gpu/drm/mediatek/mtk_mipi_tx.c only the compatible for mt2701 is defined (which is fallback in dts). So should i remove mt7623 compatible here and in the other occurences?
>
> >
> > > + "mediatek,mt2701-mipi-tx";
>
> > > +
> > > + cec: cec@10012000 {
> > > + compatible = "mediatek,mt7623-cec",
> >
> > Please explicitly define "mediatek,mt7623-cec" in mediatek,hdmi.txt [2].
> >
> > [2] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Chdmi.txt
> >
> > > + "mediatek,mt8173-cec";
>
> same here...only mt8173-cec is defined in drivers/gpu/drm/mediatek/mtk_cec.c
>
> > > cir: cir@10013000 {
> > > compatible = "mediatek,mt7623-cir";
> > > reg = <0 0x10013000 0 0x1000>;
> > > @@ -369,6 +393,18 @@ apmixedsys: syscon@10209000 {
> > > #clock-cells = <1>;
> > > };
> > >
> > > + hdmi_phy: phy@10209100 {
> > > + compatible = "mediatek,mt7623-hdmi-phy",
> >
> > Ditto.
> >
> > > + "mediatek,mt2701-hdmi-phy";
> same as above (./drivers/gpu/drm/mediatek/mtk_hdmi_phy.c)
>
> > > + hdmiddc0: i2c@11013000 {
> > > + compatible = "mediatek,mt7623-hdmi-ddc",
> >
> > Ditto.
> >
> > > + "mediatek,mt8173-hdmi-ddc";
> > > + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> > > + reg = <0 0x11013000 0 0x1C>;
> > > + clocks = <&pericfg CLK_PERI_I2C3>;
> > > + clock-names = "ddc-i2c";
> > > + status = "disabled";
> > > + };
> > > +
> > > nor_flash: spi@11014000 {
> > > compatible = "mediatek,mt7623-nor",
> > > "mediatek,mt8173-nor";
> > > @@ -766,6 +812,84 @@ mmsys: syscon@14000000 {
> > > #clock-cells = <1>;
> > > };
> > >
> > > + display_components: dispsys@14000000 {
> > > + compatible = "mediatek,mt7623-mmsys",
> > > + "mediatek,mt2701-mmsys";
> >
> > In mediatek,mmsys.txt [3], this should be:
> >
> > mmsys: syscon@14000000 {
> > compatible = "mediatek,mt7623-mmsys", "mediatek,mt2701-mmsys", "syscon"
> >
> > [3] https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/mediatek/mediatek%2Cmmsys.txt
>
> as i added this in driver, i add this to documentation (and maybe remove the fallback because it results in wrong routing?)
>
> >
> > > + reg = <0 0x14000000 0 0x1000>;
> > > + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> > > + };
> > > +
> > > + ovl@14007000 {
> > > + compatible = "mediatek,mt7623-disp-ovl",
> >
> > This is not defined in mediatek,disp.txt [4].
> >
> > [4] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdisp.txt
> also not defined in drivers/gpu/drm/mediatek/mtk_drm_drv.c so again fallback used
>
> >
> > > + "mediatek,mt2701-disp-ovl";
>
> > > + reg = <0 0x14007000 0 0x1000>;
> > > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> > > + clocks = <&mmsys CLK_MM_DISP_OVL>;
> > > + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> > > + mediatek,larb = <&larb0>;
> > > + };
> > > +
> > > + rdma0: rdma@14008000 {
> > > + compatible = "mediatek,mt7623-disp-rdma",
> > > + "mediatek,mt2701-disp-rdma";
> > > + reg = <0 0x14008000 0 0x1000>;
> > > + interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
> > > + clocks = <&mmsys CLK_MM_DISP_RDMA>;
> > > + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
> > > + mediatek,larb = <&larb0>;
> > > + };
> > > +
> > > + wdma@14009000 {
> > > + compatible = "mediatek,mt7623-disp-wdma",
> > > + "mediatek,mt2701-disp-wdma";
> > > + reg = <0 0x14009000 0 0x1000>;
> > > + interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
> > > + clocks = <&mmsys CLK_MM_DISP_WDMA>;
> > > + iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
> > > + mediatek,larb = <&larb0>;
> > > + };
> > > +
> > > + bls: pwm@1400a000 {
> > > + compatible = "mediatek,mt7623-disp-pwm",
> > > + "mediatek,mt2701-disp-pwm";
> > > + reg = <0 0x1400a000 0 0x1000>;
> > > + #pwm-cells = <2>;
> > > + clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
> > > + <&mmsys CLK_MM_DISP_BLS>;
> > > + clock-names = "main", "mm";
> > > + status = "disabled";
> > > + };
> > > +
> > > + color@1400b000 {
> > > + compatible = "mediatek,mt7623-disp-color",
> > > + "mediatek,mt2701-disp-color";
> > > + reg = <0 0x1400b000 0 0x1000>;
> > > + interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
> > > + clocks = <&mmsys CLK_MM_DISP_COLOR>;
> > > + };
> > > +
> > > + dsi: dsi@1400c000 {
> > > + compatible = "mediatek,mt7623-dsi",
> >
> > This is not defined in mediatek,dsi.txt [1].
> >
> > > + "mediatek,mt2701-dsi";
> also fallback used (drivers/gpu/drm/mediatek/mtk_drm_drv.c)
>
>
> > > + dpi0: dpi@14014000 {
> > > + compatible = "mediatek,mt7623-dpi",
> >
> > This is not defined in mediatek,dpi.txt [5].
> >
> > [5] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdpi.txt
> >
> > > + "mediatek,mt2701-dpi";
>