public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Francois Dugast <francois.dugast@intel.com>
To: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
Cc: <igt-dev@lists.freedesktop.org>, Nishit Sharma <nishit.sharma@intel.com>
Subject: Re: [PATCH i-g-t v7 4/6] tests/xe_compute: Extend compute test to cover user passed buffers
Date: Mon, 9 Mar 2026 13:54:11 +0100	[thread overview]
Message-ID: <aa7C8wf1kuZyBMBp@fdugast-desk> (raw)
In-Reply-To: <o7lhuimythzhzjqdadjylmgh3pw7e36tsbr5qwiue37hiutr6l@se3agvt6zgam>

On Mon, Mar 09, 2026 at 12:14:01PM +0100, Zbigniew Kempczyński wrote:
> On Mon, Mar 09, 2026 at 11:53:05AM +0100, Francois Dugast wrote:
> > Hi,
> > 
> > On Fri, Mar 06, 2026 at 10:14:35AM +0100, Zbigniew Kempczyński wrote:
> > > Compute library allows user to pass its own buffers. They may be
> > > normal buffer objects or system allocated buffers (SVM).
> > > 
> > > Verify input and output buffers added to user_execenv are properly
> > > handled and data within are fully on user control. Execute compute
> > > square on these buffers and check data are correct regardless
> > > allocation strategy.
> > > 
> > > Signed-off-by: Nishit Sharma <nishit.sharma@intel.com>
> > > Cc: Francois Dugast <francois.dugast@intel.com>
> > > Acked-by: Nishit Sharma <nishit.sharma@intel.com>
> > > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > > ---
> > >  tests/intel/xe_compute.c | 150 +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 150 insertions(+)
> > > 
> > > diff --git a/tests/intel/xe_compute.c b/tests/intel/xe_compute.c
> > > index 310093fc56..e5367b4c10 100644
> > > --- a/tests/intel/xe_compute.c
> > > +++ b/tests/intel/xe_compute.c
> > > @@ -510,6 +510,144 @@ test_compute_square(int fd)
> > >  		      "GPU not supported\n");
> > >  }
> > >  
> > > +/**
> > > + * SUBTEST: compute-square-userenv
> > > + * Mega feature: Compute
> > > + * Sub-category: compute tests
> > > + * Functionality: OpenCL kernel
> > > + * Description:
> > > + *	Run an openCL Kernel that returns output[i] = input[i] * input[i],
> > > + *	taking buffers from userenv.
> > > + *
> > > + * SUBTEST: compute-square-userenv-isvm
> > > + * Mega feature: Compute
> > > + * Sub-category: compute tests
> > > + * Functionality: OpenCL kernel
> > > + * Description:
> > > + *	Run an openCL Kernel that returns output[i] = input[i] * input[i],
> > > + *	taking buffers from userenv where input is svm buffer.
> > > + *
> > > + * SUBTEST: compute-square-userenv-osvm
> > > + * Mega feature: Compute
> > > + * Sub-category: compute tests
> > > + * Functionality: OpenCL kernel
> > > + * Description:
> > > + *	Run an openCL Kernel that returns output[i] = input[i] * input[i],
> > > + *	taking buffers from userenv where output buffer is svm buffer.
> > > + *
> > > + * SUBTEST: compute-square-userenv-iosvm
> > > + * Mega feature: Compute
> > > + * Sub-category: compute tests
> > > + * Functionality: OpenCL kernel
> > > + * Description:
> > > + *	Run an openCL Kernel that returns output[i] = input[i] * input[i],
> > > + *	taking buffers from userenv where input and output buffers are svm buffer.
> > > + */
> > > +
> > > +#define INPUT_IN_SVM		(1 << 0)
> > > +#define OUTPUT_IN_SVM		(1 << 1)
> > > +#define INPUT_BO_ADDR		0x30000000
> > > +#define OUTPUT_BO_ADDR		0x31000000
> > > +#define USER_FENCE_VALUE	0xdeadbeefdeadbeefull
> > > +#define FIVE_SEC		(5LL * NSEC_PER_SEC)
> > > +
> > > +#define bind_system_allocator(__sync, __num_sync)			\
> > > +	__xe_vm_bind_assert(fd, vm, 0,					\
> > > +			    0, 0, 0, 0x1ull << va_bits,			\
> > > +			    DRM_XE_VM_BIND_OP_MAP,			\
> > > +			    DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR,	\
> > > +			    (__sync), (__num_sync), 0, 0)
> > 
> > Not sure about the reason for moving system allocator tests outside of
> > xe_exec_system_allocator.c but if we go this way then this ^ should be
> > moved to libs as helper instead of being duplicated.
> 
> Because it on one leg is in xe_compute and second one on
> xe_exec_system_allocator. Tests here exercises both kind of buffers
> (bo/svm) and their input/output variations passed in userenv.
> I picked xe_compute as imo it matches better.

