public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Alessio Belle <alessio.belle@imgtec.com>
To: Frank Binns <frank.binns@imgtec.com>,
	Matt Coster <matt.coster@imgtec.com>,
	Brajesh Gupta <brajesh.gupta@imgtec.com>,
	"Alexandru Dadu" <alexandru.dadu@imgtec.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	Sumit Semwal <sumit.semwal@linaro.org>,
	Christian König <christian.koenig@amd.com>,
	Boris Brezillon <boris.brezillon@collabora.com>
Cc: <dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
	<linux-media@vger.kernel.org>, <linaro-mm-sig@lists.linaro.org>,
	"Alessio Belle" <alessio.belle@imgtec.com>,
	<stable@vger.kernel.org>
Subject: [PATCH 1/8] drm/imagination: Count paired job fence as dependency in prepare_job()
Date: Mon, 30 Mar 2026 08:56:36 +0100	[thread overview]
Message-ID: <20260330-job-submission-fixes-cleanup-v1-1-7de8c09cef8c@imgtec.com> (raw)
In-Reply-To: <20260330-job-submission-fixes-cleanup-v1-0-7de8c09cef8c@imgtec.com>

The DRM scheduler's prepare_job() callback counts the remaining
non-signaled native dependencies for a job, preventing job submission
until those (plus job data and fence update) can fit in the job queue's
CCCB.

This means checking which dependencies can be waited upon in the
firmware, i.e. whether they are backed by a UFO object, i.e. whether
their drm_sched_fence::parent has been assigned to a
pvr_queue_fence::base fence. That happens when the job owning the fence
is submitted to the firmware.

Paired geometry and fragment jobs are submitted at the same time, which
means the dependency between them can't be checked this way before
submission.

Update job_count_remaining_native_deps() to take into account the
dependency between paired jobs.

This fixes cases where prepare_job() underestimated the space left in
an almost full fragment CCCB, wrongly unblocking run_job(), which then
returned early without writing the full sequence of commands to the
CCCB.

The above lead to kernel warnings such as the following and potentially
job timeouts (depending on waiters on the missing commands):

  [  375.702979] WARNING: drivers/gpu/drm/imagination/pvr_cccb.c:178 at pvr_cccb_write_command_with_header+0x2c4/0x330 [powervr], CPU#1: kworker/u16:3/47
  [  375.703160] Modules linked in:
  [  375.703571] CPU: 1 UID: 0 PID: 47 Comm: kworker/u16:3 Tainted: G        W           7.0.0-rc2-g817eb6b11ad5 #40 PREEMPT
  [  375.703613] Tainted: [W]=WARN
  [  375.703627] Hardware name: Texas Instruments AM625 SK (DT)
  [  375.703645] Workqueue: powervr-sched drm_sched_run_job_work [gpu_sched]
  [  375.703741] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  [  375.703764] pc : pvr_cccb_write_command_with_header+0x2c4/0x330 [powervr]
  [  375.703847] lr : pvr_queue_submit_job_to_cccb+0x578/0xa70 [powervr]
  [  375.703921] sp : ffff800084a97650
  [  375.703934] x29: ffff800084a97740 x28: 0000000000000958 x27: ffff80008565d000
  [  375.703979] x26: 0000000000000030 x25: ffff800084a97680 x24: 0000000000001000
  [  375.704017] x23: ffff800084a97820 x22: 1ffff00010952ecc x21: 0000000000000008
  [  375.704056] x20: 00000000000006a8 x19: ffff00002ff7da88 x18: 0000000000000000
  [  375.704093] x17: 0000000020020000 x16: 0000000000020000 x15: 0000000000000000
  [  375.704132] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
  [  375.704168] x11: 000000000000f2f2 x10: 00000000f3000000 x9 : 00000000f3f3f3f3
  [  375.704206] x8 : 00000000f2f2f200 x7 : ffff700010952ecc x6 : 0000000000000008
  [  375.704243] x5 : 0000000000000000 x4 : 1ffff00010acba00 x3 : 0000000000000000
  [  375.704279] x2 : 0000000000000007 x1 : 0000000000000fff x0 : 000000000000002f
  [  375.704317] Call trace:
  [  375.704331]  pvr_cccb_write_command_with_header+0x2c4/0x330 [powervr] (P)
  [  375.704411]  pvr_queue_submit_job_to_cccb+0x578/0xa70 [powervr]
  [  375.704487]  pvr_queue_run_job+0x3a4/0x990 [powervr]
  [  375.704562]  drm_sched_run_job_work+0x580/0xd48 [gpu_sched]
  [  375.704623]  process_one_work+0x520/0x1288
  [  375.704658]  worker_thread+0x3f0/0xb3c
  [  375.704680]  kthread+0x334/0x3d8
  [  375.704706]  ret_from_fork+0x10/0x20
  [  375.704736] ---[ end trace 0000000000000000 ]---

