[PATCH 1/5] drm/msm: Remove pm_runtime operations from msm_iommu
Now that the IOMMU is the master of it's own power we don't need to bring
up the GPU to do IOMMU operations. This is good because bringing up a6xx
requires the GMU so calling pm_runtime_get_sync() too early in the process
gets us into some nasty circular dependency situations.
Signed-off-by: Jordan Crouse
---
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 b23d33622f37..e80c79b3bb5c 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 < 0);
return (ret == len) ? 0 : -EINVAL;
@@ -76,9 +67,7 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t iova,
{
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.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/msm: Remove pm_runtime operations from msm_iommu
On Wed, Jun 20, 2018 at 4:00 AM, Jordan Crouse wrote:
> Now that the IOMMU is the master of it's own power we don't need to bring
> up the GPU to do IOMMU operations. This is good because bringing up a6xx
> requires the GMU so calling pm_runtime_get_sync() too early in the process
> gets us into some nasty circular dependency situations.
>
> Signed-off-by: Jordan Crouse
> ---
> 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 b23d33622f37..e80c79b3bb5c 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 < 0);
>
> return (ret == len) ? 0 : -EINVAL;
> @@ -76,9 +67,7 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t
> iova,
> {
> 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;
> }
Looks good to me.
Reviewed-by: Vivek Gautam
Best regards
Vivek
> --
> 2.17.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm/msm: Remove pm_runtime operations from msm_iommu
Now that the IOMMU is the master of it's own power we don't need to bring
up the GPU to do IOMMU operations. This is good because bringing up a6xx
requires the GMU so calling pm_runtime_get_sync() too early in the process
gets us into some nasty circular dependency situations.
Signed-off-by: Jordan Crouse
---
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 b23d33622f37..e80c79b3bb5c 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 < 0);
return (ret == len) ? 0 : -EINVAL;
@@ -76,9 +67,7 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t iova,
{
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.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/msm: Remove pm_runtime operations from msm_iommu
Hi Jordan,
On Mon, Jun 11, 2018 at 11:56 PM, Jordan Crouse wrote:
> Now that the IOMMU is the master of it's own power we don't need to bring
> up the GPU to do IOMMU operations. This is good because bringing up a6xx
> requires the GMU so calling pm_runtime_get_sync() too early in the process
> gets us into some nasty circular dependency situations.
Thanks for the patch.
While you are removing these calls, we should add pm_runtime calls
to qcom_iommu_map(). That should make qcom_iommu too master of
its power control.
Then we should be good to go with this patch.
>
> Signed-off-by: Jordan Crouse
> ---
> drivers/gpu/drm/msm/msm_iommu.c | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index b23d33622f37..ccd93ac6a4d8 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -40,9 +40,7 @@ static int msm_iommu_attach(struct msm_mmu *mmu, const char
> * const *names,
> 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);
may be just do the following here.
return iommu_attach_device(iommu->domain, mmu->dev);
Rest looks good. So after the change.
Reviewed-by: Vivek Gautam
Best regards
Vivek
>
> return ret;
> }
> @@ -52,9 +50,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 +59,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 < 0);
>
> return (ret == len) ? 0 : -EINVAL;
> @@ -76,9 +70,7 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t
> iova,
> {
> 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.17.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/msm: Remove pm_runtime operations from msm_iommu
On Thu, Jun 14, 2018 at 12:30:35PM +0530, Vivek Gautam wrote: > Hi Jordan, > > On Mon, Jun 11, 2018 at 11:56 PM, Jordan Crouse > wrote: > > Now that the IOMMU is the master of it's own power we don't need to bring > > up the GPU to do IOMMU operations. This is good because bringing up a6xx > > requires the GMU so calling pm_runtime_get_sync() too early in the process > > gets us into some nasty circular dependency situations. > > Thanks for the patch. > While you are removing these calls, we should add pm_runtime calls > to qcom_iommu_map(). That should make qcom_iommu too master of > its power control. > Then we should be good to go with this patch. Right - I told Rob about that in IRC but I should have mentioned it here as well. In order to do this we need to be sure that all of of the possible MMU combinations are covered. > > > > Signed-off-by: Jordan Crouse > > --- > > drivers/gpu/drm/msm/msm_iommu.c | 8 > > 1 file changed, 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_iommu.c > > b/drivers/gpu/drm/msm/msm_iommu.c > > index b23d33622f37..ccd93ac6a4d8 100644 > > --- a/drivers/gpu/drm/msm/msm_iommu.c > > +++ b/drivers/gpu/drm/msm/msm_iommu.c > > @@ -40,9 +40,7 @@ static int msm_iommu_attach(struct msm_mmu *mmu, const > > char * const *names, > > 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); > > may be just do the following here. >return iommu_attach_device(iommu->domain, mmu->dev); I'll do that. Thanks. Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/msm: Remove pm_runtime operations from msm_iommu
On Mon, Jun 11, 2018 at 11:26 AM Jordan Crouse wrote:
>
> Now that the IOMMU is the master of it's own power we don't need to bring
> up the GPU to do IOMMU operations. This is good because bringing up a6xx
> requires the GMU so calling pm_runtime_get_sync() too early in the process
> gets us into some nasty circular dependency situations.
>
> Signed-off-by: Jordan Crouse
Reviewed-by: Kristian H. Kristensen
> ---
> drivers/gpu/drm/msm/msm_iommu.c | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index b23d33622f37..ccd93ac6a4d8 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -40,9 +40,7 @@ static int msm_iommu_attach(struct msm_mmu *mmu, const char
> * const *names,
> 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;
> }
> @@ -52,9 +50,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 +59,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 < 0);
>
> return (ret == len) ? 0 : -EINVAL;
> @@ -76,9 +70,7 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t
> iova,
> {
> 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.17.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm/msm: Remove pm_runtime operations from msm_iommu
Now that the IOMMU is the master of it's own power we don't need to bring
up the GPU to do IOMMU operations. This is good because bringing up a6xx
requires the GMU so calling pm_runtime_get_sync() too early in the process
gets us into some nasty circular dependency situations.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/msm_iommu.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index b23d33622f37..ccd93ac6a4d8 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -40,9 +40,7 @@ static int msm_iommu_attach(struct msm_mmu *mmu, const char *
const *names,
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;
}
@@ -52,9 +50,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 +59,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 < 0);
return (ret == len) ? 0 : -EINVAL;
@@ -76,9 +70,7 @@ static int msm_iommu_unmap(struct msm_mmu *mmu, uint64_t iova,
{
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.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
