Subject: [PATCH 0/3] drm/mediatek: Fixes for DDP component search/destroy

This series performs some cleanups for DDP component CRTC search and
correctly iounmaps the previously of_iomap() calls from drm_ddp_comp.

Tested on MT8195 Cherry Tomato

AngeloGioacchino Del Regno (3):
drm/mediatek: drm_ddp_comp: Fix and cleanup DDP component CRTC search
drm/mediatek: Perform iounmap on simple DDP component destruction
drm/mediatek: drm_ddp_comp: Add mtk_ddp_is_simple_comp() internal
helper

drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 113 +++++++++++++-------
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +-
3 files changed, 80 insertions(+), 38 deletions(-)

--
2.43.0



Subject: [PATCH 2/3] drm/mediatek: Perform iounmap on simple DDP component destruction

Add a new mtk_ddp_comp_destroy() function and call it in the teardown
path of mtk_drm_drv to make sure that we unmap the iospace of the
simple DDP components.

While at it, also fix iounmapping on mtk_ddp_comp_init() error path.

Fixes: ff1395609e20 ("drm/mediatek: Move mtk_ddp_comp_init() from sub driver to DRM driver")
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 38 ++++++++++++++++++++-
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 ++-
3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index c13359eeb3cd..539b526a6b0a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -626,8 +626,11 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,

priv->regs = of_iomap(node, 0);
priv->clk = of_clk_get(node, 0);
- if (IS_ERR(priv->clk))
+ if (IS_ERR(priv->clk)) {
+ iounmap(priv->regs);
+ priv->regs = NULL;
return PTR_ERR(priv->clk);
+ }

#if IS_REACHABLE(CONFIG_MTK_CMDQ)
ret = cmdq_dev_get_client_reg(comp->dev, &priv->cmdq_reg, 0);
@@ -639,3 +642,36 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,

return 0;
}
+
+void mtk_ddp_comp_destroy(struct mtk_ddp_comp *comp)
+{
+ struct mtk_ddp_comp_dev *priv;
+
+ if (!comp || !comp->dev)
+ return;
+
+ /* Complex components are destroyed with their own remove callback */
+ if (type == MTK_DISP_AAL ||
+ type == MTK_DISP_BLS ||
+ type == MTK_DISP_CCORR ||
+ type == MTK_DISP_COLOR ||
+ type == MTK_DISP_GAMMA ||
+ type == MTK_DISP_MERGE ||
+ type == MTK_DISP_OVL ||
+ type == MTK_DISP_OVL_2L ||
+ type == MTK_DISP_PWM ||
+ type == MTK_DISP_RDMA ||
+ type == MTK_DPI ||
+ type == MTK_DP_INTF ||
+ type == MTK_DSI)
+ return;
+
+ priv = dev_get_drvdata(comp->dev);
+ if (!priv)
+ return;
+
+ if (priv->regs) {
+ iounmap(priv->regs);
+ priv->regs = NULL;
+ }
+}
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index 15b2eafff438..43372b416a3f 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -318,6 +318,7 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
struct device *dev);
int mtk_ddp_comp_init(struct device_node *comp_node, struct mtk_ddp_comp *comp,
unsigned int comp_id);
+void mtk_ddp_comp_destroy(struct mtk_ddp_comp *comp);
enum mtk_ddp_comp_type mtk_ddp_comp_get_type(unsigned int comp_id);
void mtk_ddp_write(struct cmdq_pkt *cmdq_pkt, unsigned int value,
struct cmdq_client_reg *cmdq_reg, void __iomem *regs,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 14a1e0157cc4..89b6c31a1511 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -944,8 +944,10 @@ static void mtk_drm_remove(struct platform_device *pdev)
component_master_del(&pdev->dev, &mtk_drm_ops);
pm_runtime_disable(&pdev->dev);
of_node_put(private->mutex_node);
- for (i = 0; i < DDP_COMPONENT_DRM_ID_MAX; i++)
+ for (i = 0; i < DDP_COMPONENT_DRM_ID_MAX; i++) {
+ mtk_ddp_comp_destroy(&private->ddp_comp[i]);
of_node_put(private->comp_node[i]);
+ }
}

static int mtk_drm_sys_prepare(struct device *dev)
--
2.43.0


Subject: Re: [PATCH 0/3] drm/mediatek: Fixes for DDP component search/destroy

Il 01/02/24 13:53, AngeloGioacchino Del Regno ha scritto:
> This series performs some cleanups for DDP component CRTC search and
> correctly iounmaps the previously of_iomap() calls from drm_ddp_comp.
>
> Tested on MT8195 Cherry Tomato
>

Hello CK,
gentle ping for this series.

Cheers,
Angelo

> AngeloGioacchino Del Regno (3):
> drm/mediatek: drm_ddp_comp: Fix and cleanup DDP component CRTC search
> drm/mediatek: Perform iounmap on simple DDP component destruction
> drm/mediatek: drm_ddp_comp: Add mtk_ddp_is_simple_comp() internal
> helper
>
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 113 +++++++++++++-------
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +-
> 3 files changed, 80 insertions(+), 38 deletions(-)
>


2024-03-22 08:49:20

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [PATCH 2/3] drm/mediatek: Perform iounmap on simple DDP component destruction

Hi, Angelo:

On Thu, 2024-02-01 at 13:53 +0100, AngeloGioacchino Del Regno wrote:
> Add a new mtk_ddp_comp_destroy() function and call it in the teardown
> path of mtk_drm_drv to make sure that we unmap the iospace of the
> simple DDP components.
>
> While at it, also fix iounmapping on mtk_ddp_comp_init() error path.

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

>
> Fixes: ff1395609e20 ("drm/mediatek: Move mtk_ddp_comp_init() from sub
> driver to DRM driver")
> Signed-off-by: AngeloGioacchino Del Regno <
> [email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 38
> ++++++++++++++++++++-
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 ++-
> 3 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index c13359eeb3cd..539b526a6b0a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -626,8 +626,11 @@ int mtk_ddp_comp_init(struct device_node *node,
> struct mtk_ddp_comp *comp,
>
> priv->regs = of_iomap(node, 0);
> priv->clk = of_clk_get(node, 0);
> - if (IS_ERR(priv->clk))
> + if (IS_ERR(priv->clk)) {
> + iounmap(priv->regs);
> + priv->regs = NULL;
> return PTR_ERR(priv->clk);
> + }
>
> #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> ret = cmdq_dev_get_client_reg(comp->dev, &priv->cmdq_reg, 0);
> @@ -639,3 +642,36 @@ int mtk_ddp_comp_init(struct device_node *node,
> struct mtk_ddp_comp *comp,
>
> return 0;
> }
> +
> +void mtk_ddp_comp_destroy(struct mtk_ddp_comp *comp)
> +{
> + struct mtk_ddp_comp_dev *priv;
> +
> + if (!comp || !comp->dev)
> + return;
> +
> + /* Complex components are destroyed with their own remove
> callback */
> + if (type == MTK_DISP_AAL ||
> + type == MTK_DISP_BLS ||
> + type == MTK_DISP_CCORR ||
> + type == MTK_DISP_COLOR ||
> + type == MTK_DISP_GAMMA ||
> + type == MTK_DISP_MERGE ||
> + type == MTK_DISP_OVL ||
> + type == MTK_DISP_OVL_2L ||
> + type == MTK_DISP_PWM ||
> + type == MTK_DISP_RDMA ||
> + type == MTK_DPI ||
> + type == MTK_DP_INTF ||
> + type == MTK_DSI)
> + return;
> +
> + priv = dev_get_drvdata(comp->dev);
> + if (!priv)
> + return;
> +
> + if (priv->regs) {
> + iounmap(priv->regs);
> + priv->regs = NULL;
> + }
> +}
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 15b2eafff438..43372b416a3f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -318,6 +318,7 @@ unsigned int
> mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
> struct device *dev);
> int mtk_ddp_comp_init(struct device_node *comp_node, struct
> mtk_ddp_comp *comp,
> unsigned int comp_id);
> +void mtk_ddp_comp_destroy(struct mtk_ddp_comp *comp);
> enum mtk_ddp_comp_type mtk_ddp_comp_get_type(unsigned int comp_id);
> void mtk_ddp_write(struct cmdq_pkt *cmdq_pkt, unsigned int value,
> struct cmdq_client_reg *cmdq_reg, void __iomem
> *regs,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 14a1e0157cc4..89b6c31a1511 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -944,8 +944,10 @@ static void mtk_drm_remove(struct
> platform_device *pdev)
> component_master_del(&pdev->dev, &mtk_drm_ops);
> pm_runtime_disable(&pdev->dev);
> of_node_put(private->mutex_node);
> - for (i = 0; i < DDP_COMPONENT_DRM_ID_MAX; i++)
> + for (i = 0; i < DDP_COMPONENT_DRM_ID_MAX; i++) {
> + mtk_ddp_comp_destroy(&private->ddp_comp[i]);
> of_node_put(private->comp_node[i]);
> + }
> }
>
> static int mtk_drm_sys_prepare(struct device *dev)

2024-04-01 14:18:58

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH 2/3] drm/mediatek: Perform iounmap on simple DDP component destruction

Hi, Angelo:

CK Hu (胡俊光) <[email protected]> 於 2024年3月22日 週五 下午4:49寫道:
>
> Hi, Angelo:
>
> On Thu, 2024-02-01 at 13:53 +0100, AngeloGioacchino Del Regno wrote:
> > Add a new mtk_ddp_comp_destroy() function and call it in the teardown
> > path of mtk_drm_drv to make sure that we unmap the iospace of the
> > simple DDP components.
> >
> > While at it, also fix iounmapping on mtk_ddp_comp_init() error path.
>
> Reviewed-by: CK Hu <[email protected]>
>
> >
> > Fixes: ff1395609e20 ("drm/mediatek: Move mtk_ddp_comp_init() from sub
> > driver to DRM driver")
> > Signed-off-by: AngeloGioacchino Del Regno <
> > [email protected]>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 38
> > ++++++++++++++++++++-
> > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
> > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 ++-
> > 3 files changed, 41 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > index c13359eeb3cd..539b526a6b0a 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > @@ -626,8 +626,11 @@ int mtk_ddp_comp_init(struct device_node *node,
> > struct mtk_ddp_comp *comp,
> >
> > priv->regs = of_iomap(node, 0);
> > priv->clk = of_clk_get(node, 0);
> > - if (IS_ERR(priv->clk))
> > + if (IS_ERR(priv->clk)) {
> > + iounmap(priv->regs);
> > + priv->regs = NULL;
> > return PTR_ERR(priv->clk);
> > + }
> >
> > #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > ret = cmdq_dev_get_client_reg(comp->dev, &priv->cmdq_reg, 0);
> > @@ -639,3 +642,36 @@ int mtk_ddp_comp_init(struct device_node *node,
> > struct mtk_ddp_comp *comp,
> >
> > return 0;
> > }
> > +
> > +void mtk_ddp_comp_destroy(struct mtk_ddp_comp *comp)
> > +{
> > + struct mtk_ddp_comp_dev *priv;
> > +
> > + if (!comp || !comp->dev)
> > + return;
> > +
> > + /* Complex components are destroyed with their own remove
> > callback */
> > + if (type == MTK_DISP_AAL ||

Build error:

./drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c: In function
\u2018mtk_ddp_comp_destroy\u2019:
./drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c:657:13: error:
\u2018type\u2019 undeclared (first use in this function); did you mean
\u2018_ctype\u2019?
657 | if (type == MTK_DISP_AAL ||
| ^~~~
| _ctype
./drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c:657:13: note: each
undeclared identifier is reported only once for each function it
appears in

Regards,
Chun-Kuang.

> > + type == MTK_DISP_BLS ||
> > + type == MTK_DISP_CCORR ||
> > + type == MTK_DISP_COLOR ||
> > + type == MTK_DISP_GAMMA ||
> > + type == MTK_DISP_MERGE ||
> > + type == MTK_DISP_OVL ||
> > + type == MTK_DISP_OVL_2L ||
> > + type == MTK_DISP_PWM ||
> > + type == MTK_DISP_RDMA ||
> > + type == MTK_DPI ||
> > + type == MTK_DP_INTF ||
> > + type == MTK_DSI)
> > + return;
> > +
> > + priv = dev_get_drvdata(comp->dev);
> > + if (!priv)
> > + return;
> > +
> > + if (priv->regs) {
> > + iounmap(priv->regs);
> > + priv->regs = NULL;
> > + }
> > +}
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > index 15b2eafff438..43372b416a3f 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > @@ -318,6 +318,7 @@ unsigned int
> > mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
> > struct device *dev);
> > int mtk_ddp_comp_init(struct device_node *comp_node, struct
> > mtk_ddp_comp *comp,
> > unsigned int comp_id);
> > +void mtk_ddp_comp_destroy(struct mtk_ddp_comp *comp);
> > enum mtk_ddp_comp_type mtk_ddp_comp_get_type(unsigned int comp_id);
> > void mtk_ddp_write(struct cmdq_pkt *cmdq_pkt, unsigned int value,
> > struct cmdq_client_reg *cmdq_reg, void __iomem
> > *regs,
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > index 14a1e0157cc4..89b6c31a1511 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > @@ -944,8 +944,10 @@ static void mtk_drm_remove(struct
> > platform_device *pdev)
> > component_master_del(&pdev->dev, &mtk_drm_ops);
> > pm_runtime_disable(&pdev->dev);
> > of_node_put(private->mutex_node);
> > - for (i = 0; i < DDP_COMPONENT_DRM_ID_MAX; i++)
> > + for (i = 0; i < DDP_COMPONENT_DRM_ID_MAX; i++) {
> > + mtk_ddp_comp_destroy(&private->ddp_comp[i]);
> > of_node_put(private->comp_node[i]);
> > + }
> > }
> >
> > static int mtk_drm_sys_prepare(struct device *dev)