Re: [PATCH v3 08/25] drm/xe: Add helpers to access PT ops

[Francois Dugast <francois.dugast@intel.com>]

On Fri, Feb 27, 2026 at 05:34:44PM -0800, Matthew Brost wrote:
> Add helpers to access PT ops, making it easier to shuffle the location of
> the ops structures without requiring widespread code changes.
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Francois Dugast <francois.dugast@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_pt.c | 65 ++++++++++++++++++++++++++------------
>  1 file changed, 45 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 83dacc91b7b3..1f24eff75185 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1881,13 +1881,37 @@ xe_pt_commit_prepare_unbind(struct xe_vma *vma,
>  	}
>  }
>  
> +static struct xe_vm_pgtable_update_op *
> +to_pt_op(struct xe_vm_pgtable_update_ops *pt_update_ops, u32 op_idx)
> +{
> +	return &pt_update_ops->ops[op_idx];
> +}
> +
> +static u32
> +get_current_op(struct xe_vm_pgtable_update_ops *pt_update_ops)
> +{
> +	return pt_update_ops->current_op;
> +}
> +
> +static struct xe_vm_pgtable_update_op *
> +to_current_pt_op(struct xe_vm_pgtable_update_ops *pt_update_ops)
> +{
> +	return to_pt_op(pt_update_ops, get_current_op(pt_update_ops));
> +}
> +
> +static void
> +incr_current_op(struct xe_vm_pgtable_update_ops *pt_update_ops)
> +{
> +	++pt_update_ops->current_op;
> +}
> +
>  static void
>  xe_pt_update_ops_rfence_interval(struct xe_vm_pgtable_update_ops *pt_update_ops,
>  				 u64 start, u64 end)
>  {
>  	u64 last;
> -	u32 current_op = pt_update_ops->current_op;
> -	struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->ops[current_op];
> +	struct xe_vm_pgtable_update_op *pt_op =
> +		to_current_pt_op(pt_update_ops);
>  	int i, level = 0;
>  
>  	for (i = 0; i < pt_op->num_entries; i++) {
> @@ -1922,8 +1946,8 @@ static int bind_op_prepare(struct xe_vm *vm, struct xe_tile *tile,
>  			   struct xe_vm_pgtable_update_ops *pt_update_ops,
>  			   struct xe_vma *vma, bool invalidate_on_bind)
>  {
> -	u32 current_op = pt_update_ops->current_op;
> -	struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->ops[current_op];
> +	struct xe_vm_pgtable_update_op *pt_op =
> +		to_current_pt_op(pt_update_ops);
>  	int err;
>  
>  	xe_tile_assert(tile, !xe_vma_is_cpu_addr_mirror(vma));
> @@ -1952,7 +1976,7 @@ static int bind_op_prepare(struct xe_vm *vm, struct xe_tile *tile,
>  		xe_pt_update_ops_rfence_interval(pt_update_ops,
>  						 xe_vma_start(vma),
>  						 xe_vma_end(vma));
> -		++pt_update_ops->current_op;
> +		incr_current_op(pt_update_ops);
>  		pt_update_ops->needs_svm_lock |= xe_vma_is_userptr(vma);
>  
>  		/*
> @@ -1989,8 +2013,8 @@ static int bind_range_prepare(struct xe_vm *vm, struct xe_tile *tile,
>  			      struct xe_vm_pgtable_update_ops *pt_update_ops,
>  			      struct xe_vma *vma, struct xe_svm_range *range)
>  {
> -	u32 current_op = pt_update_ops->current_op;
> -	struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->ops[current_op];
> +	struct xe_vm_pgtable_update_op *pt_op =
> +		to_current_pt_op(pt_update_ops);
>  	int err;
>  
>  	xe_tile_assert(tile, xe_vma_is_cpu_addr_mirror(vma));
> @@ -2014,7 +2038,7 @@ static int bind_range_prepare(struct xe_vm *vm, struct xe_tile *tile,
>  		xe_pt_update_ops_rfence_interval(pt_update_ops,
>  						 xe_svm_range_start(range),
>  						 xe_svm_range_end(range));
> -		++pt_update_ops->current_op;
> +		incr_current_op(pt_update_ops);
>  		pt_update_ops->needs_svm_lock = true;
>  
>  		pt_op->vma = vma;
> @@ -2032,8 +2056,8 @@ static int unbind_op_prepare(struct xe_tile *tile,
>  			     struct xe_vma *vma)
>  {
>  	struct xe_device *xe = tile_to_xe(tile);
> -	u32 current_op = pt_update_ops->current_op;
> -	struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->ops[current_op];
> +	struct xe_vm_pgtable_update_op *pt_op =
> +		to_current_pt_op(pt_update_ops);
>  	int err;
>  
>  	if (!((vma->tile_present | vma->tile_staged) & BIT(tile->id)))
> @@ -2072,7 +2096,7 @@ static int unbind_op_prepare(struct xe_tile *tile,
>  				pt_op->num_entries, false);
>  	xe_pt_update_ops_rfence_interval(pt_update_ops, xe_vma_start(vma),
>  					 xe_vma_end(vma));
> -	++pt_update_ops->current_op;
> +	incr_current_op(pt_update_ops);
>  	pt_update_ops->needs_svm_lock |= xe_vma_is_userptr(vma);
>  	pt_update_ops->needs_invalidation = true;
>  
> @@ -2112,8 +2136,8 @@ static int unbind_range_prepare(struct xe_vm *vm,
>  				struct xe_vm_pgtable_update_ops *pt_update_ops,
>  				struct xe_svm_range *range)
>  {
> -	u32 current_op = pt_update_ops->current_op;
> -	struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->ops[current_op];
> +	struct xe_vm_pgtable_update_op *pt_op =
> +		to_current_pt_op(pt_update_ops);
>  
>  	if (!(range->tile_present & BIT(tile->id)))
>  		return 0;
> @@ -2134,7 +2158,7 @@ static int unbind_range_prepare(struct xe_vm *vm,
>  				pt_op->num_entries, false);
>  	xe_pt_update_ops_rfence_interval(pt_update_ops, xe_svm_range_start(range),
>  					 xe_svm_range_end(range));
> -	++pt_update_ops->current_op;
> +	incr_current_op(pt_update_ops);
>  	pt_update_ops->needs_svm_lock = true;
>  	pt_update_ops->needs_invalidation |= xe_vm_has_scratch(vm) ||
>  		xe_vm_has_valid_gpu_mapping(tile, range->tile_present,
> @@ -2282,7 +2306,7 @@ int xe_pt_update_ops_prepare(struct xe_tile *tile, struct xe_vma_ops *vops)
>  			return err;
>  	}
>  
> -	xe_tile_assert(tile, pt_update_ops->current_op <=
> +	xe_tile_assert(tile, get_current_op(pt_update_ops) <=
>  		       pt_update_ops->num_ops);
>  
>  #ifdef TEST_VM_OPS_ERROR
> @@ -2515,7 +2539,7 @@ xe_pt_update_ops_run(struct xe_tile *tile, struct xe_vma_ops *vops)
>  	lockdep_assert_held(&vm->lock);
>  	xe_vm_assert_held(vm);
>  
> -	if (!pt_update_ops->current_op) {
> +	if (!get_current_op(pt_update_ops)) {
>  		xe_tile_assert(tile, xe_vm_in_fault_mode(vm));
>  
>  		return dma_fence_get_stub();
> @@ -2583,8 +2607,9 @@ xe_pt_update_ops_run(struct xe_tile *tile, struct xe_vma_ops *vops)
>  	}
>  
>  	/* Point of no return - VM killed if failure after this */
> -	for (i = 0; i < pt_update_ops->current_op; ++i) {
> -		struct xe_vm_pgtable_update_op *pt_op = &pt_update_ops->ops[i];
> +	for (i = 0; i < get_current_op(pt_update_ops); ++i) {
> +		struct xe_vm_pgtable_update_op *pt_op =
> +			to_pt_op(pt_update_ops, i);
>  
>  		xe_pt_commit(pt_op->vma, pt_op->entries,
>  			     pt_op->num_entries, &pt_update_ops->deferred);
> @@ -2708,9 +2733,9 @@ void xe_pt_update_ops_abort(struct xe_tile *tile, struct xe_vma_ops *vops)
>  
>  	for (i = pt_update_ops->num_ops - 1; i >= 0; --i) {
>  		struct xe_vm_pgtable_update_op *pt_op =
> -			&pt_update_ops->ops[i];
> +			to_pt_op(pt_update_ops, i);
>  
> -		if (!pt_op->vma || i >= pt_update_ops->current_op)
> +		if (!pt_op->vma || i >= get_current_op(pt_update_ops))
>  			continue;
>  
>  		if (pt_op->bind)
> -- 
> 2.34.1
>