All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/etnaviv: split fence lock
@ 2022-12-01 17:48 Lucas Stach
  2022-12-01 17:48 ` [PATCH 2/2] drm/etnaviv: convert user fence tracking to XArray Lucas Stach
  2022-12-02  9:41 ` [PATCH 1/2] drm/etnaviv: split fence lock Philipp Zabel
  0 siblings, 2 replies; 4+ messages in thread
From: Lucas Stach @ 2022-12-01 17:48 UTC (permalink / raw)
  To: etnaviv; +Cc: patchwork-lst, kernel, dri-devel, Russell King

The fence lock currently protects two distinct things. It protects the fence
IDR from concurrent inserts and removes and also keeps drm_sched_job_arm and
drm_sched_entity_push_job in one atomic section to guarantee the fence seqno
monotonicity. Split the lock into those two functions.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  4 ++--
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c        |  3 ++-
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h        |  3 ++-
 drivers/gpu/drm/etnaviv/etnaviv_sched.c      | 11 +++++++----
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 1ac916b24891..2337b24b05b0 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -394,9 +394,9 @@ static void submit_cleanup(struct kref *kref)
 
 	if (submit->out_fence) {
 		/* first remove from IDR, so fence can not be found anymore */
-		mutex_lock(&submit->gpu->fence_lock);
+		mutex_lock(&submit->gpu->idr_lock);
 		idr_remove(&submit->gpu->fence_idr, submit->out_fence_id);
-		mutex_unlock(&submit->gpu->fence_lock);
+		mutex_unlock(&submit->gpu->idr_lock);
 		dma_fence_put(submit->out_fence);
 	}
 	kfree(submit->pmrs);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..30d7c1d8d6c0 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1786,7 +1786,8 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
 
 	gpu->dev = &pdev->dev;
 	mutex_init(&gpu->lock);
