2020-02-26 10:54:48

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v9 0/4] arm64: mediatek: Fix mt8173 mmsys device probing

Dear all,

Those patches are intended to solve an old standing issue on some
Mediatek devices (mt8173, mt2701 and mt2712).

Up to now both drivers, clock and drm are probed with the same device tree
compatible. But only the first driver gets probed, which in effect breaks
graphics on those devices.

The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
control clock gates (which is used in the clk driver) and some registers
to set the routing and enable the differnet blocks of the display
and MDP (Media Data Path) subsystem. On this series the clk driver is
not a pure clock controller but a system controller that can provide
access to the shared registers between the different drivers that need
it (mediatek-drm and mediatek-mdp). Hence the MMSYS clk driver was moved
to drivers/soc/mediatek and is the entry point (parent) which will trigger
the probe of the corresponding mediatek-drm driver.

**IMPORTANT** This series only fixes the issue on mt8173 to make it
simple and as is the only platform I can test. Similar changes should be
applied for mt2701 and mt2712 to have display working.

For reference, here are the links to the old discussions:
* v8: https://patchwork.kernel.org/project/linux-mediatek/list/?series=244891
* v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
* v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
* v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
* v4:
* https://patchwork.kernel.org/patch/10530871/
* https://patchwork.kernel.org/patch/10530883/
* https://patchwork.kernel.org/patch/10530885/
* https://patchwork.kernel.org/patch/10530911/
* https://patchwork.kernel.org/patch/10530913/
* v3:
* https://patchwork.kernel.org/patch/10367857/
* https://patchwork.kernel.org/patch/10367861/
* https://patchwork.kernel.org/patch/10367877/
* https://patchwork.kernel.org/patch/10367875/
* https://patchwork.kernel.org/patch/10367885/
* https://patchwork.kernel.org/patch/10367883/
* https://patchwork.kernel.org/patch/10367889/
* https://patchwork.kernel.org/patch/10367907/
* https://patchwork.kernel.org/patch/10367909/
* https://patchwork.kernel.org/patch/10367905/
* v2: No relevant discussion, see v3
* v1:
* https://patchwork.kernel.org/patch/10016497/
* https://patchwork.kernel.org/patch/10016499/
* https://patchwork.kernel.org/patch/10016505/
* https://patchwork.kernel.org/patch/10016507/

Best regards,
Enric

Changes in v9:
- Move mmsys to drivers/soc/mediatek (CK)
- Do not move the display routing from the drm driver (CK)
- Removed from this series because are not needed:
* [PATCH v8 5/6] drm/mediatek: Move MMSYS configuration to include/linux/platform_data
- Removed from this series because are applied:
* [PATCH v8 3/6] media: mtk-mdp: Check return value of of_clk_get.

Changes in v8:
- Select REGMAP and MFD_SYSCON (Randy Dunlap)
- Be a builtin_platform_driver like other mediatek mmsys drivers.
- New patch introduced in this series.

Changes in v7:
- Add R-by from CK
- Free clk_data->clks as well
- Get rid of private data structure

Enric Balletbo i Serra (1):
drm/mediatek: Fix mediatek-drm device probing

Matthias Brugger (3):
drm/mediatek: Use regmap for register access
drm/mediatek: Omit warning on probe defers
soc: mediatek: Move mt8173 MMSYS to platform driver

drivers/clk/mediatek/clk-mt8173.c | 104 ----------------
drivers/gpu/drm/mediatek/Kconfig | 2 +
drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 +-
drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 5 +-
drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 +-
drivers/gpu/drm/mediatek/mtk_dpi.c | 12 +-
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 53 ++++----
drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 45 ++++---
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +-
drivers/gpu/drm/mediatek/mtk_dsi.c | 8 +-
drivers/gpu/drm/mediatek/mtk_hdmi.c | 4 +-
drivers/soc/mediatek/Kconfig | 7 ++
drivers/soc/mediatek/Makefile | 1 +
drivers/soc/mediatek/mt8173-mmsys.c | 143 ++++++++++++++++++++++
16 files changed, 233 insertions(+), 171 deletions(-)
create mode 100644 drivers/soc/mediatek/mt8173-mmsys.c

--
2.25.0


2020-02-26 10:54:59

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v9 2/4] drm/mediatek: Omit warning on probe defers

From: Matthias Brugger <[email protected]>

It can happen that the mmsys clock drivers aren't probed before the
platform driver gets invoked. The platform driver used to print a warning
that the driver failed to get the clocks. Omit this error on
the defered probe path.

Signed-off-by: Matthias Brugger <[email protected]>
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
---

Changes in v9: None
Changes in v8: None
Changes in v7:
- Add Rv-by from CK

drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 ++++-
drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 5 ++++-
drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 ++++-
drivers/gpu/drm/mediatek/mtk_dpi.c | 12 +++++++++---
drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 ++-
drivers/gpu/drm/mediatek/mtk_dsi.c | 8 ++++++--
drivers/gpu/drm/mediatek/mtk_hdmi.c | 4 +++-
7 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c
index 6fb0d6983a4a..3ae9c810845b 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_color.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c
@@ -119,7 +119,10 @@ static int mtk_disp_color_probe(struct platform_device *pdev)
ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id,
&mtk_disp_color_funcs);
if (ret) {
- dev_err(dev, "Failed to initialize component: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to initialize component: %d\n",
+ ret);
+
return ret;
}

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 891d80c73e04..28651bc579bc 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -386,7 +386,10 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev)
ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id,
&mtk_disp_ovl_funcs);
if (ret) {
- dev_err(dev, "Failed to initialize component: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to initialize component: %d\n",
+ ret);
+
return ret;
}

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
index 0cb848d64206..e04319fedf46 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
@@ -294,7 +294,10 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev)
ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id,
&mtk_disp_rdma_funcs);
if (ret) {
- dev_err(dev, "Failed to initialize component: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to initialize component: %d\n",
+ ret);
+
return ret;
}

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 01fa8b8d763d..1b219edef541 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -701,21 +701,27 @@ static int mtk_dpi_probe(struct platform_device *pdev)
dpi->engine_clk = devm_clk_get(dev, "engine");
if (IS_ERR(dpi->engine_clk)) {
ret = PTR_ERR(dpi->engine_clk);
- dev_err(dev, "Failed to get engine clock: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get engine clock: %d\n", ret);
+
return ret;
}

dpi->pixel_clk = devm_clk_get(dev, "pixel");
if (IS_ERR(dpi->pixel_clk)) {
ret = PTR_ERR(dpi->pixel_clk);
- dev_err(dev, "Failed to get pixel clock: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get pixel clock: %d\n", ret);
+
return ret;
}

dpi->tvd_clk = devm_clk_get(dev, "pll");
if (IS_ERR(dpi->tvd_clk)) {
ret = PTR_ERR(dpi->tvd_clk);
- dev_err(dev, "Failed to get tvdpll clock: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get tvdpll clock: %d\n", ret);
+
return ret;
}

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 302753744cc6..39700b9428b9 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -620,7 +620,8 @@ static int mtk_ddp_probe(struct platform_device *pdev)
if (!ddp->data->no_clk) {
ddp->clk = devm_clk_get(dev, NULL);
if (IS_ERR(ddp->clk)) {
- dev_err(dev, "Failed to get clock\n");
+ if (PTR_ERR(ddp->clk) != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get clock\n");
return PTR_ERR(ddp->clk);
}
}
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 5fa1073cf26b..a45ed0270531 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1194,14 +1194,18 @@ static int mtk_dsi_probe(struct platform_device *pdev)
dsi->engine_clk = devm_clk_get(dev, "engine");
if (IS_ERR(dsi->engine_clk)) {
ret = PTR_ERR(dsi->engine_clk);
- dev_err(dev, "Failed to get engine clock: %d\n", ret);
+
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get engine clock: %d\n", ret);
goto err_unregister_host;
}

dsi->digital_clk = devm_clk_get(dev, "digital");
if (IS_ERR(dsi->digital_clk)) {
ret = PTR_ERR(dsi->digital_clk);
- dev_err(dev, "Failed to get digital clock: %d\n", ret);
+
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get digital clock: %d\n", ret);
goto err_unregister_host;
}

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 5e4a4dbda443..69c6a146c561 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1451,7 +1451,9 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,

ret = mtk_hdmi_get_all_clk(hdmi, np);
if (ret) {
- dev_err(dev, "Failed to get clocks: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get clocks: %d\n", ret);
+
return ret;
}

--
2.25.0

2020-02-26 10:55:11

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v9 4/4] drm/mediatek: Fix mediatek-drm device probing

In the actual implementation the same compatible string
"mediatek,<chip>-mmsys" is used to bind the clock drivers
(drivers/soc/mediatek) as well as to the gpu driver
(drivers/gpu/drm/mediatek/mtk_drm_drv.c). This ends with the problem
that the only probed driver is the clock driver and there is no display
at all.

In any case having the same compatible string for two drivers is not
correct and should be fixed. To fix this, and maintain backward
compatibility, we can consider that the mmsys driver is the top-level
entry point for the multimedia subsystem, so is not a pure clock
controller but a system controller, and the drm driver is instantiated
by that MMSYS driver.

Signed-off-by: Enric Balletbo i Serra <[email protected]>
---

Changes in v9:
- Do not move the display routing from the drm driver (CK)

Changes in v8:
- New patch introduced in this series.

Changes in v7: None

drivers/gpu/drm/mediatek/mtk_drm_drv.c | 34 ++++++++++++++------------
drivers/soc/mediatek/mt8173-mmsys.c | 6 +++++
2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index b68837ea02b3..17f118ee0e57 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -422,9 +422,21 @@ 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,mt2712-mmsys",
+ .data = &mt2712_mmsys_driver_data},
+ { .compatible = "mediatek,mt8173-mmsys",
+ .data = &mt8173_mmsys_driver_data},
+ { }
+};
+
static int mtk_drm_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ struct device_node *phandle = dev->parent->of_node;
+ const struct of_device_id *of_id;
struct mtk_drm_private *private;
struct device_node *node;
struct component_match *match = NULL;
@@ -435,15 +447,18 @@ static int mtk_drm_probe(struct platform_device *pdev)
if (!private)
return -ENOMEM;

- private->data = of_device_get_match_data(dev);
+ of_id = of_match_node(mtk_drm_of_ids, phandle);
+ if (!of_id)
+ return -ENODEV;
+
+ private->data = of_id->data;

- private->config_regs = syscon_node_to_regmap(dev->of_node);
+ private->config_regs = syscon_node_to_regmap(phandle);
if (IS_ERR(private->config_regs))
return PTR_ERR(private->config_regs);

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

@@ -576,22 +591,11 @@ static int mtk_drm_sys_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
mtk_drm_sys_resume);

-static const struct of_device_id mtk_drm_of_ids[] = {
- { .compatible = "mediatek,mt2701-mmsys",
- .data = &mt2701_mmsys_driver_data},
- { .compatible = "mediatek,mt2712-mmsys",
- .data = &mt2712_mmsys_driver_data},
- { .compatible = "mediatek,mt8173-mmsys",
- .data = &mt8173_mmsys_driver_data},
- { }
-};
-
static struct platform_driver mtk_drm_platform_driver = {
.probe = mtk_drm_probe,
.remove = mtk_drm_remove,
.driver = {
.name = "mediatek-drm",
- .of_match_table = mtk_drm_of_ids,
.pm = &mtk_drm_pm_ops,
},
};
diff --git a/drivers/soc/mediatek/mt8173-mmsys.c b/drivers/soc/mediatek/mt8173-mmsys.c
index 48e6c157d28e..c894db5b6ca9 100644
--- a/drivers/soc/mediatek/mt8173-mmsys.c
+++ b/drivers/soc/mediatek/mt8173-mmsys.c
@@ -103,6 +103,7 @@ static int mt8173_mmsys_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
struct clk_onecell_data *clk_data;
+ struct platform_device *drm;
int ret;

clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
@@ -118,6 +119,11 @@ static int mt8173_mmsys_probe(struct platform_device *pdev)
if (ret)
return ret;

+ drm = platform_device_register_data(&pdev->dev, "mediatek-drm",
+ PLATFORM_DEVID_NONE, NULL, 0);
+ if (IS_ERR(drm))
+ return PTR_ERR(drm);
+
return 0;
}

--
2.25.0

2020-02-26 10:55:58

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v9 3/4] soc: mediatek: Move mt8173 MMSYS to platform driver

From: Matthias Brugger <[email protected]>

There is no strong reason for this to use CLK_OF_DECLARE instead of
being a platform driver. Plus, this driver provides clocks but also
a shared register space for the mediatek-drm and the mediatek-mdp
driver. So move to drivers/soc/mediatek as a platform driver.

Signed-off-by: Matthias Brugger <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
---

Changes in v9:
- Move mmsys to drivers/soc/mediatek (CK)

Changes in v8:
- Be a builtin_platform_driver like other mediatek mmsys drivers.

Changes in v7:
- Free clk_data->clks as well
- Get rid of private data structure

drivers/clk/mediatek/clk-mt8173.c | 104 ---------------------
drivers/soc/mediatek/Kconfig | 7 ++
drivers/soc/mediatek/Makefile | 1 +
drivers/soc/mediatek/mt8173-mmsys.c | 137 ++++++++++++++++++++++++++++
4 files changed, 145 insertions(+), 104 deletions(-)
create mode 100644 drivers/soc/mediatek/mt8173-mmsys.c

diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 537a7f49b0f7..8f898ac476c0 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -753,93 +753,6 @@ static const struct mtk_gate img_clks[] __initconst = {
GATE_IMG(CLK_IMG_FD, "img_fd", "mm_sel", 11),
};

-static const struct mtk_gate_regs mm0_cg_regs __initconst = {
- .set_ofs = 0x0104,
- .clr_ofs = 0x0108,
- .sta_ofs = 0x0100,
-};
-
-static const struct mtk_gate_regs mm1_cg_regs __initconst = {
- .set_ofs = 0x0114,
- .clr_ofs = 0x0118,
- .sta_ofs = 0x0110,
-};
-
-#define GATE_MM0(_id, _name, _parent, _shift) { \
- .id = _id, \
- .name = _name, \
- .parent_name = _parent, \
- .regs = &mm0_cg_regs, \
- .shift = _shift, \
- .ops = &mtk_clk_gate_ops_setclr, \
- }
-
-#define GATE_MM1(_id, _name, _parent, _shift) { \
- .id = _id, \
- .name = _name, \
- .parent_name = _parent, \
- .regs = &mm1_cg_regs, \
- .shift = _shift, \
- .ops = &mtk_clk_gate_ops_setclr, \
- }
-
-static const struct mtk_gate mm_clks[] __initconst = {
- /* MM0 */
- GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
- GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
- GATE_MM0(CLK_MM_CAM_MDP, "mm_cam_mdp", "mm_sel", 2),
- GATE_MM0(CLK_MM_MDP_RDMA0, "mm_mdp_rdma0", "mm_sel", 3),
- GATE_MM0(CLK_MM_MDP_RDMA1, "mm_mdp_rdma1", "mm_sel", 4),
- GATE_MM0(CLK_MM_MDP_RSZ0, "mm_mdp_rsz0", "mm_sel", 5),
- GATE_MM0(CLK_MM_MDP_RSZ1, "mm_mdp_rsz1", "mm_sel", 6),
- GATE_MM0(CLK_MM_MDP_RSZ2, "mm_mdp_rsz2", "mm_sel", 7),
- GATE_MM0(CLK_MM_MDP_TDSHP0, "mm_mdp_tdshp0", "mm_sel", 8),
- GATE_MM0(CLK_MM_MDP_TDSHP1, "mm_mdp_tdshp1", "mm_sel", 9),
- GATE_MM0(CLK_MM_MDP_WDMA, "mm_mdp_wdma", "mm_sel", 11),
- GATE_MM0(CLK_MM_MDP_WROT0, "mm_mdp_wrot0", "mm_sel", 12),
- GATE_MM0(CLK_MM_MDP_WROT1, "mm_mdp_wrot1", "mm_sel", 13),
- GATE_MM0(CLK_MM_FAKE_ENG, "mm_fake_eng", "mm_sel", 14),
- GATE_MM0(CLK_MM_MUTEX_32K, "mm_mutex_32k", "rtc_sel", 15),
- GATE_MM0(CLK_MM_DISP_OVL0, "mm_disp_ovl0", "mm_sel", 16),
- GATE_MM0(CLK_MM_DISP_OVL1, "mm_disp_ovl1", "mm_sel", 17),
- GATE_MM0(CLK_MM_DISP_RDMA0, "mm_disp_rdma0", "mm_sel", 18),
- GATE_MM0(CLK_MM_DISP_RDMA1, "mm_disp_rdma1", "mm_sel", 19),
- GATE_MM0(CLK_MM_DISP_RDMA2, "mm_disp_rdma2", "mm_sel", 20),
- GATE_MM0(CLK_MM_DISP_WDMA0, "mm_disp_wdma0", "mm_sel", 21),
- GATE_MM0(CLK_MM_DISP_WDMA1, "mm_disp_wdma1", "mm_sel", 22),
- GATE_MM0(CLK_MM_DISP_COLOR0, "mm_disp_color0", "mm_sel", 23),
- GATE_MM0(CLK_MM_DISP_COLOR1, "mm_disp_color1", "mm_sel", 24),
- GATE_MM0(CLK_MM_DISP_AAL, "mm_disp_aal", "mm_sel", 25),
- GATE_MM0(CLK_MM_DISP_GAMMA, "mm_disp_gamma", "mm_sel", 26),
- GATE_MM0(CLK_MM_DISP_UFOE, "mm_disp_ufoe", "mm_sel", 27),
- GATE_MM0(CLK_MM_DISP_SPLIT0, "mm_disp_split0", "mm_sel", 28),
- GATE_MM0(CLK_MM_DISP_SPLIT1, "mm_disp_split1", "mm_sel", 29),
- GATE_MM0(CLK_MM_DISP_MERGE, "mm_disp_merge", "mm_sel", 30),
- GATE_MM0(CLK_MM_DISP_OD, "mm_disp_od", "mm_sel", 31),
- /* MM1 */
- GATE_MM1(CLK_MM_DISP_PWM0MM, "mm_disp_pwm0mm", "mm_sel", 0),
- GATE_MM1(CLK_MM_DISP_PWM026M, "mm_disp_pwm026m", "pwm_sel", 1),
- GATE_MM1(CLK_MM_DISP_PWM1MM, "mm_disp_pwm1mm", "mm_sel", 2),
- GATE_MM1(CLK_MM_DISP_PWM126M, "mm_disp_pwm126m", "pwm_sel", 3),
- GATE_MM1(CLK_MM_DSI0_ENGINE, "mm_dsi0_engine", "mm_sel", 4),
- GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "dsi0_dig", 5),
- GATE_MM1(CLK_MM_DSI1_ENGINE, "mm_dsi1_engine", "mm_sel", 6),
- GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "dsi1_dig", 7),
- GATE_MM1(CLK_MM_DPI_PIXEL, "mm_dpi_pixel", "dpi0_sel", 8),
- GATE_MM1(CLK_MM_DPI_ENGINE, "mm_dpi_engine", "mm_sel", 9),
- GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "lvds_pxl", 10),
- GATE_MM1(CLK_MM_DPI1_ENGINE, "mm_dpi1_engine", "mm_sel", 11),
- GATE_MM1(CLK_MM_HDMI_PIXEL, "mm_hdmi_pixel", "dpi0_sel", 12),
- GATE_MM1(CLK_MM_HDMI_PLLCK, "mm_hdmi_pllck", "hdmi_sel", 13),
- GATE_MM1(CLK_MM_HDMI_AUDIO, "mm_hdmi_audio", "apll1", 14),
- GATE_MM1(CLK_MM_HDMI_SPDIF, "mm_hdmi_spdif", "apll2", 15),
- GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "lvds_pxl", 16),
- GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "lvds_cts", 17),
- GATE_MM1(CLK_MM_SMI_LARB4, "mm_smi_larb4", "mm_sel", 18),
- GATE_MM1(CLK_MM_HDMI_HDCP, "mm_hdmi_hdcp", "hdcp_sel", 19),
- GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
-};
-
static const struct mtk_gate_regs vdec0_cg_regs __initconst = {
.set_ofs = 0x0000,
.clr_ofs = 0x0004,
@@ -1144,23 +1057,6 @@ static void __init mtk_imgsys_init(struct device_node *node)
}
CLK_OF_DECLARE(mtk_imgsys, "mediatek,mt8173-imgsys", mtk_imgsys_init);

