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

Francois Dugast
2026-04-07T15:22:54Z

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
>