[igt-dev] [PATCH i-g-t] intel/xe_exec_compute_mode: Add non-blocking subtest

[igt-dev] [PATCH i-g-t] intel/xe_exec_compute_mode: Add non-blocking subtest

[sai.gowtham.ch]

From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Fill the ring with maximum workload and expecte kernel to return
-EWOULDBLOCK error.

Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 tests/intel/xe_exec_compute_mode.c | 95 ++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/tests/intel/xe_exec_compute_mode.c b/tests/intel/xe_exec_compute_mode.c
index 7d3004d65..56bd6cb49 100644
--- a/tests/intel/xe_exec_compute_mode.c
+++ b/tests/intel/xe_exec_compute_mode.c
@@ -290,6 +290,97 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
 		close(map_fd);
 }
 
+/**
+ * SUBTEST: non-blocking
+ * Description: Fill the ring and check we get expected errors.
+ * Test category: functionality test
+ */
+static void non_block(int fd, int expect)
+{
+	struct drm_xe_sync sync = {
+		.type = DRM_XE_SYNC_TYPE_SYNCOBJ,
+		.flags = DRM_XE_SYNC_FLAG_SIGNAL,
+	};
+
+	struct drm_xe_exec exec = {
+		.num_batch_buffer = 1,
+		.num_syncs = 1,
+		.syncs = to_user_pointer(&sync),
+	};
+	struct {
+		uint32_t batch[16];
+		uint64_t pad;
+		uint32_t data;
+		uint64_t addr;
+	} *data;
+	struct drm_xe_engine *engine;
+	uint32_t vm, exec_queue, syncobj;
+	size_t bo_size;
+	int value = 0x123456;
+	uint64_t addr = 0x100000;
+	uint32_t bo = 0;
+	int b, count, intr, err;
+
+	syncobj = syncobj_create(fd, 0);
+	sync.handle = syncobj;
+
+	vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_ASYNC_DEFAULT |
+			      DRM_XE_VM_CREATE_FLAG_LR_MODE, 0);
+	bo_size = sizeof(*data);
+	bo_size = ALIGN(bo_size + xe_cs_prefetch_size(fd), xe_get_default_alignment(fd));
+
+	engine = xe_engine(fd, 1);
+	bo = xe_bo_create(fd, vm, bo_size, vram_if_possible(fd, engine->instance.gt_id),
+					   DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
+
+	xe_vm_bind_async(fd, vm, engine->instance.gt_id, bo, 0, addr, bo_size, &sync, 1);
+	exec_queue = xe_exec_queue_create(fd, vm, &engine->instance, 0);
+	data = xe_bo_map(fd, bo, bo_size);
+
+	count = 0;
+	intr = 0;
+	do {
+		uint64_t batch_offset = (char *)&(data->batch) - (char *)data;
+		uint64_t batch_addr = addr + batch_offset;
+		uint64_t sdi_offset = (char *) & (data->data) - (char *)data;
+		uint64_t sdi_addr = addr + sdi_offset;
+
+		b = 0;
+		data->batch[b++] = MI_STORE_DWORD_IMM_GEN4;
+		data->batch[b++] = sdi_addr;
+		data->batch[b++] = sdi_addr >> 32;
+		data->batch[b++] = value;
+		data->batch[b++] = MI_BATCH_BUFFER_END;
+		igt_assert(b <= ARRAY_SIZE(data->batch));
+
+		exec.exec_queue_id = exec_queue;
+		exec.address = batch_addr;
+		sync.flags &= DRM_XE_SYNC_FLAG_SIGNAL;
+
+		err = __xe_exec(fd, &exec);
+		igt_assert(syncobj_wait(fd, &sync.handle, 1,
+					INT64_MAX, 0, NULL));
+
+		if (err == -EWOULDBLOCK) {
+			if (intr != count)
+				err = 0;
+			intr = count + 1;
+		}
+		if (err)
+			break;
+		count++;
+	} while (1);
+
+	igt_assert_eq(err, expect);
+
+	syncobj_destroy(fd, syncobj);
+	munmap(data, bo_size);
+	gem_close(fd, bo);
+
+	xe_exec_queue_destroy(fd, exec_queue);
+	xe_vm_destroy(fd, vm);
+}
+
 igt_main
 {
 	struct drm_xe_engine_class_instance *hwe;
@@ -347,6 +438,10 @@ igt_main
 					  s->flags);
 	}
 
