Changes in v2:
- Fixed patch [2/3]
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.44.0
Finding a possible CRTC by DDP component is done by first checking
static routes in three paths (main, external, third/extra path) and
then, if not found, we check for dynamic connection on a per-route
basis because, for example, on some SoCs the main route may output
to either a DSI display or DisplayPort and this is finally done by
assigning a CRTC mask to `possible_crtcs`, found with function
mtk_drm_find_comp_in_ddp_conn_path(): being that a mask the possible
values are BIT(x) and, if no CRTC is possible, zero.
Problem is, both mtk_drm_find_possible_crtc_by_comp() and the
aforementioned function are trying to return a negative error value
(but it's unsigned int, so the value is never negative!) if no CRTC
was found, which is wrong for multiple obvious reasons.
Cleanup both functions, so that:
- mtk_drm_find_comp_in_ddp_conn_path() returns a signed integer
with a negative number for error, or a bit/bitmask of the found
possible CRTC; and
- mtk_drm_find_possible_crtc_by_comp() always returns either a
bitmask of the possible CRTC, or zero if none available.
Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 44 ++++++++++-----------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index a515e96cfefc..82d7a6887f44 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -514,29 +514,25 @@ static bool mtk_drm_find_comp_in_ddp(struct device *dev,
return false;
}
-static unsigned int mtk_drm_find_comp_in_ddp_conn_path(struct device *dev,
- const struct mtk_drm_route *routes,
- unsigned int num_routes,
- struct mtk_ddp_comp *ddp_comp)
+static int mtk_drm_find_comp_in_ddp_conn_path(struct device *dev,
+ const struct mtk_drm_route *routes,
+ unsigned int num_routes,
+ struct mtk_ddp_comp *ddp_comp)
{
- int ret;
- unsigned int i;
+ int i;
- if (!routes) {
- ret = -EINVAL;
- goto err;
+ if (!routes || !num_routes) {
+ DRM_ERROR("No connection routes specified!\n");
+ return -EINVAL;
}
for (i = 0; i < num_routes; i++)
if (dev == ddp_comp[routes[i].route_ddp].dev)
return BIT(routes[i].crtc_id);
- ret = -ENODEV;
-err:
-
- DRM_INFO("Failed to find comp in ddp table, ret = %d\n", ret);
+ DRM_ERROR("Failed to find component in ddp table\n");
- return 0;
+ return -ENODEV;
}
int mtk_ddp_comp_get_id(struct device_node *node,
@@ -558,22 +554,24 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
struct device *dev)
{
struct mtk_drm_private *private = drm->dev_private;
- unsigned int ret = 0;
+ int ret;
if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path, private->data->main_len,
private->ddp_comp))
- ret = BIT(0);
+ return BIT(0);
else if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
private->data->ext_len, private->ddp_comp))
- ret = BIT(1);
+ return BIT(1);
else if (mtk_drm_find_comp_in_ddp(dev, private->data->third_path,
private->data->third_len, private->ddp_comp))
- ret = BIT(2);
- else
- ret = mtk_drm_find_comp_in_ddp_conn_path(dev,
- private->data->conn_routes,
- private->data->num_conn_routes,
- private->ddp_comp);
+ return BIT(2);
+
+ ret = mtk_drm_find_comp_in_ddp_conn_path(dev, private->data->conn_routes,
+ private->data->num_conn_routes,
+ private->ddp_comp);
+ /* No CRTC is available: return a zero mask */
+ if (ret < 0)
+ return 0;
return ret;
}
--
2.44.0
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]>
Reviewed-by: CK Hu <[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 82d7a6887f44..477fc1950a0e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -627,8 +627,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);
@@ -640,3 +643,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 (mtk_ddp_matches[comp->id].type == MTK_DISP_AAL ||
+ mtk_ddp_matches[comp->id].type == MTK_DISP_BLS ||
+ mtk_ddp_matches[comp->id].type == MTK_DISP_CCORR ||
+ mtk_ddp_matches[comp->id].type == MTK_DISP_COLOR ||
+ mtk_ddp_matches[comp->id].type == MTK_DISP_GAMMA ||
+ mtk_ddp_matches[comp->id].type == MTK_DISP_MERGE ||
+ mtk_ddp_matches[comp->id].type == MTK_DISP_OVL ||
+ mtk_ddp_matches[comp->id].type == MTK_DISP_OVL_2L ||
+ mtk_ddp_matches[comp->id].type == MTK_DISP_PWM ||
+ mtk_ddp_matches[comp->id].type == MTK_DISP_RDMA ||
+ mtk_ddp_matches[comp->id].type == MTK_DPI ||
+ mtk_ddp_matches[comp->id].type == MTK_DP_INTF ||
+ mtk_ddp_matches[comp->id].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 93d79a1366e9..3d147217caa4 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -330,6 +330,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 74832c213092..7e240d16d3e5 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -946,8 +946,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.44.0