[PATCH] drm/msm: Remove pm_runtime calls from msm_iommu.c

[PATCH] drm/msm: Remove pm_runtime calls from msm_iommu.c

[Jordan Crouse]

Currently the IOMMU code calls pm_runtime_get/put on the GPU or display
device before doing a IOMMU operation. This was because usually the
IOMMU driver didn't do power control of its own and since the hardware
used the same clocks and power as the respective multimedia device it
was a easy way to make sure that the power was available.

Now two things have changed. First, the SMMU devices can do their own power
control and more important bringing up the a6xx GPU isn't as easy as
turning on some clocks. To bring the GPU up we need the GMU which itself
needs the IOMMU so we have a chicken and egg problem.

Luckily this is easily fixed by removing the pm_runtime calls from the
functions and letting the device link to the IOMMU device handle the magic.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/msm_iommu.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index 4d62790..12bb54c 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -38,13 +38,8 @@ static int msm_iommu_attach(struct msm_mmu *mmu, const char * const *names,
 			    int cnt)
 {
 	struct msm_iommu *iommu = to_msm_iommu(mmu);
-	int ret;
 
-	pm_runtime_get_sync(mmu->dev);
-	ret = iommu_attach_device(iommu->domain, mmu->dev);
-	pm_runtime_put_sync(mmu->dev);
-
-	return ret;
+	return iommu_attach_device(iommu->domain, mmu->dev);
 }
 
 static void msm_iommu_detach(struct msm_mmu *mmu, const char * const *names,
@@ -52,9 +47,7 @@ static void msm_iommu_detach(struct msm_mmu *mmu, const char * const *names,
 {
 	struct msm_iommu *iommu = to_msm_iommu(mmu);
 
-	pm_runtime_get_sync(mmu->dev);
 	iommu_detach_device(iommu->domain, mmu->dev);
-	pm_runtime_put_sync(mmu->dev);
 }
 
 static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
@@ -63,9 +56,7 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
 	struct msm_iommu *iommu = to_msm_iommu(mmu);
 	size_t ret;
 
-//	pm_runtime_get_sync(mmu->dev);
 	ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot);
-//	pm_runtime_put_sync(mmu->dev);
 	WARN_ON(!ret);
 
 	return (ret == len) ? 0 : -EINVAL;
@@ -75,9 +66,7 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t iova, unsigned len)
 {
 	struct msm_iommu *iommu = to_msm_iommu(mmu);
 
-	pm_runtime_get_sync(mmu->dev);
 	iommu_unmap(iommu->domain, iova, len);
-	pm_runtime_put_sync(mmu->dev);
 
 	return 0;
 }
-- 
2.7.4

Re: [PATCH] drm/msm: Remove pm_runtime calls from msm_iommu.c

[Bjorn Andersson]

On Tue 19 Feb 10:48 PST 2019, Jordan Crouse wrote:

> Currently the IOMMU code calls pm_runtime_get/put on the GPU or display
> device before doing a IOMMU operation. This was because usually the
> IOMMU driver didn't do power control of its own and since the hardware
> used the same clocks and power as the respective multimedia device it
> was a easy way to make sure that the power was available.
> 
> Now two things have changed. First, the SMMU devices can do their own power
> control and more important bringing up the a6xx GPU isn't as easy as
> turning on some clocks. To bring the GPU up we need the GMU which itself
> needs the IOMMU so we have a chicken and egg problem.
> 
> Luckily this is easily fixed by removing the pm_runtime calls from the
> functions and letting the device link to the IOMMU device handle the magic.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>

