Re: [PATCH v17 5/8] drm/xe: Export xe_emit_aux_table_inv

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Jan 28, 2026 at 10:13:30AM +0000, Tvrtko Ursulin wrote:
> Export the existing AuxCCS invalidation ring buffer programming helper
> which we will need to use to setup the indirect context workaround in the
> next patch.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_ring_ops.c | 67 +++++++++++++++++++++-----------
>  drivers/gpu/drm/xe/xe_ring_ops.h |  3 ++
>  2 files changed, 48 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ring_ops.c b/drivers/gpu/drm/xe/xe_ring_ops.c
> index 9867a316c12b..e6ecd70618c3 100644
> --- a/drivers/gpu/drm/xe/xe_ring_ops.c
> +++ b/drivers/gpu/drm/xe/xe_ring_ops.c
> @@ -12,6 +12,7 @@
>  #include "regs/xe_engine_regs.h"
>  #include "regs/xe_gt_regs.h"
>  #include "xe_exec_queue.h"
> +#include "xe_gt_printk.h"
>  #include "xe_gt_types.h"
>  #include "xe_lrc.h"
>  #include "xe_sched_job.h"
> @@ -48,22 +49,49 @@ static u32 preparser_disable(bool state)
>  	return MI_ARB_CHECK | BIT(8) | state;
>  }
>  
> -static int emit_aux_table_inv(struct xe_gt *gt, struct xe_reg reg,
> -			      u32 *dw, int i)
> +u32 *xe_emit_aux_table_inv(struct xe_hw_engine *hwe, u32 *cmd)

I'd prefer not to export the ring operation helpers directly as
functions, or at least not from this layer, as that opens a huge can of
worms. For example, look at the i915 ring operation helpers and the mess
they turned into—this is exactly what I'd like to avoid.

I see three options:

1. We add a xe_ring_ops_helper layer and stick this function there
2. In the next patch [1] we open code the ring programing there.
3. We add an emit_aux_table_inv vfunc to 'struct xe_ring_ops' and call
   in [1].

I'd strongly lean toward option #3, with three vfuncs: render_compute,
video_decode, and video_encode. We'd also need to split
ring_ops_gen12_video into two distinct structures. This results in more
code, but it's structurally correct, avoids open coding in xe_lrc.c, and
prevents the floodgate that would be opened by exporting ring ops helper
functions.

Matt

[1] https://patchwork.freedesktop.org/patch/701228/?series=160748&rev=1

>  {
> -	dw[i++] = MI_LOAD_REGISTER_IMM | MI_LRI_NUM_REGS(1) | MI_LRI_MMIO_REMAP_EN;
> -	dw[i++] = reg.addr + gt->mmio.adj_offset;
> -	dw[i++] = AUX_INV;
> -	dw[i++] = MI_SEMAPHORE_WAIT_TOKEN |
> -		  MI_SEMAPHORE_REGISTER_POLL |
> -		  MI_SEMAPHORE_POLL |
> -		  MI_SEMAPHORE_SAD_EQ_SDD;
> -	dw[i++] = 0;
> -	dw[i++] = reg.addr + gt->mmio.adj_offset;
> -	dw[i++] = 0;
> -	dw[i++] = 0;
> +	struct xe_gt *gt = hwe->gt;
> +	struct xe_reg reg;
>  
> -	return i;
> +	switch (hwe->class) {
> +	case XE_ENGINE_CLASS_RENDER:
> +	case XE_ENGINE_CLASS_COMPUTE:
> +		reg = CCS_AUX_INV;
> +		break;
> +	case XE_ENGINE_CLASS_VIDEO_DECODE:
> +		reg = VD0_AUX_INV;
> +		break;
> +	case XE_ENGINE_CLASS_VIDEO_ENHANCE:
> +		reg = VE0_AUX_INV;
> +		break;
> +	default:
> +		xe_gt_err_once(gt, "AuxCCS invalidation not implemented!\n");
> +		return cmd;
> +	};
> +
> +	*cmd++ = MI_LOAD_REGISTER_IMM | MI_LRI_NUM_REGS(1) |
> +		 MI_LRI_MMIO_REMAP_EN;
> +	*cmd++ = reg.addr + gt->mmio.adj_offset;
> +	*cmd++ = AUX_INV;
> +	*cmd++ = MI_SEMAPHORE_WAIT_TOKEN | MI_SEMAPHORE_REGISTER_POLL |
> +		 MI_SEMAPHORE_POLL | MI_SEMAPHORE_SAD_EQ_SDD;
> +	*cmd++ = 0;
> +	*cmd++ = reg.addr + gt->mmio.adj_offset;
> +	*cmd++ = 0;
> +	*cmd++ = 0;
> +
> +	return cmd;
> +}
> +
> +static int emit_aux_table_inv(struct xe_hw_engine *hwe, u32 *dw, int i)
> +{
> +	u32 *start, *end;
> +
> +	start = dw + i;
> +	end = xe_emit_aux_table_inv(hwe, start);
> +
> +	return i + (end - start);
>  }
>  
>  static int emit_user_interrupt(u32 *dw, int i)
> @@ -324,7 +352,6 @@ static void __emit_job_gen12_video(struct xe_sched_job *job, struct xe_lrc *lrc,
>  	u32 ppgtt_flag = get_ppgtt_flag(job);
>  	struct xe_gt *gt = job->q->gt;
>  	struct xe_device *xe = gt_to_xe(gt);
> -	bool decode = job->q->class == XE_ENGINE_CLASS_VIDEO_DECODE;
>  
>  	*head = lrc->ring.tail;
>  
> @@ -333,12 +360,8 @@ static void __emit_job_gen12_video(struct xe_sched_job *job, struct xe_lrc *lrc,
>  	dw[i++] = preparser_disable(true);
>  
>  	/* hsdes: 1809175790 */
> -	if (has_aux_ccs(xe)) {
> -		if (decode)
> -			i = emit_aux_table_inv(gt, VD0_AUX_INV, dw, i);
> -		else
> -			i = emit_aux_table_inv(gt, VE0_AUX_INV, dw, i);
> -	}
> +	if (has_aux_ccs(xe))
> +		i = emit_aux_table_inv(job->q->hwe, dw, i);
>  
>  	if (job->ring_ops_flush_tlb)
>  		i = emit_flush_imm_ggtt(xe_lrc_start_seqno_ggtt_addr(lrc),
> @@ -403,7 +426,7 @@ static void __emit_job_gen12_render_compute(struct xe_sched_job *job,
>  
>  	/* hsdes: 1809175790 */
>  	if (aux_ccs)
> -		i = emit_aux_table_inv(gt, CCS_AUX_INV, dw, i);
> +		i = emit_aux_table_inv(job->q->hwe, dw, i);
>  
>  	dw[i++] = preparser_disable(false);
>  
> diff --git a/drivers/gpu/drm/xe/xe_ring_ops.h b/drivers/gpu/drm/xe/xe_ring_ops.h
> index e942735d76a6..5a2d32f9bb25 100644
> --- a/drivers/gpu/drm/xe/xe_ring_ops.h
> +++ b/drivers/gpu/drm/xe/xe_ring_ops.h
> @@ -10,8 +10,11 @@
>  #include "xe_ring_ops_types.h"
>  
>  struct xe_gt;
> +struct xe_hw_engine;
>  
>  const struct xe_ring_ops *
>  xe_ring_ops_get(struct xe_gt *gt, enum xe_engine_class class);
>  
> +u32 *xe_emit_aux_table_inv(struct xe_hw_engine *hwe, u32 *cmd);
> +
>  #endif
> -- 
> 2.52.0
>