[igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
Ch, Sai Gowtham
sai.gowtham.ch at intel.com
Thu Apr 27 05:49:42 UTC 2023
> -----Original Message-----
> From: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>
> Sent: Thursday, April 27, 2023 11:10 AM
> To: Ch, Sai Gowtham <sai.gowtham.ch at intel.com>; igt-
> dev at lists.freedesktop.org; Kempczynski, Zbigniew
> <zbigniew.kempczynski at intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with
> Xe.
>
>
> On Thu-27-04-2023 01:46 am, sai.gowtham.ch at intel.com wrote:
> > From: Sai Gowtham Ch <sai.gowtham.ch at intel.com>
> >
> > Extending the spin_create implementation and allocator handle support
> > in xe, where it submits dummy work loads to engine. This
> > Implementation is wrapped around vm_bind and unbind as we are supposed to
> do it manually for xe.
> >
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch at intel.com>
> > ---
> > lib/xe/xe_spin.c | 67
> ++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/xe/xe_spin.h | 21 +++++++++++++--
> > 2 files changed, 86 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
> > 856d0ba2..503d440e 100644
> > --- a/lib/xe/xe_spin.c
> > +++ b/lib/xe/xe_spin.c
> > @@ -82,6 +82,73 @@ void xe_spin_end(struct xe_spin *spin)
> > spin->end = 0;
> > }
> >
> > +static xe_spin_t *
> > +xe_spin_create(int fd, struct xe_spin_factory *opt) {
> > + struct drm_xe_engine_class_instance *eci;
> > + size_t bo_size = xe_get_default_alignment(fd);
> > + uint32_t vm, bo, engine, syncobj;
> > + uint64_t ahnd = opt->ahnd, addr;
> > + struct xe_spin *spin;
> > + struct drm_xe_sync sync = {
> > + .flags = DRM_XE_SYNC_SYNCOBJ | DRM_XE_SYNC_SIGNAL,
> > + };
> > + struct drm_xe_exec exec = {
> > + .num_batch_buffer = 1,
> > + .num_syncs = 1,
> > + .syncs = to_user_pointer(&sync),
> > + };
> > +
> > + spin = calloc(1, sizeof(struct xe_spin));
> > + igt_assert(spin);
> > +
> > + vm = xe_vm_create(fd, 0, 0);
> > + bo = xe_bo_create(fd, eci->gt_id, vm, bo_size);
> > + spin = xe_bo_map(fd, bo, 0x1000);
> > + engine = xe_engine_create(fd, vm, eci, 0);
> > + syncobj = syncobj_create(fd, 0);
> > +
> > + if (ahnd)
>
> In i915, ahnd is used for relocations which 0 for XE. Is this ahnd is different than
> i915's?
ahnd here returns allocator handle from GGT.
>
> > + addr = intel_allocator_alloc_with_strategy(ahnd, bo, bo_size, 0,
> > +ALLOC_STRATEGY_LOW_TO_HIGH);
> > +
> > + xe_vm_bind_sync(fd, vm, bo, 0, addr, bo_size);
> > +
> > + xe_spin_init(spin, addr, true);
> > + exec.engine_id = engine;
> > + exec.address = addr;
> > + sync.handle = syncobj;
> > + igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> > +
> > + opt->bo = bo;
> > + opt->vm = vm;
> > + opt->engine = engine;
> > + opt->spin = spin;
> > + opt->syncobj = syncobj;
> > +
> > + return opt->spin;
> > +
> > +}
> > +
> > +xe_spin_t *
> > +__xe_spin_factory(int fd, struct xe_spin_factory *opt) {
> > + return xe_spin_create(fd, opt);
> > +}
> > +
> > +void xe_spin_sync_wait(int fd, struct xe_spin_factory *opt) {
> > + igt_assert(syncobj_wait(fd, &opt->syncobj, 1, INT64_MAX, 0,
> > + NULL));
> > +}
> > +
> > +void xe_spin_free(int fd, struct xe_spin_factory *opt) {
> > + xe_spin_end(opt->spin);
> > + xe_engine_destroy(fd, opt->engine);
> > + xe_vm_destroy(fd, opt->vm);
> > + gem_close(fd, opt->bo);
> > +}
> > +
> > void xe_cork_init(int fd, struct drm_xe_engine_class_instance *hwe,
> > struct xe_cork *cork)
> > {
> > diff --git a/lib/xe/xe_spin.h b/lib/xe/xe_spin.h index
> > 73f9a026..124fdebe 100644
> > --- a/lib/xe/xe_spin.h
> > +++ b/lib/xe/xe_spin.h
> > @@ -15,17 +15,34 @@
> > #include "xe_query.h"
> >
> > /* Mapped GPU object */
> > -struct xe_spin {
> > +struct xe_spin_factory {
> > + uint64_t ahnd;
> > + uint32_t vm;
> > + uint32_t bo;
> > + uint32_t engine;
> > + uint32_t syncobj;
> > + struct xe_spin *spin;
> > +};
> > +
> > +typedef struct xe_spin {
> > uint32_t batch[16];
> > uint64_t pad;
> > uint32_t start;
> > uint32_t end;
> > -};
> > +} xe_spin_t;
> > +
> > +xe_spin_t *
> > +__xe_spin_factory(int fd, struct xe_spin_factory *opt);
> > +
> > +#define xe_spin_new(fd, ...) \
> > + __xe_spin_factory(fd, &((xe_spin_factory *opt){__VA__ARGS}))
>
> So from kms tests, we need to call this helper as below:
>
> if (is_xe_device(fd))
> xe_spin_new();
> else
> igt_spin_new();
>
>
> I am expecting some helper as below, so that we can avoid code churn in test
> level:
> igt_spin_new() {
> if (is_xe_device(fd))
> xe_spin_new();
> else
> i915_spin_new();
> }
Initial Plan was that same, however it is difficult to do that as spin_create()
uses igt_spin_t to collect all necessary information about spinner, Instead adding this check from test
would make sense to me.
> - Bhanu
>
> >
> > void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool preempt);
> > bool xe_spin_started(struct xe_spin *spin);
> > +void xe_spin_sync_wait(int fd, struct xe_spin_factory *opt);
> > void xe_spin_wait_started(struct xe_spin *spin);
> > void xe_spin_end(struct xe_spin *spin);
> > +void xe_spin_free(int fd, struct xe_spin_factory *opt);
> >
> > struct xe_cork {
> > struct xe_spin *spin;
More information about the igt-dev
mailing list