Re: [Freedreno] [DPU PATCH 04/11] drm/msm/dpu: create new platform driver for dpu device
On Thu, May 10, 2018 at 01:59:38PM +0530, Rajesh Yadav wrote: > Current MSM display controller HW matches a tree like > hierarchy where MDSS top level wrapper is parent device > and mdp5/dpu, dsi, dp are child devices. > > Each child device like mdp5, dsi etc. have a separate driver, > but currently dpu handling is tied to a single driver which > was managing both mdss and dpu resources. > > Inorder to have the cleaner one to one device and driver > association, this change adds a new platform_driver for dpu > child device node which implements the kms functionality. > > The dpu driver implements runtime_pm support for managing clocks > and bus bandwidth etc. > > Signed-off-by: Rajesh Yadav> --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 251 > ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 + > drivers/gpu/drm/msm/msm_drv.c | 2 + > drivers/gpu/drm/msm/msm_drv.h | 3 + > 4 files changed, 214 insertions(+), 46 deletions(-) > if (!kms) { > @@ -1565,34 +1548,28 @@ static int dpu_kms_hw_init(struct msm_kms *kms) > goto end; > } > > - platformdev = to_platform_device(dev->dev); > - if (!platformdev) { > - DPU_ERROR("invalid platform device\n"); > - goto end; > - } > - > priv = dev->dev_private; > if (!priv) { > DPU_ERROR("invalid private data\n"); > goto end; > } > > - dpu_kms->mmio = msm_ioremap(platformdev, "mdp_phys", "mdp_phys"); > + dpu_kms->mmio = msm_ioremap(dpu_kms->pdev, "mdp_phys", "mdp_phys"); > if (IS_ERR(dpu_kms->mmio)) { > rc = PTR_ERR(dpu_kms->mmio); > DPU_ERROR("mdp register memory map failed: %d\n", rc); > dpu_kms->mmio = NULL; > goto error; > } > - DRM_INFO("mapped mdp address space @%p\n", dpu_kms->mmio); > - dpu_kms->mmio_len = msm_iomap_size(platformdev, "mdp_phys"); > + DRM_INFO("mapped dpu address space @%p\n", dpu_kms->mmio); This is not a useful message - move to debug or remove. In any event, please don't use %p. > + dpu_kms->mmio_len = msm_iomap_size(dpu_kms->pdev, "mdp_phys"); > rc = dpu_dbg_reg_register_base(DPU_DBG_NAME, dpu_kms->mmio, > dpu_kms->mmio_len); > if (rc) > DPU_ERROR("dbg base register kms failed: %d\n", rc); > > - dpu_kms->vbif[VBIF_RT] = msm_ioremap(platformdev, "vbif_phys", > + dpu_kms->vbif[VBIF_RT] = msm_ioremap(dpu_kms->pdev, "vbif_phys", > "vbif_phys"); > if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { > rc = PTR_ERR(dpu_kms->vbif[VBIF_RT]); > @@ -1600,20 +1577,20 @@ static int dpu_kms_hw_init(struct msm_kms *kms) > dpu_kms->vbif[VBIF_RT] = NULL; > goto error; > } > - dpu_kms->vbif_len[VBIF_RT] = msm_iomap_size(platformdev, > + dpu_kms->vbif_len[VBIF_RT] = msm_iomap_size(dpu_kms->pdev, > "vbif_phys"); > rc = dpu_dbg_reg_register_base("vbif_rt", dpu_kms->vbif[VBIF_RT], > dpu_kms->vbif_len[VBIF_RT]); > if (rc) > DPU_ERROR("dbg base register vbif_rt failed: %d\n", rc); > > - dpu_kms->vbif[VBIF_NRT] = msm_ioremap(platformdev, "vbif_nrt_phys", > + dpu_kms->vbif[VBIF_NRT] = msm_ioremap(dpu_kms->pdev, "vbif_nrt_phys", > > "vbif_nrt_phys"); > if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { > dpu_kms->vbif[VBIF_NRT] = NULL; > DPU_DEBUG("VBIF NRT is not defined"); > } else { > - dpu_kms->vbif_len[VBIF_NRT] = msm_iomap_size(platformdev, > + dpu_kms->vbif_len[VBIF_NRT] = msm_iomap_size(dpu_kms->pdev, > "vbif_nrt_phys"); > rc = dpu_dbg_reg_register_base("vbif_nrt", > dpu_kms->vbif[VBIF_NRT], > @@ -1624,13 +1601,13 @@ static int dpu_kms_hw_init(struct msm_kms *kms) > } > > #ifdef CONFIG_CHROME_REGDMA > - dpu_kms->reg_dma = msm_ioremap(platformdev, "regdma_phys", > + dpu_kms->reg_dma = msm_ioremap(dpu_kms->pdev, "regdma_phys", > "regdma_phys"); > if (IS_ERR(dpu_kms->reg_dma)) { > dpu_kms->reg_dma = NULL; > DPU_DEBUG("REG_DMA is not defined"); > } else { > - dpu_kms->reg_dma_len = msm_iomap_size(platformdev, > + dpu_kms->reg_dma_len = msm_iomap_size(dpu_kms->pdev, > "regdma_phys"); > rc = dpu_dbg_reg_register_base("reg_dma", > dpu_kms->reg_dma, > @@ -1804,14 +1781,13 @@ static int dpu_kms_hw_init(struct msm_kms *kms) >
Re: [DPU PATCH 04/11] drm/msm/dpu: create new platform driver for dpu device
On Thu, May 10, 2018 at 01:59:38PM +0530, Rajesh Yadav wrote: > Current MSM display controller HW matches a tree like > hierarchy where MDSS top level wrapper is parent device > and mdp5/dpu, dsi, dp are child devices. > > Each child device like mdp5, dsi etc. have a separate driver, > but currently dpu handling is tied to a single driver which > was managing both mdss and dpu resources. > > Inorder to have the cleaner one to one device and driver > association, this change adds a new platform_driver for dpu > child device node which implements the kms functionality. > > The dpu driver implements runtime_pm support for managing clocks > and bus bandwidth etc. > > Signed-off-by: Rajesh Yadav> --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 251 > ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 + > drivers/gpu/drm/msm/msm_drv.c | 2 + > drivers/gpu/drm/msm/msm_drv.h | 3 + > 4 files changed, 214 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index e4ab753..2cd51fc 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -1030,14 +1030,13 @@ static long dpu_kms_round_pixclk(struct msm_kms *kms, > unsigned long rate, > return rate; > } > > -static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms, > - struct platform_device *pdev) > +static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms) > { > struct drm_device *dev; > struct msm_drm_private *priv; > int i; > > - if (!dpu_kms || !pdev) > + if (!dpu_kms) This isn't possible, please remove. > return; > > dev = dpu_kms->dev; > @@ -1091,15 +1090,15 @@ static void _dpu_kms_hw_destroy(struct dpu_kms > *dpu_kms, > dpu_kms->core_client = NULL; > > if (dpu_kms->vbif[VBIF_NRT]) > - msm_iounmap(pdev, dpu_kms->vbif[VBIF_NRT]); > + msm_iounmap(dpu_kms->pdev, dpu_kms->vbif[VBIF_NRT]); > dpu_kms->vbif[VBIF_NRT] = NULL; > > if (dpu_kms->vbif[VBIF_RT]) > - msm_iounmap(pdev, dpu_kms->vbif[VBIF_RT]); > + msm_iounmap(dpu_kms->pdev, dpu_kms->vbif[VBIF_RT]); > dpu_kms->vbif[VBIF_RT] = NULL; > > if (dpu_kms->mmio) > - msm_iounmap(pdev, dpu_kms->mmio); > + msm_iounmap(dpu_kms->pdev, dpu_kms->mmio); > dpu_kms->mmio = NULL; > > dpu_reg_dma_deinit(); > +static void dpu_destroy(struct platform_device *pdev) > +{ > + struct dpu_kms *dpu_kms = platform_get_drvdata(pdev); > + struct dss_module_power *mp = _kms->mp; > + > + msm_dss_put_clk(mp->clk_config, mp->num_clk); > + devm_kfree(>dev, mp->clk_config); > + mp->num_clk = 0; > + > + if (dpu_kms->rpm_enabled) > + pm_runtime_disable(>dev); > + > + devm_kfree(>dev, dpu_kms); devm_kfrees are not necessary > +} > > - dpu_kms = kzalloc(sizeof(*dpu_kms), GFP_KERNEL); > +static int dpu_init(struct platform_device *pdev, struct drm_device *dev) > +{ > + struct msm_drm_private *priv = dev->dev_private; > + struct dpu_kms *dpu_kms; > + struct dss_module_power *mp; > + int ret = 0; > + > + dpu_kms = devm_kzalloc(>dev, sizeof(*dpu_kms), GFP_KERNEL); > if (!dpu_kms) { > DPU_ERROR("failed to allocate dpu kms\n"); > - return ERR_PTR(-ENOMEM); > + return -ENOMEM; > + } > + > + mp = _kms->mp; > + ret = msm_dss_parse_clock(pdev, mp); > + if (ret) { > + DPU_ERROR("failed to parse clocks, ret=%d\n", ret); > + goto clk_parse_error; > + } > + > + ret = msm_dss_get_clk(>dev, mp->clk_config, mp->num_clk); > + if (ret) { > + pr_err("failed to get clocks, ret=%d\n", ret); > + goto clk_get_error; > + } > + > + ret = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk); > + if (ret) { > + pr_err("failed to set clock rate, ret=%d\n", ret); > + goto clk_rate_error; > } > > + platform_set_drvdata(pdev, dpu_kms); > + > msm_kms_init(_kms->base, _funcs); > dpu_kms->dev = dev; > - dpu_kms->base.irq = irq; > + dpu_kms->pdev = pdev; > > - return _kms->base; > + pm_runtime_enable(>dev); > + dpu_kms->rpm_enabled = true; > + > + priv->kms = _kms->base; > + > + return ret; > + > +clk_rate_error: > + msm_dss_put_clk(mp->clk_config, mp->num_clk); > +clk_get_error: > + devm_kfree(>dev, mp->clk_config); > + mp->num_clk = 0; > +clk_parse_error: > + devm_kfree(>dev, dpu_kms); > + > + return ret; > +} > + > +static int dpu_bind(struct device *dev, struct device *master, void *data) > +{ > + struct drm_device *ddev = dev_get_drvdata(master); > + struct platform_device *pdev = to_platform_device(dev); > + > + return dpu_init(pdev, ddev); No need to have
[DPU PATCH 04/11] drm/msm/dpu: create new platform driver for dpu device
Current MSM display controller HW matches a tree like hierarchy where MDSS top level wrapper is parent device and mdp5/dpu, dsi, dp are child devices. Each child device like mdp5, dsi etc. have a separate driver, but currently dpu handling is tied to a single driver which was managing both mdss and dpu resources. Inorder to have the cleaner one to one device and driver association, this change adds a new platform_driver for dpu child device node which implements the kms functionality. The dpu driver implements runtime_pm support for managing clocks and bus bandwidth etc. Signed-off-by: Rajesh Yadav--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 251 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 + drivers/gpu/drm/msm/msm_drv.c | 2 + drivers/gpu/drm/msm/msm_drv.h | 3 + 4 files changed, 214 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index e4ab753..2cd51fc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1030,14 +1030,13 @@ static long dpu_kms_round_pixclk(struct msm_kms *kms, unsigned long rate, return rate; } -static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms, - struct platform_device *pdev) +static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms) { struct drm_device *dev; struct msm_drm_private *priv; int i; - if (!dpu_kms || !pdev) + if (!dpu_kms) return; dev = dpu_kms->dev; @@ -1091,15 +1090,15 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms, dpu_kms->core_client = NULL; if (dpu_kms->vbif[VBIF_NRT]) - msm_iounmap(pdev, dpu_kms->vbif[VBIF_NRT]); + msm_iounmap(dpu_kms->pdev, dpu_kms->vbif[VBIF_NRT]); dpu_kms->vbif[VBIF_NRT] = NULL; if (dpu_kms->vbif[VBIF_RT]) - msm_iounmap(pdev, dpu_kms->vbif[VBIF_RT]); + msm_iounmap(dpu_kms->pdev, dpu_kms->vbif[VBIF_RT]); dpu_kms->vbif[VBIF_RT] = NULL; if (dpu_kms->mmio) - msm_iounmap(pdev, dpu_kms->mmio); + msm_iounmap(dpu_kms->pdev, dpu_kms->mmio); dpu_kms->mmio = NULL; dpu_reg_dma_deinit(); @@ -1172,8 +1171,6 @@ int dpu_kms_mmu_attach(struct dpu_kms *dpu_kms, bool secure_only) static void dpu_kms_destroy(struct msm_kms *kms) { struct dpu_kms *dpu_kms; - struct drm_device *dev; - struct platform_device *platformdev; if (!kms) { DPU_ERROR("invalid kms\n"); @@ -1181,20 +1178,7 @@ static void dpu_kms_destroy(struct msm_kms *kms) } dpu_kms = to_dpu_kms(kms); - dev = dpu_kms->dev; - if (!dev) { - DPU_ERROR("invalid device\n"); - return; - } - - platformdev = to_platform_device(dev->dev); - if (!platformdev) { - DPU_ERROR("invalid platform device\n"); - return; - } - - _dpu_kms_hw_destroy(dpu_kms, platformdev); - kfree(dpu_kms); + _dpu_kms_hw_destroy(dpu_kms); } static void dpu_kms_preclose(struct msm_kms *kms, struct drm_file *file) @@ -1550,7 +1534,6 @@ static int dpu_kms_hw_init(struct msm_kms *kms) struct dpu_kms *dpu_kms; struct drm_device *dev; struct msm_drm_private *priv; - struct platform_device *platformdev; int i, rc = -EINVAL; if (!kms) { @@ -1565,34 +1548,28 @@ static int dpu_kms_hw_init(struct msm_kms *kms) goto end; } - platformdev = to_platform_device(dev->dev); - if (!platformdev) { - DPU_ERROR("invalid platform device\n"); - goto end; - } - priv = dev->dev_private; if (!priv) { DPU_ERROR("invalid private data\n"); goto end; } - dpu_kms->mmio = msm_ioremap(platformdev, "mdp_phys", "mdp_phys"); + dpu_kms->mmio = msm_ioremap(dpu_kms->pdev, "mdp_phys", "mdp_phys"); if (IS_ERR(dpu_kms->mmio)) { rc = PTR_ERR(dpu_kms->mmio); DPU_ERROR("mdp register memory map failed: %d\n", rc); dpu_kms->mmio = NULL; goto error; } - DRM_INFO("mapped mdp address space @%p\n", dpu_kms->mmio); - dpu_kms->mmio_len = msm_iomap_size(platformdev, "mdp_phys"); + DRM_INFO("mapped dpu address space @%p\n", dpu_kms->mmio); + dpu_kms->mmio_len = msm_iomap_size(dpu_kms->pdev, "mdp_phys"); rc = dpu_dbg_reg_register_base(DPU_DBG_NAME, dpu_kms->mmio, dpu_kms->mmio_len); if (rc) DPU_ERROR("dbg base register kms failed: %d\n", rc); - dpu_kms->vbif[VBIF_RT] = msm_ioremap(platformdev, "vbif_phys", + dpu_kms->vbif[VBIF_RT] = msm_ioremap(dpu_kms->pdev, "vbif_phys",