-static void __init mtk_mmsys_init(struct device_node *node)
-{
- struct clk_onecell_data *clk_data;
- int r;
-
- clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
-
- mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
- clk_data);
-
- r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
- if (r)
- pr_err("%s(): could not register clock provider: %d\n",
- __func__, r);
-}
-CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys", mtk_mmsys_init);
-
static void __init mtk_vdecsys_init(struct device_node *node)
{
struct clk_onecell_data *clk_data;
diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index 2114b563478c..dcd6481a14d0 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -44,4 +44,11 @@ config MTK_SCPSYS
Say yes here to add support for the MediaTek SCPSYS power domain
driver.

+config MT8173_MMSYS
+ bool "MediaTek MT8173 MMSYS Support"
+ depends on COMMON_CLK_MT8173
+ help
+ Say yes here to add support for the MediaTek MT8173 Multimedia
+ Subsystem (MMSYS).
+
endmenu
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index b01733074ad6..a185e4ee7601 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
+obj-$(CONFIG_MT8173_MMSYS) += mt8173-mmsys.o
diff --git a/drivers/soc/mediatek/mt8173-mmsys.c b/drivers/soc/mediatek/mt8173-mmsys.c
new file mode 100644
index 000000000000..48e6c157d28e
--- /dev/null
+++ b/drivers/soc/mediatek/mt8173-mmsys.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2014 MediaTek Inc.
+ * Author: James Liao <[email protected]>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+
+#include "../../clk/mediatek/clk-gate.h"
+#include "../../clk/mediatek/clk-mtk.h"
+
+#include <dt-bindings/clock/mt8173-clk.h>
+
+static const struct mtk_gate_regs mm0_cg_regs = {
+ .set_ofs = 0x0104,
+ .clr_ofs = 0x0108,
+ .sta_ofs = 0x0100,
+};
+
+static const struct mtk_gate_regs mm1_cg_regs = {
+ .set_ofs = 0x0114,
+ .clr_ofs = 0x0118,
+ .sta_ofs = 0x0110,
+};
+
+#define GATE_MM0(_id, _name, _parent, _shift) { \
+ .id = _id, \
+ .name = _name, \
+ .parent_name = _parent, \
+ .regs = &mm0_cg_regs, \
+ .shift = _shift, \
+ .ops = &mtk_clk_gate_ops_setclr, \
+ }
+
+#define GATE_MM1(_id, _name, _parent, _shift) { \
+ .id = _id, \
+ .name = _name, \
+ .parent_name = _parent, \
+ .regs = &mm1_cg_regs, \
+ .shift = _shift, \
+ .ops = &mtk_clk_gate_ops_setclr, \
+ }
+
+static const struct mtk_gate mm_clks[] = {
+ /* MM0 */
+ GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
+ GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
+ GATE_MM0(CLK_MM_CAM_MDP, "mm_cam_mdp", "mm_sel", 2),
+ GATE_MM0(CLK_MM_MDP_RDMA0, "mm_mdp_rdma0", "mm_sel", 3),
+ GATE_MM0(CLK_MM_MDP_RDMA1, "mm_mdp_rdma1", "mm_sel", 4),
+ GATE_MM0(CLK_MM_MDP_RSZ0, "mm_mdp_rsz0", "mm_sel", 5),
+ GATE_MM0(CLK_MM_MDP_RSZ1, "mm_mdp_rsz1", "mm_sel", 6),
+ GATE_MM0(CLK_MM_MDP_RSZ2, "mm_mdp_rsz2", "mm_sel", 7),
+ GATE_MM0(CLK_MM_MDP_TDSHP0, "mm_mdp_tdshp0", "mm_sel", 8),
+ GATE_MM0(CLK_MM_MDP_TDSHP1, "mm_mdp_tdshp1", "mm_sel", 9),
+ GATE_MM0(CLK_MM_MDP_WDMA, "mm_mdp_wdma", "mm_sel", 11),
+ GATE_MM0(CLK_MM_MDP_WROT0, "mm_mdp_wrot0", "mm_sel", 12),
+ GATE_MM0(CLK_MM_MDP_WROT1, "mm_mdp_wrot1", "mm_sel", 13),
+ GATE_MM0(CLK_MM_FAKE_ENG, "mm_fake_eng", "mm_sel", 14),
+ GATE_MM0(CLK_MM_MUTEX_32K, "mm_mutex_32k", "rtc_sel", 15),
+ GATE_MM0(CLK_MM_DISP_OVL0, "mm_disp_ovl0", "mm_sel", 16),
+ GATE_MM0(CLK_MM_DISP_OVL1, "mm_disp_ovl1", "mm_sel", 17),
+ GATE_MM0(CLK_MM_DISP_RDMA0, "mm_disp_rdma0", "mm_sel", 18),
+ GATE_MM0(CLK_MM_DISP_RDMA1, "mm_disp_rdma1", "mm_sel", 19),
+ GATE_MM0(CLK_MM_DISP_RDMA2, "mm_disp_rdma2", "mm_sel", 20),
+ GATE_MM0(CLK_MM_DISP_WDMA0, "mm_disp_wdma0", "mm_sel", 21),
+ GATE_MM0(CLK_MM_DISP_WDMA1, "mm_disp_wdma1", "mm_sel", 22),
+ GATE_MM0(CLK_MM_DISP_COLOR0, "mm_disp_color0", "mm_sel", 23),
+ GATE_MM0(CLK_MM_DISP_COLOR1, "mm_disp_color1", "mm_sel", 24),
+ GATE_MM0(CLK_MM_DISP_AAL, "mm_disp_aal", "mm_sel", 25),
+ GATE_MM0(CLK_MM_DISP_GAMMA, "mm_disp_gamma", "mm_sel", 26),
+ GATE_MM0(CLK_MM_DISP_UFOE, "mm_disp_ufoe", "mm_sel", 27),
+ GATE_MM0(CLK_MM_DISP_SPLIT0, "mm_disp_split0", "mm_sel", 28),
+ GATE_MM0(CLK_MM_DISP_SPLIT1, "mm_disp_split1", "mm_sel", 29),
+ GATE_MM0(CLK_MM_DISP_MERGE, "mm_disp_merge", "mm_sel", 30),
+ GATE_MM0(CLK_MM_DISP_OD, "mm_disp_od", "mm_sel", 31),
+ /* MM1 */
+ GATE_MM1(CLK_MM_DISP_PWM0MM, "mm_disp_pwm0mm", "mm_sel", 0),
+ GATE_MM1(CLK_MM_DISP_PWM026M, "mm_disp_pwm026m", "pwm_sel", 1),
+ GATE_MM1(CLK_MM_DISP_PWM1MM, "mm_disp_pwm1mm", "mm_sel", 2),
+ GATE_MM1(CLK_MM_DISP_PWM126M, "mm_disp_pwm126m", "pwm_sel", 3),
+ GATE_MM1(CLK_MM_DSI0_ENGINE, "mm_dsi0_engine", "mm_sel", 4),
+ GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "dsi0_dig", 5),
+ GATE_MM1(CLK_MM_DSI1_ENGINE, "mm_dsi1_engine", "mm_sel", 6),
+ GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "dsi1_dig", 7),
+ GATE_MM1(CLK_MM_DPI_PIXEL, "mm_dpi_pixel", "dpi0_sel", 8),
+ GATE_MM1(CLK_MM_DPI_ENGINE, "mm_dpi_engine", "mm_sel", 9),
+ GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "lvds_pxl", 10),
+ GATE_MM1(CLK_MM_DPI1_ENGINE, "mm_dpi1_engine", "mm_sel", 11),
+ GATE_MM1(CLK_MM_HDMI_PIXEL, "mm_hdmi_pixel", "dpi0_sel", 12),
+ GATE_MM1(CLK_MM_HDMI_PLLCK, "mm_hdmi_pllck", "hdmi_sel", 13),
+ GATE_MM1(CLK_MM_HDMI_AUDIO, "mm_hdmi_audio", "apll1", 14),
+ GATE_MM1(CLK_MM_HDMI_SPDIF, "mm_hdmi_spdif", "apll2", 15),
+ GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "lvds_pxl", 16),
+ GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "lvds_cts", 17),
+ GATE_MM1(CLK_MM_SMI_LARB4, "mm_smi_larb4", "mm_sel", 18),
+ GATE_MM1(CLK_MM_HDMI_HDCP, "mm_hdmi_hdcp", "hdcp_sel", 19),
+ GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
+};
+
+static int mt8173_mmsys_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct clk_onecell_data *clk_data;
+ int ret;
+
+ clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
+ if (!clk_data)
+ return -ENOMEM;
+
+ ret = mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
+ clk_data);
+ if (ret)
+ return ret;
+
+ ret = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static const struct of_device_id of_match_mt8173_mmsys[] = {
+ { .compatible = "mediatek,mt8173-mmsys", },
+ { }
+};
+
+static struct platform_driver mt8173_mmsys_drv = {
+ .driver = {
+ .name = "mt8173-mmsys",
+ .of_match_table = of_match_mt8173_mmsys,
+ },
+ .probe = mt8173_mmsys_probe,
+};
+
+builtin_platform_driver(mt8173_mmsys_drv);
--
2.25.0

2020-02-26 10:56:27

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v9 1/4] drm/mediatek: Use regmap for register access

From: Matthias Brugger <[email protected]>

The mmsys memory space is shared between the drm and the
clk driver. Use regmap to access it.

Signed-off-by: Matthias Brugger <[email protected]>
Reviewed-by: Philipp Zabel <[email protected]>
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
---

Changes in v9: None
Changes in v8:
- Select REGMAP and MFD_SYSCON (Randy Dunlap)

Changes in v7:
- Add R-by from CK

drivers/gpu/drm/mediatek/Kconfig | 2 +
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 50 +++++++++++--------------
drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 4 +-
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 13 ++-----
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +-
6 files changed, 32 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index fa5ffc4fe823..89e18a473cb5 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -10,8 +10,10 @@ config DRM_MEDIATEK
select DRM_KMS_HELPER
select DRM_MIPI_DSI
select DRM_PANEL
+ select MFD_SYSCON
select MEMORY
select MTK_SMI
+ select REGMAP
select VIDEOMODE_HELPERS
help
Choose this option if you have a Mediatek SoCs.
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 5ee74d7ce35c..a236499123aa 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -28,7 +28,7 @@
* @enabled: records whether crtc_enable succeeded
* @planes: array of 4 drm_plane structures, one for each overlay plane
* @pending_planes: whether any plane has pending changes to be applied
- * @config_regs: memory mapped mmsys configuration register space
+ * @config_regs: regmap mapped mmsys configuration register space
* @mutex: handle to one of the ten disp_mutex streams
* @ddp_comp_nr: number of components in ddp_comp
* @ddp_comp: array of pointers the mtk_ddp_comp structures used by this crtc
@@ -50,7 +50,7 @@ struct mtk_drm_crtc {
u32 cmdq_event;
#endif

- void __iomem *config_regs;
+ struct regmap *config_regs;
struct mtk_disp_mutex *mutex;
unsigned int ddp_comp_nr;
struct mtk_ddp_comp **ddp_comp;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 13035c906035..302753744cc6 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -383,61 +383,53 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
return value;
}

-static void mtk_ddp_sout_sel(void __iomem *config_regs,
+static void mtk_ddp_sout_sel(struct regmap *config_regs,
enum mtk_ddp_comp_id cur,
enum mtk_ddp_comp_id next)
{
if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
- writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
- config_regs + DISP_REG_CONFIG_OUT_SEL);
+ regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
+ BLS_TO_DSI_RDMA1_TO_DPI1);
} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
- writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI,
- config_regs + DISP_REG_CONFIG_OUT_SEL);
- writel_relaxed(DSI_SEL_IN_RDMA,
- config_regs + DISP_REG_CONFIG_DSI_SEL);
- writel_relaxed(DPI_SEL_IN_BLS,
- config_regs + DISP_REG_CONFIG_DPI_SEL);
+ regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
+ BLS_TO_DPI_RDMA1_TO_DSI);
+ regmap_write(config_regs, DISP_REG_CONFIG_DSI_SEL,
+ DSI_SEL_IN_RDMA);
+ regmap_write(config_regs, DISP_REG_CONFIG_DPI_SEL,
+ DPI_SEL_IN_BLS);
}
}

-void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
+void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
enum mtk_ddp_comp_id cur,
enum mtk_ddp_comp_id next)
{
- unsigned int addr, value, reg;
+ unsigned int addr, value;

value = mtk_ddp_mout_en(cur, next, &addr);
- if (value) {
- reg = readl_relaxed(config_regs + addr) | value;
- writel_relaxed(reg, config_regs + addr);
- }
+ if (value)
+ regmap_update_bits(config_regs, addr, value, value);

mtk_ddp_sout_sel(config_regs, cur, next);

value = mtk_ddp_sel_in(cur, next, &addr);
- if (value) {
- reg = readl_relaxed(config_regs + addr) | value;
- writel_relaxed(reg, config_regs + addr);
- }
+ if (value)
+ regmap_update_bits(config_regs, addr, value, value);
}

-void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
+void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
enum mtk_ddp_comp_id cur,
enum mtk_ddp_comp_id next)
{
- unsigned int addr, value, reg;
+ unsigned int addr, value;

value = mtk_ddp_mout_en(cur, next, &addr);
- if (value) {
- reg = readl_relaxed(config_regs + addr) & ~value;
- writel_relaxed(reg, config_regs + addr);
- }
+ if (value)
+ regmap_update_bits(config_regs, addr, value, 0);

value = mtk_ddp_sel_in(cur, next, &addr);
- if (value) {
- reg = readl_relaxed(config_regs + addr) & ~value;
- writel_relaxed(reg, config_regs + addr);
- }
+ if (value)
+ regmap_update_bits(config_regs, addr, value, 0);
}

struct mtk_disp_mutex *mtk_disp_mutex_get(struct device *dev, unsigned int id)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
index 827be424a148..01ff8b68881f 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
@@ -12,10 +12,10 @@ struct regmap;
struct device;
struct mtk_disp_mutex;

