Re: [PATCH v1 4/4] drm/msm/mdp5: move resource allocation to the _probe function

[Abhinav Kumar]

On 6/20/2022 2:30 PM, Dmitry Baryshkov wrote:

To let the probe function bail early if any of the resources is
unavailable, move resource allocattion from kms_init directly to the
probe callback.

Seems to be the common typo in all the patches of this series

allocattion -> allocation

Apart from that, this one LGTM,

Reviewed-by: Abhinav Kumar <quic_abhin...@quicinc.com>

Signed-off-by: Dmitry Baryshkov <dmitry.barysh...@linaro.org>
---
  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 97 +++++++++++-------------
  1 file changed, 45 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index daf5b5ca7233..015388f262f4 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -556,17 +556,18 @@ static int mdp5_kms_init(struct drm_device *dev)
        struct mdp5_cfg *config;
        struct msm_kms *kms;
        struct msm_gem_address_space *aspace;
-       int irq, i, ret;
+       int i, ret;
        struct device *iommu_dev;

- ret = mdp5_init(to_platform_device(dev->dev), dev);

-
        /* priv->kms would have been populated by the MDP5 driver */
        kms = priv->kms;
        if (!kms)
                return -ENOMEM;

mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));

+
+       ret = mdp5_init(to_platform_device(dev->dev), dev);
+
        pdev = mdp5_kms->pdev;

ret = mdp_kms_init(&mdp5_kms->base, &kms_funcs);

@@ -575,15 +576,6 @@ static int mdp5_kms_init(struct drm_device *dev)
                goto fail;
        }

- irq = irq_of_parse_and_map(pdev->dev.of_node, 0);

-       if (!irq) {
-               ret = -EINVAL;
-               DRM_DEV_ERROR(&pdev->dev, "failed to get irq\n");
-               goto fail;
-       }
-
-       kms->irq = irq;
-
        config = mdp5_cfg_get_config(mdp5_kms->cfg);

/* make sure things are off before attaching iommu (bootloader could

@@ -804,51 +796,17 @@ static int interface_init(struct mdp5_kms *mdp5_kms)
  static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
  {
        struct msm_drm_private *priv = dev->dev_private;
-       struct mdp5_kms *mdp5_kms;
+       struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
        struct mdp5_cfg *config;
        u32 major, minor;
        int ret;

- mdp5_kms = devm_kzalloc(&pdev->dev, sizeof(*mdp5_kms), GFP_KERNEL);

-       if (!mdp5_kms) {
-               ret = -ENOMEM;
-               goto fail;
-       }
-
-       spin_lock_init(&mdp5_kms->resource_lock);
-
        mdp5_kms->dev = dev;
-       mdp5_kms->pdev = pdev;

ret = mdp5_global_obj_init(mdp5_kms);

        if (ret)
                goto fail;

- mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys");

-       if (IS_ERR(mdp5_kms->mmio)) {
-               ret = PTR_ERR(mdp5_kms->mmio);
-               goto fail;
-       }
-
-       /* mandatory clocks: */
-       ret = get_clk(pdev, &mdp5_kms->axi_clk, "bus", true);
-       if (ret)
-               goto fail;
-       ret = get_clk(pdev, &mdp5_kms->ahb_clk, "iface", true);
-       if (ret)
-               goto fail;
-       ret = get_clk(pdev, &mdp5_kms->core_clk, "core", true);
-       if (ret)
-               goto fail;
-       ret = get_clk(pdev, &mdp5_kms->vsync_clk, "vsync", true);
-       if (ret)
-               goto fail;
-
-       /* optional clocks: */
-       get_clk(pdev, &mdp5_kms->lut_clk, "lut", false);
-       get_clk(pdev, &mdp5_kms->tbu_clk, "tbu", false);
-       get_clk(pdev, &mdp5_kms->tbu_rt_clk, "tbu_rt", false);
-
        /* we need to set a default rate before enabling.  Set a safe
         * rate first, then figure out hw revision, and then set a
         * more optimal rate:
@@ -906,9 +864,6 @@ static int mdp5_init(struct platform_device *pdev, struct 
drm_device *dev)
        if (ret)
                goto fail;

- /* set uninit-ed kms */

-       priv->kms = &mdp5_kms->base.base;
-
        return 0;
  fail:
        if (mdp5_kms)
@@ -951,15 +906,53 @@ static int mdp5_setup_interconnect(struct platform_device 
*pdev)

static int mdp5_dev_probe(struct platform_device *pdev)

  {
-       int ret;
+       struct mdp5_kms *mdp5_kms;
+       int ret, irq;

DBG(""); + mdp5_kms = devm_kzalloc(&pdev->dev, sizeof(*mdp5_kms), GFP_KERNEL);

+       if (!mdp5_kms)
+               return -ENOMEM;
+
        ret = mdp5_setup_interconnect(pdev);
        if (ret)
                return ret;

- return msm_drv_probe(&pdev->dev, mdp5_kms_init, NULL);

+       mdp5_kms->pdev = pdev;
+
+       spin_lock_init(&mdp5_kms->resource_lock);
+
+       mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys");
+       if (IS_ERR(mdp5_kms->mmio))
+               return PTR_ERR(mdp5_kms->mmio);
+
+       /* mandatory clocks: */
+       ret = get_clk(pdev, &mdp5_kms->axi_clk, "bus", true);
+       if (ret)
+               return ret;
+       ret = get_clk(pdev, &mdp5_kms->ahb_clk, "iface", true);
+       if (ret)
+               return ret;
+       ret = get_clk(pdev, &mdp5_kms->core_clk, "core", true);
+       if (ret)
+               return ret;
+       ret = get_clk(pdev, &mdp5_kms->vsync_clk, "vsync", true);
+       if (ret)
+               return ret;
+
+       /* optional clocks: */
+       get_clk(pdev, &mdp5_kms->lut_clk, "lut", false);
+       get_clk(pdev, &mdp5_kms->tbu_clk, "tbu", false);
+       get_clk(pdev, &mdp5_kms->tbu_rt_clk, "tbu_rt", false);
+
+       irq = platform_get_irq(pdev, 0);
+       if (irq < 0)
+               return dev_err_probe(&pdev->dev, irq, "failed to get irq\n");
+
+       mdp5_kms->base.base.irq = irq;
+
+       return msm_drv_probe(&pdev->dev, mdp5_kms_init, &mdp5_kms->base.base);
  }

static int mdp5_dev_remove(struct platform_device *pdev)