-	mutex_init(&gpu->fence_lock);
+	mutex_init(&gpu->sched_lock);
+	mutex_init(&gpu->idr_lock);
 
 	/* Map registers: */
 	gpu->mmio = devm_platform_ioremap_resource(pdev, 0);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 85eddd492774..267d8ec97f11 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -99,6 +99,7 @@ struct etnaviv_gpu {
 	struct etnaviv_chip_identity identity;
 	enum etnaviv_sec_mode sec_mode;
 	struct workqueue_struct *wq;
+	struct mutex sched_lock;
 	struct drm_gpu_scheduler sched;
 	bool initialized;
 	bool fe_running;
@@ -116,7 +117,7 @@ struct etnaviv_gpu {
 	u32 idle_mask;
 
 	/* Fencing support */
-	struct mutex fence_lock;
+	struct mutex idr_lock;
 	struct idr fence_idr;
 	u32 next_fence;
 	u32 completed_fence;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 72e2553fbc98..27448431a45c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -97,21 +97,24 @@ static const struct drm_sched_backend_ops etnaviv_sched_ops = {
 
 int etnaviv_sched_push_job(struct etnaviv_gem_submit *submit)
 {
+	struct etnaviv_gpu *gpu = submit->gpu;
 	int ret = 0;
 
 	/*
-	 * Hold the fence lock across the whole operation to avoid jobs being
+	 * Hold the sched lock across the whole operation to avoid jobs being
 	 * pushed out of order with regard to their sched fence seqnos as
 	 * allocated in drm_sched_job_arm.
 	 */
-	mutex_lock(&submit->gpu->fence_lock);
+	mutex_lock(&gpu->sched_lock);
 
 	drm_sched_job_arm(&submit->sched_job);
 
 	submit->out_fence = dma_fence_get(&submit->sched_job.s_fence->finished);
-	submit->out_fence_id = idr_alloc_cyclic(&submit->gpu->fence_idr,
+	mutex_lock(&gpu->idr_lock);
+	submit->out_fence_id = idr_alloc_cyclic(&gpu->fence_idr,
 						submit->out_fence, 0,
 						INT_MAX, GFP_KERNEL);
+	mutex_unlock(&gpu->idr_lock);
 	if (submit->out_fence_id < 0) {
 		drm_sched_job_cleanup(&submit->sched_job);
 		ret = -ENOMEM;
@@ -124,7 +127,7 @@ int etnaviv_sched_push_job(struct etnaviv_gem_submit *submit)
 	drm_sched_entity_push_job(&submit->sched_job);
 
 out_unlock:
-	mutex_unlock(&submit->gpu->fence_lock);
+	mutex_unlock(&gpu->sched_lock);
 
 	return ret;
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] drm/etnaviv: convert user fence tracking to XArray
  2022-12-01 17:48 [PATCH 1/2] drm/etnaviv: split fence lock Lucas Stach
@ 2022-12-01 17:48 ` Lucas Stach
  2022-12-02  9:49   ` Philipp Zabel
  2022-12-02  9:41 ` [PATCH 1/2] drm/etnaviv: split fence lock Philipp Zabel
  1 sibling, 1 reply; 4+ messages in thread
From: Lucas Stach @ 2022-12-01 17:48 UTC (permalink / raw)
  To: etnaviv; +Cc: patchwork-lst, kernel, dri-devel, Russell King

This simplifies the driver code a bit, as XArray already provides
internal locking. IDRs are implemented using XArrays anyways, so
this drops one level of unneeded abstraction.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.h        |  1 +
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  9 +++++----
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c        |  7 +++----
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h        |  4 ++--
 drivers/gpu/drm/etnaviv/etnaviv_sched.c      | 13 +++++--------
 5 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index f32f4771dada..778394e85c3d 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -11,6 +11,7 @@
 #include <linux/sizes.h>
 #include <linux/time64.h>
 #include <linux/types.h>
+#include <linux/xarray.h>
 
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem.h>
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 2337b24b05b0..cc9d8b8f76f0 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -393,10 +393,11 @@ static void submit_cleanup(struct kref *kref)
 	wake_up_all(&submit->gpu->fence_event);
 
 	if (submit->out_fence) {
-		/* first remove from IDR, so fence can not be found anymore */
-		mutex_lock(&submit->gpu->idr_lock);
-		idr_remove(&submit->gpu->fence_idr, submit->out_fence_id);
-		mutex_unlock(&submit->gpu->idr_lock);
+		/*
+		 * Remove from user fence array before dropping the reference,
+		 * so fence can not be found in lookup anymore.
+		 */
+		xa_erase(&submit->gpu->user_fences, submit->out_fence_id);
 		dma_fence_put(submit->out_fence);
 	}
 	kfree(submit->pmrs);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 30d7c1d8d6c0..f7375d9e1716 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1216,7 +1216,7 @@ int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
 	 * pretends we didn't find a fence in that case.
 	 */
 	rcu_read_lock();
-	fence = idr_find(&gpu->fence_idr, id);
+	fence = xa_load(&gpu->user_fences, id);
 	if (fence)
 		fence = dma_fence_get_rcu(fence);
 	rcu_read_unlock();
@@ -1700,7 +1700,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
 
 	gpu->drm = drm;
 	gpu->fence_context = dma_fence_context_alloc(1);
-	idr_init(&gpu->fence_idr);
+	xa_init_flags(&gpu->user_fences, XA_FLAGS_ALLOC);
 	spin_lock_init(&gpu->fence_spinlock);
 
 	INIT_WORK(&gpu->sync_point_work, sync_point_worker);
@@ -1754,7 +1754,7 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master,
 	}
 
 	gpu->drm = NULL;
-	idr_destroy(&gpu->fence_idr);
+	xa_destroy(&gpu->user_fences);
 
 	if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL))
 		thermal_cooling_device_unregister(gpu->cooling);
@@ -1787,7 +1787,6 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
 	gpu->dev = &pdev->dev;
 	mutex_init(&gpu->lock);
 	mutex_init(&gpu->sched_lock);
-	mutex_init(&gpu->idr_lock);
 
 	/* Map registers: */
 	gpu->mmio = devm_platform_ioremap_resource(pdev, 0);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 267d8ec97f11..9b6773051773 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -117,8 +117,8 @@ struct etnaviv_gpu {
 	u32 idle_mask;
 
 	/* Fencing support */
-	struct mutex idr_lock;
-	struct idr fence_idr;
+	struct xarray user_fences;
+	u32 next_user_fence;
 	u32 next_fence;
 	u32 completed_fence;
 	wait_queue_head_t fence_event;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 27448431a45c..39ca04cf2dd5 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -98,7 +98,7 @@ static const struct drm_sched_backend_ops etnaviv_sched_ops = {
 int etnaviv_sched_push_job(struct etnaviv_gem_submit *submit)
 {
 	struct etnaviv_gpu *gpu = submit->gpu;
-	int ret = 0;
+	int ret;
 
 	/*
 	 * Hold the sched lock across the whole operation to avoid jobs being
@@ -110,14 +110,11 @@ int etnaviv_sched_push_job(struct etnaviv_gem_submit *submit)
 	drm_sched_job_arm(&submit->sched_job);
 
 	submit->out_fence = dma_fence_get(&submit->sched_job.s_fence->finished);
-	mutex_lock(&gpu->idr_lock);
-	submit->out_fence_id = idr_alloc_cyclic(&gpu->fence_idr,
-						submit->out_fence, 0,
-						INT_MAX, GFP_KERNEL);
-	mutex_unlock(&gpu->idr_lock);
-	if (submit->out_fence_id < 0) {
+	ret = xa_alloc_cyclic(&gpu->user_fences, &submit->out_fence_id,
+			      submit->out_fence, xa_limit_32b,
+			      &gpu->next_user_fence, GFP_KERNEL);
+	if (ret < 0) {
 		drm_sched_job_cleanup(&submit->sched_job);
-		ret = -ENOMEM;
 		goto out_unlock;
 	}
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] drm/etnaviv: split fence lock
  2022-12-01 17:48 [PATCH 1/2] drm/etnaviv: split fence lock Lucas Stach
  2022-12-01 17:48 ` [PATCH 2/2] drm/etnaviv: convert user fence tracking to XArray Lucas Stach
@ 2022-12-02  9:41 ` Philipp Zabel
  1 sibling, 0 replies; 4+ messages in thread
From: Philipp Zabel @ 2022-12-02  9:41 UTC (permalink / raw)
  To: Lucas Stach; +Cc: etnaviv, dri-devel, patchwork-lst, kernel, Russell King

On Thu, Dec 01, 2022 at 06:48:45PM +0100, Lucas Stach wrote:
> The fence lock currently protects two distinct things. It protects the fence
> IDR from concurrent inserts and removes and also keeps drm_sched_job_arm and
> drm_sched_entity_push_job in one atomic section to guarantee the fence seqno
> monotonicity. Split the lock into those two functions.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] drm/etnaviv: convert user fence tracking to XArray
  2022-12-01 17:48 ` [PATCH 2/2] drm/etnaviv: convert user fence tracking to XArray Lucas Stach
@ 2022-12-02  9:49   ` Philipp Zabel
  0 siblings, 0 replies; 4+ messages in thread
From: Philipp Zabel @ 2022-12-02  9:49 UTC (permalink / raw)
  To: Lucas Stach; +Cc: etnaviv, dri-devel, patchwork-lst, kernel, Russell King

On Thu, Dec 01, 2022 at 06:48:46PM +0100, Lucas Stach wrote:
> This simplifies the driver code a bit, as XArray already provides
> internal locking. IDRs are implemented using XArrays anyways, so
> this drops one level of unneeded abstraction.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-12-02  9:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 17:48 [PATCH 1/2] drm/etnaviv: split fence lock Lucas Stach
2022-12-01 17:48 ` [PATCH 2/2] drm/etnaviv: convert user fence tracking to XArray Lucas Stach
2022-12-02  9:49   ` Philipp Zabel
2022-12-02  9:41 ` [PATCH 1/2] drm/etnaviv: split fence lock Philipp Zabel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.