Re: [PATCH v3 6/6] drm/msm: make mdp5/dpu devices master components

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/03/2022 00:37, Stephen Boyd wrote:
Quoting Dmitry Baryshkov (2022-03-23 02:25:38)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 38627ccf3068..ab8a35e09bc9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -381,8 +381,8 @@ static int dpu_kms_parse_data_bus_icc_path(struct dpu_kms *dpu_kms)
         struct icc_path *path1;
         struct drm_device *dev = dpu_kms->dev;

-       path0 = of_icc_get(dev->dev, "mdp0-mem");
-       path1 = of_icc_get(dev->dev, "mdp1-mem");
+       path0 = of_icc_get(dev->dev->parent, "mdp0-mem");

dev->dev->parent is long

+       path1 = of_icc_get(dev->dev->parent, "mdp1-mem");

         if (IS_ERR_OR_NULL(path0))
                 return PTR_ERR_OR_ZERO(path0);
@@ -837,6 +837,9 @@ static void dpu_kms_destroy(struct msm_kms *kms)
         _dpu_kms_hw_destroy(dpu_kms);

         msm_kms_destroy(&dpu_kms->base);
+
+       if (dpu_kms->rpm_enabled)
+               pm_runtime_disable(&dpu_kms->pdev->dev);
  }

  static irqreturn_t dpu_irq(struct msm_kms *kms)
@@ -978,7 +981,7 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
         if (!domain)
                 return 0;

-       mmu = msm_iommu_new(dpu_kms->dev->dev, domain);
+       mmu = msm_iommu_new(dpu_kms->dev->dev->parent, domain);

And dpu_kms->dev->dev->parent is longer. Can we get some local variable
or something that is more descriptive? I guess it is an 'mdss_dev'?

Yes, I'll fix these two usages.


         if (IS_ERR(mmu)) {
                 iommu_domain_free(domain);
                 return PTR_ERR(mmu);
@@ -1172,40 +1175,15 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
         return rc;
  }

-static int dpu_kms_init(struct drm_device *dev)
-{
-       struct msm_drm_private *priv;
-       struct dpu_kms *dpu_kms;
-       int irq;
-
-       if (!dev) {
-               DPU_ERROR("drm device node invalid\n");
-               return -EINVAL;
-       }
-
-       priv = dev->dev_private;
-       dpu_kms = to_dpu_kms(priv->kms);
-
-       irq = irq_of_parse_and_map(dpu_kms->pdev->dev.of_node, 0);
-       if (irq < 0) {
-               DPU_ERROR("failed to get irq: %d\n", irq);
-               return irq;
-       }
-       dpu_kms->base.irq = irq;
-
-       return 0;
-}
-
-static int dpu_bind(struct device *dev, struct device *master, void *data)
+static int dpu_kms_init(struct drm_device *ddev)
  {
-       struct msm_drm_private *priv = dev_get_drvdata(master);
+       struct msm_drm_private *priv = ddev->dev_private;
+       struct device *dev = ddev->dev;
         struct platform_device *pdev = to_platform_device(dev);
-       struct drm_device *ddev = priv->dev;
         struct dpu_kms *dpu_kms;
+       int irq;
         int ret = 0;

-       priv->kms_init = dpu_kms_init;
-
         dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL);
         if (!dpu_kms)
                 return -ENOMEM;
@@ -1227,8 +1205,6 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
         }
         dpu_kms->num_clocks = ret;

-       platform_set_drvdata(pdev, dpu_kms);
-
         ret = msm_kms_init(&dpu_kms->base, &kms_funcs);
         if (ret) {
                 DPU_ERROR("failed to init kms, ret=%d\n", ret);
@@ -1242,31 +1218,25 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)

         priv->kms = &dpu_kms->base;

-       return ret;
-}
-
-static void dpu_unbind(struct device *dev, struct device *master, void *data)
-{
-       struct platform_device *pdev = to_platform_device(dev);
-       struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
+       irq = irq_of_parse_and_map(dpu_kms->pdev->dev.of_node, 0);

Why doesn't platform_get_irq() work? This is code movement but I'm
trying to understand why OF APIs are required.

Good question, I'll take a look separately (in a followup patch).


+       if (irq < 0) {
+               DPU_ERROR("failed to get irq: %d\n", irq);
+               return irq;
+       }
+       dpu_kms->base.irq = irq;

-       if (dpu_kms->rpm_enabled)
-               pm_runtime_disable(&pdev->dev);
+       return 0;
  }

diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 1f571372e928..ab25fff271f9 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -194,9 +194,6 @@ static inline void msm_kms_destroy(struct msm_kms *kms)
                 msm_atomic_destroy_pending_timer(&kms->pending_timers[i]);
  }

-extern const struct of_device_id dpu_dt_match[];
-extern const struct of_device_id mdp5_dt_match[];
-
  #define for_each_crtc_mask(dev, crtc, crtc_mask) \
         drm_for_each_crtc(crtc, dev) \
                 for_each_if (drm_crtc_mask(crtc) & (crtc_mask))
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 7451105cbf01..9ecae833037d 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -329,14 +310,7 @@ static int mdss_probe(struct platform_device *pdev)
         if (IS_ERR(mdss))
                 return PTR_ERR(mdss);

-       priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-       if (!priv) {
-               ret = -ENOMEM;
-               goto fail;
-       }
-
-       priv->mdss = mdss;
-       platform_set_drvdata(pdev, priv);
+       platform_set_drvdata(pdev, mdss);

         /*
          * MDP5/DPU based devices don't have a flat hierarchy. There is a top
@@ -350,39 +324,18 @@ static int mdss_probe(struct platform_device *pdev)
                 goto fail;

Can the goto fail be removed? And replaced with

Ack, I'll do this.


	if (ret)
		msm_mdss_destroy(mdss)

	return ret;

         }

-       mdp_dev = device_find_child(dev, NULL, find_mdp_node);
-       if (!mdp_dev) {
-               DRM_DEV_ERROR(dev, "failed to find MDSS MDP node\n");
-               of_platform_depopulate(dev);
-               ret = -ENODEV;
-               goto fail;
-       }
-
-       /*
-        * on MDP5 based platforms, the MDSS platform device is the component
-        * that adds MDP5 and other display interface components to
-        * itself.
-        */
-       ret = msm_drv_probe(dev, mdp_dev);
-       put_device(mdp_dev);
-       if (ret)
-               goto fail;
-

I see a lot of removal of 'goto fail'.

         return 0;

  fail:
-       of_platform_depopulate(dev);
-       msm_mdss_destroy(priv->mdss);
+       msm_mdss_destroy(mdss);

         return ret;
  }



--
With best wishes
Dmitry



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux