From patchwork Fri Nov 2 18:25:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jordan Crouse X-Patchwork-Id: 10666003 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 89A3915E9 for ; Fri, 2 Nov 2018 18:25:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 743962B3FA for ; Fri, 2 Nov 2018 18:25:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 67CCE2C58E; Fri, 2 Nov 2018 18:25:37 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id EEF2C2B3FA for ; Fri, 2 Nov 2018 18:25:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C677E6E5BE; Fri, 2 Nov 2018 18:25:35 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) by gabe.freedesktop.org (Postfix) with ESMTPS id 542E46E5BD; Fri, 2 Nov 2018 18:25:34 +0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 3E53560795; Fri, 2 Nov 2018 18:25:33 +0000 (UTC) Received: from jcrouse-lnx.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: jcrouse@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 1D3006034F; Fri, 2 Nov 2018 18:25:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 1D3006034F From: Jordan Crouse To: freedreno@lists.freedesktop.org Subject: [PATCH v2] drm/msm/dpu: Don't use devm for component devices Date: Fri, 2 Nov 2018 12:25:29 -0600 Message-Id: <20181102182529.2501-1-jcrouse@codeaurora.org> X-Mailer: git-send-email 2.18.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: jshekhar@codeaurora.org, linux-arm-msm@vger.kernel.org, seanpaul@chromium.org, abhinavk@codeaurora.org, dri-devel@lists.freedesktop.org MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 compoent bind fails due to an -EPROBE_DEFER. In this case the bind would try again later and any devm managed meory allocated during the former aborted attempt would be leaked until the device itself was destroyed. Therefore, all the memory allocated during a bind should be freed during an unbind (or bind error case) and 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 more generic condition that can easily be addressed as a cleanup later and the unbind code already does the requisite devm calls to unmap it. v2: free mp->clk_config on failure Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c | 6 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 ++++++---- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 8 +++++--- 4 files changed, 16 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..14fecf00e032 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; @@ -201,5 +199,7 @@ int msm_dss_parse_clock(struct platform_device *pdev, err: msm_dss_put_clk(mp->clk_config, num_clk); + kfree(mp->clk_config); + mp->clk_config = NULL; return rc; } 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..c82347afc967 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; @@ -238,8 +240,8 @@ int dpu_mdss_init(struct drm_device *dev) _dpu_mdss_irq_domain_fini(dpu_mdss); irq_domain_error: msm_dss_put_clk(mp->clk_config, mp->num_clk); + kfree(mp->clk_config); clk_parse_err: - devm_kfree(&pdev->dev, mp->clk_config); if (dpu_mdss->mmio) devm_iounmap(&pdev->dev, dpu_mdss->mmio); dpu_mdss->mmio = NULL;