-void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
+void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
enum mtk_ddp_comp_id cur,
enum mtk_ddp_comp_id next);
-void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
+void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
enum mtk_ddp_comp_id cur,
enum mtk_ddp_comp_id next);

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 0563c6813333..b68837ea02b3 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -6,6 +6,7 @@

#include <linux/component.h>
#include <linux/iommu.h>
+#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_platform.h>
@@ -425,7 +426,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct mtk_drm_private *private;
- struct resource *mem;
struct device_node *node;
struct component_match *match = NULL;
int ret;
@@ -437,14 +437,9 @@ static int mtk_drm_probe(struct platform_device *pdev)

private->data = of_device_get_match_data(dev);

- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- private->config_regs = devm_ioremap_resource(dev, mem);
- if (IS_ERR(private->config_regs)) {
- ret = PTR_ERR(private->config_regs);
- dev_err(dev, "Failed to ioremap mmsys-config resource: %d\n",
- ret);
- return ret;
- }
+ private->config_regs = syscon_node_to_regmap(dev->of_node);
+ if (IS_ERR(private->config_regs))
+ return PTR_ERR(private->config_regs);

/* Iterate over sibling DISP function blocks */
for_each_child_of_node(dev->of_node->parent, node) {
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 17bc99b9f5d4..03201080688d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -39,7 +39,7 @@ struct mtk_drm_private {

struct device_node *mutex_node;
struct device *mutex_dev;
- void __iomem *config_regs;
+ struct regmap *config_regs;
struct device_node *comp_node[DDP_COMPONENT_ID_MAX];
struct mtk_ddp_comp *ddp_comp[DDP_COMPONENT_ID_MAX];
const struct mtk_mmsys_driver_data *data;
--
2.25.0

2020-02-26 15:41:05

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v9 3/4] soc: mediatek: Move mt8173 MMSYS to platform driver

On 2/26/20 2:54 AM, Enric Balletbo i Serra wrote:
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index 2114b563478c..dcd6481a14d0 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -44,4 +44,11 @@ config MTK_SCPSYS
> Say yes here to add support for the MediaTek SCPSYS power domain
> driver.
>
> +config MT8173_MMSYS
> + bool "MediaTek MT8173 MMSYS Support"

Hi,
Can this be tristate instead of bool?

> + depends on COMMON_CLK_MT8173
> + help
> + Say yes here to add support for the MediaTek MT8173 Multimedia
> + Subsystem (MMSYS).
> +
> endmenu

thanks.
--
~Randy

2020-02-26 20:30:41

by Ezequiel Garcia

[permalink] [raw]
Subject: Re: [PATCH v9 3/4] soc: mediatek: Move mt8173 MMSYS to platform driver

On Wed, 2020-02-26 at 07:40 -0800, Randy Dunlap wrote:
> On 2/26/20 2:54 AM, Enric Balletbo i Serra wrote:
> > diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> > index 2114b563478c..dcd6481a14d0 100644
> > --- a/drivers/soc/mediatek/Kconfig
> > +++ b/drivers/soc/mediatek/Kconfig
> > @@ -44,4 +44,11 @@ config MTK_SCPSYS
> > Say yes here to add support for the MediaTek SCPSYS power domain
> > driver.
> >
> > +config MT8173_MMSYS
> > + bool "MediaTek MT8173 MMSYS Support"
>
> Hi,
> Can this be tristate instead of bool?
>

Wouldn't that conflict with
the driver being a builtin_platform_driver,
or am I just confusing things badly?

Thanks,
Ezequiel

> + depends on COMMON_CLK_MT8173
> > + help
> > + Say yes here to add support for the MediaTek MT8173 Multimedia
> > + Subsystem (MMSYS).
> > +
> > endmenu
>
> thanks.
> --
> ~Randy
>
>


2020-02-26 20:38:22

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v9 3/4] soc: mediatek: Move mt8173 MMSYS to platform driver

On 2/26/20 12:29 PM, Ezequiel Garcia wrote:
> On Wed, 2020-02-26 at 07:40 -0800, Randy Dunlap wrote:
>> On 2/26/20 2:54 AM, Enric Balletbo i Serra wrote:
>>> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
>>> index 2114b563478c..dcd6481a14d0 100644
>>> --- a/drivers/soc/mediatek/Kconfig
>>> +++ b/drivers/soc/mediatek/Kconfig
>>> @@ -44,4 +44,11 @@ config MTK_SCPSYS
>>> Say yes here to add support for the MediaTek SCPSYS power domain
>>> driver.
>>>
>>> +config MT8173_MMSYS
>>> + bool "MediaTek MT8173 MMSYS Support"
>>
>> Hi,
>> Can this be tristate instead of bool?
>>
>
> Wouldn't that conflict with
> the driver being a builtin_platform_driver,
> or am I just confusing things badly?

OK, it probably would conflict.

Thanks.

>> + depends on COMMON_CLK_MT8173
>>> + help
>>> + Say yes here to add support for the MediaTek MT8173 Multimedia
>>> + Subsystem (MMSYS).
>>> +
>>> endmenu


--
~Randy

2020-02-27 01:11:31

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [PATCH v9 1/4] drm/mediatek: Use regmap for register access

Hi, Enric:

On Wed, 2020-02-26 at 11:54 +0100, Enric Balletbo i Serra wrote:
> From: Matthias Brugger <[email protected]>
>
> The mmsys memory space is shared between the drm and the
> clk driver. Use regmap to access it.

Once there is a mmsys driver and clock control is moved into mmsys
driver, I think we should also move routing control into mmsys driver
and we could drop this patch.

Regards,
CK

