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
