[PATCH 19/48] staging: etnaviv: switch to per-GPU fence completion implementation

From: Lucas Stach <l.stach@pengutronix.de>
To: Russell King <linux@arm.linux.org.uk>,
	Christian Gmeiner <christian.gmeiner@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: [PATCH 19/48] staging: etnaviv: switch to per-GPU fence completion implementation
Date: Fri, 25 Sep 2015 13:57:31 +0200	[thread overview]
Message-ID: <1443182280-15868-20-git-send-email-l.stach@pengutronix.de> (raw)
In-Reply-To: <1443182280-15868-1-git-send-email-l.stach@pengutronix.de>

From: Russell King <rmk+kernel@arm.linux.org.uk>

Having a global fence completion implementation in the presence of
mutliple GPU cores which can complete out of order is buggy.  Consider
the case of two processes submitting work for two GPU cores:

	GPU A				GPU B
	priv->completed_fence = 0
	gpu->submitted_fence = 0	gpu->submitted_fence = 0
	gpu->retired_fence = 0		gpu->retired_fence = 0

	process A submits work,
	allocated fence 1,
	gpu->submitted_fence = 1,
	priv->next_fence = 2
					process B submits work,
					allocated fence 2,
					gpu->submitted_fence = 2,
					priv->next_fence = 3

					GPU B finishes,
					gpu->retired_fence = 2
					runs work queue,
					priv->completed_fence = 2

At this point, GPU A's buffers have fence 1, and when tested by
etnaviv_wait_fence_interruptable(), it compares fence 1 to the global
priv->completed_fence, and decides that GPU A has also finished its
work.

This is plainly incorrect at this point.

Solve this by moving etnaviv_wait_fence_interruptable() into the GPU
code, and arrange for it to decide whether the fence has completed
based upon the GPUs submitted and retired fence markers.

This allows us to get rid of the global priv->completed_fence, and
change the global fence wait queue to a per-GPU fence wait queue, thus
avoiding disturbing sleepers on other (busy) GPUs.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/staging/etnaviv/etnaviv_drv.c | 64 ++---------------------------------
 drivers/staging/etnaviv/etnaviv_drv.h |  8 +----
 drivers/staging/etnaviv/etnaviv_gem.c |  7 ++--
 drivers/staging/etnaviv/etnaviv_gpu.c | 44 ++++++++++++++++++++++--
 drivers/staging/etnaviv/etnaviv_gpu.h | 12 ++++++-
 5 files changed, 58 insertions(+), 77 deletions(-)

diff --git a/drivers/staging/etnaviv/etnaviv_drv.c b/drivers/staging/etnaviv/etnaviv_drv.c
index 7ec4b224b589..4f97bb667278 100644
--- a/drivers/staging/etnaviv/etnaviv_drv.c
+++ b/drivers/staging/etnaviv/etnaviv_drv.c
@@ -133,8 +133,6 @@ static int etnaviv_load(struct drm_device *dev, unsigned long flags)
 		goto err_wq;
 	}
 
-	init_waitqueue_head(&priv->fence_event);
-
 	INIT_LIST_HEAD(&priv->inactive_list);
 	priv->num_gpus = 0;
 
@@ -310,64 +308,6 @@ static void etnaviv_debugfs_cleanup(struct drm_minor *minor)
 #endif
 
 /*
- * Fences:
- */
-int etnaviv_wait_fence_interruptable(struct drm_device *dev,
-		struct etnaviv_gpu *gpu, uint32_t fence,
-		struct timespec *timeout)
-{
-	struct etnaviv_drm_private *priv = dev->dev_private;
-	int ret;
-
-	if (fence_after(fence, gpu->submitted_fence)) {
-		DRM_ERROR("waiting on invalid fence: %u (of %u)\n",
-				fence, gpu->submitted_fence);
-		return -EINVAL;
-	}
-
-	if (!timeout) {
-		/* no-wait: */
-		ret = fence_completed(dev, fence) ? 0 : -EBUSY;
-	} else {
-		unsigned long timeout_jiffies = timespec_to_jiffies(timeout);
-		unsigned long start_jiffies = jiffies;
-		unsigned long remaining_jiffies;
-
-		if (time_after(start_jiffies, timeout_jiffies))
-			remaining_jiffies = 0;
-		else
-			remaining_jiffies = timeout_jiffies - start_jiffies;
-
-		ret = wait_event_interruptible_timeout(priv->fence_event,
-				fence_completed(dev, fence),
-				remaining_jiffies);
-
-		if (ret == 0) {
-			DBG("timeout waiting for fence: %u (completed: %u)",
-					fence, priv->completed_fence);
-			ret = -ETIMEDOUT;
-		} else if (ret != -ERESTARTSYS) {
-			ret = 0;
-		}
-	}
-
-	return ret;
-}
-
-/* called from workqueue */
-void etnaviv_update_fence(struct drm_device *dev, uint32_t fence)
-{
-	struct etnaviv_drm_private *priv = dev->dev_private;
-
-	mutex_lock(&dev->struct_mutex);
-	if (fence_after(fence, priv->completed_fence))
-		priv->completed_fence = fence;
-	mutex_unlock(&dev->struct_mutex);
-
-	wake_up_all(&priv->fence_event);
-}
-
-/*
  * DRM ioctls:
  */
 
@@ -478,8 +418,8 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
 	if (!gpu)
 		return -ENXIO;
 
-	return etnaviv_wait_fence_interruptable(dev, gpu,
-		args->fence, &TS(args->timeout));
+	return etnaviv_gpu_wait_fence_interruptible(gpu, args->fence,
+						    &TS(args->timeout));
 }
 
 static int etnaviv_ioctl_gem_userptr(struct drm_device *dev, void *data,
diff --git a/drivers/staging/etnaviv/etnaviv_drv.h b/drivers/staging/etnaviv/etnaviv_drv.h
index f7aff4b2562c..98536090e947 100644
--- a/drivers/staging/etnaviv/etnaviv_drv.h
+++ b/drivers/staging/etnaviv/etnaviv_drv.h
@@ -53,8 +53,7 @@ struct etnaviv_drm_private {
 	int num_gpus;
 	struct etnaviv_gpu *gpu[ETNA_MAX_PIPES];
 
-	uint32_t next_fence, completed_fence;
-	wait_queue_head_t fence_event;
+	uint32_t next_fence;
 
 	/* list of GEM objects: */
 	struct list_head inactive_list;
@@ -62,11 +61,6 @@ struct etnaviv_drm_private {
 	struct workqueue_struct *wq;
 };
 
-int etnaviv_wait_fence_interruptable(struct drm_device *dev,
-		struct etnaviv_gpu *gpu, uint32_t fence,
-		struct timespec *timeout);
-void etnaviv_update_fence(struct drm_device *dev, uint32_t fence);
-
 int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		struct drm_file *file);
 
diff --git a/drivers/staging/etnaviv/etnaviv_gem.c b/drivers/staging/etnaviv/etnaviv_gem.c
index b1984a4ac0c5..aaa0f5eaeabd 100644
--- a/drivers/staging/etnaviv/etnaviv_gem.c
+++ b/drivers/staging/etnaviv/etnaviv_gem.c
@@ -429,13 +429,11 @@ void etnaviv_gem_move_to_inactive(struct drm_gem_object *obj)
 int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op,
 		struct timespec *timeout)
 {
-
-	struct drm_device *dev = obj->dev;
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
-
 	int ret = 0;
 
 	if (is_active(etnaviv_obj)) {
+		struct etnaviv_gpu *gpu = etnaviv_obj->gpu;
 		uint32_t fence = 0;
 
 		if (op & ETNA_PREP_READ)
@@ -445,8 +443,7 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op,
 		if (op & ETNA_PREP_NOSYNC)
 			timeout = NULL;
 
-		ret = etnaviv_wait_fence_interruptable(dev, etnaviv_obj->gpu,
-						       fence, timeout);
+		ret = etnaviv_gpu_wait_fence_interruptible(gpu, fence, timeout);
 	}
 
 	/* TODO cache maintenance */
diff --git a/drivers/staging/etnaviv/etnaviv_gpu.c b/drivers/staging/etnaviv/etnaviv_gpu.c
index 2dbf25121a07..39fe69414679 100644
--- a/drivers/staging/etnaviv/etnaviv_gpu.c
+++ b/drivers/staging/etnaviv/etnaviv_gpu.c
@@ -838,8 +838,6 @@ static void retire_worker(struct work_struct *work)
 	struct drm_device *dev = gpu->drm;
 	uint32_t fence = gpu->retired_fence;
 
-	etnaviv_update_fence(gpu->drm, fence);
-
 	mutex_lock(&dev->struct_mutex);
 
 	while (!list_empty(&gpu->active_list)) {
@@ -862,6 +860,8 @@ static void retire_worker(struct work_struct *work)
 	}
 
 	mutex_unlock(&dev->struct_mutex);
+
+	wake_up_all(&gpu->fence_event);
 }
 
 /* call from irq handler to schedule work to retire bo's */
