[PATCH 0/3] drm/msm: fixes for KMS iommu handling

[PATCH 0/3] drm/msm: fixes for KMS iommu handling

[Dmitry Baryshkov]

This series started from the applied and then reverted [2] patch by
Robin Murphy [1]. After the MDSS rework [3] has landed it is now
possible to reapply the extended version of the original patch. While we
are at it, also rework the IOMMU init code for DPU and MDP5 drivers.

For MDP5 this moves iommu_domain_alloc() call and removes struct
mdp5_cfg_platform remains.

For DPU this allows specifying the iommus = <...> either in the DPU
device (like all DPU devices do) or in the MDSS device (like MDP5
devices do).

[1] https://patchwork.freedesktop.org/patch/480707/
[2] https://patchwork.freedesktop.org/patch/482453/
[3] https://patchwork.freedesktop.org/series/98525/

Dmitry Baryshkov (3):
  drm/msm/dpu: check both DPU and MDSS devices for the IOMMU
  drm/msm/mdp5: move iommu_domain_alloc() call close to its usage
  drm/msm: Stop using iommu_present()

 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 14 +++++++++++---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16 ----------------
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h |  6 ------
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |  6 ++++--
 drivers/gpu/drm/msm/msm_drv.c            | 10 ++++++++--
 5 files changed, 23 insertions(+), 29 deletions(-)

-- 
2.35.1

[PATCH 1/3] drm/msm/dpu: check both DPU and MDSS devices for the IOMMU

[Dmitry Baryshkov]

Follow the lead of MDP5 driver and check both DPU and MDSS devices for
the IOMMU specifiers.

Historically DPU devices had IOMMU specified in the MDSS device tree
node, but as some of MDP5 devices are being converted to the supported
by the DPU driver, the driver should adapt and check both devices.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 143d6643be53..5ccda0766f6c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1004,14 +1004,22 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
 	struct msm_mmu *mmu;
 	struct device *dpu_dev = dpu_kms->dev->dev;
 	struct device *mdss_dev = dpu_dev->parent;
+	struct device *iommu_dev;
 
 	domain = iommu_domain_alloc(&platform_bus_type);
 	if (!domain)
 		return 0;
 
-	/* IOMMUs are a part of MDSS device tree binding, not the
-	 * MDP/DPU device. */
-	mmu = msm_iommu_new(mdss_dev, domain);
+	/*
+	 * IOMMUs can be a part of MDSS device tree binding, or the
+	 * MDP/DPU device.
+	 */
+	if (dev_iommu_fwspec_get(dpu_dev))
+		iommu_dev = dpu_dev;
+	else
+		iommu_dev = mdss_dev;
+
+	mmu = msm_iommu_new(iommu_dev, domain);
 	if (IS_ERR(mmu)) {
 		iommu_domain_free(domain);
 		return PTR_ERR(mmu);
-- 
2.35.1

[PATCH 2/3] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage

[Dmitry Baryshkov]

Move iommu_domain_alloc() in front of adress space/IOMMU initialization.
This allows us to drop final bits of struct mdp5_cfg_platform which
remained from the pre-DT days.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16 ----------------
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h |  6 ------
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |  6 ++++--
 3 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 1bf9ff5dbabc..714effb967ff 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
 	{ .revision = 3, .config = { .hw = &sdm630_config } },
 };
 
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
-
 const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler *cfg_handler)
 {
 	return cfg_handler->config.hw;
@@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
 		uint32_t major, uint32_t minor)
 {
 	struct drm_device *dev = mdp5_kms->dev;
-	struct platform_device *pdev = to_platform_device(dev->dev);
 	struct mdp5_cfg_handler *cfg_handler;
 	const struct mdp5_cfg_handler *cfg_handlers;
-	struct mdp5_cfg_platform *pconfig;
 	int i, ret = 0, num_handlers;
 
 	cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL);
@@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
 	cfg_handler->revision = minor;
 	cfg_handler->config.hw = mdp5_cfg;
 
-	pconfig = mdp5_get_config(pdev);
-	memcpy(&cfg_handler->config.platform, pconfig, sizeof(*pconfig));
-
 	DBG("MDP5: %s hw config selected", mdp5_cfg->name);
 
 	return cfg_handler;
