2024-02-08 15:02:43

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH v4 2/4] drm/msm/dpu: support binding to the mdp5 devices

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 <[email protected]>
---
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..5de1861f608e 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 (!dev_is_platform(dpu_kms->pdev->dev.parent))
+ 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)
{

--
2.39.2



2024-02-09 18:46:44

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] drm/msm/dpu: support binding to the mdp5 devices



On 2/8/2024 7:01 AM, 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 <[email protected]>
> ---
> 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(-)
>

I thought we had agreed on a quite version of msm_ioremap_mdss for
vbif_nrt_phys?

Anyway, its not something to block this change. Hence,

Reviewed-by: Abhinav Kumar <[email protected]>

2024-02-09 19:26:44

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] drm/msm/dpu: support binding to the mdp5 devices

On Fri, 9 Feb 2024 at 20:44, Abhinav Kumar <[email protected]> wrote:
>
>
>
> On 2/8/2024 7:01 AM, 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 <[email protected]>
> > ---
> > 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(-)
> >
>
> I thought we had agreed on a quite version of msm_ioremap_mdss for
> vbif_nrt_phys?
>
> Anyway, its not something to block this change. Hence,

Excuse me, I'll fix it when applying, if I don't forget.

>
> Reviewed-by: Abhinav Kumar <[email protected]>



--
With best wishes
Dmitry

2024-02-19 11:37:51

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] drm/msm/dpu: support binding to the mdp5 devices

On Fri, 9 Feb 2024 at 20:58, Dmitry Baryshkov
<[email protected]> wrote:
>
> On Fri, 9 Feb 2024 at 20:44, Abhinav Kumar <[email protected]> wrote:
> >
> >
> >
> > On 2/8/2024 7:01 AM, 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 <[email protected]>
> > > ---
> > > 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(-)
> > >
> >
> > I thought we had agreed on a quite version of msm_ioremap_mdss for
> > vbif_nrt_phys?
> >
> > Anyway, its not something to block this change. Hence,
>
> Excuse me, I'll fix it when applying, if I don't forget.

I took another glance. The msm_ioremap_mdss is already quiet (it is
not printing warnings if the resource is not present), so this should
be fine by default. But I'll make a note that we might be able to
replace most of msm_ioremap functions with the standard devm ioremap
functions.

--
With best wishes
Dmitry