[PATCH v2 14/14] drm/v3d: Ensure atomic submissions in v3d_submit_jobs()

[Maíra Canal <mcanal@igalia.com>]

Currently, v3d_submit_jobs() arms and pushes each job one at a time,
wiring dependencies between consecutive jobs after each push. If
drm_sched_job_add_dependency() fails midway, the already-pushed jobs are
scheduler-owned and will be submitted to the GPU for execution, even though
the subsequent jobs won't be submitted.

This breaks the atomicity of the submissions, as only some of the jobs
from a submission would be submitted to the hardware, while the other part
fails.

Restructure v3d_submit_jobs() into three phases: (1) arm all jobs belonging
to a given submission, (2) wire inter-job dependencies, and (3) push all
jobs to the scheduler unconditionally. Phase (2) can fail; on failure,
it marks every armed job finished fence with an error, so that run_job()
callbacks skip hardware execution.

This guarantees that every armed job is always pushed, either to run
or to be skipped, and it also ensures the atomicity of a submission.

Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_sched.c  |  6 ++++
 drivers/gpu/drm/v3d/v3d_submit.c | 76 +++++++++++++++++++++++-----------------
 2 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index cc3212e2cb5d..6e890b8547e5 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -670,6 +670,9 @@ v3d_cpu_job_run(struct drm_sched_job *sched_job)
 	struct v3d_cpu_job *job = to_cpu_job(sched_job);
 	struct v3d_dev *v3d = job->base.v3d;
 
+	if (unlikely(job->base.base.s_fence->finished.error))
+		return NULL;
+
 	if (job->job_type >= ARRAY_SIZE(cpu_job_function)) {
 		drm_dbg(&v3d->drm, "Unknown CPU job: %d\n", job->job_type);
 		return NULL;
@@ -693,6 +696,9 @@ v3d_cache_clean_job_run(struct drm_sched_job *sched_job)
 	struct v3d_job *job = to_v3d_job(sched_job);
 	struct v3d_dev *v3d = job->v3d;
 
+	if (unlikely(job->base.s_fence->finished.error))
+		return NULL;
+
 	v3d_job_start_stats(job);
 
 	v3d_clean_caches(v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index afe54e73a461..11ba3c977e2d 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -292,19 +292,6 @@ v3d_submit_attach_object_fences(struct v3d_submit *submit)
 	}
 }
 
-static void
-v3d_push_job(struct v3d_job *job)
-{
-	drm_sched_job_arm(&job->base);
-
-	job->done_fence = dma_fence_get(&job->base.s_fence->finished);
-
-	/* put by scheduler job completion */
-	kref_get(&job->refcount);
-
-	drm_sched_entity_push_job(&job->base);
-}
-
 static int
 v3d_submit_jobs(struct v3d_submit *submit)
 {
@@ -316,16 +303,23 @@ v3d_submit_jobs(struct v3d_submit *submit)
 	for (int i = 0; i < submit->job_count; i++) {
 		struct v3d_job *job = submit->jobs[i];
 
-		v3d_push_job(job);
+		drm_sched_job_arm(&job->base);
+		job->done_fence = dma_fence_get(&job->base.s_fence->finished);
 
-		if (i + 1 < submit->job_count) {
-			ret = drm_sched_job_add_dependency(&submit->jobs[i + 1]->base,
-							   dma_fence_get(job->done_fence));
-			if (ret)
-				goto err;
-		}
+		/* put by scheduler job completion */
+		kref_get(&job->refcount);
 	}
 
+	for (int i = 0; i + 1 < submit->job_count; i++) {
+		ret = drm_sched_job_add_dependency(&submit->jobs[i + 1]->base,
+						   dma_fence_get(submit->jobs[i]->done_fence));
+		if (ret)
+			goto err;
+	}
+
+	for (int i = 0; i < submit->job_count; i++)
+		drm_sched_entity_push_job(&submit->jobs[i]->base);
+
 	mutex_unlock(&v3d->sched_lock);
 
 	v3d_submit_attach_object_fences(submit);
@@ -334,7 +328,17 @@ v3d_submit_jobs(struct v3d_submit *submit)
 	return 0;
 
 err:
+	/* Mark every armed job as failed so run_job() skips execution */
+	for (int i = 0; i < submit->job_count; i++)
+		dma_fence_set_error(&submit->jobs[i]->base.s_fence->finished, ret);
+
+	for (int i = 0; i < submit->job_count; i++)
+		drm_sched_entity_push_job(&submit->jobs[i]->base);
+
 	mutex_unlock(&v3d->sched_lock);
+
+	v3d_submit_unlock_reservations(submit);
+
 	return ret;
 }
 
@@ -1068,8 +1072,11 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
 		goto fail_unreserve;
 
 	ret = v3d_submit_jobs(&submit);
-	if (ret)
-		goto fail_unreserve;
+	if (ret) {
+		v3d_submit_put_jobs(&submit);
+		v3d_submit_put_post_deps(sync_out, &se);
+		return ret;
+	}
 
 	v3d_submit_process_post_deps(&submit, sync_out, &se);
 	v3d_submit_put_jobs(&submit);
@@ -1167,16 +1174,17 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
 		goto fail;
 
 	ret = v3d_submit_jobs(&submit);
-	if (ret)
-		goto fail_unreserve;
+	if (ret) {
+		v3d_submit_put_jobs(&submit);
+		v3d_submit_put_post_deps(sync_out, &se);
+		return ret;
+	}
 
 	v3d_submit_process_post_deps(&submit, sync_out, &se);
 	v3d_submit_put_jobs(&submit);
 
 	return 0;
 
-fail_unreserve:
-	v3d_submit_unlock_reservations(&submit);
 fail:
 	v3d_submit_cleanup_jobs(&submit);
 	v3d_submit_put_post_deps(sync_out, &se);
@@ -1241,8 +1249,11 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
 		goto fail_unreserve;
 
 	ret = v3d_submit_jobs(&submit);
-	if (ret)
-		goto fail_unreserve;
+	if (ret) {
+		v3d_submit_put_jobs(&submit);
+		v3d_submit_put_post_deps(sync_out, &se);
+		return ret;
+	}
 
 	v3d_submit_process_post_deps(&submit, sync_out, &se);
 	v3d_submit_put_jobs(&submit);
@@ -1349,16 +1360,17 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
 	}
 
 	ret = v3d_submit_jobs(&submit);
-	if (ret)
-		goto fail_unreserve;
+	if (ret) {
+		v3d_submit_put_jobs(&submit);
+		v3d_submit_put_post_deps(NULL, &se);
+		return ret;
+	}
 
 	v3d_submit_process_post_deps(&submit, NULL, &se);
 	v3d_submit_put_jobs(&submit);
 
 	return 0;
 
-fail_unreserve:
-	v3d_submit_unlock_reservations(&submit);
 fail:
 	v3d_submit_cleanup_jobs(&submit);
 	v3d_submit_put_post_deps(NULL, &se);

-- 
2.54.0