@@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
 
 	return ERR_PTR(ret);
 }
-
-static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
-{
-	static struct mdp5_cfg_platform config = {};
-
-	config.iommu = iommu_domain_alloc(&platform_bus_type);
-
-	return &config;
-}
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
index 6b03d7899309..c2502cc33864 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
@@ -104,14 +104,8 @@ struct mdp5_cfg_hw {
 	uint32_t max_clk;
 };
 
-/* platform config data (ie. from DT, or pdata) */
-struct mdp5_cfg_platform {
-	struct iommu_domain *iommu;
-};
-
 struct mdp5_cfg {
 	const struct mdp5_cfg_hw *hw;
-	struct mdp5_cfg_platform platform;
 };
 
 struct mdp5_kms;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 9b7bbc3adb97..1c67c2c828cd 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev)
 	struct msm_gem_address_space *aspace;
 	int irq, i, ret;
 	struct device *iommu_dev;
+	struct iommu_domain *iommu;
 
 	ret = mdp5_init(to_platform_device(dev->dev), dev);
 
@@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev)
 	}
 	mdelay(16);
 
-	if (config->platform.iommu) {
+	iommu = iommu_domain_alloc(&platform_bus_type);
+	if (iommu) {
 		struct msm_mmu *mmu;
 
 		iommu_dev = &pdev->dev;
 		if (!dev_iommu_fwspec_get(iommu_dev))
 			iommu_dev = iommu_dev->parent;
 
-		mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
+		mmu = msm_iommu_new(iommu_dev, iommu);
 
 		aspace = msm_gem_address_space_create(mmu, "mdp5",
 			0x1000, 0x100000000 - 0x1000);
-- 
2.35.1

[PATCH 3/3] drm/msm: Stop using iommu_present()

[Dmitry Baryshkov]

Even if some IOMMU has registered itself on the platform "bus", that
doesn't necessarily mean it provides translation for the device we
care about. Replace iommu_present() with a more appropriate check.

On Qualcomm platforms the IOMMU can be specified either for the MDP/DPU
device or for its parent MDSS device depending on the actual platform.
Check both of them, since that is how both DPU and MDP5 drivers work.

Co-developed-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_drv.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 4a3dda23e3e0..a37a3bbc04d9 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -266,8 +266,14 @@ bool msm_use_mmu(struct drm_device *dev)
 {
 	struct msm_drm_private *priv = dev->dev_private;
 
-	/* a2xx comes with its own MMU */
-	return priv->is_a2xx || iommu_present(&platform_bus_type);
+	/*
+	 * a2xx comes with its own MMU
+	 * On other platforms IOMMU can be declared specified either for the
+	 * MDP/DPU device or for its parent, MDSS device.
+	 */
+	return priv->is_a2xx ||
+		device_iommu_mapped(dev->dev) ||
+		device_iommu_mapped(dev->dev->parent);
 }
 
 static int msm_init_vram(struct drm_device *dev)
-- 
2.35.1

Re: [PATCH 2/3] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage

[Robin Murphy]

On 2022-05-01 11:10, Dmitry Baryshkov wrote:
> Move iommu_domain_alloc() in front of adress space/IOMMU initialization.
> This allows us to drop final bits of struct mdp5_cfg_platform which
> remained from the pre-DT days.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16 ----------------
>   drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h |  6 ------
>   drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |  6 ++++--
>   3 files changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> index 1bf9ff5dbabc..714effb967ff 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> @@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
>   	{ .revision = 3, .config = { .hw = &sdm630_config } },
>   };
>   
> -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
> -
>   const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler *cfg_handler)
>   {
>   	return cfg_handler->config.hw;
> @@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>   		uint32_t major, uint32_t minor)
>   {
>   	struct drm_device *dev = mdp5_kms->dev;
> -	struct platform_device *pdev = to_platform_device(dev->dev);
>   	struct mdp5_cfg_handler *cfg_handler;
>   	const struct mdp5_cfg_handler *cfg_handlers;
> -	struct mdp5_cfg_platform *pconfig;
>   	int i, ret = 0, num_handlers;
>   
>   	cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL);
> @@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>   	cfg_handler->revision = minor;
>   	cfg_handler->config.hw = mdp5_cfg;
>   
> -	pconfig = mdp5_get_config(pdev);
> -	memcpy(&cfg_handler->config.platform, pconfig, sizeof(*pconfig));
> -
>   	DBG("MDP5: %s hw config selected", mdp5_cfg->name);
>   
>   	return cfg_handler;
> @@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>   
>   	return ERR_PTR(ret);
>   }
> -
> -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
> -{
> -	static struct mdp5_cfg_platform config = {};
> -
> -	config.iommu = iommu_domain_alloc(&platform_bus_type);
> -
> -	return &config;
> -}
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> index 6b03d7899309..c2502cc33864 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> @@ -104,14 +104,8 @@ struct mdp5_cfg_hw {
>   	uint32_t max_clk;
>   };
>   
> -/* platform config data (ie. from DT, or pdata) */
> -struct mdp5_cfg_platform {
> -	struct iommu_domain *iommu;
> -};
> -
>   struct mdp5_cfg {
>   	const struct mdp5_cfg_hw *hw;
> -	struct mdp5_cfg_platform platform;
>   };
>   
>   struct mdp5_kms;
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 9b7bbc3adb97..1c67c2c828cd 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev)
>   	struct msm_gem_address_space *aspace;
>   	int irq, i, ret;
>   	struct device *iommu_dev;
> +	struct iommu_domain *iommu;
>   
>   	ret = mdp5_init(to_platform_device(dev->dev), dev);
>   
> @@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev)
>   	}
>   	mdelay(16);
>   
> -	if (config->platform.iommu) {
> +	iommu = iommu_domain_alloc(&platform_bus_type);

To preempt the next change down the line as well, could this be 
rearranged to work as iommu_domain_alloc(iommu_dev->bus)?

> +	if (iommu) {
>   		struct msm_mmu *mmu;
>   
>   		iommu_dev = &pdev->dev;
>   		if (!dev_iommu_fwspec_get(iommu_dev))

The fwspec helpers are more of an internal thing between the IOMMU 
drivers and the respective firmware code - I'd rather that external API 
users stuck consistently to using device_iommu_mapped() (it should give 
the same result).

Otherwise, thanks for sorting this out!

Robin.

>   			iommu_dev = iommu_dev->parent;
>   
> -		mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
> +		mmu = msm_iommu_new(iommu_dev, iommu);
>   
>   		aspace = msm_gem_address_space_create(mmu, "mdp5",
>   			0x1000, 0x100000000 - 0x1000);

Re: [PATCH 2/3] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage

[Dmitry Baryshkov]

On Tue, 3 May 2022 at 13:57, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2022-05-01 11:10, Dmitry Baryshkov wrote:
> > Move iommu_domain_alloc() in front of adress space/IOMMU initialization.
> > This allows us to drop final bits of struct mdp5_cfg_platform which
> > remained from the pre-DT days.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16 ----------------
> >   drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h |  6 ------
> >   drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |  6 ++++--
> >   3 files changed, 4 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> > index 1bf9ff5dbabc..714effb967ff 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> > @@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
> >       { .revision = 3, .config = { .hw = &sdm630_config } },
> >   };
> >
> > -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
> > -
> >   const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler *cfg_handler)
> >   {
> >       return cfg_handler->config.hw;
> > @@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
> >               uint32_t major, uint32_t minor)
> >   {
> >       struct drm_device *dev = mdp5_kms->dev;
> > -     struct platform_device *pdev = to_platform_device(dev->dev);
> >       struct mdp5_cfg_handler *cfg_handler;
> >       const struct mdp5_cfg_handler *cfg_handlers;
> > -     struct mdp5_cfg_platform *pconfig;
> >       int i, ret = 0, num_handlers;
> >
> >       cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL);
> > @@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
> >       cfg_handler->revision = minor;
> >       cfg_handler->config.hw = mdp5_cfg;
> >
> > -     pconfig = mdp5_get_config(pdev);
> > -     memcpy(&cfg_handler->config.platform, pconfig, sizeof(*pconfig));
> > -
> >       DBG("MDP5: %s hw config selected", mdp5_cfg->name);
> >
> >       return cfg_handler;
> > @@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
> >
> >       return ERR_PTR(ret);
> >   }
> > -
> > -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
> > -{
> > -     static struct mdp5_cfg_platform config = {};
> > -
> > -     config.iommu = iommu_domain_alloc(&platform_bus_type);
> > -
> > -     return &config;
> > -}
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> > index 6b03d7899309..c2502cc33864 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
> > @@ -104,14 +104,8 @@ struct mdp5_cfg_hw {
> >       uint32_t max_clk;
> >   };
> >
> > -/* platform config data (ie. from DT, or pdata) */
> > -struct mdp5_cfg_platform {
> > -     struct iommu_domain *iommu;
> > -};
> > -
> >   struct mdp5_cfg {
> >       const struct mdp5_cfg_hw *hw;
> > -     struct mdp5_cfg_platform platform;
> >   };
> >
> >   struct mdp5_kms;
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > index 9b7bbc3adb97..1c67c2c828cd 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> > @@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev)
> >       struct msm_gem_address_space *aspace;
> >       int irq, i, ret;
> >       struct device *iommu_dev;
> > +     struct iommu_domain *iommu;
> >
> >       ret = mdp5_init(to_platform_device(dev->dev), dev);
> >
> > @@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev)
> >       }
> >       mdelay(16);
> >
> > -     if (config->platform.iommu) {
> > +     iommu = iommu_domain_alloc(&platform_bus_type);
>
> To preempt the next change down the line as well, could this be
> rearranged to work as iommu_domain_alloc(iommu_dev->bus)?

I'd prefer to split this into the separate change, if you don't mind.

>
> > +     if (iommu) {
> >               struct msm_mmu *mmu;
> >
> >               iommu_dev = &pdev->dev;
> >               if (!dev_iommu_fwspec_get(iommu_dev))
>
> The fwspec helpers are more of an internal thing between the IOMMU
> drivers and the respective firmware code - I'd rather that external API
> users stuck consistently to using device_iommu_mapped() (it should give
> the same result).

Let me check that it works correctly and spin a v2 afterwards.

>
> Otherwise, thanks for sorting this out!
>
> Robin.
>
> >                       iommu_dev = iommu_dev->parent;
> >
> > -             mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
> > +             mmu = msm_iommu_new(iommu_dev, iommu);
> >
> >               aspace = msm_gem_address_space_create(mmu, "mdp5",
> >                       0x1000, 0x100000000 - 0x1000);



-- 
With best wishes
Dmitry

Re: [PATCH 2/3] drm/msm/mdp5: move iommu_domain_alloc() call close to its usage

[Robin Murphy]

On 2022-05-03 14:30, Dmitry Baryshkov wrote:
> On Tue, 3 May 2022 at 13:57, Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2022-05-01 11:10, Dmitry Baryshkov wrote:
>>> Move iommu_domain_alloc() in front of adress space/IOMMU initialization.
>>> This allows us to drop final bits of struct mdp5_cfg_platform which
>>> remained from the pre-DT days.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>    drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 16 ----------------
>>>    drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h |  6 ------
>>>    drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |  6 ++++--
>>>    3 files changed, 4 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
>>> index 1bf9ff5dbabc..714effb967ff 100644
>>> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
>>> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
>>> @@ -1248,8 +1248,6 @@ static const struct mdp5_cfg_handler cfg_handlers_v3[] = {
>>>        { .revision = 3, .config = { .hw = &sdm630_config } },
>>>    };
>>>
>>> -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
>>> -
>>>    const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler *cfg_handler)
>>>    {
>>>        return cfg_handler->config.hw;
>>> @@ -1274,10 +1272,8 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>>>                uint32_t major, uint32_t minor)
>>>    {
>>>        struct drm_device *dev = mdp5_kms->dev;
>>> -     struct platform_device *pdev = to_platform_device(dev->dev);
>>>        struct mdp5_cfg_handler *cfg_handler;
>>>        const struct mdp5_cfg_handler *cfg_handlers;
>>> -     struct mdp5_cfg_platform *pconfig;
>>>        int i, ret = 0, num_handlers;
>>>
>>>        cfg_handler = kzalloc(sizeof(*cfg_handler), GFP_KERNEL);
>>> @@ -1320,9 +1316,6 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>>>        cfg_handler->revision = minor;
>>>        cfg_handler->config.hw = mdp5_cfg;
>>>
>>> -     pconfig = mdp5_get_config(pdev);
>>> -     memcpy(&cfg_handler->config.platform, pconfig, sizeof(*pconfig));
>>> -
>>>        DBG("MDP5: %s hw config selected", mdp5_cfg->name);
>>>
>>>        return cfg_handler;
>>> @@ -1333,12 +1326,3 @@ struct mdp5_cfg_handler *mdp5_cfg_init(struct mdp5_kms *mdp5_kms,
>>>
>>>        return ERR_PTR(ret);
>>>    }
>>> -
>>> -static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
>>> -{
>>> -     static struct mdp5_cfg_platform config = {};
>>> -
>>> -     config.iommu = iommu_domain_alloc(&platform_bus_type);
>>> -
>>> -     return &config;
>>> -}
>>> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
>>> index 6b03d7899309..c2502cc33864 100644
>>> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
>>> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.h
>>> @@ -104,14 +104,8 @@ struct mdp5_cfg_hw {
>>>        uint32_t max_clk;
>>>    };
>>>
>>> -/* platform config data (ie. from DT, or pdata) */
>>> -struct mdp5_cfg_platform {
>>> -     struct iommu_domain *iommu;
>>> -};
>>> -
>>>    struct mdp5_cfg {
>>>        const struct mdp5_cfg_hw *hw;
>>> -     struct mdp5_cfg_platform platform;
>>>    };
>>>
>>>    struct mdp5_kms;
>>> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
>>> index 9b7bbc3adb97..1c67c2c828cd 100644
>>> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
>>> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
>>> @@ -558,6 +558,7 @@ static int mdp5_kms_init(struct drm_device *dev)
>>>        struct msm_gem_address_space *aspace;
>>>        int irq, i, ret;
>>>        struct device *iommu_dev;
>>> +     struct iommu_domain *iommu;
>>>
>>>        ret = mdp5_init(to_platform_device(dev->dev), dev);
>>>
>>> @@ -601,14 +602,15 @@ static int mdp5_kms_init(struct drm_device *dev)
>>>        }
>>>        mdelay(16);
>>>
>>> -     if (config->platform.iommu) {
>>> +     iommu = iommu_domain_alloc(&platform_bus_type);
>>
>> To preempt the next change down the line as well, could this be
>> rearranged to work as iommu_domain_alloc(iommu_dev->bus)?
> 
> I'd prefer to split this into the separate change, if you don't mind.

Oh, for sure, divide the patches however you see fit - I'm just hoping 
to save your time overall by getting all the IOMMU-related refactoring 
done now as a single series rather than risk me coming back and breaking 
things again in a few months :)

Cheers,
Robin.

> 
>>
>>> +     if (iommu) {
>>>                struct msm_mmu *mmu;
>>>
>>>                iommu_dev = &pdev->dev;
>>>                if (!dev_iommu_fwspec_get(iommu_dev))
>>
>> The fwspec helpers are more of an internal thing between the IOMMU
>> drivers and the respective firmware code - I'd rather that external API
>> users stuck consistently to using device_iommu_mapped() (it should give
>> the same result).
> 
> Let me check that it works correctly and spin a v2 afterwards.
> 
>>
>> Otherwise, thanks for sorting this out!
>>
>> Robin.
>>
>>>                        iommu_dev = iommu_dev->parent;
>>>
>>> -             mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
>>> +             mmu = msm_iommu_new(iommu_dev, iommu);
>>>
>>>                aspace = msm_gem_address_space_create(mmu, "mdp5",
>>>                        0x1000, 0x100000000 - 0x1000);
> 
> 
>