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

[Tvrtko Ursulin]

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.

>   	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>

Regards,

Tvrtko