Fixes: eaf01ee5ba28 ("drm/imagination: Implement job submission and scheduling")
Cc: stable@vger.kernel.org
Signed-off-by: Alessio Belle <alessio.belle@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_queue.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
index dd88949f6194..836feaa0b295 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.c
+++ b/drivers/gpu/drm/imagination/pvr_queue.c
@@ -179,7 +179,7 @@ static const struct dma_fence_ops pvr_queue_job_fence_ops = {
 
 /**
  * to_pvr_queue_job_fence() - Return a pvr_queue_fence object if the fence is
- * backed by a UFO.
+ * already backed by a UFO.
  * @f: The dma_fence to turn into a pvr_queue_fence.
  *
  * Return:
@@ -356,6 +356,15 @@ static u32 job_cmds_size(struct pvr_job *job, u32 ufo_wait_count)
 	       pvr_cccb_get_size_of_cmd_with_hdr(job->cmd_len);
 }
 
+static bool
+is_paired_job_fence(struct dma_fence *fence, struct pvr_job *job)
+{
+	/* This assumes "fence" is one of "job"'s drm_sched_job::dependencies */
+	return job->type == DRM_PVR_JOB_TYPE_FRAGMENT &&
+	       job->paired_job &&
+	       &job->paired_job->base.s_fence->scheduled == fence;
+}
+
 /**
  * job_count_remaining_native_deps() - Count the number of non-signaled native dependencies.
  * @job: Job to operate on.
@@ -371,6 +380,17 @@ static unsigned long job_count_remaining_native_deps(struct pvr_job *job)
 	xa_for_each(&job->base.dependencies, index, fence) {
 		struct pvr_queue_fence *jfence;
 
+		if (is_paired_job_fence(fence, job)) {
+			/*
+			 * A fence between paired jobs won't resolve to a pvr_queue_fence (i.e.
+			 * be backed by a UFO) until the jobs have been submitted, together.
+			 * The submitting code will insert a partial render fence command for this.
+			 */
+			WARN_ON(dma_fence_is_signaled(fence));
+			remaining_count++;
+			continue;
+		}
+
 		jfence = to_pvr_queue_job_fence(fence);
 		if (!jfence)
 			continue;
@@ -630,9 +650,8 @@ static void pvr_queue_submit_job_to_cccb(struct pvr_job *job)
 		if (!jfence)
 			continue;
 
-		/* Skip the partial render fence, we will place it at the end. */
-		if (job->type == DRM_PVR_JOB_TYPE_FRAGMENT && job->paired_job &&
-		    &job->paired_job->base.s_fence->scheduled == fence)
+		/* This fence will be placed last, as partial render fence. */
+		if (is_paired_job_fence(fence, job))
 			continue;
 
 		if (dma_fence_is_signaled(&jfence->base))

-- 
2.43.0


  reply	other threads:[~2026-03-30  7:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-30  7:56 [PATCH 0/8] drm/imagination: Job submission fixes and cleanup Alessio Belle
2026-03-30  7:56 ` Alessio Belle [this message]
2026-03-31  7:33   ` Claude review: drm/imagination: Count paired job fence as dependency in prepare_job() Claude Code Review Bot
2026-03-30  7:56 ` [PATCH 2/8] drm/imagination: Fit paired fragment job in the correct CCCB Alessio Belle
2026-03-31  7:33   ` Claude review: " Claude Code Review Bot
2026-03-30  7:56 ` [PATCH 3/8] drm/imagination: Skip check on paired job fence during job submission Alessio Belle
2026-03-31  7:33   ` Claude review: " Claude Code Review Bot
2026-03-30  7:56 ` [PATCH 4/8] drm/imagination: Rename pvr_queue_fence_is_ufo_backed() to reflect usage Alessio Belle
2026-03-31  7:33   ` Claude review: " Claude Code Review Bot
2026-03-30  7:56 ` [PATCH 5/8] drm/imagination: Rename fence returned by pvr_queue_job_arm() Alessio Belle
2026-03-31  7:33   ` Claude review: " Claude Code Review Bot
2026-03-30  7:56 ` [PATCH 6/8] drm/imagination: Move repeated job fence check to its own function Alessio Belle
2026-03-31  7:33   ` Claude review: " Claude Code Review Bot
2026-03-30  7:56 ` [PATCH 7/8] drm/imagination: Update check to skip prepare_job() for fragment jobs Alessio Belle
2026-03-31  7:33   ` Claude review: " Claude Code Review Bot
2026-03-30  7:56 ` [PATCH 8/8] drm/imagination: Minor improvements to job submission code documentation Alessio Belle
2026-03-31  7:33   ` Claude review: " Claude Code Review Bot
2026-03-31  7:33 ` Claude review: drm/imagination: Job submission fixes and cleanup Claude Code Review Bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260330-job-submission-fixes-cleanup-v1-1-7de8c09cef8c@imgtec.com \
    --to=alessio.belle@imgtec.com \
    --cc=airlied@gmail.com \
    --cc=alexandru.dadu@imgtec.com \
    --cc=boris.brezillon@collabora.com \
    --cc=brajesh.gupta@imgtec.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=frank.binns@imgtec.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=matt.coster@imgtec.com \
    --cc=mripard@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=stable@vger.kernel.org \
    --cc=sumit.semwal@linaro.org \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox