diff mbox series

[v2,5/5] drm/msm: switch msm_kms_init_aspace() to use device_iommu_mapped()

Message ID 20220505001605.1268483-6-dmitry.baryshkov@linaro.org (mailing list archive)
State New, archived
Headers show
Series drm/msm: fixes for KMS iommu handling | expand

Commit Message

Dmitry Baryshkov May 5, 2022, 12:16 a.m. UTC
Change msm_kms_init_aspace() to use generic function
device_iommu_mapped() instead of the fwnode-specific interface
dev_iommu_fwspec_get(). While we are at it, stop referencing
platform_bus_type directly and use the bus of the IOMMU device.

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

Comments

Robin Murphy May 5, 2022, 10:27 a.m. UTC | #1
On 2022-05-05 01:16, Dmitry Baryshkov wrote:
> Change msm_kms_init_aspace() to use generic function
> device_iommu_mapped() instead of the fwnode-specific interface
> dev_iommu_fwspec_get(). While we are at it, stop referencing
> platform_bus_type directly and use the bus of the IOMMU device.

FWIW, I'd have squashed these changes across the previous patches, such 
that the dodgy fwspec calls are never introduced in the first place, but 
it's your driver, and if that's the way you want to work it and Rob's 
happy with it too, then fine by me.

For the end result,

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

I'm guessing MDP4 could probably use msm_kms_init_aspace() now as well, 
but unless there's any other reason to respin this series, that's 
something we could do as a follow-up. Thanks for sorting this out!

Robin.

> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/msm_drv.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 98ae0036ab57..2fc3f820cd59 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -272,21 +272,21 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev)
>   	struct device *mdss_dev = mdp_dev->parent;
>   	struct device *iommu_dev;
>   
> -	domain = iommu_domain_alloc(&platform_bus_type);
> -	if (!domain) {
> -		drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n");
> -		return NULL;
> -	}
> -
>   	/*
>   	 * IOMMUs can be a part of MDSS device tree binding, or the
>   	 * MDP/DPU device.
>   	 */
> -	if (dev_iommu_fwspec_get(mdp_dev))
> +	if (device_iommu_mapped(mdp_dev))
>   		iommu_dev = mdp_dev;
>   	else
>   		iommu_dev = mdss_dev;
>   
> +	domain = iommu_domain_alloc(iommu_dev->bus);
> +	if (!domain) {
> +		drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n");
> +		return NULL;
> +	}
> +
>   	mmu = msm_iommu_new(iommu_dev, domain);
>   	if (IS_ERR(mmu)) {
>   		iommu_domain_free(domain);
Dmitry Baryshkov May 5, 2022, 11:49 a.m. UTC | #2
On Thu, 5 May 2022 at 13:27, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2022-05-05 01:16, Dmitry Baryshkov wrote:
> > Change msm_kms_init_aspace() to use generic function
> > device_iommu_mapped() instead of the fwnode-specific interface
> > dev_iommu_fwspec_get(). While we are at it, stop referencing
> > platform_bus_type directly and use the bus of the IOMMU device.
>
> FWIW, I'd have squashed these changes across the previous patches, such
> that the dodgy fwspec calls are never introduced in the first place, but
> it's your driver, and if that's the way you want to work it and Rob's
> happy with it too, then fine by me.

I thought about this. But as the calls were already there (in the
mdp5), it was easier for me to merge the code and to update it
afterwards.

>
> For the end result,
>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>
> I'm guessing MDP4 could probably use msm_kms_init_aspace() now as well,
> but unless there's any other reason to respin this series, that's
> something we could do as a follow-up. Thanks for sorting this out!

Not really. MDP4 doesn't have the parent MDSS device, so it doesn't
need all these troubles.

>
> Robin.
>
> > Suggested-by: Robin Murphy <robin.murphy@arm.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/msm_drv.c | 14 +++++++-------
> >   1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index 98ae0036ab57..2fc3f820cd59 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -272,21 +272,21 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev)
> >       struct device *mdss_dev = mdp_dev->parent;
> >       struct device *iommu_dev;
> >
> > -     domain = iommu_domain_alloc(&platform_bus_type);
> > -     if (!domain) {
> > -             drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n");
> > -             return NULL;
> > -     }
> > -
> >       /*
> >        * IOMMUs can be a part of MDSS device tree binding, or the
> >        * MDP/DPU device.
> >        */
> > -     if (dev_iommu_fwspec_get(mdp_dev))
> > +     if (device_iommu_mapped(mdp_dev))
> >               iommu_dev = mdp_dev;
> >       else
> >               iommu_dev = mdss_dev;
> >
> > +     domain = iommu_domain_alloc(iommu_dev->bus);
> > +     if (!domain) {
> > +             drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n");
> > +             return NULL;
> > +     }
> > +
> >       mmu = msm_iommu_new(iommu_dev, domain);
> >       if (IS_ERR(mmu)) {
> >               iommu_domain_free(domain);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 98ae0036ab57..2fc3f820cd59 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -272,21 +272,21 @@  struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev)
 	struct device *mdss_dev = mdp_dev->parent;
 	struct device *iommu_dev;
 
-	domain = iommu_domain_alloc(&platform_bus_type);
-	if (!domain) {
-		drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n");
-		return NULL;
-	}
-
 	/*
 	 * IOMMUs can be a part of MDSS device tree binding, or the
 	 * MDP/DPU device.
 	 */
-	if (dev_iommu_fwspec_get(mdp_dev))
+	if (device_iommu_mapped(mdp_dev))
 		iommu_dev = mdp_dev;
 	else
 		iommu_dev = mdss_dev;
 
+	domain = iommu_domain_alloc(iommu_dev->bus);
+	if (!domain) {
+		drm_info(dev, "no IOMMU, fallback to phys contig buffers for scanout\n");
+		return NULL;
+	}
+
 	mmu = msm_iommu_new(iommu_dev, domain);
 	if (IS_ERR(mmu)) {
 		iommu_domain_free(domain);