Ok, agreed.

> 
> Regarding macro - yes, this is copy/paste and depends implicitly
> on fd, vm, va_bits. May this be follow up series or you want to
> refactor it now?

Follow-up is fine, I just want to prevent that more tests follow this
pattern and we end up with duplications all over the place.

> 
> > 
> > > +
> > > +static void
> > > +test_compute_square_userenv(int fd, uint32_t flags)
> > > +{
> > > +	struct user_execenv env = {};
> > > +	uint32_t input_bo, output_bo, vm, size = SZ_4K;
> > > +	int va_bits = xe_va_bits(fd);
> > > +	float *input, *output;
> > > +	int i;
> > > +
> > > +	size = ALIGN(size, xe_get_default_alignment(fd));
> > > +	env.array_size = size / sizeof(float);
> > > +
> > > +	vm = env.vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_LR_MODE |
> > > +				   DRM_XE_VM_CREATE_FLAG_FAULT_MODE, 0);
> > > +
> > > +	if ((flags & INPUT_IN_SVM) || (flags & OUTPUT_IN_SVM)) {
> > > +		struct drm_xe_sync sync = {
> > > +			.type = DRM_XE_SYNC_TYPE_USER_FENCE,
> > > +			.flags = DRM_XE_SYNC_FLAG_SIGNAL,
> > > +			.timeline_value = USER_FENCE_VALUE,
> > > +		};
> > > +		struct bo_sync {
> > > +			uint64_t sync;
> > > +		} *bo_sync;
> > > +
> > > +		bo_sync = aligned_alloc(xe_get_default_alignment(fd), sizeof(*bo_sync));
> > > +		igt_assert(bo_sync);
> > > +		sync.addr = to_user_pointer(&bo_sync->sync);
> > > +		bind_system_allocator(&sync, 1);
> > > +		xe_wait_ufence(fd, &bo_sync->sync, USER_FENCE_VALUE, 0, FIVE_SEC);
> > > +		free(bo_sync);
> > > +	}
> > > +
> > > +	if (flags & INPUT_IN_SVM) {
> > > +		input = aligned_alloc(xe_get_default_alignment(fd), size);
> > > +		env.input_addr = to_user_pointer(input);
> > > +
> > > +	} else {
> > > +		input_bo = xe_bo_create(fd, env.vm, size, vram_if_possible(fd, 0),
> > > +					DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
> > > +		input = xe_bo_map(fd, input_bo, size);
> > > +		env.input_bo = input_bo;
> > > +		env.input_addr = INPUT_BO_ADDR;
> > > +	}
> > > +
> > > +	if (flags & OUTPUT_IN_SVM) {
> > > +		output = aligned_alloc(xe_get_default_alignment(fd), size);
> > > +		env.output_addr = to_user_pointer(output);
> > > +	} else {
> > > +		output_bo = xe_bo_create(fd, env.vm, size, vram_if_possible(fd, 0),
> > > +					 DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
> > > +		output = xe_bo_map(fd, output_bo, size);
> > > +		env.output_bo = output_bo;
> > > +		env.output_addr = OUTPUT_BO_ADDR;
> > > +	}
> > > +
> > > +	env.loop_count = env.array_size / 2;
> > > +	env.skip_results_check = true;
> > > +
> > > +	for (i = 0; i < env.array_size; i++)
> > > +		input[i] = (float)i + 2.0f;
> > > +
> > > +	run_intel_compute_kernel(fd, &env, EXECENV_PREF_SYSTEM);
> > > +
> > > +	for (i = 0; i < env.loop_count; i++) {
> > > +		float expected_output = input[i] * input[i];
> > > +
> > > +		if (output[i] != expected_output || output[i] == 0.0f)
> > > +			igt_debug("[%4d] input:%f output:%f expected_output:%f\n",
> > > +				  i, input[i], output[i], expected_output);
> > > +		igt_assert_eq_double(output[i], expected_output);
> > > +	}
> > 
> > This seems like a copy/paste from bo_check_square(). Besides, a few lines
> > above we set:
> > 
> >     env.skip_results_check = true
> > 
> > to skip the result check, which seems contradictory. Is there a reason for
> > this duplication?
> 
> Results check assumes square what if we provide own kernel might not
> be true. And in the test I show how to control input/output set/check
> from userenv side.

Please as comment the reason your test does not rely on bo_check_square().

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

> 
> --
> Zbigniew
> 
> > 
> > Thanks,
> > Francois
> > 
> > > +
> > > +	if (flags & INPUT_IN_SVM) {
> > > +		free(input);
> > > +	} else {
> > > +		munmap(input, size);
> > > +		gem_close(fd, input_bo);
> > > +	}
> > > +
> > > +	if (flags & OUTPUT_IN_SVM) {
> > > +		free(output);
> > > +	} else {
> > > +		munmap(output, size);
> > > +		gem_close(fd, output_bo);
> > > +	}
> > > +
> > > +	xe_vm_destroy(fd, env.vm);
> > > +}
> > > +
> > >  int igt_main()
> > >  {
> > >  	int xe, ccs_mode[4];
> > > @@ -525,6 +663,18 @@ int igt_main()
> > >  	igt_subtest("compute-square")
> > >  		test_compute_square(xe);
> > >  
> > > +	igt_subtest("compute-square-userenv")
> > > +		test_compute_square_userenv(xe, 0);
> > > +
> > > +	igt_subtest("compute-square-userenv-isvm")
> > > +		test_compute_square_userenv(xe, INPUT_IN_SVM);
> > > +
> > > +	igt_subtest("compute-square-userenv-osvm")
> > > +		test_compute_square_userenv(xe, OUTPUT_IN_SVM);
> > > +
> > > +	igt_subtest("compute-square-userenv-iosvm")
> > > +		test_compute_square_userenv(xe, INPUT_IN_SVM | OUTPUT_IN_SVM);
> > > +
> > >  	igt_fixture()
> > >  		drm_close_driver(xe);
> > >  
> > > -- 
> > > 2.43.0
> > > 

  reply	other threads:[~2026-03-09 12:54 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-06  9:14 [PATCH i-g-t v7 0/6] Extend compute userenv to support user passed buffers Zbigniew Kempczyński
2026-03-06  9:14 ` [PATCH i-g-t v7 1/6] lib/intel_compute: Add types for input and output buffers Zbigniew Kempczyński
2026-03-06  9:14 ` [PATCH i-g-t v7 2/6] lib/intel_compute: Extend userenv by adding input and output bos Zbigniew Kempczyński
2026-03-06  9:14 ` [PATCH i-g-t v7 3/6] lib/intel_compute: Enhance Xe3p pipeline to accept user data Zbigniew Kempczyński
2026-03-06  9:14 ` [PATCH i-g-t v7 4/6] tests/xe_compute: Extend compute test to cover user passed buffers Zbigniew Kempczyński
2026-03-09 10:53   ` Francois Dugast
2026-03-09 11:14     ` Zbigniew Kempczyński
2026-03-09 12:54       ` Francois Dugast [this message]
2026-03-06  9:14 ` [PATCH i-g-t v7 5/6] tests/xe_compute: Use appropriate feature in compute-square tests Zbigniew Kempczyński
2026-03-06  9:14 ` [PATCH i-g-t v7 6/6] tests/xe_compute: Change shader and local max x on PVC Zbigniew Kempczyński
2026-03-10 15:11   ` Hajda, Andrzej
2026-03-06 11:30 ` ✓ i915.CI.BAT: success for Extend compute userenv to support user passed buffers (rev7) Patchwork
2026-03-06 12:07 ` ✗ Xe.CI.BAT: failure " Patchwork
2026-03-06 14:06 ` ✓ Xe.CI.BAT: success for Extend compute userenv to support user passed buffers (rev8) Patchwork
2026-03-06 14:22 ` ✓ i915.CI.BAT: " Patchwork
2026-03-07 12:10 ` ✗ i915.CI.Full: failure for Extend compute userenv to support user passed buffers (rev7) Patchwork
2026-03-07 13:08 ` ✗ Xe.CI.FULL: " Patchwork
2026-03-07 14:51 ` ✗ Xe.CI.FULL: failure for Extend compute userenv to support user passed buffers (rev8) Patchwork
2026-03-07 15:13 ` ✗ i915.CI.Full: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aa7C8wf1kuZyBMBp@fdugast-desk \
    --to=francois.dugast@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=nishit.sharma@intel.com \
    --cc=zbigniew.kempczynski@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox