diff mbox series

[v3,3/8] drm/panfrost: Remove unnecessary mmu->lock mutex

Message ID 20190826223317.28509-4-robh@kernel.org
State Accepted
Commit e316f08f1abf5f1116118e25dce7bc3e9ab03246
Headers show
Series panfrost: Locking and runtime PM fixes | expand

Commit Message

Rob Herring (Arm) Aug. 26, 2019, 10:33 p.m. UTC 
There's no need to serialize io-pgtable calls and the as_lock is
sufficient to serialize flush operations, so we can remove the per
page table lock.

Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces")
Suggested-by: Robin Murphy <robin.murphy@arm.com>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Rob Herring <robh@kernel.org>
---
v3:
 - new patch

 drivers/gpu/drm/panfrost/panfrost_device.h | 1 -
 drivers/gpu/drm/panfrost/panfrost_mmu.c    | 9 ---------
 2 files changed, 10 deletions(-)

--
2.20.1

Comments

Robin Murphy Aug. 27, 2019, 11 a.m. UTC | #1 
On 26/08/2019 23:33, Rob Herring wrote:
> There's no need to serialize io-pgtable calls and the as_lock is
> sufficient to serialize flush operations, so we can remove the per
> page table lock.

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

> Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces")
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v3:
>   - new patch
> 
>   drivers/gpu/drm/panfrost/panfrost_device.h | 1 -
>   drivers/gpu/drm/panfrost/panfrost_mmu.c    | 9 ---------
>   2 files changed, 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> index f503c566e99f..b7fa08ed3a23 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -104,7 +104,6 @@ struct panfrost_device {
>   struct panfrost_mmu {
>   	struct io_pgtable_cfg pgtbl_cfg;
>   	struct io_pgtable_ops *pgtbl_ops;
> -	struct mutex lock;
>   	int as;
>   	atomic_t as_count;
>   	struct list_head list;
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 842bdd7cf6be..3a8bcfa7e7b6 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -228,8 +228,6 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu,
>   	struct io_pgtable_ops *ops = mmu->pgtbl_ops;
>   	u64 start_iova = iova;
> 
> -	mutex_lock(&mmu->lock);
> -
>   	for_each_sg(sgt->sgl, sgl, sgt->nents, count) {
>   		unsigned long paddr = sg_dma_address(sgl);
>   		size_t len = sg_dma_len(sgl);
> @@ -249,8 +247,6 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu,
>   	mmu_hw_do_operation(pfdev, mmu, start_iova, iova - start_iova,
>   			    AS_COMMAND_FLUSH_PT);
> 
> -	mutex_unlock(&mmu->lock);
> -
>   	return 0;
>   }
> 
> @@ -304,8 +300,6 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo)
>   	if (ret < 0)
>   		return;
> 
> -	mutex_lock(&bo->mmu->lock);
> -
>   	while (unmapped_len < len) {
>   		size_t unmapped_page;
>   		size_t pgsize = get_pgsize(iova, len - unmapped_len);
> @@ -321,8 +315,6 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo)
>   	mmu_hw_do_operation(pfdev, bo->mmu, bo->node.start << PAGE_SHIFT,
>   			    bo->node.size << PAGE_SHIFT, AS_COMMAND_FLUSH_PT);
> 
> -	mutex_unlock(&bo->mmu->lock);
> -
>   	pm_runtime_mark_last_busy(pfdev->dev);
>   	pm_runtime_put_autosuspend(pfdev->dev);
>   	bo->is_mapped = false;
> @@ -356,7 +348,6 @@ int panfrost_mmu_pgtable_alloc(struct panfrost_file_priv *priv)
>   	struct panfrost_mmu *mmu = &priv->mmu;
>   	struct panfrost_device *pfdev = priv->pfdev;
> 
> -	mutex_init(&mmu->lock);
>   	INIT_LIST_HEAD(&mmu->list);
>   	mmu->as = -1;
> 
> --
> 2.20.1
>
Steven Price Aug. 28, 2019, 10:42 a.m. UTC | #2 
On 26/08/2019 23:33, Rob Herring wrote:
> There's no need to serialize io-pgtable calls and the as_lock is
> sufficient to serialize flush operations, so we can remove the per
> page table lock.
> 
> Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces")
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Rob Herring <robh@kernel.org>

Reviewed-by: Steven Price <steven.price@arm.com>

Steve

> ---
> v3:
>  - new patch
> 
>  drivers/gpu/drm/panfrost/panfrost_device.h | 1 -
>  drivers/gpu/drm/panfrost/panfrost_mmu.c    | 9 ---------
>  2 files changed, 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> index f503c566e99f..b7fa08ed3a23 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -104,7 +104,6 @@ struct panfrost_device {
>  struct panfrost_mmu {
>  	struct io_pgtable_cfg pgtbl_cfg;
>  	struct io_pgtable_ops *pgtbl_ops;
> -	struct mutex lock;
>  	int as;
>  	atomic_t as_count;
>  	struct list_head list;
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 842bdd7cf6be..3a8bcfa7e7b6 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -228,8 +228,6 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu,
>  	struct io_pgtable_ops *ops = mmu->pgtbl_ops;
>  	u64 start_iova = iova;
> 
> -	mutex_lock(&mmu->lock);
> -
>  	for_each_sg(sgt->sgl, sgl, sgt->nents, count) {
>  		unsigned long paddr = sg_dma_address(sgl);
>  		size_t len = sg_dma_len(sgl);
> @@ -249,8 +247,6 @@ static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu,
>  	mmu_hw_do_operation(pfdev, mmu, start_iova, iova - start_iova,
>  			    AS_COMMAND_FLUSH_PT);
> 
> -	mutex_unlock(&mmu->lock);
> -
>  	return 0;
>  }
> 
> @@ -304,8 +300,6 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo)
>  	if (ret < 0)
>  		return;
> 
> -	mutex_lock(&bo->mmu->lock);
> -
>  	while (unmapped_len < len) {
>  		size_t unmapped_page;
>  		size_t pgsize = get_pgsize(iova, len - unmapped_len);
> @@ -321,8 +315,6 @@ void panfrost_mmu_unmap(struct panfrost_gem_object *bo)
>  	mmu_hw_do_operation(pfdev, bo->mmu, bo->node.start << PAGE_SHIFT,
>  			    bo->node.size << PAGE_SHIFT, AS_COMMAND_FLUSH_PT);
> 
> -	mutex_unlock(&bo->mmu->lock);
> -
>  	pm_runtime_mark_last_busy(pfdev->dev);
>  	pm_runtime_put_autosuspend(pfdev->dev);
>  	bo->is_mapped = false;
> @@ -356,7 +348,6 @@ int panfrost_mmu_pgtable_alloc(struct panfrost_file_priv *priv)
>  	struct panfrost_mmu *mmu = &priv->mmu;
>  	struct panfrost_device *pfdev = priv->pfdev;
> 
> -	mutex_init(&mmu->lock);
>  	INIT_LIST_HEAD(&mmu->list);
>  	mmu->as = -1;
> 
> --
> 2.20.1
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index f503c566e99f..b7fa08ed3a23 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -104,7 +104,6 @@  struct panfrost_device {
 struct panfrost_mmu {
 	struct io_pgtable_cfg pgtbl_cfg;
 	struct io_pgtable_ops *pgtbl_ops;
-	struct mutex lock;
 	int as;
 	atomic_t as_count;
 	struct list_head list;
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index 842bdd7cf6be..3a8bcfa7e7b6 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -228,8 +228,6 @@  static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu,
 	struct io_pgtable_ops *ops = mmu->pgtbl_ops;
 	u64 start_iova = iova;

-	mutex_lock(&mmu->lock);
-
 	for_each_sg(sgt->sgl, sgl, sgt->nents, count) {
 		unsigned long paddr = sg_dma_address(sgl);
 		size_t len = sg_dma_len(sgl);
@@ -249,8 +247,6 @@  static int mmu_map_sg(struct panfrost_device *pfdev, struct panfrost_mmu *mmu,
 	mmu_hw_do_operation(pfdev, mmu, start_iova, iova - start_iova,
 			    AS_COMMAND_FLUSH_PT);

-	mutex_unlock(&mmu->lock);
-
 	return 0;
 }

@@ -304,8 +300,6 @@  void panfrost_mmu_unmap(struct panfrost_gem_object *bo)
 	if (ret < 0)
 		return;

-	mutex_lock(&bo->mmu->lock);
-
 	while (unmapped_len < len) {
 		size_t unmapped_page;
 		size_t pgsize = get_pgsize(iova, len - unmapped_len);
@@ -321,8 +315,6 @@  void panfrost_mmu_unmap(struct panfrost_gem_object *bo)
 	mmu_hw_do_operation(pfdev, bo->mmu, bo->node.start << PAGE_SHIFT,
 			    bo->node.size << PAGE_SHIFT, AS_COMMAND_FLUSH_PT);

-	mutex_unlock(&bo->mmu->lock);
-
 	pm_runtime_mark_last_busy(pfdev->dev);
 	pm_runtime_put_autosuspend(pfdev->dev);
 	bo->is_mapped = false;
@@ -356,7 +348,6 @@  int panfrost_mmu_pgtable_alloc(struct panfrost_file_priv *priv)
 	struct panfrost_mmu *mmu = &priv->mmu;
 	struct panfrost_device *pfdev = priv->pfdev;

-	mutex_init(&mmu->lock);
 	INIT_LIST_HEAD(&mmu->list);
 	mmu->as = -1;