>
> Signed-off-by: Matthias Brugger <[email protected]>
> Reviewed-by: Philipp Zabel <[email protected]>
> Reviewed-by: CK Hu <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
>
> Changes in v9: None
> Changes in v8:
> - Select REGMAP and MFD_SYSCON (Randy Dunlap)
>
> Changes in v7:
> - Add R-by from CK
>
> drivers/gpu/drm/mediatek/Kconfig | 2 +
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +-
> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 50 +++++++++++--------------
> drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 4 +-
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 13 ++-----
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +-
> 6 files changed, 32 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
> index fa5ffc4fe823..89e18a473cb5 100644
> --- a/drivers/gpu/drm/mediatek/Kconfig
> +++ b/drivers/gpu/drm/mediatek/Kconfig
> @@ -10,8 +10,10 @@ config DRM_MEDIATEK
> select DRM_KMS_HELPER
> select DRM_MIPI_DSI
> select DRM_PANEL
> + select MFD_SYSCON
> select MEMORY
> select MTK_SMI
> + select REGMAP
> select VIDEOMODE_HELPERS
> help
> Choose this option if you have a Mediatek SoCs.
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 5ee74d7ce35c..a236499123aa 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -28,7 +28,7 @@
> * @enabled: records whether crtc_enable succeeded
> * @planes: array of 4 drm_plane structures, one for each overlay plane
> * @pending_planes: whether any plane has pending changes to be applied
> - * @config_regs: memory mapped mmsys configuration register space
> + * @config_regs: regmap mapped mmsys configuration register space
> * @mutex: handle to one of the ten disp_mutex streams
> * @ddp_comp_nr: number of components in ddp_comp
> * @ddp_comp: array of pointers the mtk_ddp_comp structures used by this crtc
> @@ -50,7 +50,7 @@ struct mtk_drm_crtc {
> u32 cmdq_event;
> #endif
>
> - void __iomem *config_regs;
> + struct regmap *config_regs;
> struct mtk_disp_mutex *mutex;
> unsigned int ddp_comp_nr;
> struct mtk_ddp_comp **ddp_comp;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> index 13035c906035..302753744cc6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> @@ -383,61 +383,53 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
> return value;
> }
>
> -static void mtk_ddp_sout_sel(void __iomem *config_regs,
> +static void mtk_ddp_sout_sel(struct regmap *config_regs,
> enum mtk_ddp_comp_id cur,
> enum mtk_ddp_comp_id next)
> {
> if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> - writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
> - config_regs + DISP_REG_CONFIG_OUT_SEL);
> + regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
> + BLS_TO_DSI_RDMA1_TO_DPI1);
> } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
> - writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI,
> - config_regs + DISP_REG_CONFIG_OUT_SEL);
> - writel_relaxed(DSI_SEL_IN_RDMA,
> - config_regs + DISP_REG_CONFIG_DSI_SEL);
> - writel_relaxed(DPI_SEL_IN_BLS,
> - config_regs + DISP_REG_CONFIG_DPI_SEL);
> + regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
> + BLS_TO_DPI_RDMA1_TO_DSI);
> + regmap_write(config_regs, DISP_REG_CONFIG_DSI_SEL,
> + DSI_SEL_IN_RDMA);
> + regmap_write(config_regs, DISP_REG_CONFIG_DPI_SEL,
> + DPI_SEL_IN_BLS);
> }
> }
>
> -void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
> +void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
> enum mtk_ddp_comp_id cur,
> enum mtk_ddp_comp_id next)
> {
> - unsigned int addr, value, reg;
> + unsigned int addr, value;
>
> value = mtk_ddp_mout_en(cur, next, &addr);
> - if (value) {
> - reg = readl_relaxed(config_regs + addr) | value;
> - writel_relaxed(reg, config_regs + addr);
> - }
> + if (value)
> + regmap_update_bits(config_regs, addr, value, value);
>
> mtk_ddp_sout_sel(config_regs, cur, next);
>
> value = mtk_ddp_sel_in(cur, next, &addr);
> - if (value) {
> - reg = readl_relaxed(config_regs + addr) | value;
> - writel_relaxed(reg, config_regs + addr);
> - }
> + if (value)
> + regmap_update_bits(config_regs, addr, value, value);
> }
>
> -void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
> +void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
> enum mtk_ddp_comp_id cur,
> enum mtk_ddp_comp_id next)
> {
> - unsigned int addr, value, reg;
> + unsigned int addr, value;
>
> value = mtk_ddp_mout_en(cur, next, &addr);
> - if (value) {
> - reg = readl_relaxed(config_regs + addr) & ~value;
> - writel_relaxed(reg, config_regs + addr);
> - }
> + if (value)
> + regmap_update_bits(config_regs, addr, value, 0);
>
> value = mtk_ddp_sel_in(cur, next, &addr);
> - if (value) {
> - reg = readl_relaxed(config_regs + addr) & ~value;
> - writel_relaxed(reg, config_regs + addr);
> - }
> + if (value)
> + regmap_update_bits(config_regs, addr, value, 0);
> }
>
> struct mtk_disp_mutex *mtk_disp_mutex_get(struct device *dev, unsigned int id)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
> index 827be424a148..01ff8b68881f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
> @@ -12,10 +12,10 @@ struct regmap;
> struct device;
> struct mtk_disp_mutex;
>
> -void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
> +void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
> enum mtk_ddp_comp_id cur,
> enum mtk_ddp_comp_id next);
> -void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
> +void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
> enum mtk_ddp_comp_id cur,
> enum mtk_ddp_comp_id next);
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 0563c6813333..b68837ea02b3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -6,6 +6,7 @@
>
> #include <linux/component.h>
> #include <linux/iommu.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of_address.h>
> #include <linux/of_platform.h>
> @@ -425,7 +426,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct mtk_drm_private *private;
> - struct resource *mem;
> struct device_node *node;
> struct component_match *match = NULL;
> int ret;
> @@ -437,14 +437,9 @@ static int mtk_drm_probe(struct platform_device *pdev)
>
> private->data = of_device_get_match_data(dev);
>
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - private->config_regs = devm_ioremap_resource(dev, mem);
> - if (IS_ERR(private->config_regs)) {
> - ret = PTR_ERR(private->config_regs);
> - dev_err(dev, "Failed to ioremap mmsys-config resource: %d\n",
> - ret);
> - return ret;
> - }
> + private->config_regs = syscon_node_to_regmap(dev->of_node);
> + if (IS_ERR(private->config_regs))
> + return PTR_ERR(private->config_regs);
>
> /* Iterate over sibling DISP function blocks */
> for_each_child_of_node(dev->of_node->parent, node) {
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 17bc99b9f5d4..03201080688d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -39,7 +39,7 @@ struct mtk_drm_private {
>
> struct device_node *mutex_node;
> struct device *mutex_dev;
> - void __iomem *config_regs;
> + struct regmap *config_regs;
> struct device_node *comp_node[DDP_COMPONENT_ID_MAX];
> struct mtk_ddp_comp *ddp_comp[DDP_COMPONENT_ID_MAX];
> const struct mtk_mmsys_driver_data *data;

2020-02-27 01:19:11

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [PATCH v9 3/4] soc: mediatek: Move mt8173 MMSYS to platform driver

Hi, Enric:

On Wed, 2020-02-26 at 11:54 +0100, Enric Balletbo i Serra wrote:
> From: Matthias Brugger <[email protected]>
>
> There is no strong reason for this to use CLK_OF_DECLARE instead of
> being a platform driver. Plus, this driver provides clocks but also
> a shared register space for the mediatek-drm and the mediatek-mdp
> driver. So move to drivers/soc/mediatek as a platform driver.
>

Reviewed-by: CK Hu <[email protected]>

> Signed-off-by: Matthias Brugger <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
>
> Changes in v9:
> - Move mmsys to drivers/soc/mediatek (CK)
>
> Changes in v8:
> - Be a builtin_platform_driver like other mediatek mmsys drivers.
>
> Changes in v7:
> - Free clk_data->clks as well
> - Get rid of private data structure
>
> drivers/clk/mediatek/clk-mt8173.c | 104 ---------------------
> drivers/soc/mediatek/Kconfig | 7 ++
> drivers/soc/mediatek/Makefile | 1 +
> drivers/soc/mediatek/mt8173-mmsys.c | 137 ++++++++++++++++++++++++++++
> 4 files changed, 145 insertions(+), 104 deletions(-)
> create mode 100644 drivers/soc/mediatek/mt8173-mmsys.c
>
> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
> index 537a7f49b0f7..8f898ac476c0 100644
> --- a/drivers/clk/mediatek/clk-mt8173.c
> +++ b/drivers/clk/mediatek/clk-mt8173.c
> @@ -753,93 +753,6 @@ static const struct mtk_gate img_clks[] __initconst = {
> GATE_IMG(CLK_IMG_FD, "img_fd", "mm_sel", 11),
> };
>
> -static const struct mtk_gate_regs mm0_cg_regs __initconst = {
> - .set_ofs = 0x0104,
> - .clr_ofs = 0x0108,
> - .sta_ofs = 0x0100,
> -};
> -
> -static const struct mtk_gate_regs mm1_cg_regs __initconst = {
> - .set_ofs = 0x0114,
> - .clr_ofs = 0x0118,
> - .sta_ofs = 0x0110,
> -};
> -
> -#define GATE_MM0(_id, _name, _parent, _shift) { \
> - .id = _id, \
> - .name = _name, \
> - .parent_name = _parent, \
> - .regs = &mm0_cg_regs, \
> - .shift = _shift, \
> - .ops = &mtk_clk_gate_ops_setclr, \
> - }
> -
> -#define GATE_MM1(_id, _name, _parent, _shift) { \
> - .id = _id, \
> - .name = _name, \
> - .parent_name = _parent, \
> - .regs = &mm1_cg_regs, \
> - .shift = _shift, \
> - .ops = &mtk_clk_gate_ops_setclr, \
> - }
> -
> -static const struct mtk_gate mm_clks[] __initconst = {
> - /* MM0 */
> - GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
> - GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
> - GATE_MM0(CLK_MM_CAM_MDP, "mm_cam_mdp", "mm_sel", 2),
> - GATE_MM0(CLK_MM_MDP_RDMA0, "mm_mdp_rdma0", "mm_sel", 3),
> - GATE_MM0(CLK_MM_MDP_RDMA1, "mm_mdp_rdma1", "mm_sel", 4),
> - GATE_MM0(CLK_MM_MDP_RSZ0, "mm_mdp_rsz0", "mm_sel", 5),
> - GATE_MM0(CLK_MM_MDP_RSZ1, "mm_mdp_rsz1", "mm_sel", 6),
> - GATE_MM0(CLK_MM_MDP_RSZ2, "mm_mdp_rsz2", "mm_sel", 7),
> - GATE_MM0(CLK_MM_MDP_TDSHP0, "mm_mdp_tdshp0", "mm_sel", 8),
> - GATE_MM0(CLK_MM_MDP_TDSHP1, "mm_mdp_tdshp1", "mm_sel", 9),
> - GATE_MM0(CLK_MM_MDP_WDMA, "mm_mdp_wdma", "mm_sel", 11),
> - GATE_MM0(CLK_MM_MDP_WROT0, "mm_mdp_wrot0", "mm_sel", 12),
> - GATE_MM0(CLK_MM_MDP_WROT1, "mm_mdp_wrot1", "mm_sel", 13),
> - GATE_MM0(CLK_MM_FAKE_ENG, "mm_fake_eng", "mm_sel", 14),
> - GATE_MM0(CLK_MM_MUTEX_32K, "mm_mutex_32k", "rtc_sel", 15),
> - GATE_MM0(CLK_MM_DISP_OVL0, "mm_disp_ovl0", "mm_sel", 16),
> - GATE_MM0(CLK_MM_DISP_OVL1, "mm_disp_ovl1", "mm_sel", 17),
> - GATE_MM0(CLK_MM_DISP_RDMA0, "mm_disp_rdma0", "mm_sel", 18),
> - GATE_MM0(CLK_MM_DISP_RDMA1, "mm_disp_rdma1", "mm_sel", 19),
> - GATE_MM0(CLK_MM_DISP_RDMA2, "mm_disp_rdma2", "mm_sel", 20),
> - GATE_MM0(CLK_MM_DISP_WDMA0, "mm_disp_wdma0", "mm_sel", 21),
> - GATE_MM0(CLK_MM_DISP_WDMA1, "mm_disp_wdma1", "mm_sel", 22),
> - GATE_MM0(CLK_MM_DISP_COLOR0, "mm_disp_color0", "mm_sel", 23),
> - GATE_MM0(CLK_MM_DISP_COLOR1, "mm_disp_color1", "mm_sel", 24),
> - GATE_MM0(CLK_MM_DISP_AAL, "mm_disp_aal", "mm_sel", 25),
> - GATE_MM0(CLK_MM_DISP_GAMMA, "mm_disp_gamma", "mm_sel", 26),
> - GATE_MM0(CLK_MM_DISP_UFOE, "mm_disp_ufoe", "mm_sel", 27),
> - GATE_MM0(CLK_MM_DISP_SPLIT0, "mm_disp_split0", "mm_sel", 28),
> - GATE_MM0(CLK_MM_DISP_SPLIT1, "mm_disp_split1", "mm_sel", 29),
> - GATE_MM0(CLK_MM_DISP_MERGE, "mm_disp_merge", "mm_sel", 30),
> - GATE_MM0(CLK_MM_DISP_OD, "mm_disp_od", "mm_sel", 31),
> - /* MM1 */
> - GATE_MM1(CLK_MM_DISP_PWM0MM, "mm_disp_pwm0mm", "mm_sel", 0),
> - GATE_MM1(CLK_MM_DISP_PWM026M, "mm_disp_pwm026m", "pwm_sel", 1),
> - GATE_MM1(CLK_MM_DISP_PWM1MM, "mm_disp_pwm1mm", "mm_sel", 2),
> - GATE_MM1(CLK_MM_DISP_PWM126M, "mm_disp_pwm126m", "pwm_sel", 3),
> - GATE_MM1(CLK_MM_DSI0_ENGINE, "mm_dsi0_engine", "mm_sel", 4),
> - GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "dsi0_dig", 5),
> - GATE_MM1(CLK_MM_DSI1_ENGINE, "mm_dsi1_engine", "mm_sel", 6),
> - GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "dsi1_dig", 7),
> - GATE_MM1(CLK_MM_DPI_PIXEL, "mm_dpi_pixel", "dpi0_sel", 8),
> - GATE_MM1(CLK_MM_DPI_ENGINE, "mm_dpi_engine", "mm_sel", 9),
> - GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "lvds_pxl", 10),
> - GATE_MM1(CLK_MM_DPI1_ENGINE, "mm_dpi1_engine", "mm_sel", 11),
> - GATE_MM1(CLK_MM_HDMI_PIXEL, "mm_hdmi_pixel", "dpi0_sel", 12),
> - GATE_MM1(CLK_MM_HDMI_PLLCK, "mm_hdmi_pllck", "hdmi_sel", 13),
> - GATE_MM1(CLK_MM_HDMI_AUDIO, "mm_hdmi_audio", "apll1", 14),
> - GATE_MM1(CLK_MM_HDMI_SPDIF, "mm_hdmi_spdif", "apll2", 15),
> - GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "lvds_pxl", 16),
> - GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "lvds_cts", 17),
> - GATE_MM1(CLK_MM_SMI_LARB4, "mm_smi_larb4", "mm_sel", 18),
> - GATE_MM1(CLK_MM_HDMI_HDCP, "mm_hdmi_hdcp", "hdcp_sel", 19),
> - GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
> -};
> -
> static const struct mtk_gate_regs vdec0_cg_regs __initconst = {
> .set_ofs = 0x0000,
> .clr_ofs = 0x0004,
> @@ -1144,23 +1057,6 @@ static void __init mtk_imgsys_init(struct device_node *node)
> }
> CLK_OF_DECLARE(mtk_imgsys, "mediatek,mt8173-imgsys", mtk_imgsys_init);
>
> -static void __init mtk_mmsys_init(struct device_node *node)
> -{
> - struct clk_onecell_data *clk_data;
> - int r;
> -
> - clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
> -
> - mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
> - clk_data);
> -
> - r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> - if (r)
> - pr_err("%s(): could not register clock provider: %d\n",
> - __func__, r);
> -}
> -CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys", mtk_mmsys_init);
> -
> static void __init mtk_vdecsys_init(struct device_node *node)
> {
> struct clk_onecell_data *clk_data;
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index 2114b563478c..dcd6481a14d0 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -44,4 +44,11 @@ config MTK_SCPSYS
> Say yes here to add support for the MediaTek SCPSYS power domain
> driver.
>
> +config MT8173_MMSYS
> + bool "MediaTek MT8173 MMSYS Support"
> + depends on COMMON_CLK_MT8173
> + help
> + Say yes here to add support for the MediaTek MT8173 Multimedia
> + Subsystem (MMSYS).
> +
> endmenu
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index b01733074ad6..a185e4ee7601 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -3,3 +3,4 @@ obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
> obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
> obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
> obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> +obj-$(CONFIG_MT8173_MMSYS) += mt8173-mmsys.o
> diff --git a/drivers/soc/mediatek/mt8173-mmsys.c b/drivers/soc/mediatek/mt8173-mmsys.c
> new file mode 100644
> index 000000000000..48e6c157d28e
> --- /dev/null
> +++ b/drivers/soc/mediatek/mt8173-mmsys.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: James Liao <[email protected]>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/platform_device.h>
> +
> +#include "../../clk/mediatek/clk-gate.h"
> +#include "../../clk/mediatek/clk-mtk.h"
> +
> +#include <dt-bindings/clock/mt8173-clk.h>
> +
> +static const struct mtk_gate_regs mm0_cg_regs = {
> + .set_ofs = 0x0104,
> + .clr_ofs = 0x0108,
> + .sta_ofs = 0x0100,
> +};
> +
> +static const struct mtk_gate_regs mm1_cg_regs = {
> + .set_ofs = 0x0114,
> + .clr_ofs = 0x0118,
> + .sta_ofs = 0x0110,
> +};
> +
> +#define GATE_MM0(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &mm0_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr, \
> + }
> +
> +#define GATE_MM1(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &mm1_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr, \
> + }
> +
> +static const struct mtk_gate mm_clks[] = {
> + /* MM0 */
> + GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
> + GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
> + GATE_MM0(CLK_MM_CAM_MDP, "mm_cam_mdp", "mm_sel", 2),
> + GATE_MM0(CLK_MM_MDP_RDMA0, "mm_mdp_rdma0", "mm_sel", 3),
> + GATE_MM0(CLK_MM_MDP_RDMA1, "mm_mdp_rdma1", "mm_sel", 4),
> + GATE_MM0(CLK_MM_MDP_RSZ0, "mm_mdp_rsz0", "mm_sel", 5),
> + GATE_MM0(CLK_MM_MDP_RSZ1, "mm_mdp_rsz1", "mm_sel", 6),
> + GATE_MM0(CLK_MM_MDP_RSZ2, "mm_mdp_rsz2", "mm_sel", 7),
> + GATE_MM0(CLK_MM_MDP_TDSHP0, "mm_mdp_tdshp0", "mm_sel", 8),
> + GATE_MM0(CLK_MM_MDP_TDSHP1, "mm_mdp_tdshp1", "mm_sel", 9),
> + GATE_MM0(CLK_MM_MDP_WDMA, "mm_mdp_wdma", "mm_sel", 11),
> + GATE_MM0(CLK_MM_MDP_WROT0, "mm_mdp_wrot0", "mm_sel", 12),
> + GATE_MM0(CLK_MM_MDP_WROT1, "mm_mdp_wrot1", "mm_sel", 13),
> + GATE_MM0(CLK_MM_FAKE_ENG, "mm_fake_eng", "mm_sel", 14),
> + GATE_MM0(CLK_MM_MUTEX_32K, "mm_mutex_32k", "rtc_sel", 15),
> + GATE_MM0(CLK_MM_DISP_OVL0, "mm_disp_ovl0", "mm_sel", 16),
> + GATE_MM0(CLK_MM_DISP_OVL1, "mm_disp_ovl1", "mm_sel", 17),
> + GATE_MM0(CLK_MM_DISP_RDMA0, "mm_disp_rdma0", "mm_sel", 18),
> + GATE_MM0(CLK_MM_DISP_RDMA1, "mm_disp_rdma1", "mm_sel", 19),
> + GATE_MM0(CLK_MM_DISP_RDMA2, "mm_disp_rdma2", "mm_sel", 20),
> + GATE_MM0(CLK_MM_DISP_WDMA0, "mm_disp_wdma0", "mm_sel", 21),
> + GATE_MM0(CLK_MM_DISP_WDMA1, "mm_disp_wdma1", "mm_sel", 22),
> + GATE_MM0(CLK_MM_DISP_COLOR0, "mm_disp_color0", "mm_sel", 23),
> + GATE_MM0(CLK_MM_DISP_COLOR1, "mm_disp_color1", "mm_sel", 24),
> + GATE_MM0(CLK_MM_DISP_AAL, "mm_disp_aal", "mm_sel", 25),
> + GATE_MM0(CLK_MM_DISP_GAMMA, "mm_disp_gamma", "mm_sel", 26),
> + GATE_MM0(CLK_MM_DISP_UFOE, "mm_disp_ufoe", "mm_sel", 27),
> + GATE_MM0(CLK_MM_DISP_SPLIT0, "mm_disp_split0", "mm_sel", 28),
> + GATE_MM0(CLK_MM_DISP_SPLIT1, "mm_disp_split1", "mm_sel", 29),
> + GATE_MM0(CLK_MM_DISP_MERGE, "mm_disp_merge", "mm_sel", 30),
> + GATE_MM0(CLK_MM_DISP_OD, "mm_disp_od", "mm_sel", 31),
> + /* MM1 */
> + GATE_MM1(CLK_MM_DISP_PWM0MM, "mm_disp_pwm0mm", "mm_sel", 0),
> + GATE_MM1(CLK_MM_DISP_PWM026M, "mm_disp_pwm026m", "pwm_sel", 1),
> + GATE_MM1(CLK_MM_DISP_PWM1MM, "mm_disp_pwm1mm", "mm_sel", 2),
> + GATE_MM1(CLK_MM_DISP_PWM126M, "mm_disp_pwm126m", "pwm_sel", 3),
> + GATE_MM1(CLK_MM_DSI0_ENGINE, "mm_dsi0_engine", "mm_sel", 4),
> + GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "dsi0_dig", 5),
> + GATE_MM1(CLK_MM_DSI1_ENGINE, "mm_dsi1_engine", "mm_sel", 6),
> + GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "dsi1_dig", 7),
> + GATE_MM1(CLK_MM_DPI_PIXEL, "mm_dpi_pixel", "dpi0_sel", 8),
> + GATE_MM1(CLK_MM_DPI_ENGINE, "mm_dpi_engine", "mm_sel", 9),
> + GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "lvds_pxl", 10),
> + GATE_MM1(CLK_MM_DPI1_ENGINE, "mm_dpi1_engine", "mm_sel", 11),
> + GATE_MM1(CLK_MM_HDMI_PIXEL, "mm_hdmi_pixel", "dpi0_sel", 12),
> + GATE_MM1(CLK_MM_HDMI_PLLCK, "mm_hdmi_pllck", "hdmi_sel", 13),
> + GATE_MM1(CLK_MM_HDMI_AUDIO, "mm_hdmi_audio", "apll1", 14),
> + GATE_MM1(CLK_MM_HDMI_SPDIF, "mm_hdmi_spdif", "apll2", 15),
> + GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "lvds_pxl", 16),
> + GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "lvds_cts", 17),
> + GATE_MM1(CLK_MM_SMI_LARB4, "mm_smi_larb4", "mm_sel", 18),
> + GATE_MM1(CLK_MM_HDMI_HDCP, "mm_hdmi_hdcp", "hdcp_sel", 19),
> + GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
> +};
> +
> +static int mt8173_mmsys_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct clk_onecell_data *clk_data;
> + int ret;
> +
> + clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
> + if (!clk_data)
> + return -ENOMEM;
> +
> + ret = mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
> + clk_data);
> + if (ret)
> + return ret;
> +
> + ret = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id of_match_mt8173_mmsys[] = {
> + { .compatible = "mediatek,mt8173-mmsys", },
> + { }
> +};
> +
> +static struct platform_driver mt8173_mmsys_drv = {
> + .driver = {
> + .name = "mt8173-mmsys",
> + .of_match_table = of_match_mt8173_mmsys,
> + },
> + .probe = mt8173_mmsys_probe,
> +};
> +
> +builtin_platform_driver(mt8173_mmsys_drv);