@@ -872,6 +872,45 @@ void etnaviv_gpu_retire(struct etnaviv_gpu *gpu)
 	queue_work(priv->wq, &gpu->retire_work);
 }
 
+int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
+	uint32_t fence, struct timespec *timeout)
+{
+	int ret;
+
+	if (fence_after(fence, gpu->submitted_fence)) {
+		DRM_ERROR("waiting on invalid fence: %u (of %u)\n",
+				fence, gpu->submitted_fence);
+		return -EINVAL;
+	}
+
+	if (!timeout) {
+		/* No timeout was requested: just test for completion */
+		ret = fence_completed(gpu, fence) ? 0 : -EBUSY;
+	} else {
+		unsigned long timeout_jiffies = timespec_to_jiffies(timeout);
+		unsigned long start_jiffies = jiffies;
+		unsigned long remaining_jiffies;
+
+		if (time_after(start_jiffies, timeout_jiffies))
+			remaining_jiffies = 0;
+		else
+			remaining_jiffies = timeout_jiffies - start_jiffies;
+
+		ret = wait_event_interruptible_timeout(gpu->fence_event,
+						fence_completed(gpu, fence),
+						remaining_jiffies);
+		if (ret == 0) {
+			DBG("timeout waiting for fence: %u (completed: %u)",
+					fence, gpu->retired_fence);
+			ret = -ETIMEDOUT;
+		} else if (ret != -ERESTARTSYS) {
+			ret = 0;
+		}
+	}
+
+	return ret;
+}
+
 int etnaviv_gpu_pm_get_sync(struct etnaviv_gpu *gpu)
 {
 	return pm_runtime_get_sync(gpu->dev);
@@ -1095,6 +1134,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
 	INIT_LIST_HEAD(&gpu->active_list);
 	INIT_WORK(&gpu->retire_work, retire_worker);
 	INIT_WORK(&gpu->recover_work, recover_worker);
+	init_waitqueue_head(&gpu->fence_event);
 
 	setup_timer(&gpu->hangcheck_timer, hangcheck_handler,
 			(unsigned long)gpu);
diff --git a/drivers/staging/etnaviv/etnaviv_gpu.h b/drivers/staging/etnaviv/etnaviv_gpu.h
index 67f6097ba576..49f369b5a4b5 100644
--- a/drivers/staging/etnaviv/etnaviv_gpu.h
+++ b/drivers/staging/etnaviv/etnaviv_gpu.h
@@ -105,9 +105,12 @@ struct etnaviv_gpu {
 	struct list_head active_list;
 
 	uint32_t idle_mask;
+	uint32_t last_ring_pos;
+
+	/* Fencing support */
 	uint32_t submitted_fence;
 	uint32_t retired_fence;
-	uint32_t last_ring_pos;
+	wait_queue_head_t fence_event;
 
 	/* worker for handling active-list retiring: */
 	struct work_struct retire_work;
@@ -141,6 +144,11 @@ static inline u32 gpu_read(struct etnaviv_gpu *gpu, u32 reg)
 	return etnaviv_readl(gpu->mmio + reg);
 }
 
+static inline bool fence_completed(struct etnaviv_gpu *gpu, uint32_t fence)
+{
+	return fence_after_eq(gpu->retired_fence, fence);
+}
+
 int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, uint32_t param,
 	uint64_t *value);
 
@@ -151,6 +159,8 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m);
 #endif
 
 void etnaviv_gpu_retire(struct etnaviv_gpu *gpu);
+int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
+	uint32_t fence, struct timespec *timeout);
 int etnaviv_gpu_submit(struct etnaviv_gpu *gpu,
 	struct etnaviv_gem_submit *submit, struct etnaviv_file_private *ctx);
 int etnaviv_gpu_pm_get_sync(struct etnaviv_gpu *gpu);
-- 
2.5.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