Thanks for the patch Jordan, this resolves the previously seen splat and
the GPU driver is now happily initialized.

Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
> 
>  drivers/gpu/drm/msm/msm_iommu.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index 4d62790..12bb54c 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -38,13 +38,8 @@ static int msm_iommu_attach(struct msm_mmu *mmu, const char * const *names,
>  			    int cnt)
>  {
>  	struct msm_iommu *iommu = to_msm_iommu(mmu);
> -	int ret;
>  
> -	pm_runtime_get_sync(mmu->dev);
> -	ret = iommu_attach_device(iommu->domain, mmu->dev);
> -	pm_runtime_put_sync(mmu->dev);
> -
> -	return ret;
> +	return iommu_attach_device(iommu->domain, mmu->dev);
>  }
>  
>  static void msm_iommu_detach(struct msm_mmu *mmu, const char * const *names,
> @@ -52,9 +47,7 @@ static void msm_iommu_detach(struct msm_mmu *mmu, const char * const *names,
>  {
>  	struct msm_iommu *iommu = to_msm_iommu(mmu);
>  
> -	pm_runtime_get_sync(mmu->dev);
>  	iommu_detach_device(iommu->domain, mmu->dev);
> -	pm_runtime_put_sync(mmu->dev);
>  }
>  
>  static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
> @@ -63,9 +56,7 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
>  	struct msm_iommu *iommu = to_msm_iommu(mmu);
>  	size_t ret;
>  
> -//	pm_runtime_get_sync(mmu->dev);
>  	ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot);
> -//	pm_runtime_put_sync(mmu->dev);
>  	WARN_ON(!ret);
>  
>  	return (ret == len) ? 0 : -EINVAL;
> @@ -75,9 +66,7 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t iova, unsigned len)
>  {
>  	struct msm_iommu *iommu = to_msm_iommu(mmu);
>  
> -	pm_runtime_get_sync(mmu->dev);
>  	iommu_unmap(iommu->domain, iova, len);
> -	pm_runtime_put_sync(mmu->dev);
>  
>  	return 0;
>  }
> -- 
> 2.7.4
> 
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm: Remove pm_runtime calls from msm_iommu.c

[Bjorn Andersson]

On Tue 19 Feb 10:48 PST 2019, Jordan Crouse wrote:

> Currently the IOMMU code calls pm_runtime_get/put on the GPU or display
> device before doing a IOMMU operation. This was because usually the
> IOMMU driver didn't do power control of its own and since the hardware
> used the same clocks and power as the respective multimedia device it
> was a easy way to make sure that the power was available.
> 
> Now two things have changed. First, the SMMU devices can do their own power
> control and more important bringing up the a6xx GPU isn't as easy as
> turning on some clocks. To bring the GPU up we need the GMU which itself
> needs the IOMMU so we have a chicken and egg problem.
> 
> Luckily this is easily fixed by removing the pm_runtime calls from the
> functions and letting the device link to the IOMMU device handle the magic.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>

Thanks for the patch Jordan, this resolves the previously seen splat and
the GPU driver is now happily initialized.

Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
> 
>  drivers/gpu/drm/msm/msm_iommu.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index 4d62790..12bb54c 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -38,13 +38,8 @@ static int msm_iommu_attach(struct msm_mmu *mmu, const char * const *names,
>  			    int cnt)
>  {
>  	struct msm_iommu *iommu = to_msm_iommu(mmu);
> -	int ret;
>  
> -	pm_runtime_get_sync(mmu->dev);
> -	ret = iommu_attach_device(iommu->domain, mmu->dev);
> -	pm_runtime_put_sync(mmu->dev);
> -
> -	return ret;
> +	return iommu_attach_device(iommu->domain, mmu->dev);
>  }
>  
>  static void msm_iommu_detach(struct msm_mmu *mmu, const char * const *names,
> @@ -52,9 +47,7 @@ static void msm_iommu_detach(struct msm_mmu *mmu, const char * const *names,
>  {
>  	struct msm_iommu *iommu = to_msm_iommu(mmu);
>  
> -	pm_runtime_get_sync(mmu->dev);
>  	iommu_detach_device(iommu->domain, mmu->dev);
> -	pm_runtime_put_sync(mmu->dev);
>  }
>  
>  static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
> @@ -63,9 +56,7 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova,
>  	struct msm_iommu *iommu = to_msm_iommu(mmu);
>  	size_t ret;
>  
> -//	pm_runtime_get_sync(mmu->dev);
>  	ret = iommu_map_sg(iommu->domain, iova, sgt->sgl, sgt->nents, prot);
> -//	pm_runtime_put_sync(mmu->dev);
>  	WARN_ON(!ret);
>  
>  	return (ret == len) ? 0 : -EINVAL;
> @@ -75,9 +66,7 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t iova, unsigned len)
>  {
>  	struct msm_iommu *iommu = to_msm_iommu(mmu);
>  
> -	pm_runtime_get_sync(mmu->dev);
>  	iommu_unmap(iommu->domain, iova, len);
> -	pm_runtime_put_sync(mmu->dev);
>  
>  	return 0;
>  }
> -- 
> 2.7.4
>