2020-02-27 01:34:26

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [PATCH v9 4/4] drm/mediatek: Fix mediatek-drm device probing

Hi, Enric:

On Wed, 2020-02-26 at 11:54 +0100, Enric Balletbo i Serra wrote:
> In the actual implementation the same compatible string
> "mediatek,<chip>-mmsys" is used to bind the clock drivers
> (drivers/soc/mediatek) as well as to the gpu driver
> (drivers/gpu/drm/mediatek/mtk_drm_drv.c). This ends with the problem
> that the only probed driver is the clock driver and there is no display
> at all.
>
> In any case having the same compatible string for two drivers is not
> correct and should be fixed. To fix this, and maintain backward
> compatibility, we can consider that the mmsys driver is the top-level
> entry point for the multimedia subsystem, so is not a pure clock
> controller but a system controller, and the drm driver is instantiated
> by that MMSYS driver.
>

Reviewed-by: CK Hu <[email protected]>

> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
>
> Changes in v9:
> - Do not move the display routing from the drm driver (CK)
>
> Changes in v8:
> - New patch introduced in this series.
>
> Changes in v7: None
>
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 34 ++++++++++++++------------
> drivers/soc/mediatek/mt8173-mmsys.c | 6 +++++
> 2 files changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index b68837ea02b3..17f118ee0e57 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -422,9 +422,21 @@ 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,mt2712-mmsys",
> + .data = &mt2712_mmsys_driver_data},
> + { .compatible = "mediatek,mt8173-mmsys",
> + .data = &mt8173_mmsys_driver_data},
> + { }
> +};
> +
> static int mtk_drm_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> + struct device_node *phandle = dev->parent->of_node;
> + const struct of_device_id *of_id;
> struct mtk_drm_private *private;
> struct device_node *node;
> struct component_match *match = NULL;
> @@ -435,15 +447,18 @@ static int mtk_drm_probe(struct platform_device *pdev)
> if (!private)
> return -ENOMEM;
>
> - private->data = of_device_get_match_data(dev);
> + of_id = of_match_node(mtk_drm_of_ids, phandle);
> + if (!of_id)
> + return -ENODEV;
> +
> + private->data = of_id->data;
>
> - private->config_regs = syscon_node_to_regmap(dev->of_node);
> + private->config_regs = syscon_node_to_regmap(phandle);
> if (IS_ERR(private->config_regs))
> return PTR_ERR(private->config_regs);
>
> /* Iterate over sibling DISP function blocks */
> - for_each_child_of_node(dev->of_node->parent, node) {
> - const struct of_device_id *of_id;
> + for_each_child_of_node(phandle->parent, node) {
> enum mtk_ddp_comp_type comp_type;
> int comp_id;
>
> @@ -576,22 +591,11 @@ static int mtk_drm_sys_resume(struct device *dev)
> static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
> mtk_drm_sys_resume);
>
> -static const struct of_device_id mtk_drm_of_ids[] = {
> - { .compatible = "mediatek,mt2701-mmsys",
> - .data = &mt2701_mmsys_driver_data},
> - { .compatible = "mediatek,mt2712-mmsys",
> - .data = &mt2712_mmsys_driver_data},
> - { .compatible = "mediatek,mt8173-mmsys",
> - .data = &mt8173_mmsys_driver_data},
> - { }
> -};
> -
> static struct platform_driver mtk_drm_platform_driver = {
> .probe = mtk_drm_probe,
> .remove = mtk_drm_remove,
> .driver = {
> .name = "mediatek-drm",
> - .of_match_table = mtk_drm_of_ids,
> .pm = &mtk_drm_pm_ops,
> },
> };
> diff --git a/drivers/soc/mediatek/mt8173-mmsys.c b/drivers/soc/mediatek/mt8173-mmsys.c
> index 48e6c157d28e..c894db5b6ca9 100644
> --- a/drivers/soc/mediatek/mt8173-mmsys.c
> +++ b/drivers/soc/mediatek/mt8173-mmsys.c
> @@ -103,6 +103,7 @@ static int mt8173_mmsys_probe(struct platform_device *pdev)
> {
> struct device_node *node = pdev->dev.of_node;
> struct clk_onecell_data *clk_data;
> + struct platform_device *drm;
> int ret;
>
> clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
> @@ -118,6 +119,11 @@ static int mt8173_mmsys_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + drm = platform_device_register_data(&pdev->dev, "mediatek-drm",
> + PLATFORM_DEVID_NONE, NULL, 0);
> + if (IS_ERR(drm))
> + return PTR_ERR(drm);
> +
> return 0;
> }
>

2020-02-27 02:20:38

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [PATCH v9 0/4] arm64: mediatek: Fix mt8173 mmsys device probing

Hi, Enric:

I would like you to modify mmsys binding document. In current document,
mmsys is a clock controller, but I think it's a system controller
including clock control, routing control, and miscellaneous control in
mmsys partition.

Regards,
CK

On Wed, 2020-02-26 at 11:54 +0100, Enric Balletbo i Serra wrote:
> Dear all,
>
> Those patches are intended to solve an old standing issue on some
> Mediatek devices (mt8173, mt2701 and mt2712).
>
> Up to now both drivers, clock and drm are probed with the same device tree
> compatible. But only the first driver gets probed, which in effect breaks
> graphics on those devices.
>
> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
> control clock gates (which is used in the clk driver) and some registers
> to set the routing and enable the differnet blocks of the display
> and MDP (Media Data Path) subsystem. On this series the clk driver is
> not a pure clock controller but a system controller that can provide
> access to the shared registers between the different drivers that need
> it (mediatek-drm and mediatek-mdp). Hence the MMSYS clk driver was moved
> to drivers/soc/mediatek and is the entry point (parent) which will trigger
> the probe of the corresponding mediatek-drm driver.
>
> **IMPORTANT** This series only fixes the issue on mt8173 to make it
> simple and as is the only platform I can test. Similar changes should be
> applied for mt2701 and mt2712 to have display working.
>
> For reference, here are the links to the old discussions:
> * v8: https://patchwork.kernel.org/project/linux-mediatek/list/?series=244891
> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
> * v4:
> * https://patchwork.kernel.org/patch/10530871/
> * https://patchwork.kernel.org/patch/10530883/
> * https://patchwork.kernel.org/patch/10530885/
> * https://patchwork.kernel.org/patch/10530911/
> * https://patchwork.kernel.org/patch/10530913/
> * v3:
> * https://patchwork.kernel.org/patch/10367857/
> * https://patchwork.kernel.org/patch/10367861/
> * https://patchwork.kernel.org/patch/10367877/
> * https://patchwork.kernel.org/patch/10367875/
> * https://patchwork.kernel.org/patch/10367885/
> * https://patchwork.kernel.org/patch/10367883/
> * https://patchwork.kernel.org/patch/10367889/
> * https://patchwork.kernel.org/patch/10367907/
> * https://patchwork.kernel.org/patch/10367909/
> * https://patchwork.kernel.org/patch/10367905/
> * v2: No relevant discussion, see v3
> * v1:
> * https://patchwork.kernel.org/patch/10016497/
> * https://patchwork.kernel.org/patch/10016499/
> * https://patchwork.kernel.org/patch/10016505/
> * https://patchwork.kernel.org/patch/10016507/
>
> Best regards,
> Enric
>
> Changes in v9:
> - Move mmsys to drivers/soc/mediatek (CK)
> - Do not move the display routing from the drm driver (CK)
> - Removed from this series because are not needed:
> * [PATCH v8 5/6] drm/mediatek: Move MMSYS configuration to include/linux/platform_data
> - Removed from this series because are applied:
> * [PATCH v8 3/6] media: mtk-mdp: Check return value of of_clk_get.
>
> Changes in v8:
> - Select REGMAP and MFD_SYSCON (Randy Dunlap)
> - Be a builtin_platform_driver like other mediatek mmsys drivers.
> - New patch introduced in this series.
>
> Changes in v7:
> - Add R-by from CK
> - Free clk_data->clks as well
> - Get rid of private data structure
>
> Enric Balletbo i Serra (1):
> drm/mediatek: Fix mediatek-drm device probing
>
> Matthias Brugger (3):
> drm/mediatek: Use regmap for register access
> drm/mediatek: Omit warning on probe defers
> soc: mediatek: Move mt8173 MMSYS to platform driver
>
> drivers/clk/mediatek/clk-mt8173.c | 104 ----------------
> drivers/gpu/drm/mediatek/Kconfig | 2 +
> drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 +-
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 5 +-
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 +-
> drivers/gpu/drm/mediatek/mtk_dpi.c | 12 +-
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +-
> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 53 ++++----
> drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 4 +-
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 45 ++++---
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +-
> drivers/gpu/drm/mediatek/mtk_dsi.c | 8 +-
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 4 +-
> drivers/soc/mediatek/Kconfig | 7 ++
> drivers/soc/mediatek/Makefile | 1 +
> drivers/soc/mediatek/mt8173-mmsys.c | 143 ++++++++++++++++++++++
> 16 files changed, 233 insertions(+), 171 deletions(-)
> create mode 100644 drivers/soc/mediatek/mt8173-mmsys.c
>

2020-02-27 08:45:28

by Enric Balletbo i Serra

[permalink] [raw]
Subject: Re: [PATCH v9 1/4] drm/mediatek: Use regmap for register access

Hi CK,

On 27/2/20 2:10, CK Hu wrote:
> Hi, Enric:
>
> On Wed, 2020-02-26 at 11:54 +0100, Enric Balletbo i Serra wrote:
>> From: Matthias Brugger <[email protected]>
>>
>> The mmsys memory space is shared between the drm and the
>> clk driver. Use regmap to access it.
>
> Once there is a mmsys driver and clock control is moved into mmsys
> driver, I think we should also move routing control into mmsys driver
> and we could drop this patch.
>

Do you want me do this in this series or later?

Thanks,
Enric

> Regards,
> CK
>
>>
>> Signed-off-by: Matthias Brugger <[email protected]>
>> Reviewed-by: Philipp Zabel <[email protected]>
>> Reviewed-by: CK Hu <[email protected]>
>> Signed-off-by: Enric Balletbo i Serra <[email protected]>
>> ---
>>
>> Changes in v9: None
>> Changes in v8:
>> - Select REGMAP and MFD_SYSCON (Randy Dunlap)
>>
>> Changes in v7:
>> - Add R-by from CK
>>
>> drivers/gpu/drm/mediatek/Kconfig | 2 +
>> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +-
>> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 50 +++++++++++--------------
>> drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 4 +-
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 13 ++-----
>> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +-
>> 6 files changed, 32 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
>> index fa5ffc4fe823..89e18a473cb5 100644
>> --- a/drivers/gpu/drm/mediatek/Kconfig
>> +++ b/drivers/gpu/drm/mediatek/Kconfig
>> @@ -10,8 +10,10 @@ config DRM_MEDIATEK
>> select DRM_KMS_HELPER
>> select DRM_MIPI_DSI
>> select DRM_PANEL
>> + select MFD_SYSCON
>> select MEMORY
>> select MTK_SMI
>> + select REGMAP
>> select VIDEOMODE_HELPERS
>> help
>> Choose this option if you have a Mediatek SoCs.
>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>> index 5ee74d7ce35c..a236499123aa 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>> @@ -28,7 +28,7 @@
>> * @enabled: records whether crtc_enable succeeded
>> * @planes: array of 4 drm_plane structures, one for each overlay plane
>> * @pending_planes: whether any plane has pending changes to be applied
>> - * @config_regs: memory mapped mmsys configuration register space
>> + * @config_regs: regmap mapped mmsys configuration register space
>> * @mutex: handle to one of the ten disp_mutex streams
>> * @ddp_comp_nr: number of components in ddp_comp
>> * @ddp_comp: array of pointers the mtk_ddp_comp structures used by this crtc
>> @@ -50,7 +50,7 @@ struct mtk_drm_crtc {
>> u32 cmdq_event;
>> #endif
>>
>> - void __iomem *config_regs;
>> + struct regmap *config_regs;
>> struct mtk_disp_mutex *mutex;
>> unsigned int ddp_comp_nr;
>> struct mtk_ddp_comp **ddp_comp;
>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
>> index 13035c906035..302753744cc6 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
>> @@ -383,61 +383,53 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
>> return value;
>> }
>>
>> -static void mtk_ddp_sout_sel(void __iomem *config_regs,
>> +static void mtk_ddp_sout_sel(struct regmap *config_regs,
>> enum mtk_ddp_comp_id cur,
>> enum mtk_ddp_comp_id next)
>> {
>> if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
>> - writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
>> - config_regs + DISP_REG_CONFIG_OUT_SEL);
>> + regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
>> + BLS_TO_DSI_RDMA1_TO_DPI1);
>> } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
>> - writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI,
>> - config_regs + DISP_REG_CONFIG_OUT_SEL);
>> - writel_relaxed(DSI_SEL_IN_RDMA,
>> - config_regs + DISP_REG_CONFIG_DSI_SEL);
>> - writel_relaxed(DPI_SEL_IN_BLS,
>> - config_regs + DISP_REG_CONFIG_DPI_SEL);
>> + regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
>> + BLS_TO_DPI_RDMA1_TO_DSI);
>> + regmap_write(config_regs, DISP_REG_CONFIG_DSI_SEL,
>> + DSI_SEL_IN_RDMA);
>> + regmap_write(config_regs, DISP_REG_CONFIG_DPI_SEL,
>> + DPI_SEL_IN_BLS);
>> }
>> }
>>
>> -void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
>> +void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
>> enum mtk_ddp_comp_id cur,
>> enum mtk_ddp_comp_id next)
>> {
>> - unsigned int addr, value, reg;
>> + unsigned int addr, value;
>>
>> value = mtk_ddp_mout_en(cur, next, &addr);
>> - if (value) {
>> - reg = readl_relaxed(config_regs + addr) | value;
>> - writel_relaxed(reg, config_regs + addr);
>> - }
>> + if (value)
>> + regmap_update_bits(config_regs, addr, value, value);
>>
>> mtk_ddp_sout_sel(config_regs, cur, next);
>>
>> value = mtk_ddp_sel_in(cur, next, &addr);
>> - if (value) {
>> - reg = readl_relaxed(config_regs + addr) | value;
>> - writel_relaxed(reg, config_regs + addr);
>> - }
>> + if (value)
>> + regmap_update_bits(config_regs, addr, value, value);
>> }
>>
>> -void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
>> +void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
>> enum mtk_ddp_comp_id cur,
>> enum mtk_ddp_comp_id next)
>> {
>> - unsigned int addr, value, reg;
>> + unsigned int addr, value;
>>
>> value = mtk_ddp_mout_en(cur, next, &addr);
>> - if (value) {
>> - reg = readl_relaxed(config_regs + addr) & ~value;
>> - writel_relaxed(reg, config_regs + addr);
>> - }
>> + if (value)
>> + regmap_update_bits(config_regs, addr, value, 0);
>>
>> value = mtk_ddp_sel_in(cur, next, &addr);
>> - if (value) {
>> - reg = readl_relaxed(config_regs + addr) & ~value;
>> - writel_relaxed(reg, config_regs + addr);
>> - }
>> + if (value)
>> + regmap_update_bits(config_regs, addr, value, 0);
>> }
>>
>> struct mtk_disp_mutex *mtk_disp_mutex_get(struct device *dev, unsigned int id)
>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
>> index 827be424a148..01ff8b68881f 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
>> @@ -12,10 +12,10 @@ struct regmap;
>> struct device;
>> struct mtk_disp_mutex;
>>
>> -void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
>> +void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
>> enum mtk_ddp_comp_id cur,
>> enum mtk_ddp_comp_id next);
>> -void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
>> +void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
>> enum mtk_ddp_comp_id cur,
>> enum mtk_ddp_comp_id next);
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>> index 0563c6813333..b68837ea02b3 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>> @@ -6,6 +6,7 @@
>>
>> #include <linux/component.h>
>> #include <linux/iommu.h>
>> +#include <linux/mfd/syscon.h>
>> #include <linux/module.h>
>> #include <linux/of_address.h>
>> #include <linux/of_platform.h>
>> @@ -425,7 +426,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> struct mtk_drm_private *private;
>> - struct resource *mem;
>> struct device_node *node;
>> struct component_match *match = NULL;
>> int ret;
>> @@ -437,14 +437,9 @@ static int mtk_drm_probe(struct platform_device *pdev)
>>
>> private->data = of_device_get_match_data(dev);
>>
>> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - private->config_regs = devm_ioremap_resource(dev, mem);
>> - if (IS_ERR(private->config_regs)) {
>> - ret = PTR_ERR(private->config_regs);
>> - dev_err(dev, "Failed to ioremap mmsys-config resource: %d\n",
>> - ret);
>> - return ret;
>> - }
>> + private->config_regs = syscon_node_to_regmap(dev->of_node);
>> + if (IS_ERR(private->config_regs))
>> + return PTR_ERR(private->config_regs);
>>
>> /* Iterate over sibling DISP function blocks */
>> for_each_child_of_node(dev->of_node->parent, node) {
>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
>> index 17bc99b9f5d4..03201080688d 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
>> @@ -39,7 +39,7 @@ struct mtk_drm_private {
>>
>> struct device_node *mutex_node;
>> struct device *mutex_dev;
>> - void __iomem *config_regs;
>> + struct regmap *config_regs;
>> struct device_node *comp_node[DDP_COMPONENT_ID_MAX];
>> struct mtk_ddp_comp *ddp_comp[DDP_COMPONENT_ID_MAX];
>> const struct mtk_mmsys_driver_data *data;
>

2020-02-27 09:05:05

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [PATCH v9 1/4] drm/mediatek: Use regmap for register access

Hi, Enric:

On Thu, 2020-02-27 at 09:45 +0100, Enric Balletbo i Serra wrote:
> Hi CK,
>
> On 27/2/20 2:10, CK Hu wrote:
> > Hi, Enric:
> >
> > On Wed, 2020-02-26 at 11:54 +0100, Enric Balletbo i Serra wrote:
> >> From: Matthias Brugger <[email protected]>
> >>
> >> The mmsys memory space is shared between the drm and the
> >> clk driver. Use regmap to access it.
> >
> > Once there is a mmsys driver and clock control is moved into mmsys
> > driver, I think we should also move routing control into mmsys driver
> > and we could drop this patch.
> >
>
> Do you want me do this in this series or later?

I would like you to do it in this series. If you move routing control to
mmsys driver, you need not to use regmap any more. What you need to move
is what you modify in this patch. mmsys may provide mtk_mmsys_connect()
and mtk_mmsys_disconnect() function to replace
mtk_ddp_add_comp_to_path() and mtk_ddp_remove_comp_from_path(). DRM
driver need not to map mmsys's register and just keep mmsys device
pointer. You could move routing control after clock control has been
moved.

Regards,
CK

>
> Thanks,
> Enric
>
> > Regards,
> > CK
> >
> >>
> >> Signed-off-by: Matthias Brugger <[email protected]>
> >> Reviewed-by: Philipp Zabel <[email protected]>
> >> Reviewed-by: CK Hu <[email protected]>
> >> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> >> ---
> >>
> >> Changes in v9: None
> >> Changes in v8:
> >> - Select REGMAP and MFD_SYSCON (Randy Dunlap)
> >>
> >> Changes in v7:
> >> - Add R-by from CK
> >>
> >> drivers/gpu/drm/mediatek/Kconfig | 2 +
> >> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +-
> >> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 50 +++++++++++--------------
> >> drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 4 +-
> >> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 13 ++-----
> >> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +-
> >> 6 files changed, 32 insertions(+), 43 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
> >> index fa5ffc4fe823..89e18a473cb5 100644
> >> --- a/drivers/gpu/drm/mediatek/Kconfig
> >> +++ b/drivers/gpu/drm/mediatek/Kconfig
> >> @@ -10,8 +10,10 @@ config DRM_MEDIATEK
> >> select DRM_KMS_HELPER
> >> select DRM_MIPI_DSI
> >> select DRM_PANEL
> >> + select MFD_SYSCON
> >> select MEMORY
> >> select MTK_SMI
> >> + select REGMAP
> >> select VIDEOMODE_HELPERS
> >> help
> >> Choose this option if you have a Mediatek SoCs.
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> >> index 5ee74d7ce35c..a236499123aa 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> >> @@ -28,7 +28,7 @@
> >> * @enabled: records whether crtc_enable succeeded
> >> * @planes: array of 4 drm_plane structures, one for each overlay plane
> >> * @pending_planes: whether any plane has pending changes to be applied
> >> - * @config_regs: memory mapped mmsys configuration register space
> >> + * @config_regs: regmap mapped mmsys configuration register space
> >> * @mutex: handle to one of the ten disp_mutex streams
> >> * @ddp_comp_nr: number of components in ddp_comp
> >> * @ddp_comp: array of pointers the mtk_ddp_comp structures used by this crtc
> >> @@ -50,7 +50,7 @@ struct mtk_drm_crtc {
> >> u32 cmdq_event;
> >> #endif
> >>
> >> - void __iomem *config_regs;
> >> + struct regmap *config_regs;
> >> struct mtk_disp_mutex *mutex;
> >> unsigned int ddp_comp_nr;
> >> struct mtk_ddp_comp **ddp_comp;
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> >> index 13035c906035..302753744cc6 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> >> @@ -383,61 +383,53 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
> >> return value;
> >> }
> >>
> >> -static void mtk_ddp_sout_sel(void __iomem *config_regs,
> >> +static void mtk_ddp_sout_sel(struct regmap *config_regs,
> >> enum mtk_ddp_comp_id cur,
> >> enum mtk_ddp_comp_id next)
> >> {
> >> if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> >> - writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
> >> - config_regs + DISP_REG_CONFIG_OUT_SEL);
> >> + regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
> >> + BLS_TO_DSI_RDMA1_TO_DPI1);
> >> } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
> >> - writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI,
> >> - config_regs + DISP_REG_CONFIG_OUT_SEL);
> >> - writel_relaxed(DSI_SEL_IN_RDMA,
> >> - config_regs + DISP_REG_CONFIG_DSI_SEL);
> >> - writel_relaxed(DPI_SEL_IN_BLS,
> >> - config_regs + DISP_REG_CONFIG_DPI_SEL);
> >> + regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
> >> + BLS_TO_DPI_RDMA1_TO_DSI);
> >> + regmap_write(config_regs, DISP_REG_CONFIG_DSI_SEL,
> >> + DSI_SEL_IN_RDMA);
> >> + regmap_write(config_regs, DISP_REG_CONFIG_DPI_SEL,
> >> + DPI_SEL_IN_BLS);
> >> }
> >> }
> >>
> >> -void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
> >> +void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
> >> enum mtk_ddp_comp_id cur,
> >> enum mtk_ddp_comp_id next)
> >> {
> >> - unsigned int addr, value, reg;
> >> + unsigned int addr, value;
> >>
> >> value = mtk_ddp_mout_en(cur, next, &addr);
> >> - if (value) {
> >> - reg = readl_relaxed(config_regs + addr) | value;
> >> - writel_relaxed(reg, config_regs + addr);
> >> - }
> >> + if (value)
> >> + regmap_update_bits(config_regs, addr, value, value);
> >>
> >> mtk_ddp_sout_sel(config_regs, cur, next);
> >>
> >> value = mtk_ddp_sel_in(cur, next, &addr);
> >> - if (value) {
> >> - reg = readl_relaxed(config_regs + addr) | value;
> >> - writel_relaxed(reg, config_regs + addr);
> >> - }
> >> + if (value)
> >> + regmap_update_bits(config_regs, addr, value, value);
> >> }
> >>
> >> -void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
> >> +void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
> >> enum mtk_ddp_comp_id cur,
> >> enum mtk_ddp_comp_id next)
> >> {
> >> - unsigned int addr, value, reg;
> >> + unsigned int addr, value;
> >>
> >> value = mtk_ddp_mout_en(cur, next, &addr);
> >> - if (value) {
> >> - reg = readl_relaxed(config_regs + addr) & ~value;
> >> - writel_relaxed(reg, config_regs + addr);
> >> - }
> >> + if (value)
> >> + regmap_update_bits(config_regs, addr, value, 0);
> >>
> >> value = mtk_ddp_sel_in(cur, next, &addr);
> >> - if (value) {
> >> - reg = readl_relaxed(config_regs + addr) & ~value;
> >> - writel_relaxed(reg, config_regs + addr);
> >> - }
> >> + if (value)
> >> + regmap_update_bits(config_regs, addr, value, 0);
> >> }
> >>
> >> struct mtk_disp_mutex *mtk_disp_mutex_get(struct device *dev, unsigned int id)
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
> >> index 827be424a148..01ff8b68881f 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
> >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
> >> @@ -12,10 +12,10 @@ struct regmap;
> >> struct device;
> >> struct mtk_disp_mutex;
> >>
> >> -void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
> >> +void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
> >> enum mtk_ddp_comp_id cur,
> >> enum mtk_ddp_comp_id next);
> >> -void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
> >> +void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
> >> enum mtk_ddp_comp_id cur,
> >> enum mtk_ddp_comp_id next);
> >>
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> >> index 0563c6813333..b68837ea02b3 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> >> @@ -6,6 +6,7 @@
> >>
> >> #include <linux/component.h>
> >> #include <linux/iommu.h>
> >> +#include <linux/mfd/syscon.h>
> >> #include <linux/module.h>
> >> #include <linux/of_address.h>
> >> #include <linux/of_platform.h>
> >> @@ -425,7 +426,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
> >> {
> >> struct device *dev = &pdev->dev;
> >> struct mtk_drm_private *private;
> >> - struct resource *mem;
> >> struct device_node *node;
> >> struct component_match *match = NULL;
> >> int ret;
> >> @@ -437,14 +437,9 @@ static int mtk_drm_probe(struct platform_device *pdev)
> >>
> >> private->data = of_device_get_match_data(dev);
> >>
> >> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> - private->config_regs = devm_ioremap_resource(dev, mem);
> >> - if (IS_ERR(private->config_regs)) {
> >> - ret = PTR_ERR(private->config_regs);
> >> - dev_err(dev, "Failed to ioremap mmsys-config resource: %d\n",
> >> - ret);
> >> - return ret;
> >> - }
> >> + private->config_regs = syscon_node_to_regmap(dev->of_node);
> >> + if (IS_ERR(private->config_regs))
> >> + return PTR_ERR(private->config_regs);
> >>
> >> /* Iterate over sibling DISP function blocks */
> >> for_each_child_of_node(dev->of_node->parent, node) {
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> >> index 17bc99b9f5d4..03201080688d 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> >> @@ -39,7 +39,7 @@ struct mtk_drm_private {
> >>
> >> struct device_node *mutex_node;
> >> struct device *mutex_dev;
> >> - void __iomem *config_regs;
> >> + struct regmap *config_regs;
> >> struct device_node *comp_node[DDP_COMPONENT_ID_MAX];
> >> struct mtk_ddp_comp *ddp_comp[DDP_COMPONENT_ID_MAX];
> >> const struct mtk_mmsys_driver_data *data;
> >