[PATCH 4/7] drm/panfrost: Don't resume a soft-stopped job after a GPU reset

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@xxxxxxxxx>

panfrost_reset() soft-stops every slot before it resets the GPU. A job
that reaches a task boundary in time reports STOPPED,
panfrost_job_handle_err() moves its jc to JS_TAIL, and
drm_sched_resubmit_jobs() runs it again from there once the GPU has
been reset. But the progress of a stopped job lives in the GPU: the
hardware's update of the job header and the results of the tasks that
completed since the last cache write-back sit in the L2, and
panfrost_device_reset() soft-resets the GPU without cleaning it first.
The resumed job then completes with stale data in the buffers it still
had to write, and no error is reported.

Count the GPU resets and record the count when a job is stopped. A job
that was stopped before the last reset is failed with -ECANCELED in
panfrost_job_run() instead of being resumed, as the Arm driver fails
every atom that was on the hardware when the GPU was reset. A job that
is soft-stopped and resumed without a reset in between is unaffected.

Today the window is small, since panfrost_reset() hardly waits for the
soft-stops (see the next patch), but the next patches make every
soft-stopped job reach STOPPED.

Fixes: 030761e097b4 ("drm/panfrost: Queue jobs on the hardware")
Signed-off-by: Arun Iyengar <aiyengar1990@xxxxxxxxx>
---
 drivers/gpu/drm/panfrost/panfrost_device.h |  5 +++++
 drivers/gpu/drm/panfrost/panfrost_job.c    | 14 ++++++++++++++
 drivers/gpu/drm/panfrost/panfrost_job.h    |  8 ++++++++
 3 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index 63452febb..fd960a436 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -167,6 +167,11 @@ struct panfrost_device {
 		struct work_struct work;
 		struct rw_semaphore lock;
 		atomic_t pending;
+		/* Bumped by every GPU reset. A soft-stopped job may only be
+		 * resumed while this still reads what it did when the job was
+		 * stopped (see panfrost_job_run()).
+		 */
+		unsigned int count;
 	} reset;
 
 	struct mutex shrinker_lock;
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
index 3a46dd61c..e1f3e5d71 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -415,6 +415,17 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job)
 	if (!job->jc)
 		return NULL;
 
+	/* A soft-stopped job carries its progress in the job header and the
+	 * GPU caches, which a GPU reset throws away. Resumed from JS_TAIL after
+	 * a reset it would complete with the results of the tasks since the
+	 * last cache write-back missing, and no error. Fail it instead, as the
+	 * Arm driver fails every atom that was on the hardware at a reset.
+	 */
+	if (job->stopped && job->stop_reset_count != pfdev->reset.count) {
+		dma_fence_set_error(&job->base.s_fence->finished, -ECANCELED);
+		return NULL;
+	}
+
 	fence = panfrost_fence_create(pfdev, slot);
 	if (IS_ERR(fence))
 		return fence;
@@ -475,6 +486,8 @@ static void panfrost_job_handle_err(struct panfrost_device *pfdev,
 		/* Update the job head so we can resume */
 		job->jc = job_read(pfdev, JS_TAIL_LO(js)) |
 			  ((u64)job_read(pfdev, JS_TAIL_HI(js)) << 32);
+		job->stopped = true;
+		job->stop_reset_count = pfdev->reset.count;
 
 		/* The job will be resumed, don't signal the fence */
 		signal_fence = false;
@@ -728,6 +741,7 @@ panfrost_reset(struct panfrost_device *pfdev,
 			panfrost_devfreq_record_idle(&pfdev->pfdevfreq);
 		}
 	}
+	pfdev->reset.count++;
 	memset(pfdev->jobs, 0, sizeof(pfdev->jobs));
 	spin_unlock(&pfdev->js->job_lock);
 
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.h b/drivers/gpu/drm/panfrost/panfrost_job.h
index 3e3b97178..9bbc0c262 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.h
+++ b/drivers/gpu/drm/panfrost/panfrost_job.h
@@ -27,6 +27,14 @@ struct panfrost_job {
 	__u32 requirements;
 	__u32 flush_id;
 
+	/* Set once the job has been soft-stopped: jc then points into a partly
+	 * executed chain whose progress lives in the job header and the GPU
+	 * caches, and is only valid while the GPU has not been reset since
+	 * (pfdev->reset.count at the time of the stop).
+	 */
+	bool stopped;
+	unsigned int stop_reset_count;
+
 	struct panfrost_gem_mapping **mappings;
 	struct drm_gem_object **bos;
 	u32 bo_count;

-- 
2.55.0