| Message ID | 20240106-fd-migrate-mdp5-v3-2-3d2750378063@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm: provide migration path from MDP5 to DPU driver | expand |
On 1/5/2024 3:34 PM, Dmitry Baryshkov wrote: > Existing MDP5 devices have slightly different bindings. The main > register region is called `mdp_phys' instead of `mdp'. Also vbif > register regions are a part of the parent, MDSS device. Add support for > handling this binding differences. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 98 ++++++++++++++++++++++++++------- > drivers/gpu/drm/msm/msm_drv.h | 3 + > drivers/gpu/drm/msm/msm_io_utils.c | 13 +++++ > 3 files changed, 93 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 723cc1d82143..aa9e0ad33ebb 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -1197,6 +1197,78 @@ static int dpu_kms_init(struct drm_device *ddev) > return 0; > } > > +static int dpu_kms_mmap_mdp5(struct dpu_kms *dpu_kms) > +{ > + struct platform_device *pdev = dpu_kms->pdev; > + struct platform_device *mdss_dev; > + int ret; > + > + if (dpu_kms->pdev->dev.bus != &platform_bus_type) > + return -EINVAL; > + !dev_is_platform() perhaps? But I would like to understand this check a bit more. Can you pls explain for which case this check was added? > + mdss_dev = to_platform_device(dpu_kms->pdev->dev.parent); > + > + dpu_kms->mmio = msm_ioremap(pdev, "mdp_phys"); > + if (IS_ERR(dpu_kms->mmio)) { > + ret = PTR_ERR(dpu_kms->mmio); > + DPU_ERROR("mdp register memory map failed: %d\n", ret); > + dpu_kms->mmio = NULL; > + return ret; > + } > + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); > + > + dpu_kms->vbif[VBIF_RT] = msm_ioremap_mdss(mdss_dev, > + dpu_kms->pdev, > + "vbif_phys"); > + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { > + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); > + DPU_ERROR("vbif register memory map failed: %d\n", ret); > + dpu_kms->vbif[VBIF_RT] = NULL; > + return ret; > + } > + > + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_mdss(mdss_dev, > + dpu_kms->pdev, > + "vbif_nrt_phys"); Do you think a "quiet" version would be better? > + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { > + dpu_kms->vbif[VBIF_NRT] = NULL; > + DPU_DEBUG("VBIF NRT is not defined"); > + } > + > + return 0; > +} > + > +static int dpu_kms_mmap_dpu(struct dpu_kms *dpu_kms) > +{ > + struct platform_device *pdev = dpu_kms->pdev; > + int ret; > + > + dpu_kms->mmio = msm_ioremap(pdev, "mdp"); > + if (IS_ERR(dpu_kms->mmio)) { > + ret = PTR_ERR(dpu_kms->mmio); > + DPU_ERROR("mdp register memory map failed: %d\n", ret); > + dpu_kms->mmio = NULL; > + return ret; > + } > + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); > + > + dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif"); > + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { > + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); > + DPU_ERROR("vbif register memory map failed: %d\n", ret); > + dpu_kms->vbif[VBIF_RT] = NULL; > + return ret; > + } > + > + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt"); > + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { > + dpu_kms->vbif[VBIF_NRT] = NULL; > + DPU_DEBUG("VBIF NRT is not defined"); > + } > + > + return 0; > +} > + > static int dpu_dev_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -1230,28 +1302,12 @@ static int dpu_dev_probe(struct platform_device *pdev) > > dpu_kms->base.irq = irq; > > - dpu_kms->mmio = msm_ioremap(pdev, "mdp"); > - if (IS_ERR(dpu_kms->mmio)) { > - ret = PTR_ERR(dpu_kms->mmio); > - DPU_ERROR("mdp register memory map failed: %d\n", ret); > - dpu_kms->mmio = NULL; > - return ret; > - } > - DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); > - > - dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif"); > - if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { > - ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); > - DPU_ERROR("vbif register memory map failed: %d\n", ret); > - dpu_kms->vbif[VBIF_RT] = NULL; > + if (of_device_is_compatible(dpu_kms->pdev->dev.of_node, "qcom,mdp5")) > + ret = dpu_kms_mmap_mdp5(dpu_kms); > + else > + ret = dpu_kms_mmap_dpu(dpu_kms); > + if (ret) > return ret; > - } > - > - dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt"); > - if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { > - dpu_kms->vbif[VBIF_NRT] = NULL; > - DPU_DEBUG("VBIF NRT is not defined"); > - } > > ret = dpu_kms_parse_data_bus_icc_path(dpu_kms); > if (ret) > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h > index 16a7cbc0b7dd..01e783130054 100644 > --- a/drivers/gpu/drm/msm/msm_drv.h > +++ b/drivers/gpu/drm/msm/msm_drv.h > @@ -476,6 +476,9 @@ void __iomem *msm_ioremap(struct platform_device *pdev, const char *name); > void __iomem *msm_ioremap_size(struct platform_device *pdev, const char *name, > phys_addr_t *size); > void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name); > +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev, > + struct platform_device *dev, > + const char *name); > > struct icc_path *msm_icc_get(struct device *dev, const char *name); > > diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c > index 59d2788c4510..afedd61c3e28 100644 > --- a/drivers/gpu/drm/msm/msm_io_utils.c > +++ b/drivers/gpu/drm/msm/msm_io_utils.c > @@ -50,6 +50,19 @@ struct clk *msm_clk_get(struct platform_device *pdev, const char *name) > return clk; > } > > +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev, > + struct platform_device *pdev, > + const char *name) > +{ > + struct resource *res; > + > + res = platform_get_resource_byname(mdss_pdev, IORESOURCE_MEM, name); > + if (!res) > + return ERR_PTR(-EINVAL); > + > + return devm_ioremap_resource(&pdev->dev, res); > +} > + > static void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name, > bool quiet, phys_addr_t *psize) > { >
On Wed, 7 Feb 2024 at 20:48, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 1/5/2024 3:34 PM, Dmitry Baryshkov wrote: > > Existing MDP5 devices have slightly different bindings. The main > > register region is called `mdp_phys' instead of `mdp'. Also vbif > > register regions are a part of the parent, MDSS device. Add support for > > handling this binding differences. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 98 ++++++++++++++++++++++++++------- > > drivers/gpu/drm/msm/msm_drv.h | 3 + > > drivers/gpu/drm/msm/msm_io_utils.c | 13 +++++ > > 3 files changed, 93 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > index 723cc1d82143..aa9e0ad33ebb 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > > @@ -1197,6 +1197,78 @@ static int dpu_kms_init(struct drm_device *ddev) > > return 0; > > } > > > > +static int dpu_kms_mmap_mdp5(struct dpu_kms *dpu_kms) > > +{ > > + struct platform_device *pdev = dpu_kms->pdev; > > + struct platform_device *mdss_dev; > > + int ret; > > + > > + if (dpu_kms->pdev->dev.bus != &platform_bus_type) > > + return -EINVAL; > > + > > !dev_is_platform() perhaps? looks good > > But I would like to understand this check a bit more. Can you pls > explain for which case this check was added? It is necessary to be sure that we can perform to_platform_device() on the next line. > > > + mdss_dev = to_platform_device(dpu_kms->pdev->dev.parent); > > + > > + dpu_kms->mmio = msm_ioremap(pdev, "mdp_phys"); > > + if (IS_ERR(dpu_kms->mmio)) { > > + ret = PTR_ERR(dpu_kms->mmio); > > + DPU_ERROR("mdp register memory map failed: %d\n", ret); > > + dpu_kms->mmio = NULL; > > + return ret; > > + } > > + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); > > + > > + dpu_kms->vbif[VBIF_RT] = msm_ioremap_mdss(mdss_dev, > > + dpu_kms->pdev, > > + "vbif_phys"); > > + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { > > + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); > > + DPU_ERROR("vbif register memory map failed: %d\n", ret); > > + dpu_kms->vbif[VBIF_RT] = NULL; > > + return ret; > > + } > > + > > + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_mdss(mdss_dev, > > + dpu_kms->pdev, > > + "vbif_nrt_phys"); > > Do you think a "quiet" version would be better? Yep, why not. > > > > + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { > > + dpu_kms->vbif[VBIF_NRT] = NULL; > > + DPU_DEBUG("VBIF NRT is not defined"); > > + } > > + > > + return 0; > > +} > > + > > +static int dpu_kms_mmap_dpu(struct dpu_kms *dpu_kms) > > +{ > > + struct platform_device *pdev = dpu_kms->pdev; > > + int ret; > > + > > + dpu_kms->mmio = msm_ioremap(pdev, "mdp"); > > + if (IS_ERR(dpu_kms->mmio)) { > > + ret = PTR_ERR(dpu_kms->mmio); > > + DPU_ERROR("mdp register memory map failed: %d\n", ret); > > + dpu_kms->mmio = NULL; > > + return ret; > > + } > > + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); > > + > > + dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif"); > > + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { > > + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); > > + DPU_ERROR("vbif register memory map failed: %d\n", ret); > > + dpu_kms->vbif[VBIF_RT] = NULL; > > + return ret; > > + } > > + > > + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt"); > > + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { > > + dpu_kms->vbif[VBIF_NRT] = NULL; > > + DPU_DEBUG("VBIF NRT is not defined"); > > + } > > + > > + return 0; > > +} > > + > > static int dpu_dev_probe(struct platform_device *pdev) > > { > > struct device *dev = &pdev->dev; > > @@ -1230,28 +1302,12 @@ static int dpu_dev_probe(struct platform_device *pdev) > > > > dpu_kms->base.irq = irq; > > > > - dpu_kms->mmio = msm_ioremap(pdev, "mdp"); > > - if (IS_ERR(dpu_kms->mmio)) { > > - ret = PTR_ERR(dpu_kms->mmio); > > - DPU_ERROR("mdp register memory map failed: %d\n", ret); > > - dpu_kms->mmio = NULL; > > - return ret; > > - } > > - DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); > > - > > - dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif"); > > - if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { > > - ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); > > - DPU_ERROR("vbif register memory map failed: %d\n", ret); > > - dpu_kms->vbif[VBIF_RT] = NULL; > > + if (of_device_is_compatible(dpu_kms->pdev->dev.of_node, "qcom,mdp5")) > > + ret = dpu_kms_mmap_mdp5(dpu_kms); > > + else > > + ret = dpu_kms_mmap_dpu(dpu_kms); > > + if (ret) > > return ret; > > - } > > - > > - dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt"); > > - if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { > > - dpu_kms->vbif[VBIF_NRT] = NULL; > > - DPU_DEBUG("VBIF NRT is not defined"); > > - } > > > > ret = dpu_kms_parse_data_bus_icc_path(dpu_kms); > > if (ret) > > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h > > index 16a7cbc0b7dd..01e783130054 100644 > > --- a/drivers/gpu/drm/msm/msm_drv.h > > +++ b/drivers/gpu/drm/msm/msm_drv.h > > @@ -476,6 +476,9 @@ void __iomem *msm_ioremap(struct platform_device *pdev, const char *name); > > void __iomem *msm_ioremap_size(struct platform_device *pdev, const char *name, > > phys_addr_t *size); > > void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name); > > +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev, > > + struct platform_device *dev, > > + const char *name); > > > > struct icc_path *msm_icc_get(struct device *dev, const char *name); > > > > diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c > > index 59d2788c4510..afedd61c3e28 100644 > > --- a/drivers/gpu/drm/msm/msm_io_utils.c > > +++ b/drivers/gpu/drm/msm/msm_io_utils.c > > @@ -50,6 +50,19 @@ struct clk *msm_clk_get(struct platform_device *pdev, const char *name) > > return clk; > > } > > > > +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev, > > + struct platform_device *pdev, > > + const char *name) > > +{ > > + struct resource *res; > > + > > + res = platform_get_resource_byname(mdss_pdev, IORESOURCE_MEM, name); > > + if (!res) > > + return ERR_PTR(-EINVAL); > > + > > + return devm_ioremap_resource(&pdev->dev, res); > > +} > > + > > static void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name, > > bool quiet, phys_addr_t *psize) > > { > >
On 2/7/2024 11:56 AM, Dmitry Baryshkov wrote: > On Wed, 7 Feb 2024 at 20:48, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 1/5/2024 3:34 PM, Dmitry Baryshkov wrote: >>> Existing MDP5 devices have slightly different bindings. The main >>> register region is called `mdp_phys' instead of `mdp'. Also vbif >>> register regions are a part of the parent, MDSS device. Add support for >>> handling this binding differences. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 98 ++++++++++++++++++++++++++------- >>> drivers/gpu/drm/msm/msm_drv.h | 3 + >>> drivers/gpu/drm/msm/msm_io_utils.c | 13 +++++ >>> 3 files changed, 93 insertions(+), 21 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >>> index 723cc1d82143..aa9e0ad33ebb 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >>> @@ -1197,6 +1197,78 @@ static int dpu_kms_init(struct drm_device *ddev) >>> return 0; >>> } >>> >>> +static int dpu_kms_mmap_mdp5(struct dpu_kms *dpu_kms) >>> +{ >>> + struct platform_device *pdev = dpu_kms->pdev; >>> + struct platform_device *mdss_dev; >>> + int ret; >>> + >>> + if (dpu_kms->pdev->dev.bus != &platform_bus_type) >>> + return -EINVAL; >>> + >> >> !dev_is_platform() perhaps? > > looks good > >> >> But I would like to understand this check a bit more. Can you pls >> explain for which case this check was added? > > It is necessary to be sure that we can perform to_platform_device() on > the next line. > Got it but on the next line we are using to_platform_device on dpu's parent device. So shouldnt the check also be dpu_kms->pdev->dev.parent.bus? >> >>> + mdss_dev = to_platform_device(dpu_kms->pdev->dev.parent); >>> + >>> + dpu_kms->mmio = msm_ioremap(pdev, "mdp_phys"); >>> + if (IS_ERR(dpu_kms->mmio)) { >>> + ret = PTR_ERR(dpu_kms->mmio); >>> + DPU_ERROR("mdp register memory map failed: %d\n", ret); >>> + dpu_kms->mmio = NULL; >>> + return ret; >>> + } >>> + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); >>> + >>> + dpu_kms->vbif[VBIF_RT] = msm_ioremap_mdss(mdss_dev, >>> + dpu_kms->pdev, >>> + "vbif_phys"); >>> + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { >>> + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); >>> + DPU_ERROR("vbif register memory map failed: %d\n", ret); >>> + dpu_kms->vbif[VBIF_RT] = NULL; >>> + return ret; >>> + } >>> + >>> + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_mdss(mdss_dev, >>> + dpu_kms->pdev, >>> + "vbif_nrt_phys"); >> >> Do you think a "quiet" version would be better? > > Yep, why not. > >> >> >>> + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { >>> + dpu_kms->vbif[VBIF_NRT] = NULL; >>> + DPU_DEBUG("VBIF NRT is not defined"); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int dpu_kms_mmap_dpu(struct dpu_kms *dpu_kms) >>> +{ >>> + struct platform_device *pdev = dpu_kms->pdev; >>> + int ret; >>> + >>> + dpu_kms->mmio = msm_ioremap(pdev, "mdp"); >>> + if (IS_ERR(dpu_kms->mmio)) { >>> + ret = PTR_ERR(dpu_kms->mmio); >>> + DPU_ERROR("mdp register memory map failed: %d\n", ret); >>> + dpu_kms->mmio = NULL; >>> + return ret; >>> + } >>> + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); >>> + >>> + dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif"); >>> + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { >>> + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); >>> + DPU_ERROR("vbif register memory map failed: %d\n", ret); >>> + dpu_kms->vbif[VBIF_RT] = NULL; >>> + return ret; >>> + } >>> + >>> + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt"); >>> + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { >>> + dpu_kms->vbif[VBIF_NRT] = NULL; >>> + DPU_DEBUG("VBIF NRT is not defined"); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static int dpu_dev_probe(struct platform_device *pdev) >>> { >>> struct device *dev = &pdev->dev; >>> @@ -1230,28 +1302,12 @@ static int dpu_dev_probe(struct platform_device *pdev) >>> >>> dpu_kms->base.irq = irq; >>> >>> - dpu_kms->mmio = msm_ioremap(pdev, "mdp"); >>> - if (IS_ERR(dpu_kms->mmio)) { >>> - ret = PTR_ERR(dpu_kms->mmio); >>> - DPU_ERROR("mdp register memory map failed: %d\n", ret); >>> - dpu_kms->mmio = NULL; >>> - return ret; >>> - } >>> - DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); >>> - >>> - dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif"); >>> - if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { >>> - ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); >>> - DPU_ERROR("vbif register memory map failed: %d\n", ret); >>> - dpu_kms->vbif[VBIF_RT] = NULL; >>> + if (of_device_is_compatible(dpu_kms->pdev->dev.of_node, "qcom,mdp5")) >>> + ret = dpu_kms_mmap_mdp5(dpu_kms); >>> + else >>> + ret = dpu_kms_mmap_dpu(dpu_kms); >>> + if (ret) >>> return ret; >>> - } >>> - >>> - dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt"); >>> - if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { >>> - dpu_kms->vbif[VBIF_NRT] = NULL; >>> - DPU_DEBUG("VBIF NRT is not defined"); >>> - } >>> >>> ret = dpu_kms_parse_data_bus_icc_path(dpu_kms); >>> if (ret) >>> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h >>> index 16a7cbc0b7dd..01e783130054 100644 >>> --- a/drivers/gpu/drm/msm/msm_drv.h >>> +++ b/drivers/gpu/drm/msm/msm_drv.h >>> @@ -476,6 +476,9 @@ void __iomem *msm_ioremap(struct platform_device *pdev, const char *name); >>> void __iomem *msm_ioremap_size(struct platform_device *pdev, const char *name, >>> phys_addr_t *size); >>> void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name); >>> +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev, >>> + struct platform_device *dev, >>> + const char *name); >>> >>> struct icc_path *msm_icc_get(struct device *dev, const char *name); >>> >>> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c >>> index 59d2788c4510..afedd61c3e28 100644 >>> --- a/drivers/gpu/drm/msm/msm_io_utils.c >>> +++ b/drivers/gpu/drm/msm/msm_io_utils.c >>> @@ -50,6 +50,19 @@ struct clk *msm_clk_get(struct platform_device *pdev, const char *name) >>> return clk; >>> } >>> >>> +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev, >>> + struct platform_device *pdev, >>> + const char *name) >>> +{ >>> + struct resource *res; >>> + >>> + res = platform_get_resource_byname(mdss_pdev, IORESOURCE_MEM, name); >>> + if (!res) >>> + return ERR_PTR(-EINVAL); >>> + >>> + return devm_ioremap_resource(&pdev->dev, res); >>> +} >>> + >>> static void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name, >>> bool quiet, phys_addr_t *psize) >>> { >>> > > >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 723cc1d82143..aa9e0ad33ebb 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1197,6 +1197,78 @@ static int dpu_kms_init(struct drm_device *ddev) return 0; } +static int dpu_kms_mmap_mdp5(struct dpu_kms *dpu_kms) +{ + struct platform_device *pdev = dpu_kms->pdev; + struct platform_device *mdss_dev; + int ret; + + if (dpu_kms->pdev->dev.bus != &platform_bus_type) + return -EINVAL; + + mdss_dev = to_platform_device(dpu_kms->pdev->dev.parent); + + dpu_kms->mmio = msm_ioremap(pdev, "mdp_phys"); + if (IS_ERR(dpu_kms->mmio)) { + ret = PTR_ERR(dpu_kms->mmio); + DPU_ERROR("mdp register memory map failed: %d\n", ret); + dpu_kms->mmio = NULL; + return ret; + } + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); + + dpu_kms->vbif[VBIF_RT] = msm_ioremap_mdss(mdss_dev, + dpu_kms->pdev, + "vbif_phys"); + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); + DPU_ERROR("vbif register memory map failed: %d\n", ret); + dpu_kms->vbif[VBIF_RT] = NULL; + return ret; + } + + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_mdss(mdss_dev, + dpu_kms->pdev, + "vbif_nrt_phys"); + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { + dpu_kms->vbif[VBIF_NRT] = NULL; + DPU_DEBUG("VBIF NRT is not defined"); + } + + return 0; +} + +static int dpu_kms_mmap_dpu(struct dpu_kms *dpu_kms) +{ + struct platform_device *pdev = dpu_kms->pdev; + int ret; + + dpu_kms->mmio = msm_ioremap(pdev, "mdp"); + if (IS_ERR(dpu_kms->mmio)) { + ret = PTR_ERR(dpu_kms->mmio); + DPU_ERROR("mdp register memory map failed: %d\n", ret); + dpu_kms->mmio = NULL; + return ret; + } + DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); + + dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif"); + if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { + ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); + DPU_ERROR("vbif register memory map failed: %d\n", ret); + dpu_kms->vbif[VBIF_RT] = NULL; + return ret; + } + + dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt"); + if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { + dpu_kms->vbif[VBIF_NRT] = NULL; + DPU_DEBUG("VBIF NRT is not defined"); + } + + return 0; +} + static int dpu_dev_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -1230,28 +1302,12 @@ static int dpu_dev_probe(struct platform_device *pdev) dpu_kms->base.irq = irq; - dpu_kms->mmio = msm_ioremap(pdev, "mdp"); - if (IS_ERR(dpu_kms->mmio)) { - ret = PTR_ERR(dpu_kms->mmio); - DPU_ERROR("mdp register memory map failed: %d\n", ret); - dpu_kms->mmio = NULL; - return ret; - } - DRM_DEBUG("mapped dpu address space @%pK\n", dpu_kms->mmio); - - dpu_kms->vbif[VBIF_RT] = msm_ioremap(pdev, "vbif"); - if (IS_ERR(dpu_kms->vbif[VBIF_RT])) { - ret = PTR_ERR(dpu_kms->vbif[VBIF_RT]); - DPU_ERROR("vbif register memory map failed: %d\n", ret); - dpu_kms->vbif[VBIF_RT] = NULL; + if (of_device_is_compatible(dpu_kms->pdev->dev.of_node, "qcom,mdp5")) + ret = dpu_kms_mmap_mdp5(dpu_kms); + else + ret = dpu_kms_mmap_dpu(dpu_kms); + if (ret) return ret; - } - - dpu_kms->vbif[VBIF_NRT] = msm_ioremap_quiet(pdev, "vbif_nrt"); - if (IS_ERR(dpu_kms->vbif[VBIF_NRT])) { - dpu_kms->vbif[VBIF_NRT] = NULL; - DPU_DEBUG("VBIF NRT is not defined"); - } ret = dpu_kms_parse_data_bus_icc_path(dpu_kms); if (ret) diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 16a7cbc0b7dd..01e783130054 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -476,6 +476,9 @@ void __iomem *msm_ioremap(struct platform_device *pdev, const char *name); void __iomem *msm_ioremap_size(struct platform_device *pdev, const char *name, phys_addr_t *size); void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name); +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev, + struct platform_device *dev, + const char *name); struct icc_path *msm_icc_get(struct device *dev, const char *name); diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c index 59d2788c4510..afedd61c3e28 100644 --- a/drivers/gpu/drm/msm/msm_io_utils.c +++ b/drivers/gpu/drm/msm/msm_io_utils.c @@ -50,6 +50,19 @@ struct clk *msm_clk_get(struct platform_device *pdev, const char *name) return clk; } +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev, + struct platform_device *pdev, + const char *name) +{ + struct resource *res; + + res = platform_get_resource_byname(mdss_pdev, IORESOURCE_MEM, name); + if (!res) + return ERR_PTR(-EINVAL); + + return devm_ioremap_resource(&pdev->dev, res); +} + static void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name, bool quiet, phys_addr_t *psize) {
Existing MDP5 devices have slightly different bindings. The main register region is called `mdp_phys' instead of `mdp'. Also vbif register regions are a part of the parent, MDSS device. Add support for handling this binding differences. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 98 ++++++++++++++++++++++++++------- drivers/gpu/drm/msm/msm_drv.h | 3 + drivers/gpu/drm/msm/msm_io_utils.c | 13 +++++ 3 files changed, 93 insertions(+), 21 deletions(-)