+	igt_subtest("non-blocking")
+		non_block(fd, -EWOULDBLOCK);
+
+
 	igt_fixture
 		drm_close_driver(fd);
 }
-- 
2.39.1

[igt-dev] ✗ Fi.CI.BAT: failure for intel/xe_exec_compute_mode: Add non-blocking subtest

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 4366 bytes --]

== Series Details ==

Series: intel/xe_exec_compute_mode: Add non-blocking subtest
URL   : https://patchwork.freedesktop.org/series/127477/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13991 -> IGTPW_10365
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_10365 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_10365, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/index.html

Participating hosts (36 -> 33)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (4): bat-kbl-2 bat-adlp-11 fi-snb-2520m bat-dg1-5 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_10365:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_force_connector_basic@force-connector-state:
    - bat-mtlp-6:         [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13991/bat-mtlp-6/igt@kms_force_connector_basic@force-connector-state.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/bat-mtlp-6/igt@kms_force_connector_basic@force-connector-state.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-adlp-9:         NOTRUN -> [SKIP][3] +3 other tests skip
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/bat-adlp-9/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  
Known issues
------------

  Here are the changes found in IGTPW_10365 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_lmem_swapping@basic:
    - fi-pnv-d510:        NOTRUN -> [SKIP][4] ([fdo#109271]) +32 other tests skip
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/fi-pnv-d510/igt@gem_lmem_swapping@basic.html

  * igt@kms_pm_backlight@basic-brightness@edp-1:
    - bat-rplp-1:         NOTRUN -> [ABORT][5] ([i915#8668])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/bat-rplp-1/igt@kms_pm_backlight@basic-brightness@edp-1.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-cfl-8109u:       [DMESG-FAIL][6] ([i915#5334]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13991/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/fi-cfl-8109u/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_hdmi_inject@inject-audio:
    - fi-kbl-guc:         [FAIL][8] ([IGT#3]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13991/fi-kbl-guc/igt@kms_hdmi_inject@inject-audio.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/fi-kbl-guc/igt@kms_hdmi_inject@inject-audio.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
    - bat-rplp-1:         [ABORT][10] ([i915#8668]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13991/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html

  
  [IGT#3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7626 -> IGTPW_10365

  CI-20190529: 20190529
  CI_DRM_13991: f13e9802d9ba176a81962cfa3aa2799a84418b15 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_10365: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/index.html
  IGT_7626: 154b7288552cd7ed3033f8ef396e88d0bd1b7646 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


Testlist changes
----------------

+igt@xe_exec_compute_mode@non-blocking
-igt@kms_vrr@flip-basic-fastset
-igt@kms_vrr@seamless-rr-switch-drrs
-igt@kms_vrr@seamless-rr-switch-vrr

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/index.html

[-- Attachment #2: Type: text/html, Size: 5250 bytes --]

[igt-dev] ✓ CI.xeBAT: success for intel/xe_exec_compute_mode: Add non-blocking subtest

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2552 bytes --]

== Series Details ==

Series: intel/xe_exec_compute_mode: Add non-blocking subtest
URL   : https://patchwork.freedesktop.org/series/127477/
State : success

== Summary ==

CI Bug Log - changes from XEIGT_7626_BAT -> XEIGTPW_10365_BAT
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (4 -> 4)
------------------------------

  No changes in participating hosts

Known issues
------------

  Here are the changes found in XEIGTPW_10365_BAT that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - bat-adlp-7:         [PASS][1] -> [FAIL][2] ([i915#2346])
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7626/bat-adlp-7/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10365/bat-adlp-7/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - bat-adlp-7:         [PASS][3] -> [FAIL][4] ([Intel XE#480]) +1 other test fail
   [3]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7626/bat-adlp-7/igt@kms_flip@basic-flip-vs-wf_vblank.html
   [4]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10365/bat-adlp-7/igt@kms_flip@basic-flip-vs-wf_vblank.html

  
#### Possible fixes ####

  * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
    - bat-adlp-7:         [FAIL][5] ([i915#2346]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7626/bat-adlp-7/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html
   [6]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10365/bat-adlp-7/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html

  
  [Intel XE#480]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/480
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346


Build changes
-------------

  * IGT: IGT_7626 -> IGTPW_10365
  * Linux: xe-556-d33ebd7511c2b53c5dc006e2daa25214fc8a5921 -> xe-557-668d13abebbbc3812de86be1f8477475e1d90728

  IGTPW_10365: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10365/index.html
  IGT_7626: 154b7288552cd7ed3033f8ef396e88d0bd1b7646 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  xe-556-d33ebd7511c2b53c5dc006e2daa25214fc8a5921: d33ebd7511c2b53c5dc006e2daa25214fc8a5921
  xe-557-668d13abebbbc3812de86be1f8477475e1d90728: 668d13abebbbc3812de86be1f8477475e1d90728

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10365/index.html

[-- Attachment #2: Type: text/html, Size: 3261 bytes --]

Re: [igt-dev] [PATCH i-g-t] intel/xe_exec_compute_mode: Add non-blocking subtest

[Daniel Mrzyglod]

On Thu, Dec 07, 2023 at 12:35:03PM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Fill the ring with maximum workload and expecte kernel to return
> -EWOULDBLOCK error.
> 
LGTM,
Reviewed-by: Daniel Mrzyglod <daniel.t.mrzyglod@intel.com>


> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  tests/intel/xe_exec_compute_mode.c | 95 ++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/tests/intel/xe_exec_compute_mode.c b/tests/intel/xe_exec_compute_mode.c
> index 7d3004d65..56bd6cb49 100644
> --- a/tests/intel/xe_exec_compute_mode.c
> +++ b/tests/intel/xe_exec_compute_mode.c
> @@ -290,6 +290,97 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
>  		close(map_fd);
>  }
>  
> +/**
> + * SUBTEST: non-blocking
> + * Description: Fill the ring and check we get expected errors.
> + * Test category: functionality test
> + */
> +static void non_block(int fd, int expect)
> +{
> +	struct drm_xe_sync sync = {
> +		.type = DRM_XE_SYNC_TYPE_SYNCOBJ,
> +		.flags = DRM_XE_SYNC_FLAG_SIGNAL,
> +	};
> +
> +	struct drm_xe_exec exec = {
> +		.num_batch_buffer = 1,
> +		.num_syncs = 1,
> +		.syncs = to_user_pointer(&sync),
> +	};
> +	struct {
> +		uint32_t batch[16];
> +		uint64_t pad;
> +		uint32_t data;
> +		uint64_t addr;
> +	} *data;
> +	struct drm_xe_engine *engine;
> +	uint32_t vm, exec_queue, syncobj;
> +	size_t bo_size;
> +	int value = 0x123456;
> +	uint64_t addr = 0x100000;
> +	uint32_t bo = 0;
> +	int b, count, intr, err;
> +
> +	syncobj = syncobj_create(fd, 0);
> +	sync.handle = syncobj;
> +
> +	vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_ASYNC_DEFAULT |
> +			      DRM_XE_VM_CREATE_FLAG_LR_MODE, 0);
> +	bo_size = sizeof(*data);
> +	bo_size = ALIGN(bo_size + xe_cs_prefetch_size(fd), xe_get_default_alignment(fd));
> +
> +	engine = xe_engine(fd, 1);
> +	bo = xe_bo_create(fd, vm, bo_size, vram_if_possible(fd, engine->instance.gt_id),
> +					   DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
> +
> +	xe_vm_bind_async(fd, vm, engine->instance.gt_id, bo, 0, addr, bo_size, &sync, 1);
> +	exec_queue = xe_exec_queue_create(fd, vm, &engine->instance, 0);
> +	data = xe_bo_map(fd, bo, bo_size);
> +
> +	count = 0;
> +	intr = 0;
> +	do {
> +		uint64_t batch_offset = (char *)&(data->batch) - (char *)data;
> +		uint64_t batch_addr = addr + batch_offset;
> +		uint64_t sdi_offset = (char *) & (data->data) - (char *)data;
> +		uint64_t sdi_addr = addr + sdi_offset;
> +
> +		b = 0;
> +		data->batch[b++] = MI_STORE_DWORD_IMM_GEN4;
> +		data->batch[b++] = sdi_addr;
> +		data->batch[b++] = sdi_addr >> 32;
> +		data->batch[b++] = value;
> +		data->batch[b++] = MI_BATCH_BUFFER_END;
> +		igt_assert(b <= ARRAY_SIZE(data->batch));
> +
> +		exec.exec_queue_id = exec_queue;
> +		exec.address = batch_addr;
> +		sync.flags &= DRM_XE_SYNC_FLAG_SIGNAL;
> +
> +		err = __xe_exec(fd, &exec);
> +		igt_assert(syncobj_wait(fd, &sync.handle, 1,
> +					INT64_MAX, 0, NULL));
> +
> +		if (err == -EWOULDBLOCK) {
> +			if (intr != count)
> +				err = 0;
> +			intr = count + 1;
> +		}
> +		if (err)
> +			break;
> +		count++;
> +	} while (1);
> +
> +	igt_assert_eq(err, expect);
> +
> +	syncobj_destroy(fd, syncobj);
> +	munmap(data, bo_size);
> +	gem_close(fd, bo);
> +
> +	xe_exec_queue_destroy(fd, exec_queue);
> +	xe_vm_destroy(fd, vm);
> +}
> +
>  igt_main
>  {
>  	struct drm_xe_engine_class_instance *hwe;
> @@ -347,6 +438,10 @@ igt_main
>  					  s->flags);
>  	}
>  
> +	igt_subtest("non-blocking")
> +		non_block(fd, -EWOULDBLOCK);
> +
> +
>  	igt_fixture
>  		drm_close_driver(fd);
>  }
> -- 
> 2.39.1
> 

Re: [igt-dev] [PATCH i-g-t] intel/xe_exec_compute_mode: Add non-blocking subtest

[Matthew Brost]

On Thu, Dec 07, 2023 at 12:35:03PM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Fill the ring with maximum workload and expecte kernel to return
> -EWOULDBLOCK error.
> 
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  tests/intel/xe_exec_compute_mode.c | 95 ++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/tests/intel/xe_exec_compute_mode.c b/tests/intel/xe_exec_compute_mode.c
> index 7d3004d65..56bd6cb49 100644
> --- a/tests/intel/xe_exec_compute_mode.c
> +++ b/tests/intel/xe_exec_compute_mode.c
> @@ -290,6 +290,97 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
>  		close(map_fd);
>  }
>  
> +/**
> + * SUBTEST: non-blocking
> + * Description: Fill the ring and check we get expected errors.
> + * Test category: functionality test
> + */
> +static void non_block(int fd, int expect)
> +{
> +	struct drm_xe_sync sync = {
> +		.type = DRM_XE_SYNC_TYPE_SYNCOBJ,

DRM_XE_SYNC_TYPE_SYNCOBJ this type shouldn't work with LR VMs. Does this
test pass? AFAIK this shouldn't work... (e.g. bind and execs should
return -EOPNOTSUPP here).

> +		.flags = DRM_XE_SYNC_FLAG_SIGNAL,
> +	};
> +
> +	struct drm_xe_exec exec = {
> +		.num_batch_buffer = 1,
> +		.num_syncs = 1,
> +		.syncs = to_user_pointer(&sync),
> +	};
> +	struct {
> +		uint32_t batch[16];
> +		uint64_t pad;
> +		uint32_t data;
> +		uint64_t addr;
> +	} *data;
> +	struct drm_xe_engine *engine;
> +	uint32_t vm, exec_queue, syncobj;
> +	size_t bo_size;
> +	int value = 0x123456;
> +	uint64_t addr = 0x100000;
> +	uint32_t bo = 0;
> +	int b, count, intr, err;
> +
> +	syncobj = syncobj_create(fd, 0);
> +	sync.handle = syncobj;
> +
> +	vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_ASYNC_DEFAULT |
> +			      DRM_XE_VM_CREATE_FLAG_LR_MODE, 0);
> +	bo_size = sizeof(*data);
> +	bo_size = ALIGN(bo_size + xe_cs_prefetch_size(fd), xe_get_default_alignment(fd));
> +
> +	engine = xe_engine(fd, 1);
> +	bo = xe_bo_create(fd, vm, bo_size, vram_if_possible(fd, engine->instance.gt_id),
> +					   DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
> +
> +	xe_vm_bind_async(fd, vm, engine->instance.gt_id, bo, 0, addr, bo_size, &sync, 1);
> +	exec_queue = xe_exec_queue_create(fd, vm, &engine->instance, 0);
> +	data = xe_bo_map(fd, bo, bo_size);
> +
> +	count = 0;
> +	intr = 0;

The below loop doesn't make tons of sense. It would make a lot more
sense to issue a spin batch first and then a bunch of execs until
-EWOULDBLOCK is hit. Then release the spin batch, wait that batch to
complete, issue another exec and verify it works.

Matt

> +	do {
> +		uint64_t batch_offset = (char *)&(data->batch) - (char *)data;
> +		uint64_t batch_addr = addr + batch_offset;
> +		uint64_t sdi_offset = (char *) & (data->data) - (char *)data;
> +		uint64_t sdi_addr = addr + sdi_offset;
> +
> +		b = 0;
> +		data->batch[b++] = MI_STORE_DWORD_IMM_GEN4;
> +		data->batch[b++] = sdi_addr;
> +		data->batch[b++] = sdi_addr >> 32;
> +		data->batch[b++] = value;
> +		data->batch[b++] = MI_BATCH_BUFFER_END;
> +		igt_assert(b <= ARRAY_SIZE(data->batch));
> +
> +		exec.exec_queue_id = exec_queue;
> +		exec.address = batch_addr;
> +		sync.flags &= DRM_XE_SYNC_FLAG_SIGNAL;
> +
> +		err = __xe_exec(fd, &exec);
> +		igt_assert(syncobj_wait(fd, &sync.handle, 1,
> +					INT64_MAX, 0, NULL));
> +
> +		if (err == -EWOULDBLOCK) {
> +			if (intr != count)
> +				err = 0;
> +			intr = count + 1;
> +		}
> +		if (err)
> +			break;
> +		count++;
> +	} while (1);
> +
> +	igt_assert_eq(err, expect);
> +
> +	syncobj_destroy(fd, syncobj);
> +	munmap(data, bo_size);
> +	gem_close(fd, bo);
> +
> +	xe_exec_queue_destroy(fd, exec_queue);
> +	xe_vm_destroy(fd, vm);
> +}
> +
>  igt_main
>  {
>  	struct drm_xe_engine_class_instance *hwe;
> @@ -347,6 +438,10 @@ igt_main
>  					  s->flags);
>  	}
>  
> +	igt_subtest("non-blocking")
> +		non_block(fd, -EWOULDBLOCK);
> +
> +
>  	igt_fixture
>  		drm_close_driver(fd);
>  }
> -- 
> 2.39.1
> 

RE: [igt-dev] [PATCH i-g-t] intel/xe_exec_compute_mode: Add non-blocking subtest

[Dandamudi, Priyanka]

> -----Original Message-----
> From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of
> sai.gowtham.ch@intel.com
> Sent: Thursday, December 7, 2023 12:35 PM
> To: igt-dev@lists.freedesktop.org; Ch, Sai Gowtham
> <sai.gowtham.ch@intel.com>
> Subject: [igt-dev] [PATCH i-g-t] intel/xe_exec_compute_mode: Add non-
> blocking subtest
> 
> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Fill the ring with maximum workload and expecte kernel to return -
> EWOULDBLOCK error.
> 
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  tests/intel/xe_exec_compute_mode.c | 95
> ++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
> 
> diff --git a/tests/intel/xe_exec_compute_mode.c
> b/tests/intel/xe_exec_compute_mode.c
> index 7d3004d65..56bd6cb49 100644
> --- a/tests/intel/xe_exec_compute_mode.c
> +++ b/tests/intel/xe_exec_compute_mode.c
> @@ -290,6 +290,97 @@ test_exec(int fd, struct
> drm_xe_engine_class_instance *eci,
>  		close(map_fd);
>  }
> 
> +/**
> + * SUBTEST: non-blocking
> + * Description: Fill the ring and check we get expected errors.
> + * Test category: functionality test
> + */
> +static void non_block(int fd, int expect) {
> +	struct drm_xe_sync sync = {
> +		.type = DRM_XE_SYNC_TYPE_SYNCOBJ,
> +		.flags = DRM_XE_SYNC_FLAG_SIGNAL,
> +	};
> +
> +	struct drm_xe_exec exec = {
> +		.num_batch_buffer = 1,
> +		.num_syncs = 1,
> +		.syncs = to_user_pointer(&sync),
> +	};
> +	struct {
> +		uint32_t batch[16];
> +		uint64_t pad;
> +		uint32_t data;
> +		uint64_t addr;
> +	} *data;
> +	struct drm_xe_engine *engine;
> +	uint32_t vm, exec_queue, syncobj;
> +	size_t bo_size;
> +	int value = 0x123456;
> +	uint64_t addr = 0x100000;
> +	uint32_t bo = 0;
> +	int b, count, intr, err;
> +
> +	syncobj = syncobj_create(fd, 0);
> +	sync.handle = syncobj;
> +
> +	vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_ASYNC_DEFAULT
> |
> +			      DRM_XE_VM_CREATE_FLAG_LR_MODE, 0);
> +	bo_size = sizeof(*data);
> +	bo_size = ALIGN(bo_size + xe_cs_prefetch_size(fd),
> +xe_get_default_alignment(fd));
> +
> +	engine = xe_engine(fd, 1);
> +	bo = xe_bo_create(fd, vm, bo_size, vram_if_possible(fd, engine-
> >instance.gt_id),
> +
> DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
> +
> +	xe_vm_bind_async(fd, vm, engine->instance.gt_id, bo, 0, addr,
> bo_size, &sync, 1);
> +	exec_queue = xe_exec_queue_create(fd, vm, &engine->instance, 0);
> +	data = xe_bo_map(fd, bo, bo_size);
> +
> +	count = 0;
> +	intr = 0;
> +	do {
> +		uint64_t batch_offset = (char *)&(data->batch) - (char *)data;
> +		uint64_t batch_addr = addr + batch_offset;
> +		uint64_t sdi_offset = (char *) & (data->data) - (char *)data;
> +		uint64_t sdi_addr = addr + sdi_offset;
> +
> +		b = 0;
> +		data->batch[b++] = MI_STORE_DWORD_IMM_GEN4;
No point in declaring data structure above as only one member is being used that is batch, others like addr are declared again separately.
So, just create batch array separately.
It's better to create batch array locally inside the loop as its scope is within the loop.
--Priyanka
> +		data->batch[b++] = sdi_addr;
> +		data->batch[b++] = sdi_addr >> 32;
> +		data->batch[b++] = value;
> +		data->batch[b++] = MI_BATCH_BUFFER_END;
> +		igt_assert(b <= ARRAY_SIZE(data->batch));
> +
> +		exec.exec_queue_id = exec_queue;
> +		exec.address = batch_addr;
> +		sync.flags &= DRM_XE_SYNC_FLAG_SIGNAL;
> +
> +		err = __xe_exec(fd, &exec);
> +		igt_assert(syncobj_wait(fd, &sync.handle, 1,
> +					INT64_MAX, 0, NULL));
> +
> +		if (err == -EWOULDBLOCK) {
> +			if (intr != count)
> +				err = 0;
> +			intr = count + 1;

This logic seems incorrect,  as what is the point in incrementing intr after the if condition as it is not used after it anywhere.
-- Priyanka
> +		}
> +		if (err)
> +			break;
> +		count++;
> +	} while (1);
> +
> +	igt_assert_eq(err, expect);
> +
> +	syncobj_destroy(fd, syncobj);
> +	munmap(data, bo_size);
> +	gem_close(fd, bo);
> +
> +	xe_exec_queue_destroy(fd, exec_queue);
> +	xe_vm_destroy(fd, vm);
> +}
> +
>  igt_main
>  {
>  	struct drm_xe_engine_class_instance *hwe; @@ -347,6 +438,10 @@
> igt_main
>  					  s->flags);
>  	}
> 
> +	igt_subtest("non-blocking")
> +		non_block(fd, -EWOULDBLOCK);
> +
> +
>  	igt_fixture
>  		drm_close_driver(fd);
>  }
> --
> 2.39.1

RE: [igt-dev] [PATCH i-g-t] intel/xe_exec_compute_mode: Add non-blocking subtest

[Ch, Sai Gowtham]

>-----Original Message-----
>From: Brost, Matthew <matthew.brost@intel.com>
>Sent: Wednesday, December 13, 2023 9:24 AM
>To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>
>Cc: igt-dev@lists.freedesktop.org
>Subject: Re: [igt-dev] [PATCH i-g-t] intel/xe_exec_compute_mode: Add non-
>blocking subtest
>
>On Thu, Dec 07, 2023 at 12:35:03PM +0530, sai.gowtham.ch@intel.com wrote:
>> From: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
>>
>> Fill the ring with maximum workload and expecte kernel to return
>> -EWOULDBLOCK error.
>>
>> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
>> ---
>>  tests/intel/xe_exec_compute_mode.c | 95
>> ++++++++++++++++++++++++++++++
>>  1 file changed, 95 insertions(+)
>>
>> diff --git a/tests/intel/xe_exec_compute_mode.c
>> b/tests/intel/xe_exec_compute_mode.c
>> index 7d3004d65..56bd6cb49 100644
>> --- a/tests/intel/xe_exec_compute_mode.c
>> +++ b/tests/intel/xe_exec_compute_mode.c
>> @@ -290,6 +290,97 @@ test_exec(int fd, struct drm_xe_engine_class_instance
>*eci,
>>  		close(map_fd);
>>  }
>>
>> +/**
>> + * SUBTEST: non-blocking
>> + * Description: Fill the ring and check we get expected errors.
>> + * Test category: functionality test
>> + */
>> +static void non_block(int fd, int expect) {
>> +	struct drm_xe_sync sync = {
>> +		.type = DRM_XE_SYNC_TYPE_SYNCOBJ,
>
>DRM_XE_SYNC_TYPE_SYNCOBJ this type shouldn't work with LR VMs. Does this
>test pass? AFAIK this shouldn't work... (e.g. bind and execs should return -
>EOPNOTSUPP here).
>
>> +		.flags = DRM_XE_SYNC_FLAG_SIGNAL,
>> +	};
>> +
>> +	struct drm_xe_exec exec = {
>> +		.num_batch_buffer = 1,
>> +		.num_syncs = 1,
>> +		.syncs = to_user_pointer(&sync),
>> +	};
>> +	struct {
>> +		uint32_t batch[16];
>> +		uint64_t pad;
>> +		uint32_t data;
>> +		uint64_t addr;
>> +	} *data;
>> +	struct drm_xe_engine *engine;
>> +	uint32_t vm, exec_queue, syncobj;
>> +	size_t bo_size;
>> +	int value = 0x123456;
>> +	uint64_t addr = 0x100000;
>> +	uint32_t bo = 0;
>> +	int b, count, intr, err;
>> +
>> +	syncobj = syncobj_create(fd, 0);
>> +	sync.handle = syncobj;
>> +
>> +	vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_ASYNC_DEFAULT |
>> +			      DRM_XE_VM_CREATE_FLAG_LR_MODE, 0);
>> +	bo_size = sizeof(*data);
>> +	bo_size = ALIGN(bo_size + xe_cs_prefetch_size(fd),
>> +xe_get_default_alignment(fd));
>> +
>> +	engine = xe_engine(fd, 1);
>> +	bo = xe_bo_create(fd, vm, bo_size, vram_if_possible(fd, engine-
>>instance.gt_id),
>> +
>DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
>> +
>> +	xe_vm_bind_async(fd, vm, engine->instance.gt_id, bo, 0, addr, bo_size,
>&sync, 1);
>> +	exec_queue = xe_exec_queue_create(fd, vm, &engine->instance, 0);
>> +	data = xe_bo_map(fd, bo, bo_size);
>> +
>> +	count = 0;
>> +	intr = 0;
>
>The below loop doesn't make tons of sense. It would make a lot more sense to
>issue a spin batch first and then a bunch of execs until -EWOULDBLOCK is hit. Then
>release the spin batch, wait that batch to complete, issue another exec and verify
>it works.
>
May be issuing another exec and verify it would make sense to me, However I'm wondering why do we need to issue a spin batch first ? 
Isn't just submitting bunch of exec in a loop till it hits -EWOULDBLOCK wouldn't be enough ?

--
Gowtham
>Matt
>
>> +	do {
>> +		uint64_t batch_offset = (char *)&(data->batch) - (char *)data;
>> +		uint64_t batch_addr = addr + batch_offset;
>> +		uint64_t sdi_offset = (char *) & (data->data) - (char *)data;
>> +		uint64_t sdi_addr = addr + sdi_offset;
>> +
>> +		b = 0;
>> +		data->batch[b++] = MI_STORE_DWORD_IMM_GEN4;
>> +		data->batch[b++] = sdi_addr;
>> +		data->batch[b++] = sdi_addr >> 32;
>> +		data->batch[b++] = value;
>> +		data->batch[b++] = MI_BATCH_BUFFER_END;
>> +		igt_assert(b <= ARRAY_SIZE(data->batch));
>> +
>> +		exec.exec_queue_id = exec_queue;
>> +		exec.address = batch_addr;
>> +		sync.flags &= DRM_XE_SYNC_FLAG_SIGNAL;
>> +
>> +		err = __xe_exec(fd, &exec);
>> +		igt_assert(syncobj_wait(fd, &sync.handle, 1,
>> +					INT64_MAX, 0, NULL));
>> +
>> +		if (err == -EWOULDBLOCK) {
>> +			if (intr != count)
>> +				err = 0;
>> +			intr = count + 1;
>> +		}
>> +		if (err)
>> +			break;
>> +		count++;
>> +	} while (1);
>> +
>> +	igt_assert_eq(err, expect);
>> +
>> +	syncobj_destroy(fd, syncobj);
>> +	munmap(data, bo_size);
>> +	gem_close(fd, bo);
>> +
>> +	xe_exec_queue_destroy(fd, exec_queue);
>> +	xe_vm_destroy(fd, vm);
>> +}
>> +
>>  igt_main
>>  {
>>  	struct drm_xe_engine_class_instance *hwe; @@ -347,6 +438,10 @@
>> igt_main
>>  					  s->flags);
>>  	}
>>
>> +	igt_subtest("non-blocking")
>> +		non_block(fd, -EWOULDBLOCK);
>> +
>> +
>>  	igt_fixture
>>  		drm_close_driver(fd);
>>  }
>> --
>> 2.39.1
>>