[PATCH 3/7] drm/xe/gt: Extract emit_job_sync()

[Lucas De Marchi]

On Fri, Jul 04, 2025 at 11:35:02AM +0100, Tvrtko Ursulin wrote:
>
>On 03/07/2025 23:41, Lucas De Marchi wrote:
>>Both the nop and wa jobs are going through the same boiler plate calls
>>to emit the job with a timeout and handling error for both bb and job.
>>Extract emit_job_sync() so those functions create the bb, handling
>>possible errors and delegate the part about really emitting the job
>>and waiting for its completion.
>>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>---
>>  drivers/gpu/drm/xe/xe_gt.c | 59 +++++++++++++++++++---------------------------
>>  1 file changed, 24 insertions(+), 35 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>>index a926f560f2e36..67425e37c2187 100644
>>--- a/drivers/gpu/drm/xe/xe_gt.c
>>+++ b/drivers/gpu/drm/xe/xe_gt.c
>>@@ -146,30 +146,23 @@ static void xe_gt_disable_host_l2_vram(struct xe_gt *gt)
>>  static void gt_reset_worker(struct work_struct *w);
>>-static int emit_nop_job(struct xe_gt *gt, struct xe_exec_queue *q)
>>+static int emit_job_sync(struct xe_exec_queue *q, struct xe_bb *bb,
>>+			 long timeout_jiffies)
>>  {
>>  	struct xe_sched_job *job;
>>-	struct xe_bb *bb;
>>  	struct dma_fence *fence;
>>  	long timeout;
>>-	bb = xe_bb_new(gt, 4, false);
>>-	if (IS_ERR(bb))
>>-		return PTR_ERR(bb);
>>-
>>  	job = xe_bb_create_job(q, bb);
>>-	if (IS_ERR(job)) {
>>-		xe_bb_free(bb, NULL);
>>+	if (IS_ERR(job))
>>  		return PTR_ERR(job);
>>-	}
>>  	xe_sched_job_arm(job);
>>  	fence = dma_fence_get(&job->drm.s_fence->finished);
>>  	xe_sched_job_push(job);
>>-	timeout = dma_fence_wait_timeout(fence, false, HZ);
>>+	timeout = dma_fence_wait_timeout(fence, false, timeout_jiffies);
>>  	dma_fence_put(fence);
>>-	xe_bb_free(bb, NULL);
>>  	if (timeout < 0)
>>  		return timeout;
>>  	else if (!timeout)
>>@@ -178,17 +171,28 @@ static int emit_nop_job(struct xe_gt *gt, struct xe_exec_queue *q)
>>  	return 0;
>>  }
>>+static int emit_nop_job(struct xe_gt *gt, struct xe_exec_queue *q)
>>+{
>>+	struct xe_bb *bb;
>>+	int ret;
>>+
>>+	bb = xe_bb_new(gt, 4, false);
>>+	if (IS_ERR(bb))
>>+		return PTR_ERR(bb);
>>+
>>+	ret = emit_job_sync(q, bb, HZ);
>>+	xe_bb_free(bb, NULL);
>>+
>>+	return ret;
>>+}
>>+
>>  static int emit_wa_job(struct xe_gt *gt, struct xe_exec_queue *q)
>>  {
>>  	struct xe_reg_sr *sr = &q->hwe->reg_lrc;
>>  	struct xe_reg_sr_entry *entry;
>>+	int count_rmw = 0, count = 0, ret;
>
>Nit - might as well width sort the declarations while touching. Up to you.

it's for very few chars, and  would separate the related
xe_reg_sr{,_entry} variables.... so in this case I'd rather leave it
like that.

>
>>  	unsigned long idx;
>>-	struct xe_sched_job *job;
>>  	struct xe_bb *bb;
>>-	struct dma_fence *fence;
>>-	long timeout;
>>-	int count_rmw = 0;
>>-	int count = 0;
>>  	size_t bb_len;
>>  	/* count RMW registers as those will be handled separately */
>>@@ -199,9 +203,6 @@ static int emit_wa_job(struct xe_gt *gt, struct xe_exec_queue *q)
>>  			++count_rmw;
>>  	}
>>-	if (count || count_rmw)
>>-		xe_gt_dbg(gt, "LRC WA %s save-restore batch\n", sr->name);
>>-
>>  	bb_len = count * 2;
>>  	if (count_rmw)
>>  		bb_len += count_rmw * 20 + 7;
>>@@ -292,25 +293,13 @@ static int emit_wa_job(struct xe_gt *gt, struct xe_exec_queue *q)
>>  	xe_lrc_emit_hwe_state_instructions(q, bb);
>>-	job = xe_bb_create_job(q, bb);
>>-	if (IS_ERR(job)) {
>>-		xe_bb_free(bb, NULL);
>>-		return PTR_ERR(job);
>>-	}
>>+	if (bb->len)
>>+		xe_gt_dbg(gt, "LRC WA %s save-restore batch: %u dw", sr->name, bb->len);
>>-	xe_sched_job_arm(job);
>>-	fence = dma_fence_get(&job->drm.s_fence->finished);
>>-	xe_sched_job_push(job);
>>-
>>-	timeout = dma_fence_wait_timeout(fence, false, HZ);
>>-	dma_fence_put(fence);
>>+	ret = emit_job_sync(q, bb, HZ);
>>  	xe_bb_free(bb, NULL);
>>-	if (timeout < 0)
>>-		return timeout;
>>-	else if (!timeout)
>>-		return -ETIME;
>>-	return 0;
>>+	return ret;
>>  }
>>  int xe_gt_record_default_lrcs(struct xe_gt *gt)
>>
>
>Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>

thanks
Lucas De Marchi

>
>Regards,
>
>Tvrtko
>