[PATCH] drm/msm/dpu: Don't use devm for component devices

[Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

Devices that are bound as components should not use devm since
device managed memory is not freed when the component is
unbound.

In particular this is an issue if the component bind fails
due to an -EPROBE_DEFER. In this case the bind would try again
later and any devm managed memory allocated during the former
aborted attempt would be leaked until the device itself was
destroyed. Since all the memory allocated during a bind
should be freed during an unbind (or bind error case) there isn't
any reason to use devm for resources that have a explicit
teardown step.

This doesn't remove devm for all resources - in particular
msm_ioremap() still uses devm_ioremap() but thats a generic
issue that can easily be addressed as a cleanup later and the
unbind code already does the requisite devm calls to unmap it.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  4 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c |  4 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 10 ++++++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c    |  8 +++++---
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 82c55efb500f..287d4c3e58c3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2220,14 +2220,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
 	struct dpu_encoder_virt *dpu_enc = NULL;
 	int rc = 0;
 
-	dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
+	dpu_enc = kzalloc(sizeof(*dpu_enc), GFP_KERNEL);
 	if (!dpu_enc)
 		return ERR_PTR(ENOMEM);
 
 	rc = drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
 			drm_enc_mode, NULL);
 	if (rc) {
-		devm_kfree(dev->dev, dpu_enc);
+		kfree(dpu_enc);
 		return ERR_PTR(rc);
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
index 89ee4b36beff..90b53e9508f2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
@@ -155,9 +155,7 @@ int msm_dss_parse_clock(struct platform_device *pdev,
 		return 0;
 	}
 
-	mp->clk_config = devm_kzalloc(&pdev->dev,
-				      sizeof(struct dss_clk) * num_clk,
-				      GFP_KERNEL);
+	mp->clk_config = kcalloc(num_clk, sizeof(struct dss_clk), GFP_KERNEL);
 	if (!mp->clk_config)
 		return -ENOMEM;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 985c855796ae..5ac3c3f3b08d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1086,13 +1086,14 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
 	struct dss_module_power *mp;
 	int ret = 0;
 
-	dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL);
+	dpu_kms = kzalloc(sizeof(*dpu_kms), GFP_KERNEL);
 	if (!dpu_kms)
 		return -ENOMEM;
 
 	mp = &dpu_kms->mp;
 	ret = msm_dss_parse_clock(pdev, mp);
 	if (ret) {
+		kfree(dpu_kms);
 		DPU_ERROR("failed to parse clocks, ret=%d\n", ret);
 		return ret;
 	}
@@ -1109,7 +1110,7 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
 	dpu_kms->rpm_enabled = true;
 
 	priv->kms = &dpu_kms->base;
-	return ret;
+	return 0;
 }
 
 static void dpu_unbind(struct device *dev, struct device *master, void *data)
@@ -1120,11 +1121,12 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data)
 
 	dpu_power_resource_deinit(pdev, &dpu_kms->phandle);
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
-	devm_kfree(&pdev->dev, mp->clk_config);
-	mp->num_clk = 0;
+	kfree(mp->clk_config);
 
 	if (dpu_kms->rpm_enabled)
 		pm_runtime_disable(&pdev->dev);
+
+	kfree(dpu_kms);
 }
 
 static const struct component_ops dpu_ops = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index 2235ef8129f4..34ab489b1a5b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -161,7 +161,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
 	free_irq(platform_get_irq(pdev, 0), dpu_mdss);
 
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
-	devm_kfree(&pdev->dev, mp->clk_config);
+	kfree(mp->clk_config);
 
 	if (dpu_mdss->mmio)
 		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
@@ -169,6 +169,8 @@ static void dpu_mdss_destroy(struct drm_device *dev)
 
 	pm_runtime_disable(dev->dev);
 	priv->mdss = NULL;
+
+	kfree(dpu_mdss);
 }
 
 static const struct msm_mdss_funcs mdss_funcs = {
@@ -186,7 +188,7 @@ int dpu_mdss_init(struct drm_device *dev)
 	struct dss_module_power *mp;
 	int ret = 0;
 
-	dpu_mdss = devm_kzalloc(dev->dev, sizeof(*dpu_mdss), GFP_KERNEL);
+	dpu_mdss = kzalloc(sizeof(*dpu_mdss), GFP_KERNEL);
 	if (!dpu_mdss)
 		return -ENOMEM;
 
@@ -239,7 +241,7 @@ int dpu_mdss_init(struct drm_device *dev)
 irq_domain_error:
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
 clk_parse_err:
-	devm_kfree(&pdev->dev, mp->clk_config);
+	kfree(mp->clk_config);
 	if (dpu_mdss->mmio)
 		devm_iounmap(&pdev->dev, dpu_mdss->mmio);
 	dpu_mdss->mmio = NULL;
-- 
2.18.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno