[PATCH v2] drm/imagination: Update the trace point pvr_job_submit_fw()

[PATCH v2] drm/imagination: Update the trace point pvr_job_submit_fw()

[Brajesh Gupta]

Trace point pvr_job_submit_fw() is used to trace job submission to
the FW. Currently it is recorded when a command is written to the Client
circular buffer.

Move trace recording after writing command to the Kernel circular buffer to
better represent command submission to the FW.

Fixes: c1079aebb4de ("drm/imagination: Add support for trace points")
Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>

Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
Changes in v2:
- Updated safer trace point recording for fragment job to handle
Sashiko-bot reported possible use-after-free case.
- Due to depenedency between grometry and fragment jobs, fragment job
will not be scheduled till geometry is scheduled so even V1 solution is safe.
- Link to v1: https://lore.kernel.org/r/20260722-b4-tracepoint-fix-v1-1-f75747fa591a@imgtec.com
---
 drivers/gpu/drm/imagination/pvr_queue.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
index 941c017399fc..fd968d7def66 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.c
+++ b/drivers/gpu/drm/imagination/pvr_queue.c
@@ -726,8 +726,6 @@ static void pvr_queue_submit_job_to_cccb(struct pvr_job *job)
 		cmd->partial_render_geom_frag_fence.value = job->done_fence->seqno - 1;
 	}
 
-	trace_pvr_job_submit_fw(job);
-
 	/* Submit job to FW */
 	pvr_cccb_write_command_with_header(cccb, job->fw_ccb_cmd_type, job->cmd_len, job->cmd,
 					   job->id, job->id);
@@ -758,6 +756,7 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
 	 */
 	if (job->paired_job && job->type == DRM_PVR_JOB_TYPE_FRAGMENT &&
 	    job->done_fence->ops) {
+		trace_pvr_job_submit_fw(job);
 		return dma_fence_get(job->done_fence);
 	}
 
@@ -810,6 +809,7 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
 					pvr_context_get_fw_addr(job->ctx) + queue->ctx_offset,
 					job->hwrt);
 	}
+	trace_pvr_job_submit_fw(job);
 
 	return dma_fence_get(job->done_fence);
 }

---
base-commit: e5320be8a585a9286f231305d0d443d59c24e46c
change-id: 20260722-b4-tracepoint-fix-0fc961d7023d

Best regards,
-- 
Brajesh Gupta <brajesh.gupta@imgtec.com>

Re: [PATCH v2] drm/imagination: Update the trace point pvr_job_submit_fw()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `pvr_job_submit_fw` tracepoint for fragment jobs in paired submissions is emitted asynchronously from actual firmware submission, breaking the tracepoint's intended semantics.
--

--- Patch [1]: [PATCH v2] drm/imagination: Update the trace point pvr_job_submit_fw() ---
commit f1ff23a928ba06505b47c3b88a51019e853a537e
Author: Brajesh Gupta <brajesh.gupta@imgtec.com>
drm/imagination: Update the trace point pvr_job_submit_fw()

This patch moves the trace recording for pvr_job_submit_fw() to occur after
writing the command to the Kernel circular buffer. The intention is to more
accurately represent the exact moment a command is submitted to the firmware.

> diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> index 941c017399fc3..fd968d7def660 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.c
> +++ b/drivers/gpu/drm/imagination/pvr_queue.c
[ ... ]
> @@ -758,6 +756,7 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
>  	 */
>  	if (job->paired_job && job->type == DRM_PVR_JOB_TYPE_FRAGMENT &&
>  	    job->done_fence->ops) {
> +		trace_pvr_job_submit_fw(job);

[Severity: Medium]
For paired submissions, the geometry scheduler thread submits both the
geometry and fragment jobs to the firmware simultaneously in a combined kick.

Since this path executes in the independent fragment scheduler thread, doesn't
emitting the tracepoint here mean it happens asynchronously from the actual
firmware submission?

Because the fragment scheduler thread runs independently, it may wake up and
emit this tracepoint an arbitrary amount of time after the combined hardware
submission (or even after the hardware has finished the job).

Would it better represent the submission time to emit the fragment job's
tracepoint from the geometry job's pvr_queue_run_job() execution, where both
are actually submitted to the firmware via pvr_cccb_send_kccb_combined_kick()?

>  		return dma_fence_get(job->done_fence);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-b4-tracepoint-fix-v2-1-e6a81a348394@imgtec.com?part=1