Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
@ 2023-04-24 11:54 sai.gowtham.ch
  2023-04-24 12:47 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: sai.gowtham.ch @ 2023-04-24 11:54 UTC (permalink / raw)
  To: igt-dev, zbigniew.kempczynski, sai.gowtham.ch

From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 lib/xe/xe_spin.c | 39 +++++++++++++++++++++++++++++++++++++++
 lib/xe/xe_spin.h |  9 +++++++++
 2 files changed, 48 insertions(+)

diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
index 856d0ba2..e0629419 100644
--- a/lib/xe/xe_spin.c
+++ b/lib/xe/xe_spin.c
@@ -82,6 +82,45 @@ void xe_spin_end(struct xe_spin *spin)
 	spin->end = 0;
 }
 
+void xe_spin_new(int fd, struct xe_spin *spin, struct drm_xe_engine_class_instance *eci)
+{
+	size_t bo_size = xe_get_default_alignment(fd);
+	uint32_t syncobj;
+	uint64_t ahnd = spin->ahnd, addr;
+	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->vm = xe_vm_create(fd, 0, 0);
+	spin->bo = xe_bo_create(fd, eci->gt_id, spin->vm, bo_size);
+	spin->engine = xe_engine_create(fd, spin->vm, eci, 0);
+	syncobj = syncobj_create(fd, 0);
+
+	if (ahnd)
+		addr = intel_allocator_alloc_with_strategy(ahnd, spin->bo, bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
+
+	xe_vm_bind_sync(fd, spin->vm, spin->bo, 0, addr, bo_size);
+
+	xe_spin_init(spin, addr, true);
+	exec.engine_id = spin->engine;
+	exec.address = addr;
+	sync.handle = syncobj;
+
+	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
+
+}
+
+void xe_spin_free(struct xe_spin *spin)
+{
+	xe_engine_destroy(spin->fd, spin->engine);
+	xe_vm_destroy(spin->fd, spin->vm);
+	gem_close(spin->fd, spin->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..12116445 100644
--- a/lib/xe/xe_spin.h
+++ b/lib/xe/xe_spin.h
@@ -17,15 +17,24 @@
 /* Mapped GPU object */
 struct xe_spin {
 	uint32_t batch[16];
+	int fd;
 	uint64_t pad;
 	uint32_t start;
 	uint32_t end;
+	uint64_t ahnd;
+	uint32_t engine;
+	uint32_t vm;
+	uint32_t bo;
+
 };
 
 void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool preempt);
 bool xe_spin_started(struct xe_spin *spin);
 void xe_spin_wait_started(struct xe_spin *spin);
 void xe_spin_end(struct xe_spin *spin);
+void xe_spin_new(int fd, struct xe_spin *spin,
+		struct drm_xe_engine_class_instance *hwe);
+void xe_spin_free(struct xe_spin *spin);
 
 struct xe_cork {
 	struct xe_spin *spin;
-- 
2.39.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-04-24 11:54 [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe sai.gowtham.ch
@ 2023-04-24 12:47 ` Patchwork
  2023-04-24 15:44 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  2023-04-25  3:36 ` [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
  2 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2023-04-24 12:47 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

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

== Series Details ==

Series: lib/xe/xe_spin: Integrate igt_spin_new with Xe.
URL   : https://patchwork.freedesktop.org/series/116880/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13053 -> IGTPW_8844
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (38 -> 38)
------------------------------

  Additional (1): bat-mtlp-8 
  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@workarounds:
    - bat-rpls-1:         [PASS][1] -> [INCOMPLETE][2] ([i915#4983] / [i915#7677])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/bat-rpls-1/igt@i915_selftest@live@workarounds.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/bat-rpls-1/igt@i915_selftest@live@workarounds.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-adln-1:         NOTRUN -> [SKIP][3] ([i915#7828])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/bat-adln-1/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  
#### Possible fixes ####

  * igt@dmabuf@all-tests@dma_fence:
    - bat-adln-1:         [FAIL][4] ([i915#8064]) -> [PASS][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/bat-adln-1/igt@dmabuf@all-tests@dma_fence.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/bat-adln-1/igt@dmabuf@all-tests@dma_fence.html

  * igt@dmabuf@all-tests@sanitycheck:
    - bat-adln-1:         [ABORT][6] ([i915#8058] / [i915#8144]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/bat-adln-1/igt@dmabuf@all-tests@sanitycheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/bat-adln-1/igt@dmabuf@all-tests@sanitycheck.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      [DMESG-FAIL][8] ([i915#5334] / [i915#7872]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@migrate:
    - bat-adlp-9:         [DMESG-FAIL][10] ([i915#7699]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/bat-adlp-9/igt@i915_selftest@live@migrate.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/bat-adlp-9/igt@i915_selftest@live@migrate.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-1:
    - fi-rkl-11600:       [FAIL][12] ([fdo#103375]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/fi-rkl-11600/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-1.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/fi-rkl-11600/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-b-hdmi-a-1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#7677]: https://gitlab.freedesktop.org/drm/intel/issues/7677
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
  [i915#8058]: https://gitlab.freedesktop.org/drm/intel/issues/8058
  [i915#8064]: https://gitlab.freedesktop.org/drm/intel/issues/8064
  [i915#8144]: https://gitlab.freedesktop.org/drm/intel/issues/8144
  [i915#8361]: https://gitlab.freedesktop.org/drm/intel/issues/8361


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7266 -> IGTPW_8844

  CI-20190529: 20190529
  CI_DRM_13053: 63a6d68541b38408d5be8345f3e4a561661f68cb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8844: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/index.html
  IGT_7266: 94411dd85f9ad6d76fb7b2097197122703a3914c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

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

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-04-24 11:54 [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe sai.gowtham.ch
  2023-04-24 12:47 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
@ 2023-04-24 15:44 ` Patchwork
  2023-04-25  3:36 ` [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
  2 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2023-04-24 15:44 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

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

== Series Details ==

Series: lib/xe/xe_spin: Integrate igt_spin_new with Xe.
URL   : https://patchwork.freedesktop.org/series/116880/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13053_full -> IGTPW_8844_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with IGTPW_8844_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_8844_full, please notify your bug team 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_8844/index.html

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-pri-shrfb-draw-mmap-cpu:
    - shard-snb:          [SKIP][1] ([fdo#109271]) -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-snb4/igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-pri-shrfb-draw-mmap-cpu.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-snb1/igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-pri-shrfb-draw-mmap-cpu.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@hang:
    - shard-snb:          NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1099])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-snb6/igt@gem_ctx_persistence@hang.html

  * igt@gem_exec_capture@capture-invisible@smem0:
    - shard-apl:          NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#6334])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-apl3/igt@gem_exec_capture@capture-invisible@smem0.html

  * igt@gem_exec_fair@basic-deadline:
    - shard-apl:          [PASS][5] -> [FAIL][6] ([i915#2846])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-apl1/igt@gem_exec_fair@basic-deadline.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-apl2/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [PASS][7] -> [FAIL][8] ([i915#2842]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-glk7/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-glk1/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-apl:          [PASS][9] -> [FAIL][10] ([i915#2842])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-apl2/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-apl6/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_lmem_swapping@heavy-verify-multi:
    - shard-apl:          NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#4613])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-apl2/igt@gem_lmem_swapping@heavy-verify-multi.html

  * igt@i915_pm_dc@dc9-dpms:
    - shard-apl:          [PASS][12] -> [SKIP][13] ([fdo#109271])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-apl6/igt@i915_pm_dc@dc9-dpms.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-apl1/igt@i915_pm_dc@dc9-dpms.html

  * igt@kms_ccs@pipe-a-bad-pixel-format-4_tiled_dg2_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][14] ([fdo#109271]) +45 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-apl6/igt@kms_ccs@pipe-a-bad-pixel-format-4_tiled_dg2_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_mc_ccs:
    - shard-glk:          NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#3886]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-glk5/igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-crc-primary-basic-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#3886])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-apl7/igt@kms_ccs@pipe-c-crc-primary-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-rotation@pipe-a-vga-1:
    - shard-snb:          NOTRUN -> [SKIP][17] ([fdo#109271]) +99 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-snb6/igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-rotation@pipe-a-vga-1.html

  * igt@v3d/v3d_submit_csd@multiple-job-submission:
    - shard-glk:          NOTRUN -> [SKIP][18] ([fdo#109271]) +12 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-glk2/igt@v3d/v3d_submit_csd@multiple-job-submission.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - {shard-rkl}:        [FAIL][19] ([i915#2842]) -> [PASS][20] +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-rkl-6/igt@gem_exec_fair@basic-throttle@rcs0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-rkl-2/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-apl:          [ABORT][21] ([i915#5566]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-apl1/igt@gen9_exec_parse@allowed-single.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-apl4/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
    - {shard-rkl}:        [SKIP][23] ([i915#1937]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-rkl-4/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-rkl-7/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html

  * igt@i915_pm_rc6_residency@rc6-idle@bcs0:
    - {shard-dg1}:        [FAIL][25] ([i915#3591]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-dg1-15/igt@i915_pm_rc6_residency@rc6-idle@bcs0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-dg1-16/igt@i915_pm_rc6_residency@rc6-idle@bcs0.html

  * igt@i915_pm_rpm@modeset-non-lpsp-stress:
    - {shard-rkl}:        [SKIP][27] ([i915#1397]) -> [PASS][28] +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-rkl-7/igt@i915_pm_rpm@modeset-non-lpsp-stress.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-rkl-3/igt@i915_pm_rpm@modeset-non-lpsp-stress.html

  * igt@i915_pm_rps@reset:
    - shard-snb:          [INCOMPLETE][29] ([i915#7790]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-snb5/igt@i915_pm_rps@reset.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-snb6/igt@i915_pm_rps@reset.html

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-apl:          [DMESG-FAIL][31] ([i915#5334]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-apl6/igt@i915_selftest@live@gt_heartbeat.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-apl6/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_cursor_legacy@forked-move@pipe-b:
    - {shard-dg1}:        [INCOMPLETE][33] ([i915#8011] / [i915#8347]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-dg1-14/igt@kms_cursor_legacy@forked-move@pipe-b.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-dg1-17/igt@kms_cursor_legacy@forked-move@pipe-b.html

  * igt@kms_cursor_legacy@single-move@pipe-b:
    - {shard-rkl}:        [INCOMPLETE][35] ([i915#8011]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-rkl-7/igt@kms_cursor_legacy@single-move@pipe-b.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-rkl-1/igt@kms_cursor_legacy@single-move@pipe-b.html

  * igt@kms_plane_scaling@i915-max-src-size@pipe-a-hdmi-a-2:
    - {shard-rkl}:        [FAIL][37] ([i915#8292]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-rkl-3/igt@kms_plane_scaling@i915-max-src-size@pipe-a-hdmi-a-2.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-rkl-4/igt@kms_plane_scaling@i915-max-src-size@pipe-a-hdmi-a-2.html

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-glk:          [DMESG-FAIL][39] ([i915#118]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13053/shard-glk4/igt@kms_rotation_crc@multiplane-rotation-cropping-top.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/shard-glk6/igt@kms_rotation_crc@multiplane-rotation-cropping-top.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [IGT#2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/2
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
  [fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
  [fdo#109313]: https://bugs.freedesktop.org/show_bug.cgi?id=109313
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111656]: https://bugs.freedesktop.org/show_bug.cgi?id=111656
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1099]: https://gitlab.freedesktop.org/drm/intel/issues/1099
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1937]: https://gitlab.freedesktop.org/drm/intel/issues/1937
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
  [i915#3778]: https://gitlab.freedesktop.org/drm/intel/issues/3778
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3936]: https://gitlab.freedesktop.org/drm/intel/issues/3936
  [i915#404]: https://gitlab.freedesktop.org/drm/intel/issues/404
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4816]: https://gitlab.freedesktop.org/drm/intel/issues/4816
  [i915#4818]: https://gitlab.freedesktop.org/drm/intel/issues/4818
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4854]: https://gitlab.freedesktop.org/drm/intel/issues/4854
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
  [i915#4881]: https://gitlab.freedesktop.org/drm/intel/issues/4881
  [i915#4884]: https://gitlab.freedesktop.org/drm/intel/issues/4884
  [i915#4885]: https://gitlab.freedesktop.org/drm/intel/issues/4885
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5431]: https://gitlab.freedesktop.org/drm/intel/issues/5431
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6245]: https://gitlab.freedesktop.org/drm/intel/issues/6245
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#6334]: https://gitlab.freedesktop.org/drm/intel/issues/6334
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#6493]: https://gitlab.freedesktop.org/drm/intel/issues/6493
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7790]: https://gitlab.freedesktop.org/drm/intel/issues/7790
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8253]: https://gitlab.freedesktop.org/drm/intel/issues/8253
  [i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
  [i915#8381]: https://gitlab.freedesktop.org/drm/intel/issues/8381


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7266 -> IGTPW_8844
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_13053: 63a6d68541b38408d5be8345f3e4a561661f68cb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8844: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8844/index.html
  IGT_7266: 94411dd85f9ad6d76fb7b2097197122703a3914c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-04-24 11:54 [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe sai.gowtham.ch
  2023-04-24 12:47 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
  2023-04-24 15:44 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2023-04-25  3:36 ` Zbigniew Kempczyński
  2023-04-25  7:07   ` Ch, Sai Gowtham
  2 siblings, 1 reply; 23+ messages in thread
From: Zbigniew Kempczyński @ 2023-04-25  3:36 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

On Mon, Apr 24, 2023 at 05:24:23PM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  lib/xe/xe_spin.c | 39 +++++++++++++++++++++++++++++++++++++++
>  lib/xe/xe_spin.h |  9 +++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index 856d0ba2..e0629419 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -82,6 +82,45 @@ void xe_spin_end(struct xe_spin *spin)
>  	spin->end = 0;
>  }
>  
> +void xe_spin_new(int fd, struct xe_spin *spin, struct drm_xe_engine_class_instance *eci)
> +{
> +	size_t bo_size = xe_get_default_alignment(fd);
> +	uint32_t syncobj;
> +	uint64_t ahnd = spin->ahnd, addr;
> +	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->vm = xe_vm_create(fd, 0, 0);
> +	spin->bo = xe_bo_create(fd, eci->gt_id, spin->vm, bo_size);
> +	spin->engine = xe_engine_create(fd, spin->vm, eci, 0);
> +	syncobj = syncobj_create(fd, 0);
> +
> +	if (ahnd)
> +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->bo, bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> +
> +	xe_vm_bind_sync(fd, spin->vm, spin->bo, 0, addr, bo_size);
> +
> +	xe_spin_init(spin, addr, true);
> +	exec.engine_id = spin->engine;
> +	exec.address = addr;
> +	sync.handle = syncobj;
> +
> +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> +
> +}
> +
> +void xe_spin_free(struct xe_spin *spin)
> +{
> +	xe_engine_destroy(spin->fd, spin->engine);
> +	xe_vm_destroy(spin->fd, spin->vm);
> +	gem_close(spin->fd, spin->bo);
> +}

You need to embed this into igt_spin_new(). Take a look to kms_cursor_legacy.c.
We want to have this syntax intact:

	spin = igt_spin_new(display->drm_fd,
			    .ahnd = ahnd,
			    .dependency = fb_info.gem_handle);

--
Zbigniew

>  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..12116445 100644
> --- a/lib/xe/xe_spin.h
> +++ b/lib/xe/xe_spin.h
> @@ -17,15 +17,24 @@
>  /* Mapped GPU object */
>  struct xe_spin {
>  	uint32_t batch[16];
> +	int fd;
>  	uint64_t pad;
>  	uint32_t start;
>  	uint32_t end;
> +	uint64_t ahnd;
> +	uint32_t engine;
> +	uint32_t vm;
> +	uint32_t bo;
> +
>  };
>  
>  void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool preempt);
>  bool xe_spin_started(struct xe_spin *spin);
>  void xe_spin_wait_started(struct xe_spin *spin);
>  void xe_spin_end(struct xe_spin *spin);
> +void xe_spin_new(int fd, struct xe_spin *spin,
> +		struct drm_xe_engine_class_instance *hwe);
> +void xe_spin_free(struct xe_spin *spin);
>  
>  struct xe_cork {
>  	struct xe_spin *spin;
> -- 
> 2.39.1
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-04-25  3:36 ` [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
@ 2023-04-25  7:07   ` Ch, Sai Gowtham
  2023-04-25 10:48     ` Zbigniew Kempczyński
  0 siblings, 1 reply; 23+ messages in thread
From: Ch, Sai Gowtham @ 2023-04-25  7:07 UTC (permalink / raw)
  To: Kempczynski, Zbigniew; +Cc: igt-dev@lists.freedesktop.org



> -----Original Message-----
> From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
> Sent: Tuesday, April 25, 2023 9:07 AM
> To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>
> Cc: igt-dev@lists.freedesktop.org
> Subject: Re: [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
> 
> On Mon, Apr 24, 2023 at 05:24:23PM +0530, sai.gowtham.ch@intel.com wrote:
> > From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
> > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > ---
> >  lib/xe/xe_spin.c | 39 +++++++++++++++++++++++++++++++++++++++
> >  lib/xe/xe_spin.h |  9 +++++++++
> >  2 files changed, 48 insertions(+)
> >
> > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
> > 856d0ba2..e0629419 100644
> > --- a/lib/xe/xe_spin.c
> > +++ b/lib/xe/xe_spin.c
> > @@ -82,6 +82,45 @@ void xe_spin_end(struct xe_spin *spin)
> >  	spin->end = 0;
> >  }
> >
> > +void xe_spin_new(int fd, struct xe_spin *spin, struct
> > +drm_xe_engine_class_instance *eci) {
> > +	size_t bo_size = xe_get_default_alignment(fd);
> > +	uint32_t syncobj;
> > +	uint64_t ahnd = spin->ahnd, addr;
> > +	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->vm = xe_vm_create(fd, 0, 0);
> > +	spin->bo = xe_bo_create(fd, eci->gt_id, spin->vm, bo_size);
> > +	spin->engine = xe_engine_create(fd, spin->vm, eci, 0);
> > +	syncobj = syncobj_create(fd, 0);
> > +
> > +	if (ahnd)
> > +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->bo,
> bo_size,
> > +0, ALLOC_STRATEGY_LOW_TO_HIGH);
> > +
> > +	xe_vm_bind_sync(fd, spin->vm, spin->bo, 0, addr, bo_size);
> > +
> > +	xe_spin_init(spin, addr, true);
> > +	exec.engine_id = spin->engine;
> > +	exec.address = addr;
> > +	sync.handle = syncobj;
> > +
> > +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> > +
> > +}
> > +
> > +void xe_spin_free(struct xe_spin *spin) {
> > +	xe_engine_destroy(spin->fd, spin->engine);
> > +	xe_vm_destroy(spin->fd, spin->vm);
> > +	gem_close(spin->fd, spin->bo);
> > +}
> 
> You need to embed this into igt_spin_new(). Take a look to
> kms_cursor_legacy.c.
> We want to have this syntax intact:
> 
> 	spin = igt_spin_new(display->drm_fd,
> 			    .ahnd = ahnd,
> 			    .dependency = fb_info.gem_handle);
> 
> --
> Zbigniew
> 
Will be calling this code from igt_spin_factory it self, with a check is being called like below 

igt_spin_t *
__igt_spin_factory(int fd, const struct igt_spin_factory *opts)
{
        if (is_xe_device)
                xe_spin_create(fd, opts);
        else
                spin_create(fd, opts);
}
And similarly in igt_spin_free:

What are your thoughts on this ? Do you think this works ? 
-----
Sai Gowtham

> >  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..12116445 100644
> > --- a/lib/xe/xe_spin.h
> > +++ b/lib/xe/xe_spin.h
> > @@ -17,15 +17,24 @@
> >  /* Mapped GPU object */
> >  struct xe_spin {
> >  	uint32_t batch[16];
> > +	int fd;
> >  	uint64_t pad;
> >  	uint32_t start;
> >  	uint32_t end;
> > +	uint64_t ahnd;
> > +	uint32_t engine;
> > +	uint32_t vm;
> > +	uint32_t bo;
> > +
> >  };
> >
> >  void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool preempt);
> > bool xe_spin_started(struct xe_spin *spin);  void
> > xe_spin_wait_started(struct xe_spin *spin);  void xe_spin_end(struct
> > xe_spin *spin);
> > +void xe_spin_new(int fd, struct xe_spin *spin,
> > +		struct drm_xe_engine_class_instance *hwe); void
> xe_spin_free(struct
> > +xe_spin *spin);
> >
> >  struct xe_cork {
> >  	struct xe_spin *spin;
> > --
> > 2.39.1
> >

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-04-25  7:07   ` Ch, Sai Gowtham
@ 2023-04-25 10:48     ` Zbigniew Kempczyński
  0 siblings, 0 replies; 23+ messages in thread
From: Zbigniew Kempczyński @ 2023-04-25 10:48 UTC (permalink / raw)
  To: Ch, Sai Gowtham; +Cc: igt-dev@lists.freedesktop.org

On Tue, Apr 25, 2023 at 09:07:15AM +0200, Ch, Sai Gowtham wrote:
> 
> 
> > -----Original Message-----
> > From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
> > Sent: Tuesday, April 25, 2023 9:07 AM
> > To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>
> > Cc: igt-dev@lists.freedesktop.org
> > Subject: Re: [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
> > 
> > On Mon, Apr 24, 2023 at 05:24:23PM +0530, sai.gowtham.ch@intel.com wrote:
> > > From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
> > > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > > ---
> > >  lib/xe/xe_spin.c | 39 +++++++++++++++++++++++++++++++++++++++
> > >  lib/xe/xe_spin.h |  9 +++++++++
> > >  2 files changed, 48 insertions(+)
> > >
> > > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
> > > 856d0ba2..e0629419 100644
> > > --- a/lib/xe/xe_spin.c
> > > +++ b/lib/xe/xe_spin.c
> > > @@ -82,6 +82,45 @@ void xe_spin_end(struct xe_spin *spin)
> > >  	spin->end = 0;
> > >  }
> > >
> > > +void xe_spin_new(int fd, struct xe_spin *spin, struct
> > > +drm_xe_engine_class_instance *eci) {
> > > +	size_t bo_size = xe_get_default_alignment(fd);
> > > +	uint32_t syncobj;
> > > +	uint64_t ahnd = spin->ahnd, addr;
> > > +	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->vm = xe_vm_create(fd, 0, 0);
> > > +	spin->bo = xe_bo_create(fd, eci->gt_id, spin->vm, bo_size);
> > > +	spin->engine = xe_engine_create(fd, spin->vm, eci, 0);
> > > +	syncobj = syncobj_create(fd, 0);
> > > +
> > > +	if (ahnd)
> > > +		addr = intel_allocator_alloc_with_strategy(ahnd, spin->bo,
> > bo_size,
> > > +0, ALLOC_STRATEGY_LOW_TO_HIGH);
> > > +
> > > +	xe_vm_bind_sync(fd, spin->vm, spin->bo, 0, addr, bo_size);
> > > +
> > > +	xe_spin_init(spin, addr, true);
> > > +	exec.engine_id = spin->engine;
> > > +	exec.address = addr;
> > > +	sync.handle = syncobj;
> > > +
> > > +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> > > +
> > > +}
> > > +
> > > +void xe_spin_free(struct xe_spin *spin) {
> > > +	xe_engine_destroy(spin->fd, spin->engine);
> > > +	xe_vm_destroy(spin->fd, spin->vm);
> > > +	gem_close(spin->fd, spin->bo);
> > > +}
> > 
> > You need to embed this into igt_spin_new(). Take a look to
> > kms_cursor_legacy.c.
> > We want to have this syntax intact:
> > 
> > 	spin = igt_spin_new(display->drm_fd,
> > 			    .ahnd = ahnd,
> > 			    .dependency = fb_info.gem_handle);
> > 
> > --
> > Zbigniew
> > 
> Will be calling this code from igt_spin_factory it self, with a check is being called like below 
> 
> igt_spin_t *
> __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
> {
>         if (is_xe_device)
>                 xe_spin_create(fd, opts);
>         else
>                 spin_create(fd, opts);
> }
> And similarly in igt_spin_free:
> 
> What are your thoughts on this ? Do you think this works ? 

We need adopt the code to make this work. Be aware that spin_create()
uses igt_spin_t to collect all necessary information about spinner
so you need to do this for xe_spin_create() too.

--
Zbigniew


> -----
> Sai Gowtham
> 
> > >  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..12116445 100644
> > > --- a/lib/xe/xe_spin.h
> > > +++ b/lib/xe/xe_spin.h
> > > @@ -17,15 +17,24 @@
> > >  /* Mapped GPU object */
> > >  struct xe_spin {
> > >  	uint32_t batch[16];
> > > +	int fd;
> > >  	uint64_t pad;
> > >  	uint32_t start;
> > >  	uint32_t end;
> > > +	uint64_t ahnd;
> > > +	uint32_t engine;
> > > +	uint32_t vm;
> > > +	uint32_t bo;
> > > +
> > >  };
> > >
> > >  void xe_spin_init(struct xe_spin *spin, uint64_t addr, bool preempt);
> > > bool xe_spin_started(struct xe_spin *spin);  void
> > > xe_spin_wait_started(struct xe_spin *spin);  void xe_spin_end(struct
> > > xe_spin *spin);
> > > +void xe_spin_new(int fd, struct xe_spin *spin,
> > > +		struct drm_xe_engine_class_instance *hwe); void
> > xe_spin_free(struct
> > > +xe_spin *spin);
> > >
> > >  struct xe_cork {
> > >  	struct xe_spin *spin;
> > > --
> > > 2.39.1
> > >

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
@ 2023-04-26 20:16 sai.gowtham.ch
  2023-04-27  5:40 ` Modem, Bhanuprakash
  0 siblings, 1 reply; 23+ messages in thread
From: sai.gowtham.ch @ 2023-04-26 20:16 UTC (permalink / raw)
  To: igt-dev, zbigniew.kempczynski, sai.gowtham.ch

From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@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)
+		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}))
 
 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;
-- 
2.39.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-04-26 20:16 sai.gowtham.ch
@ 2023-04-27  5:40 ` Modem, Bhanuprakash
  2023-04-27  5:49   ` Ch, Sai Gowtham
  0 siblings, 1 reply; 23+ messages in thread
From: Modem, Bhanuprakash @ 2023-04-27  5:40 UTC (permalink / raw)
  To: sai.gowtham.ch, igt-dev, zbigniew.kempczynski


On Thu-27-04-2023 01:46 am, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@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?

> +		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();
}

- 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;

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-04-27  5:40 ` Modem, Bhanuprakash
@ 2023-04-27  5:49   ` Ch, Sai Gowtham
  2023-04-27  7:09     ` Zbigniew Kempczyński
  0 siblings, 1 reply; 23+ messages in thread
From: Ch, Sai Gowtham @ 2023-04-27  5:49 UTC (permalink / raw)
  To: Modem, Bhanuprakash, igt-dev@lists.freedesktop.org,
	Kempczynski, Zbigniew



> -----Original Message-----
> From: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> Sent: Thursday, April 27, 2023 11:10 AM
> To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>; igt-
> dev@lists.freedesktop.org; Kempczynski, Zbigniew
> <zbigniew.kempczynski@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@intel.com wrote:
> > From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
> > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@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;

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-04-27  5:49   ` Ch, Sai Gowtham
@ 2023-04-27  7:09     ` Zbigniew Kempczyński
  0 siblings, 0 replies; 23+ messages in thread
From: Zbigniew Kempczyński @ 2023-04-27  7:09 UTC (permalink / raw)
  To: Ch, Sai Gowtham; +Cc: igt-dev@lists.freedesktop.org

On Thu, Apr 27, 2023 at 07:49:42AM +0200, Ch, Sai Gowtham wrote:
> 
> 
> > -----Original Message-----
> > From: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> > Sent: Thursday, April 27, 2023 11:10 AM
> > To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>; igt-
> > dev@lists.freedesktop.org; Kempczynski, Zbigniew
> > <zbigniew.kempczynski@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@intel.com wrote:
> > > From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
> > > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@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),
> > > +	};
> > > +

Check ahnd here.

> > > +	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.

For xe ahnd must be != 0. So assert on the beginning and this will be fine.

> > 
> > > +		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.

Don't hesitate and also return igt_spin_t from xe spinner. Just add fields
you need for xe spinner and it should work.

--
Zbigniew


> > - 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;

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
@ 2023-05-05  5:35 sai.gowtham.ch
  2023-05-08 12:19 ` Hogander, Jouni
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: sai.gowtham.ch @ 2023-05-05  5:35 UTC (permalink / raw)
  To: igt-dev, zbigniew.kempczynski, sai.gowtham.ch

From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 lib/igt_dummyload.c | 28 ++++++++++++++------
 lib/igt_dummyload.h |  8 ++++++
 lib/xe/xe_spin.c    | 63 +++++++++++++++++++++++++++++++++++++++++++++
 lib/xe/xe_spin.h    |  7 +++++
 4 files changed, 98 insertions(+), 8 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 740a58f3..92507819 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -46,6 +46,7 @@
 #include "intel_reg.h"
 #include "ioctl_wrappers.h"
 #include "sw_sync.h"
+#include "xe/xe_spin.h"
 
 /**
  * SECTION:igt_dummyload
@@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory *opts)
 igt_spin_t *
 __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
 {
-	return spin_create(fd, opts);
+	if (is_xe_device(fd))
+		return xe_spin_create(fd, opts);
+	else
+		return spin_create(fd, opts);
 }
 
 /**
@@ -480,8 +484,13 @@ igt_spin_factory(int fd, const struct igt_spin_factory *opts)
 		igt_require(!gem_engine_has_cmdparser(fd, &opts->ctx->cfg,
 						      opts->engine));
 	}
-
-	spin = spin_create(fd, opts);
+	if (is_xe_device(fd)) {
+		spin = xe_spin_create(fd, opts);
+		return spin;
+	} else {
+		spin = spin_create(fd, opts);
+		return spin;
+	}
 
 	if (!(opts->flags & IGT_SPIN_INVALID_CS)) {
 		/*
@@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
 	if (!spin)
 		return;
 
-	pthread_mutex_lock(&list_lock);
-	igt_list_del(&spin->link);
-	pthread_mutex_unlock(&list_lock);
-
-	__igt_spin_free(fd, spin);
+	if (is_xe_device(fd)) {
+		xe_spin_free(fd, spin);
+	} else {
+		pthread_mutex_lock(&list_lock);
+		igt_list_del(&spin->link);
+		pthread_mutex_unlock(&list_lock);
+		__igt_spin_free(fd, spin);
+	}
 }
 
 void igt_terminate_spins(void)
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index b247ab02..7efbb464 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -83,6 +83,14 @@ typedef struct igt_spin {
 #define SPIN_CLFLUSH (1 << 0)
 
 	struct igt_spin_factory opts;
+	union {
+		uint64_t pad;
+		uint32_t start;
+		uint32_t end;
+		uint32_t vm;
+		uint32_t syncobj;
+	};
+
 } igt_spin_t;
 
 
diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
index 856d0ba2..b79b5a0c 100644
--- a/lib/xe/xe_spin.c
+++ b/lib/xe/xe_spin.c
@@ -15,6 +15,7 @@
 #include "intel_reg.h"
 #include "xe_ioctl.h"
 #include "xe_spin.h"
+#include "lib/igt_dummyload.h"
 
 /**
  * xe_spin_init:
@@ -82,6 +83,68 @@ void xe_spin_end(struct xe_spin *spin)
 	spin->end = 0;
 }
 
+igt_spin_t *
+xe_spin_create(int fd, const struct igt_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 igt_spin *spin;
+	struct xe_spin *xe_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),
+	};
+
+	igt_assert(ahnd);
+	xe_spin = calloc(1, sizeof(struct xe_spin));
+	igt_assert(xe_spin);
+	spin = calloc(1, sizeof(struct igt_spin));
+	igt_assert(spin);
+
+	bo = xe_bo_create(fd, eci->gt_id, vm, bo_size);
+	spin = xe_bo_map(fd, bo, 0x1000);
+	spin->handle = bo;
+	spin->vm = xe_vm_create(fd, 0, 0);
+	engine = xe_engine_create(fd, vm, eci, 0);
+	spin->syncobj = syncobj_create(fd, 0);
+
+	if (ahnd)
+		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(xe_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);
+
+	spin->batch = xe_spin->batch;
+	spin->start = xe_spin->start;
+	spin->end = xe_spin->end;
+	return spin;
+
+}
+
+void xe_spin_sync_wait(int fd, struct igt_spin *spin)
+{
+	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
+				NULL));
+}
+
+void xe_spin_free(int fd, struct igt_spin *spin)
+{
+	spin->end = 0;
+	xe_vm_destroy(fd, spin->vm);
+	gem_close(fd, spin->handle);
+}
+
 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..48867eb8 100644
--- a/lib/xe/xe_spin.h
+++ b/lib/xe/xe_spin.h
@@ -13,19 +13,26 @@
 #include <stdbool.h>
 
 #include "xe_query.h"
+#include "lib/igt_dummyload.h"
 
 /* Mapped GPU object */
+
 struct xe_spin {
 	uint32_t batch[16];
 	uint64_t pad;
 	uint32_t start;
 	uint32_t end;
+
 };
 
+igt_spin_t *
+xe_spin_create(int fd, const struct igt_spin_factory *opt);
 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 igt_spin *spin);
 void xe_spin_wait_started(struct xe_spin *spin);
 void xe_spin_end(struct xe_spin *spin);
+void xe_spin_free(int fd, struct igt_spin *spin);
 
 struct xe_cork {
 	struct xe_spin *spin;
-- 
2.39.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-05  5:35 sai.gowtham.ch
@ 2023-05-08 12:19 ` Hogander, Jouni
  2023-05-08 12:22   ` Ch, Sai Gowtham
  2023-05-08 19:55 ` Zbigniew Kempczyński
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 23+ messages in thread
From: Hogander, Jouni @ 2023-05-08 12:19 UTC (permalink / raw)
  To: Kempczynski, Zbigniew, Ch, Sai Gowtham,
	igt-dev@lists.freedesktop.org

On Fri, 2023-05-05 at 11:05 +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@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.

Hello Sai,

Do you have some example testcase demonstrating usage of this Xe
spinner? I've been trying to adapt my testcase here to work with Xe and
on top of your spinner patch:

https://patchwork.freedesktop.org/series/116130/

Without success. Seeing crash...

BR,

Jouni Högander

> 
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  lib/igt_dummyload.c | 28 ++++++++++++++------
>  lib/igt_dummyload.h |  8 ++++++
>  lib/xe/xe_spin.c    | 63
> +++++++++++++++++++++++++++++++++++++++++++++
>  lib/xe/xe_spin.h    |  7 +++++
>  4 files changed, 98 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 740a58f3..92507819 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -46,6 +46,7 @@
>  #include "intel_reg.h"
>  #include "ioctl_wrappers.h"
>  #include "sw_sync.h"
> +#include "xe/xe_spin.h"
>  
>  /**
>   * SECTION:igt_dummyload
> @@ -447,7 +448,10 @@ spin_create(int fd, const struct
> igt_spin_factory *opts)
>  igt_spin_t *
>  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  {
> -       return spin_create(fd, opts);
> +       if (is_xe_device(fd))
> +               return xe_spin_create(fd, opts);
> +       else
> +               return spin_create(fd, opts);
>  }
>  
>  /**
> @@ -480,8 +484,13 @@ igt_spin_factory(int fd, const struct
> igt_spin_factory *opts)
>                 igt_require(!gem_engine_has_cmdparser(fd, &opts->ctx-
> >cfg,
>                                                       opts->engine));
>         }
> -
> -       spin = spin_create(fd, opts);
> +       if (is_xe_device(fd)) {
> +               spin = xe_spin_create(fd, opts);
> +               return spin;
> +       } else {
> +               spin = spin_create(fd, opts);
> +               return spin;
> +       }
>  
>         if (!(opts->flags & IGT_SPIN_INVALID_CS)) {
>                 /*
> @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
>         if (!spin)
>                 return;
>  
> -       pthread_mutex_lock(&list_lock);
> -       igt_list_del(&spin->link);
> -       pthread_mutex_unlock(&list_lock);
> -
> -       __igt_spin_free(fd, spin);
> +       if (is_xe_device(fd)) {
> +               xe_spin_free(fd, spin);
> +       } else {
> +               pthread_mutex_lock(&list_lock);
> +               igt_list_del(&spin->link);
> +               pthread_mutex_unlock(&list_lock);
> +               __igt_spin_free(fd, spin);
> +       }
>  }
>  
>  void igt_terminate_spins(void)
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index b247ab02..7efbb464 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -83,6 +83,14 @@ typedef struct igt_spin {
>  #define SPIN_CLFLUSH (1 << 0)
>  
>         struct igt_spin_factory opts;
> +       union {
> +               uint64_t pad;
> +               uint32_t start;
> +               uint32_t end;
> +               uint32_t vm;
> +               uint32_t syncobj;
> +       };
> +
>  } igt_spin_t;
>  
>  
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index 856d0ba2..b79b5a0c 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -15,6 +15,7 @@
>  #include "intel_reg.h"
>  #include "xe_ioctl.h"
>  #include "xe_spin.h"
> +#include "lib/igt_dummyload.h"
>  
>  /**
>   * xe_spin_init:
> @@ -82,6 +83,68 @@ void xe_spin_end(struct xe_spin *spin)
>         spin->end = 0;
>  }
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_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 igt_spin *spin;
> +       struct xe_spin *xe_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),
> +       };
> +
> +       igt_assert(ahnd);
> +       xe_spin = calloc(1, sizeof(struct xe_spin));
> +       igt_assert(xe_spin);
> +       spin = calloc(1, sizeof(struct igt_spin));
> +       igt_assert(spin);
> +
> +       bo = xe_bo_create(fd, eci->gt_id, vm, bo_size);
> +       spin = xe_bo_map(fd, bo, 0x1000);
> +       spin->handle = bo;
> +       spin->vm = xe_vm_create(fd, 0, 0);
> +       engine = xe_engine_create(fd, vm, eci, 0);
> +       spin->syncobj = syncobj_create(fd, 0);
> +
> +       if (ahnd)
> +               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(xe_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);
> +
> +       spin->batch = xe_spin->batch;
> +       spin->start = xe_spin->start;
> +       spin->end = xe_spin->end;
> +       return spin;
> +
> +}
> +
> +void xe_spin_sync_wait(int fd, struct igt_spin *spin)
> +{
> +       igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> +                               NULL));
> +}
> +
> +void xe_spin_free(int fd, struct igt_spin *spin)
> +{
> +       spin->end = 0;
> +       xe_vm_destroy(fd, spin->vm);
> +       gem_close(fd, spin->handle);
> +}
> +
>  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..48867eb8 100644
> --- a/lib/xe/xe_spin.h
> +++ b/lib/xe/xe_spin.h
> @@ -13,19 +13,26 @@
>  #include <stdbool.h>
>  
>  #include "xe_query.h"
> +#include "lib/igt_dummyload.h"
>  
>  /* Mapped GPU object */
> +
>  struct xe_spin {
>         uint32_t batch[16];
>         uint64_t pad;
>         uint32_t start;
>         uint32_t end;
> +
>  };
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_spin_factory *opt);
>  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 igt_spin *spin);
>  void xe_spin_wait_started(struct xe_spin *spin);
>  void xe_spin_end(struct xe_spin *spin);
> +void xe_spin_free(int fd, struct igt_spin *spin);
>  
>  struct xe_cork {
>         struct xe_spin *spin;


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-08 12:19 ` Hogander, Jouni
@ 2023-05-08 12:22   ` Ch, Sai Gowtham
  0 siblings, 0 replies; 23+ messages in thread
From: Ch, Sai Gowtham @ 2023-05-08 12:22 UTC (permalink / raw)
  To: Hogander, Jouni, Kempczynski, Zbigniew,
	igt-dev@lists.freedesktop.org



> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Monday, May 8, 2023 5:50 PM
> To: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>; Ch, Sai
> Gowtham <sai.gowtham.ch@intel.com>; igt-dev@lists.freedesktop.org
> Subject: Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with
> Xe.
> 
> On Fri, 2023-05-05 at 11:05 +0530, sai.gowtham.ch@intel.com wrote:
> > From: Sai Gowtham Ch <sai.gowtham.ch@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.
> 
> Hello Sai,
> 
> Do you have some example testcase demonstrating usage of this Xe spinner?
> I've been trying to adapt my testcase here to work with Xe and on top of your
> spinner patch:
> 
> https://patchwork.freedesktop.org/series/116130/
> 
> Without success. Seeing crash...
> 
> BR,
> 
> Jouni Högander
Hi Jouni Högander,

It need few fixes Will be sending a new patch series in sometime, you can use that. 

Thanks,
Sai Gowtham
> 
> >
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > ---
> >  lib/igt_dummyload.c | 28 ++++++++++++++------
> >  lib/igt_dummyload.h |  8 ++++++
> >  lib/xe/xe_spin.c    | 63
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  lib/xe/xe_spin.h    |  7 +++++
> >  4 files changed, 98 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c index
> > 740a58f3..92507819 100644
> > --- a/lib/igt_dummyload.c
> > +++ b/lib/igt_dummyload.c
> > @@ -46,6 +46,7 @@
> >  #include "intel_reg.h"
> >  #include "ioctl_wrappers.h"
> >  #include "sw_sync.h"
> > +#include "xe/xe_spin.h"
> >
> >  /**
> >   * SECTION:igt_dummyload
> > @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory
> > *opts)
> >  igt_spin_t *
> >  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
> >  {
> > -       return spin_create(fd, opts);
> > +       if (is_xe_device(fd))
> > +               return xe_spin_create(fd, opts);
> > +       else
> > +               return spin_create(fd, opts);
> >  }
> >
> >  /**
> > @@ -480,8 +484,13 @@ igt_spin_factory(int fd, const struct
> > igt_spin_factory *opts)
> >                 igt_require(!gem_engine_has_cmdparser(fd, &opts->ctx-
> > >cfg,
> >                                                       opts->engine));
> >         }
> > -
> > -       spin = spin_create(fd, opts);
> > +       if (is_xe_device(fd)) {
> > +               spin = xe_spin_create(fd, opts);
> > +               return spin;
> > +       } else {
> > +               spin = spin_create(fd, opts);
> > +               return spin;
> > +       }
> >
> >         if (!(opts->flags & IGT_SPIN_INVALID_CS)) {
> >                 /*
> > @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
> >         if (!spin)
> >                 return;
> >
> > -       pthread_mutex_lock(&list_lock);
> > -       igt_list_del(&spin->link);
> > -       pthread_mutex_unlock(&list_lock);
> > -
> > -       __igt_spin_free(fd, spin);
> > +       if (is_xe_device(fd)) {
> > +               xe_spin_free(fd, spin);
> > +       } else {
> > +               pthread_mutex_lock(&list_lock);
> > +               igt_list_del(&spin->link);
> > +               pthread_mutex_unlock(&list_lock);
> > +               __igt_spin_free(fd, spin);
> > +       }
> >  }
> >
> >  void igt_terminate_spins(void)
> > diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h index
> > b247ab02..7efbb464 100644
> > --- a/lib/igt_dummyload.h
> > +++ b/lib/igt_dummyload.h
> > @@ -83,6 +83,14 @@ typedef struct igt_spin {
> >  #define SPIN_CLFLUSH (1 << 0)
> >
> >         struct igt_spin_factory opts;
> > +       union {
> > +               uint64_t pad;
> > +               uint32_t start;
> > +               uint32_t end;
> > +               uint32_t vm;
> > +               uint32_t syncobj;
> > +       };
> > +
> >  } igt_spin_t;
> >
> >
> > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
> > 856d0ba2..b79b5a0c 100644
> > --- a/lib/xe/xe_spin.c
> > +++ b/lib/xe/xe_spin.c
> > @@ -15,6 +15,7 @@
> >  #include "intel_reg.h"
> >  #include "xe_ioctl.h"
> >  #include "xe_spin.h"
> > +#include "lib/igt_dummyload.h"
> >
> >  /**
> >   * xe_spin_init:
> > @@ -82,6 +83,68 @@ void xe_spin_end(struct xe_spin *spin)
> >         spin->end = 0;
> >  }
> >
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_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 igt_spin *spin;
> > +       struct xe_spin *xe_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),
> > +       };
> > +
> > +       igt_assert(ahnd);
> > +       xe_spin = calloc(1, sizeof(struct xe_spin));
> > +       igt_assert(xe_spin);
> > +       spin = calloc(1, sizeof(struct igt_spin));
> > +       igt_assert(spin);
> > +
> > +       bo = xe_bo_create(fd, eci->gt_id, vm, bo_size);
> > +       spin = xe_bo_map(fd, bo, 0x1000);
> > +       spin->handle = bo;
> > +       spin->vm = xe_vm_create(fd, 0, 0);
> > +       engine = xe_engine_create(fd, vm, eci, 0);
> > +       spin->syncobj = syncobj_create(fd, 0);
> > +
> > +       if (ahnd)
> > +               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(xe_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);
> > +
> > +       spin->batch = xe_spin->batch;
> > +       spin->start = xe_spin->start;
> > +       spin->end = xe_spin->end;
> > +       return spin;
> > +
> > +}
> > +
> > +void xe_spin_sync_wait(int fd, struct igt_spin *spin) {
> > +       igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> > +                               NULL)); }
> > +
> > +void xe_spin_free(int fd, struct igt_spin *spin) {
> > +       spin->end = 0;
> > +       xe_vm_destroy(fd, spin->vm);
> > +       gem_close(fd, spin->handle);
> > +}
> > +
> >  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..48867eb8 100644
> > --- a/lib/xe/xe_spin.h
> > +++ b/lib/xe/xe_spin.h
> > @@ -13,19 +13,26 @@
> >  #include <stdbool.h>
> >
> >  #include "xe_query.h"
> > +#include "lib/igt_dummyload.h"
> >
> >  /* Mapped GPU object */
> > +
> >  struct xe_spin {
> >         uint32_t batch[16];
> >         uint64_t pad;
> >         uint32_t start;
> >         uint32_t end;
> > +
> >  };
> >
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_spin_factory *opt);
> >  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 igt_spin *spin);
> >  void xe_spin_wait_started(struct xe_spin *spin);
> >  void xe_spin_end(struct xe_spin *spin);
> > +void xe_spin_free(int fd, struct igt_spin *spin);
> >
> >  struct xe_cork {
> >         struct xe_spin *spin;


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-05  5:35 sai.gowtham.ch
  2023-05-08 12:19 ` Hogander, Jouni
@ 2023-05-08 19:55 ` Zbigniew Kempczyński
  2023-05-09  6:36 ` Zbigniew Kempczyński
  2023-06-08 14:10 ` Matthew Brost
  3 siblings, 0 replies; 23+ messages in thread
From: Zbigniew Kempczyński @ 2023-05-08 19:55 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

On Fri, May 05, 2023 at 11:05:28AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  lib/igt_dummyload.c | 28 ++++++++++++++------
>  lib/igt_dummyload.h |  8 ++++++
>  lib/xe/xe_spin.c    | 63 +++++++++++++++++++++++++++++++++++++++++++++
>  lib/xe/xe_spin.h    |  7 +++++
>  4 files changed, 98 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 740a58f3..92507819 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -46,6 +46,7 @@
>  #include "intel_reg.h"
>  #include "ioctl_wrappers.h"
>  #include "sw_sync.h"
> +#include "xe/xe_spin.h"
>  
>  /**
>   * SECTION:igt_dummyload
> @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory *opts)
>  igt_spin_t *
>  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  {
> -	return spin_create(fd, opts);
> +	if (is_xe_device(fd))
> +		return xe_spin_create(fd, opts);
> +	else
> +		return spin_create(fd, opts);
>  }
>  
>  /**
> @@ -480,8 +484,13 @@ igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  		igt_require(!gem_engine_has_cmdparser(fd, &opts->ctx->cfg,
>  						      opts->engine));
>  	}
> -
> -	spin = spin_create(fd, opts);
> +	if (is_xe_device(fd)) {
> +		spin = xe_spin_create(fd, opts);
> +		return spin;
> +	} else {
> +		spin = spin_create(fd, opts);
> +		return spin;
> +	}
>  
>  	if (!(opts->flags & IGT_SPIN_INVALID_CS)) {
>  		/*
> @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
>  	if (!spin)
>  		return;
>  
> -	pthread_mutex_lock(&list_lock);
> -	igt_list_del(&spin->link);
> -	pthread_mutex_unlock(&list_lock);
> -
> -	__igt_spin_free(fd, spin);
> +	if (is_xe_device(fd)) {
> +		xe_spin_free(fd, spin);
> +	} else {
> +		pthread_mutex_lock(&list_lock);
> +		igt_list_del(&spin->link);
> +		pthread_mutex_unlock(&list_lock);
> +		__igt_spin_free(fd, spin);
> +	}
>  }
>  
>  void igt_terminate_spins(void)
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index b247ab02..7efbb464 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -83,6 +83,14 @@ typedef struct igt_spin {
>  #define SPIN_CLFLUSH (1 << 0)
>  
>  	struct igt_spin_factory opts;
> +	union {
> +		uint64_t pad;
> +		uint32_t start;
> +		uint32_t end;
> +		uint32_t vm;
> +		uint32_t syncobj;
> +	};
> +

Union? Marker guaranteed.

>  } igt_spin_t;
>  
>  
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index 856d0ba2..b79b5a0c 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -15,6 +15,7 @@
>  #include "intel_reg.h"
>  #include "xe_ioctl.h"
>  #include "xe_spin.h"
> +#include "lib/igt_dummyload.h"
>  
>  /**
>   * xe_spin_init:
> @@ -82,6 +83,68 @@ void xe_spin_end(struct xe_spin *spin)
>  	spin->end = 0;
>  }
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_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 igt_spin *spin;
> +	struct xe_spin *xe_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),
> +	};
> +
> +	igt_assert(ahnd);
> +	xe_spin = calloc(1, sizeof(struct xe_spin));
> +	igt_assert(xe_spin);
> +	spin = calloc(1, sizeof(struct igt_spin));
> +	igt_assert(spin);
> +
> +	bo = xe_bo_create(fd, eci->gt_id, vm, bo_size);
> +	spin = xe_bo_map(fd, bo, 0x1000);
> +	spin->handle = bo;
> +	spin->vm = xe_vm_create(fd, 0, 0);
> +	engine = xe_engine_create(fd, vm, eci, 0);
> +	spin->syncobj = syncobj_create(fd, 0);
> +
> +	if (ahnd)
> +		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(xe_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);
> +
> +	spin->batch = xe_spin->batch;
> +	spin->start = xe_spin->start;
> +	spin->end = xe_spin->end;
> +	return spin;
> +
> +}
> +
> +void xe_spin_sync_wait(int fd, struct igt_spin *spin)
> +{
> +	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> +				NULL));
> +}
> +
> +void xe_spin_free(int fd, struct igt_spin *spin)
> +{
> +	spin->end = 0;
> +	xe_vm_destroy(fd, spin->vm);

I see no usecase working right now but spin->end without synchronous
unbinding before will likely produce gpu hang.

--
Zbigniew

> +	gem_close(fd, spin->handle);
> +}
> +
>  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..48867eb8 100644
> --- a/lib/xe/xe_spin.h
> +++ b/lib/xe/xe_spin.h
> @@ -13,19 +13,26 @@
>  #include <stdbool.h>
>  
>  #include "xe_query.h"
> +#include "lib/igt_dummyload.h"
>  
>  /* Mapped GPU object */
> +
>  struct xe_spin {
>  	uint32_t batch[16];
>  	uint64_t pad;
>  	uint32_t start;
>  	uint32_t end;
> +
>  };
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_spin_factory *opt);
>  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 igt_spin *spin);
>  void xe_spin_wait_started(struct xe_spin *spin);
>  void xe_spin_end(struct xe_spin *spin);
> +void xe_spin_free(int fd, struct igt_spin *spin);
>  
>  struct xe_cork {
>  	struct xe_spin *spin;
> -- 
> 2.39.1
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-05  5:35 sai.gowtham.ch
  2023-05-08 12:19 ` Hogander, Jouni
  2023-05-08 19:55 ` Zbigniew Kempczyński
@ 2023-05-09  6:36 ` Zbigniew Kempczyński
  2023-06-08 14:10 ` Matthew Brost
  3 siblings, 0 replies; 23+ messages in thread
From: Zbigniew Kempczyński @ 2023-05-09  6:36 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

On Fri, May 05, 2023 at 11:05:28AM +0530, sai.gowtham.ch@intel.com wrote:
<cut>

> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_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 igt_spin *spin;
> +	struct xe_spin *xe_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),
> +	};
> +
> +	igt_assert(ahnd);
> +	xe_spin = calloc(1, sizeof(struct xe_spin));
> +	igt_assert(xe_spin);
> +	spin = calloc(1, sizeof(struct igt_spin));
> +	igt_assert(spin);
> +
> +	bo = xe_bo_create(fd, eci->gt_id, vm, bo_size);
> +	spin = xe_bo_map(fd, bo, 0x1000);
> +	spin->handle = bo;
> +	spin->vm = xe_vm_create(fd, 0, 0);
> +	engine = xe_engine_create(fd, vm, eci, 0);
> +	spin->syncobj = syncobj_create(fd, 0);
> +
> +	if (ahnd)
> +		addr = intel_allocator_alloc_with_strategy(ahnd, bo, bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);

if not necessary as you've assert on the beginning.

That's all I've noticed without testcode.

--
Zbigniew

> +
> +	xe_vm_bind_sync(fd, vm, bo, 0, addr, bo_size);
> +
> +	xe_spin_init(xe_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);
> +
> +	spin->batch = xe_spin->batch;
> +	spin->start = xe_spin->start;
> +	spin->end = xe_spin->end;
> +	return spin;
> +
> +}

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
@ 2023-05-10  5:55 sai.gowtham.ch
  2023-05-11  5:08 ` Zbigniew Kempczyński
  2023-05-11  6:43 ` Hogander, Jouni
  0 siblings, 2 replies; 23+ messages in thread
From: sai.gowtham.ch @ 2023-05-10  5:55 UTC (permalink / raw)
  To: igt-dev, zbigniew.kempczynski, sai.gowtham.ch

From: Sai Gowtham Ch <sai.gowtham.ch@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: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
---
 lib/igt_dummyload.c | 24 ++++++++++++----
 lib/igt_dummyload.h | 10 +++++++
 lib/xe/xe_spin.c    | 67 +++++++++++++++++++++++++++++++++++++++++++++
 lib/xe/xe_spin.h    |  7 +++++
 4 files changed, 102 insertions(+), 6 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 740a58f3..6e89b72d 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -46,6 +46,7 @@
 #include "intel_reg.h"
 #include "ioctl_wrappers.h"
 #include "sw_sync.h"
+#include "xe/xe_spin.h"
 
 /**
  * SECTION:igt_dummyload
@@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory *opts)
 igt_spin_t *
 __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
 {
-	return spin_create(fd, opts);
+	if (is_xe_device(fd))
+		return xe_spin_create(fd, opts);
+	else
+		return spin_create(fd, opts);
 }
 
 /**
@@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct igt_spin_factory *opts)
 {
 	igt_spin_t *spin;
 
+	if (is_xe_device(fd)) {
+		spin = xe_spin_create(fd, opts);
+		return spin;
+	}
+
 	if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine != ALL_ENGINES) {
 		unsigned int class;
 
@@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
 	if (!spin)
 		return;
 
-	pthread_mutex_lock(&list_lock);
-	igt_list_del(&spin->link);
-	pthread_mutex_unlock(&list_lock);
-
-	__igt_spin_free(fd, spin);
+	if (is_xe_device(fd)) {
+		xe_spin_free(fd, spin);
+	} else {
+		pthread_mutex_lock(&list_lock);
+		igt_list_del(&spin->link);
+		pthread_mutex_unlock(&list_lock);
+		__igt_spin_free(fd, spin);
+	}
 }
 
 void igt_terminate_spins(void)
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index b247ab02..c2900a8f 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -51,6 +51,7 @@ typedef struct igt_spin_factory {
 	uint32_t dependency;
 	uint64_t dependency_size;
 	unsigned int engine;
+	struct drm_xe_engine_class_instance *hwe;
 	unsigned int flags;
 	int fence;
 	uint64_t ahnd;
@@ -83,6 +84,15 @@ typedef struct igt_spin {
 #define SPIN_CLFLUSH (1 << 0)
 
 	struct igt_spin_factory opts;
+
+	uint32_t start;
+	uint32_t end;
+	size_t bo_size;
+	uint64_t address;
+	unsigned int engine;
+	uint32_t vm;
+	uint32_t syncobj;
+
 } igt_spin_t;
 
 
diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
index 856d0ba2..5f3a3b4f 100644
--- a/lib/xe/xe_spin.c
+++ b/lib/xe/xe_spin.c
@@ -15,6 +15,7 @@
 #include "intel_reg.h"
 #include "xe_ioctl.h"
 #include "xe_spin.h"
+#include "lib/igt_dummyload.h"
 
 /**
  * xe_spin_init:
@@ -82,6 +83,72 @@ void xe_spin_end(struct xe_spin *spin)
 	spin->end = 0;
 }
 
+igt_spin_t *
+xe_spin_create(int fd, const struct igt_spin_factory *opt)
+{
+	size_t bo_size = xe_get_default_alignment(fd);
+	uint32_t bo;
+	uint64_t ahnd = opt->ahnd, addr;
+	struct igt_spin *spin;
+	struct xe_spin *xe_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),
+	};
+
+	igt_assert(ahnd);
+	xe_spin = calloc(1, sizeof(struct xe_spin));
+	igt_assert(xe_spin);
+	spin = calloc(1, sizeof(struct igt_spin));
+	igt_assert(spin);
+
+	spin->vm = xe_vm_create(fd, 0, 0);
+	bo = xe_bo_create(fd, opt->hwe->gt_id, spin->vm, bo_size);
+	spin = xe_bo_map(fd, bo, 0x1000);
+	spin->handle = bo;
+	if (opt->engine)
+		spin->engine = opt->engine;
+	else
+		spin->engine = xe_engine_create(fd, spin->vm, opt->hwe, 0);
+
+	spin->syncobj = syncobj_create(fd, 0);
+	addr = intel_allocator_alloc_with_strategy(ahnd, bo, bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
+	xe_vm_bind_sync(fd, spin->vm, bo, 0, addr, bo_size);
+
+	xe_spin_init(xe_spin, addr, true);
+	exec.engine_id = spin->engine;
+	exec.address = addr;
+	sync.handle = spin->syncobj;
+	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
+
+	spin->batch = xe_spin->batch;
+	spin->start = xe_spin->start;
+	spin->end = xe_spin->end;
+	spin->bo_size = bo_size;
+	spin->address = addr;
+	return spin;
+
+}
+
+void xe_spin_sync_wait(int fd, struct igt_spin *spin)
+{
+	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
+				NULL));
+}
+
+void xe_spin_free(int fd, struct igt_spin *spin)
+{
+	xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin->bo_size);
+	spin->end = 0;
+	syncobj_destroy(fd, spin->syncobj);
+	xe_engine_destroy(fd, spin->engine);
+	gem_close(fd, spin->handle);
+}
+
 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..48867eb8 100644
--- a/lib/xe/xe_spin.h
+++ b/lib/xe/xe_spin.h
@@ -13,19 +13,26 @@
 #include <stdbool.h>
 
 #include "xe_query.h"
+#include "lib/igt_dummyload.h"
 
 /* Mapped GPU object */
+
 struct xe_spin {
 	uint32_t batch[16];
 	uint64_t pad;
 	uint32_t start;
 	uint32_t end;
+
 };
 
+igt_spin_t *
+xe_spin_create(int fd, const struct igt_spin_factory *opt);
 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 igt_spin *spin);
 void xe_spin_wait_started(struct xe_spin *spin);
 void xe_spin_end(struct xe_spin *spin);
+void xe_spin_free(int fd, struct igt_spin *spin);
 
 struct xe_cork {
 	struct xe_spin *spin;
-- 
2.39.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-10  5:55 sai.gowtham.ch
@ 2023-05-11  5:08 ` Zbigniew Kempczyński
  2023-05-11 10:03   ` Ch, Sai Gowtham
  2023-05-11  6:43 ` Hogander, Jouni
  1 sibling, 1 reply; 23+ messages in thread
From: Zbigniew Kempczyński @ 2023-05-11  5:08 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

On Wed, May 10, 2023 at 11:25:29AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@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: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>

Please provide some test I can run with this on xe and verify
this library extension is working.

--
Zbigniew

> ---
>  lib/igt_dummyload.c | 24 ++++++++++++----
>  lib/igt_dummyload.h | 10 +++++++
>  lib/xe/xe_spin.c    | 67 +++++++++++++++++++++++++++++++++++++++++++++
>  lib/xe/xe_spin.h    |  7 +++++
>  4 files changed, 102 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 740a58f3..6e89b72d 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -46,6 +46,7 @@
>  #include "intel_reg.h"
>  #include "ioctl_wrappers.h"
>  #include "sw_sync.h"
> +#include "xe/xe_spin.h"
>  
>  /**
>   * SECTION:igt_dummyload
> @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory *opts)
>  igt_spin_t *
>  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  {
> -	return spin_create(fd, opts);
> +	if (is_xe_device(fd))
> +		return xe_spin_create(fd, opts);
> +	else
> +		return spin_create(fd, opts);
>  }
>  
>  /**
> @@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  {
>  	igt_spin_t *spin;
>  
> +	if (is_xe_device(fd)) {
> +		spin = xe_spin_create(fd, opts);
> +		return spin;
> +	}
> +
>  	if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine != ALL_ENGINES) {
>  		unsigned int class;
>  
> @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
>  	if (!spin)
>  		return;
>  
> -	pthread_mutex_lock(&list_lock);
> -	igt_list_del(&spin->link);
> -	pthread_mutex_unlock(&list_lock);
> -
> -	__igt_spin_free(fd, spin);
> +	if (is_xe_device(fd)) {
> +		xe_spin_free(fd, spin);
> +	} else {
> +		pthread_mutex_lock(&list_lock);
> +		igt_list_del(&spin->link);
> +		pthread_mutex_unlock(&list_lock);
> +		__igt_spin_free(fd, spin);
> +	}
>  }
>  
>  void igt_terminate_spins(void)
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index b247ab02..c2900a8f 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -51,6 +51,7 @@ typedef struct igt_spin_factory {
>  	uint32_t dependency;
>  	uint64_t dependency_size;
>  	unsigned int engine;
> +	struct drm_xe_engine_class_instance *hwe;
>  	unsigned int flags;
>  	int fence;
>  	uint64_t ahnd;
> @@ -83,6 +84,15 @@ typedef struct igt_spin {
>  #define SPIN_CLFLUSH (1 << 0)
>  
>  	struct igt_spin_factory opts;
> +
> +	uint32_t start;
> +	uint32_t end;
> +	size_t bo_size;
> +	uint64_t address;
> +	unsigned int engine;
> +	uint32_t vm;
> +	uint32_t syncobj;
> +
>  } igt_spin_t;
>  
>  
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index 856d0ba2..5f3a3b4f 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -15,6 +15,7 @@
>  #include "intel_reg.h"
>  #include "xe_ioctl.h"
>  #include "xe_spin.h"
> +#include "lib/igt_dummyload.h"
>  
>  /**
>   * xe_spin_init:
> @@ -82,6 +83,72 @@ void xe_spin_end(struct xe_spin *spin)
>  	spin->end = 0;
>  }
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_spin_factory *opt)
> +{
> +	size_t bo_size = xe_get_default_alignment(fd);
> +	uint32_t bo;
> +	uint64_t ahnd = opt->ahnd, addr;
> +	struct igt_spin *spin;
> +	struct xe_spin *xe_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),
> +	};
> +
> +	igt_assert(ahnd);
> +	xe_spin = calloc(1, sizeof(struct xe_spin));
> +	igt_assert(xe_spin);
> +	spin = calloc(1, sizeof(struct igt_spin));
> +	igt_assert(spin);
> +
> +	spin->vm = xe_vm_create(fd, 0, 0);
> +	bo = xe_bo_create(fd, opt->hwe->gt_id, spin->vm, bo_size);
> +	spin = xe_bo_map(fd, bo, 0x1000);
> +	spin->handle = bo;
> +	if (opt->engine)
> +		spin->engine = opt->engine;
> +	else
> +		spin->engine = xe_engine_create(fd, spin->vm, opt->hwe, 0);
> +
> +	spin->syncobj = syncobj_create(fd, 0);
> +	addr = intel_allocator_alloc_with_strategy(ahnd, bo, bo_size, 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> +	xe_vm_bind_sync(fd, spin->vm, bo, 0, addr, bo_size);
> +
> +	xe_spin_init(xe_spin, addr, true);
> +	exec.engine_id = spin->engine;
> +	exec.address = addr;
> +	sync.handle = spin->syncobj;
> +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> +
> +	spin->batch = xe_spin->batch;
> +	spin->start = xe_spin->start;
> +	spin->end = xe_spin->end;
> +	spin->bo_size = bo_size;
> +	spin->address = addr;
> +	return spin;
> +
> +}
> +
> +void xe_spin_sync_wait(int fd, struct igt_spin *spin)
> +{
> +	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> +				NULL));
> +}
> +
> +void xe_spin_free(int fd, struct igt_spin *spin)
> +{
> +	xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin->bo_size);
> +	spin->end = 0;
> +	syncobj_destroy(fd, spin->syncobj);
> +	xe_engine_destroy(fd, spin->engine);
> +	gem_close(fd, spin->handle);
> +}
> +
>  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..48867eb8 100644
> --- a/lib/xe/xe_spin.h
> +++ b/lib/xe/xe_spin.h
> @@ -13,19 +13,26 @@
>  #include <stdbool.h>
>  
>  #include "xe_query.h"
> +#include "lib/igt_dummyload.h"
>  
>  /* Mapped GPU object */
> +
>  struct xe_spin {
>  	uint32_t batch[16];
>  	uint64_t pad;
>  	uint32_t start;
>  	uint32_t end;
> +
>  };
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_spin_factory *opt);
>  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 igt_spin *spin);
>  void xe_spin_wait_started(struct xe_spin *spin);
>  void xe_spin_end(struct xe_spin *spin);
> +void xe_spin_free(int fd, struct igt_spin *spin);
>  
>  struct xe_cork {
>  	struct xe_spin *spin;
> -- 
> 2.39.1
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-10  5:55 sai.gowtham.ch
  2023-05-11  5:08 ` Zbigniew Kempczyński
@ 2023-05-11  6:43 ` Hogander, Jouni
  2023-05-11 10:02   ` Ch, Sai Gowtham
  1 sibling, 1 reply; 23+ messages in thread
From: Hogander, Jouni @ 2023-05-11  6:43 UTC (permalink / raw)
  To: Kempczynski, Zbigniew, Ch, Sai Gowtham,
	igt-dev@lists.freedesktop.org

Hello Sai, see my comments inline below:

On Wed, 2023-05-10 at 11:25 +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@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: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> ---
>  lib/igt_dummyload.c | 24 ++++++++++++----
>  lib/igt_dummyload.h | 10 +++++++
>  lib/xe/xe_spin.c    | 67
> +++++++++++++++++++++++++++++++++++++++++++++
>  lib/xe/xe_spin.h    |  7 +++++
>  4 files changed, 102 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 740a58f3..6e89b72d 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -46,6 +46,7 @@
>  #include "intel_reg.h"
>  #include "ioctl_wrappers.h"
>  #include "sw_sync.h"
> +#include "xe/xe_spin.h"
>  
>  /**
>   * SECTION:igt_dummyload
> @@ -447,7 +448,10 @@ spin_create(int fd, const struct
> igt_spin_factory *opts)
>  igt_spin_t *
>  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  {
> -       return spin_create(fd, opts);
> +       if (is_xe_device(fd))
> +               return xe_spin_create(fd, opts);
> +       else
> +               return spin_create(fd, opts);
>  }
>  
>  /**
> @@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct
> igt_spin_factory *opts)
>  {
>         igt_spin_t *spin;
>  
> +       if (is_xe_device(fd)) {
> +               spin = xe_spin_create(fd, opts);
> +               return spin;
> +       }
> +
>         if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine !=
> ALL_ENGINES) {
>                 unsigned int class;
>  
> @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
>         if (!spin)
>                 return;
>  
> -       pthread_mutex_lock(&list_lock);
> -       igt_list_del(&spin->link);
> -       pthread_mutex_unlock(&list_lock);
> -
> -       __igt_spin_free(fd, spin);
> +       if (is_xe_device(fd)) {
> +               xe_spin_free(fd, spin);
> +       } else {
> +               pthread_mutex_lock(&list_lock);
> +               igt_list_del(&spin->link);
> +               pthread_mutex_unlock(&list_lock);
> +               __igt_spin_free(fd, spin);
> +       }
>  }
>  
>  void igt_terminate_spins(void)
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index b247ab02..c2900a8f 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -51,6 +51,7 @@ typedef struct igt_spin_factory {
>         uint32_t dependency;
>         uint64_t dependency_size;
>         unsigned int engine;
> +       struct drm_xe_engine_class_instance *hwe;
>         unsigned int flags;
>         int fence;
>         uint64_t ahnd;
> @@ -83,6 +84,15 @@ typedef struct igt_spin {
>  #define SPIN_CLFLUSH (1 << 0)
>  
>         struct igt_spin_factory opts;
> +
> +       uint32_t start;
> +       uint32_t end;
> +       size_t bo_size;
> +       uint64_t address;
> +       unsigned int engine;
> +       uint32_t vm;
> +       uint32_t syncobj;
> +
>  } igt_spin_t;
>  
>  
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index 856d0ba2..5f3a3b4f 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -15,6 +15,7 @@
>  #include "intel_reg.h"
>  #include "xe_ioctl.h"
>  #include "xe_spin.h"
> +#include "lib/igt_dummyload.h"
>  
>  /**
>   * xe_spin_init:
> @@ -82,6 +83,72 @@ void xe_spin_end(struct xe_spin *spin)
>         spin->end = 0;
>  }
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_spin_factory *opt)
> +{
> +       size_t bo_size = xe_get_default_alignment(fd);
> +       uint32_t bo;
> +       uint64_t ahnd = opt->ahnd, addr;
> +       struct igt_spin *spin;
> +       struct xe_spin *xe_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),
> +       };
> +
> +       igt_assert(ahnd);
> +       xe_spin = calloc(1, sizeof(struct xe_spin));
> +       igt_assert(xe_spin);
> +       spin = calloc(1, sizeof(struct igt_spin));
> +       igt_assert(spin);
> +
> +       spin->vm = xe_vm_create(fd, 0, 0);
> +       bo = xe_bo_create(fd, opt->hwe->gt_id, spin->vm, bo_size);
> +       spin = xe_bo_map(fd, bo, 0x1000);

This looks a bit strange. You are allocating spin above and then
setting the pointer here.

I tried your code again with my testcase. Still I'm seeing
crashes/failures. Do you have some example testcase how this is
supposed to be used?

> +       spin->handle = bo;
> +       if (opt->engine)
> +               spin->engine = opt->engine;
> +       else
> +               spin->engine = xe_engine_create(fd, spin->vm, opt-
> >hwe, 0);
> +
> +       spin->syncobj = syncobj_create(fd, 0);
> +       addr = intel_allocator_alloc_with_strategy(ahnd, bo, bo_size,
> 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> +       xe_vm_bind_sync(fd, spin->vm, bo, 0, addr, bo_size);
> +
> +       xe_spin_init(xe_spin, addr, true);
> +       exec.engine_id = spin->engine;
> +       exec.address = addr;
> +       sync.handle = spin->syncobj;
> +       igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> +
> +       spin->batch = xe_spin->batch;
> +       spin->start = xe_spin->start;
> +       spin->end = xe_spin->end;
> +       spin->bo_size = bo_size;
> +       spin->address = addr;
> +       return spin;
> +
> +}
> +
> +void xe_spin_sync_wait(int fd, struct igt_spin *spin)
> +{
> +       igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> +                               NULL));
> +}
> +
> +void xe_spin_free(int fd, struct igt_spin *spin)
> +{
> +       xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin-
> >bo_size);
> +       spin->end = 0;
> +       syncobj_destroy(fd, spin->syncobj);
> +       xe_engine_destroy(fd, spin->engine);
> +       gem_close(fd, spin->handle);
> +}
> +
>  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..48867eb8 100644
> --- a/lib/xe/xe_spin.h
> +++ b/lib/xe/xe_spin.h
> @@ -13,19 +13,26 @@
>  #include <stdbool.h>
>  
>  #include "xe_query.h"
> +#include "lib/igt_dummyload.h"
>  
>  /* Mapped GPU object */
> +
>  struct xe_spin {
>         uint32_t batch[16];
>         uint64_t pad;
>         uint32_t start;
>         uint32_t end;
> +
>  };
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_spin_factory *opt);
>  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 igt_spin *spin);
>  void xe_spin_wait_started(struct xe_spin *spin);
>  void xe_spin_end(struct xe_spin *spin);
> +void xe_spin_free(int fd, struct igt_spin *spin);
>  
>  struct xe_cork {
>         struct xe_spin *spin;


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-11  6:43 ` Hogander, Jouni
@ 2023-05-11 10:02   ` Ch, Sai Gowtham
  2023-05-11 10:04     ` Hogander, Jouni
  0 siblings, 1 reply; 23+ messages in thread
From: Ch, Sai Gowtham @ 2023-05-11 10:02 UTC (permalink / raw)
  To: Hogander, Jouni, Kempczynski, Zbigniew,
	igt-dev@lists.freedesktop.org



> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Thursday, May 11, 2023 12:14 PM
> To: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>; Ch, Sai
> Gowtham <sai.gowtham.ch@intel.com>; igt-dev@lists.freedesktop.org
> Subject: Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with
> Xe.
> 
> Hello Sai, see my comments inline below:
> 
> On Wed, 2023-05-10 at 11:25 +0530, sai.gowtham.ch@intel.com wrote:
> > From: Sai Gowtham Ch <sai.gowtham.ch@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: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > ---
> >  lib/igt_dummyload.c | 24 ++++++++++++----
> >  lib/igt_dummyload.h | 10 +++++++
> >  lib/xe/xe_spin.c    | 67
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  lib/xe/xe_spin.h    |  7 +++++
> >  4 files changed, 102 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c index
> > 740a58f3..6e89b72d 100644
> > --- a/lib/igt_dummyload.c
> > +++ b/lib/igt_dummyload.c
> > @@ -46,6 +46,7 @@
> >  #include "intel_reg.h"
> >  #include "ioctl_wrappers.h"
> >  #include "sw_sync.h"
> > +#include "xe/xe_spin.h"
> >
> >  /**
> >   * SECTION:igt_dummyload
> > @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory
> > *opts)
> >  igt_spin_t *
> >  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
> >  {
> > -       return spin_create(fd, opts);
> > +       if (is_xe_device(fd))
> > +               return xe_spin_create(fd, opts);
> > +       else
> > +               return spin_create(fd, opts);
> >  }
> >
> >  /**
> > @@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct
> > igt_spin_factory *opts)
> >  {
> >         igt_spin_t *spin;
> >
> > +       if (is_xe_device(fd)) {
> > +               spin = xe_spin_create(fd, opts);
> > +               return spin;
> > +       }
> > +
> >         if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine !=
> > ALL_ENGINES) {
> >                 unsigned int class;
> >
> > @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
> >         if (!spin)
> >                 return;
> >
> > -       pthread_mutex_lock(&list_lock);
> > -       igt_list_del(&spin->link);
> > -       pthread_mutex_unlock(&list_lock);
> > -
> > -       __igt_spin_free(fd, spin);
> > +       if (is_xe_device(fd)) {
> > +               xe_spin_free(fd, spin);
> > +       } else {
> > +               pthread_mutex_lock(&list_lock);
> > +               igt_list_del(&spin->link);
> > +               pthread_mutex_unlock(&list_lock);
> > +               __igt_spin_free(fd, spin);
> > +       }
> >  }
> >
> >  void igt_terminate_spins(void)
> > diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h index
> > b247ab02..c2900a8f 100644
> > --- a/lib/igt_dummyload.h
> > +++ b/lib/igt_dummyload.h
> > @@ -51,6 +51,7 @@ typedef struct igt_spin_factory {
> >         uint32_t dependency;
> >         uint64_t dependency_size;
> >         unsigned int engine;
> > +       struct drm_xe_engine_class_instance *hwe;
> >         unsigned int flags;
> >         int fence;
> >         uint64_t ahnd;
> > @@ -83,6 +84,15 @@ typedef struct igt_spin {
> >  #define SPIN_CLFLUSH (1 << 0)
> >
> >         struct igt_spin_factory opts;
> > +
> > +       uint32_t start;
> > +       uint32_t end;
> > +       size_t bo_size;
> > +       uint64_t address;
> > +       unsigned int engine;
> > +       uint32_t vm;
> > +       uint32_t syncobj;
> > +
> >  } igt_spin_t;
> >
> >
> > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
> > 856d0ba2..5f3a3b4f 100644
> > --- a/lib/xe/xe_spin.c
> > +++ b/lib/xe/xe_spin.c
> > @@ -15,6 +15,7 @@
> >  #include "intel_reg.h"
> >  #include "xe_ioctl.h"
> >  #include "xe_spin.h"
> > +#include "lib/igt_dummyload.h"
> >
> >  /**
> >   * xe_spin_init:
> > @@ -82,6 +83,72 @@ void xe_spin_end(struct xe_spin *spin)
> >         spin->end = 0;
> >  }
> >
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_spin_factory *opt) {
> > +       size_t bo_size = xe_get_default_alignment(fd);
> > +       uint32_t bo;
> > +       uint64_t ahnd = opt->ahnd, addr;
> > +       struct igt_spin *spin;
> > +       struct xe_spin *xe_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),
> > +       };
> > +
> > +       igt_assert(ahnd);
> > +       xe_spin = calloc(1, sizeof(struct xe_spin));
> > +       igt_assert(xe_spin);
> > +       spin = calloc(1, sizeof(struct igt_spin));
> > +       igt_assert(spin);
> > +
> > +       spin->vm = xe_vm_create(fd, 0, 0);
> > +       bo = xe_bo_create(fd, opt->hwe->gt_id, spin->vm, bo_size);
> > +       spin = xe_bo_map(fd, bo, 0x1000);
> 
> This looks a bit strange. You are allocating spin above and then setting the
> pointer here.
> 
> I tried your code again with my testcase. Still I'm seeing crashes/failures. Do you
> have some example testcase how this is supposed to be used?
Does your test using i915 or xe ?? if i915 then there is no change in the functionality it's the same existing code, because this change is specific to only xe. 

Because from this patch https://patchwork.freedesktop.org/series/116130/ it looks like you are using spinner for i915.

Thanks,
Gowtham
> 
> > +       spin->handle = bo;
> > +       if (opt->engine)
> > +               spin->engine = opt->engine;
> > +       else
> > +               spin->engine = xe_engine_create(fd, spin->vm, opt-
> > >hwe, 0);
> > +
> > +       spin->syncobj = syncobj_create(fd, 0);
> > +       addr = intel_allocator_alloc_with_strategy(ahnd, bo, bo_size,
> > 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> > +       xe_vm_bind_sync(fd, spin->vm, bo, 0, addr, bo_size);
> > +
> > +       xe_spin_init(xe_spin, addr, true);
> > +       exec.engine_id = spin->engine;
> > +       exec.address = addr;
> > +       sync.handle = spin->syncobj;
> > +       igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> > +
> > +       spin->batch = xe_spin->batch;
> > +       spin->start = xe_spin->start;
> > +       spin->end = xe_spin->end;
> > +       spin->bo_size = bo_size;
> > +       spin->address = addr;
> > +       return spin;
> > +
> > +}
> > +
> > +void xe_spin_sync_wait(int fd, struct igt_spin *spin) {
> > +       igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> > +                               NULL)); }
> > +
> > +void xe_spin_free(int fd, struct igt_spin *spin) {
> > +       xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin-
> > >bo_size);
> > +       spin->end = 0;
> > +       syncobj_destroy(fd, spin->syncobj);
> > +       xe_engine_destroy(fd, spin->engine);
> > +       gem_close(fd, spin->handle);
> > +}
> > +
> >  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..48867eb8 100644
> > --- a/lib/xe/xe_spin.h
> > +++ b/lib/xe/xe_spin.h
> > @@ -13,19 +13,26 @@
> >  #include <stdbool.h>
> >
> >  #include "xe_query.h"
> > +#include "lib/igt_dummyload.h"
> >
> >  /* Mapped GPU object */
> > +
> >  struct xe_spin {
> >         uint32_t batch[16];
> >         uint64_t pad;
> >         uint32_t start;
> >         uint32_t end;
> > +
> >  };
> >
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_spin_factory *opt);
> >  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 igt_spin *spin);
> >  void xe_spin_wait_started(struct xe_spin *spin);
> >  void xe_spin_end(struct xe_spin *spin);
> > +void xe_spin_free(int fd, struct igt_spin *spin);
> >
> >  struct xe_cork {
> >         struct xe_spin *spin;


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-11  5:08 ` Zbigniew Kempczyński
@ 2023-05-11 10:03   ` Ch, Sai Gowtham
  0 siblings, 0 replies; 23+ messages in thread
From: Ch, Sai Gowtham @ 2023-05-11 10:03 UTC (permalink / raw)
  To: Kempczynski, Zbigniew; +Cc: igt-dev@lists.freedesktop.org



> -----Original Message-----
> From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
> Sent: Thursday, May 11, 2023 10:38 AM
> To: Ch, Sai Gowtham <sai.gowtham.ch@intel.com>
> Cc: igt-dev@lists.freedesktop.org
> Subject: Re: [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
> 
> On Wed, May 10, 2023 at 11:25:29AM +0530, sai.gowtham.ch@intel.com
> wrote:
> > From: Sai Gowtham Ch <sai.gowtham.ch@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: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> Please provide some test I can run with this on xe and verify this library
> extension is working.
> 
Will be coming up with a simple test which exercises this lib. 

Thanks,
Gowtham
> --
> Zbigniew
> 
> > ---
> >  lib/igt_dummyload.c | 24 ++++++++++++----  lib/igt_dummyload.h | 10
> > +++++++
> >  lib/xe/xe_spin.c    | 67 +++++++++++++++++++++++++++++++++++++++++++++
> >  lib/xe/xe_spin.h    |  7 +++++
> >  4 files changed, 102 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c index
> > 740a58f3..6e89b72d 100644
> > --- a/lib/igt_dummyload.c
> > +++ b/lib/igt_dummyload.c
> > @@ -46,6 +46,7 @@
> >  #include "intel_reg.h"
> >  #include "ioctl_wrappers.h"
> >  #include "sw_sync.h"
> > +#include "xe/xe_spin.h"
> >
> >  /**
> >   * SECTION:igt_dummyload
> > @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory
> > *opts)  igt_spin_t *  __igt_spin_factory(int fd, const struct
> > igt_spin_factory *opts)  {
> > -	return spin_create(fd, opts);
> > +	if (is_xe_device(fd))
> > +		return xe_spin_create(fd, opts);
> > +	else
> > +		return spin_create(fd, opts);
> >  }
> >
> >  /**
> > @@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct
> > igt_spin_factory *opts)  {
> >  	igt_spin_t *spin;
> >
> > +	if (is_xe_device(fd)) {
> > +		spin = xe_spin_create(fd, opts);
> > +		return spin;
> > +	}
> > +
> >  	if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine !=
> ALL_ENGINES) {
> >  		unsigned int class;
> >
> > @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
> >  	if (!spin)
> >  		return;
> >
> > -	pthread_mutex_lock(&list_lock);
> > -	igt_list_del(&spin->link);
> > -	pthread_mutex_unlock(&list_lock);
> > -
> > -	__igt_spin_free(fd, spin);
> > +	if (is_xe_device(fd)) {
> > +		xe_spin_free(fd, spin);
> > +	} else {
> > +		pthread_mutex_lock(&list_lock);
> > +		igt_list_del(&spin->link);
> > +		pthread_mutex_unlock(&list_lock);
> > +		__igt_spin_free(fd, spin);
> > +	}
> >  }
> >
> >  void igt_terminate_spins(void)
> > diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h index
> > b247ab02..c2900a8f 100644
> > --- a/lib/igt_dummyload.h
> > +++ b/lib/igt_dummyload.h
> > @@ -51,6 +51,7 @@ typedef struct igt_spin_factory {
> >  	uint32_t dependency;
> >  	uint64_t dependency_size;
> >  	unsigned int engine;
> > +	struct drm_xe_engine_class_instance *hwe;
> >  	unsigned int flags;
> >  	int fence;
> >  	uint64_t ahnd;
> > @@ -83,6 +84,15 @@ typedef struct igt_spin {  #define SPIN_CLFLUSH (1
> > << 0)
> >
> >  	struct igt_spin_factory opts;
> > +
> > +	uint32_t start;
> > +	uint32_t end;
> > +	size_t bo_size;
> > +	uint64_t address;
> > +	unsigned int engine;
> > +	uint32_t vm;
> > +	uint32_t syncobj;
> > +
> >  } igt_spin_t;
> >
> >
> > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
> > 856d0ba2..5f3a3b4f 100644
> > --- a/lib/xe/xe_spin.c
> > +++ b/lib/xe/xe_spin.c
> > @@ -15,6 +15,7 @@
> >  #include "intel_reg.h"
> >  #include "xe_ioctl.h"
> >  #include "xe_spin.h"
> > +#include "lib/igt_dummyload.h"
> >
> >  /**
> >   * xe_spin_init:
> > @@ -82,6 +83,72 @@ void xe_spin_end(struct xe_spin *spin)
> >  	spin->end = 0;
> >  }
> >
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_spin_factory *opt) {
> > +	size_t bo_size = xe_get_default_alignment(fd);
> > +	uint32_t bo;
> > +	uint64_t ahnd = opt->ahnd, addr;
> > +	struct igt_spin *spin;
> > +	struct xe_spin *xe_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),
> > +	};
> > +
> > +	igt_assert(ahnd);
> > +	xe_spin = calloc(1, sizeof(struct xe_spin));
> > +	igt_assert(xe_spin);
> > +	spin = calloc(1, sizeof(struct igt_spin));
> > +	igt_assert(spin);
> > +
> > +	spin->vm = xe_vm_create(fd, 0, 0);
> > +	bo = xe_bo_create(fd, opt->hwe->gt_id, spin->vm, bo_size);
> > +	spin = xe_bo_map(fd, bo, 0x1000);
> > +	spin->handle = bo;
> > +	if (opt->engine)
> > +		spin->engine = opt->engine;
> > +	else
> > +		spin->engine = xe_engine_create(fd, spin->vm, opt->hwe, 0);
> > +
> > +	spin->syncobj = syncobj_create(fd, 0);
> > +	addr = intel_allocator_alloc_with_strategy(ahnd, bo, bo_size, 0,
> ALLOC_STRATEGY_LOW_TO_HIGH);
> > +	xe_vm_bind_sync(fd, spin->vm, bo, 0, addr, bo_size);
> > +
> > +	xe_spin_init(xe_spin, addr, true);
> > +	exec.engine_id = spin->engine;
> > +	exec.address = addr;
> > +	sync.handle = spin->syncobj;
> > +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec), 0);
> > +
> > +	spin->batch = xe_spin->batch;
> > +	spin->start = xe_spin->start;
> > +	spin->end = xe_spin->end;
> > +	spin->bo_size = bo_size;
> > +	spin->address = addr;
> > +	return spin;
> > +
> > +}
> > +
> > +void xe_spin_sync_wait(int fd, struct igt_spin *spin) {
> > +	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> > +				NULL));
> > +}
> > +
> > +void xe_spin_free(int fd, struct igt_spin *spin) {
> > +	xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin->bo_size);
> > +	spin->end = 0;
> > +	syncobj_destroy(fd, spin->syncobj);
> > +	xe_engine_destroy(fd, spin->engine);
> > +	gem_close(fd, spin->handle);
> > +}
> > +
> >  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..48867eb8 100644
> > --- a/lib/xe/xe_spin.h
> > +++ b/lib/xe/xe_spin.h
> > @@ -13,19 +13,26 @@
> >  #include <stdbool.h>
> >
> >  #include "xe_query.h"
> > +#include "lib/igt_dummyload.h"
> >
> >  /* Mapped GPU object */
> > +
> >  struct xe_spin {
> >  	uint32_t batch[16];
> >  	uint64_t pad;
> >  	uint32_t start;
> >  	uint32_t end;
> > +
> >  };
> >
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_spin_factory *opt);
> >  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 igt_spin *spin);
> >  void xe_spin_wait_started(struct xe_spin *spin);  void
> > xe_spin_end(struct xe_spin *spin);
> > +void xe_spin_free(int fd, struct igt_spin *spin);
> >
> >  struct xe_cork {
> >  	struct xe_spin *spin;
> > --
> > 2.39.1
> >

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-11 10:02   ` Ch, Sai Gowtham
@ 2023-05-11 10:04     ` Hogander, Jouni
  0 siblings, 0 replies; 23+ messages in thread
From: Hogander, Jouni @ 2023-05-11 10:04 UTC (permalink / raw)
  To: Kempczynski, Zbigniew, Ch, Sai Gowtham,
	igt-dev@lists.freedesktop.org

On Thu, 2023-05-11 at 10:02 +0000, Ch, Sai Gowtham wrote:
> 
> 
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander@intel.com>
> > Sent: Thursday, May 11, 2023 12:14 PM
> > To: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>; Ch, Sai
> > Gowtham <sai.gowtham.ch@intel.com>; igt-dev@lists.freedesktop.org
> > Subject: Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate
> > igt_spin_new with
> > Xe.
> > 
> > Hello Sai, see my comments inline below:
> > 
> > On Wed, 2023-05-10 at 11:25 +0530, sai.gowtham.ch@intel.com wrote:
> > > From: Sai Gowtham Ch <sai.gowtham.ch@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: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> > > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> > > ---
> > >  lib/igt_dummyload.c | 24 ++++++++++++----
> > >  lib/igt_dummyload.h | 10 +++++++
> > >  lib/xe/xe_spin.c    | 67
> > > +++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/xe/xe_spin.h    |  7 +++++
> > >  4 files changed, 102 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c index
> > > 740a58f3..6e89b72d 100644
> > > --- a/lib/igt_dummyload.c
> > > +++ b/lib/igt_dummyload.c
> > > @@ -46,6 +46,7 @@
> > >  #include "intel_reg.h"
> > >  #include "ioctl_wrappers.h"
> > >  #include "sw_sync.h"
> > > +#include "xe/xe_spin.h"
> > > 
> > >  /**
> > >   * SECTION:igt_dummyload
> > > @@ -447,7 +448,10 @@ spin_create(int fd, const struct
> > > igt_spin_factory
> > > *opts)
> > >  igt_spin_t *
> > >  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
> > >  {
> > > -       return spin_create(fd, opts);
> > > +       if (is_xe_device(fd))
> > > +               return xe_spin_create(fd, opts);
> > > +       else
> > > +               return spin_create(fd, opts);
> > >  }
> > > 
> > >  /**
> > > @@ -467,6 +471,11 @@ igt_spin_factory(int fd, const struct
> > > igt_spin_factory *opts)
> > >  {
> > >         igt_spin_t *spin;
> > > 
> > > +       if (is_xe_device(fd)) {
> > > +               spin = xe_spin_create(fd, opts);
> > > +               return spin;
> > > +       }
> > > +
> > >         if ((opts->flags & IGT_SPIN_POLL_RUN) && opts->engine !=
> > > ALL_ENGINES) {
> > >                 unsigned int class;
> > > 
> > > @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t
> > > *spin)
> > >         if (!spin)
> > >                 return;
> > > 
> > > -       pthread_mutex_lock(&list_lock);
> > > -       igt_list_del(&spin->link);
> > > -       pthread_mutex_unlock(&list_lock);
> > > -
> > > -       __igt_spin_free(fd, spin);
> > > +       if (is_xe_device(fd)) {
> > > +               xe_spin_free(fd, spin);
> > > +       } else {
> > > +               pthread_mutex_lock(&list_lock);
> > > +               igt_list_del(&spin->link);
> > > +               pthread_mutex_unlock(&list_lock);
> > > +               __igt_spin_free(fd, spin);
> > > +       }
> > >  }
> > > 
> > >  void igt_terminate_spins(void)
> > > diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h index
> > > b247ab02..c2900a8f 100644
> > > --- a/lib/igt_dummyload.h
> > > +++ b/lib/igt_dummyload.h
> > > @@ -51,6 +51,7 @@ typedef struct igt_spin_factory {
> > >         uint32_t dependency;
> > >         uint64_t dependency_size;
> > >         unsigned int engine;
> > > +       struct drm_xe_engine_class_instance *hwe;
> > >         unsigned int flags;
> > >         int fence;
> > >         uint64_t ahnd;
> > > @@ -83,6 +84,15 @@ typedef struct igt_spin {
> > >  #define SPIN_CLFLUSH (1 << 0)
> > > 
> > >         struct igt_spin_factory opts;
> > > +
> > > +       uint32_t start;
> > > +       uint32_t end;
> > > +       size_t bo_size;
> > > +       uint64_t address;
> > > +       unsigned int engine;
> > > +       uint32_t vm;
> > > +       uint32_t syncobj;
> > > +
> > >  } igt_spin_t;
> > > 
> > > 
> > > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c index
> > > 856d0ba2..5f3a3b4f 100644
> > > --- a/lib/xe/xe_spin.c
> > > +++ b/lib/xe/xe_spin.c
> > > @@ -15,6 +15,7 @@
> > >  #include "intel_reg.h"
> > >  #include "xe_ioctl.h"
> > >  #include "xe_spin.h"
> > > +#include "lib/igt_dummyload.h"
> > > 
> > >  /**
> > >   * xe_spin_init:
> > > @@ -82,6 +83,72 @@ void xe_spin_end(struct xe_spin *spin)
> > >         spin->end = 0;
> > >  }
> > > 
> > > +igt_spin_t *
> > > +xe_spin_create(int fd, const struct igt_spin_factory *opt) {
> > > +       size_t bo_size = xe_get_default_alignment(fd);
> > > +       uint32_t bo;
> > > +       uint64_t ahnd = opt->ahnd, addr;
> > > +       struct igt_spin *spin;
> > > +       struct xe_spin *xe_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),
> > > +       };
> > > +
> > > +       igt_assert(ahnd);
> > > +       xe_spin = calloc(1, sizeof(struct xe_spin));
> > > +       igt_assert(xe_spin);
> > > +       spin = calloc(1, sizeof(struct igt_spin));
> > > +       igt_assert(spin);
> > > +
> > > +       spin->vm = xe_vm_create(fd, 0, 0);
> > > +       bo = xe_bo_create(fd, opt->hwe->gt_id, spin->vm,
> > > bo_size);
> > > +       spin = xe_bo_map(fd, bo, 0x1000);
> > 
> > This looks a bit strange. You are allocating spin above and then
> > setting the
> > pointer here.
> > 
> > I tried your code again with my testcase. Still I'm seeing
> > crashes/failures. Do you
> > have some example testcase how this is supposed to be used?
> Does your test using i915 or xe ?? if i915 then there is no change in
> the functionality it's the same existing code, because this change is
> specific to only xe. 
> 
> Because from this patch
> https://patchwork.freedesktop.org/series/116130/ it looks like you
> are using spinner for i915.

I'm trying to port that test for Xe on top of your changes. I.e. I'm
using Xe.

> 
> Thanks,
> Gowtham
> > 
> > > +       spin->handle = bo;
> > > +       if (opt->engine)
> > > +               spin->engine = opt->engine;
> > > +       else
> > > +               spin->engine = xe_engine_create(fd, spin->vm,
> > > opt-
> > > > hwe, 0);
> > > +
> > > +       spin->syncobj = syncobj_create(fd, 0);
> > > +       addr = intel_allocator_alloc_with_strategy(ahnd, bo,
> > > bo_size,
> > > 0, ALLOC_STRATEGY_LOW_TO_HIGH);
> > > +       xe_vm_bind_sync(fd, spin->vm, bo, 0, addr, bo_size);
> > > +
> > > +       xe_spin_init(xe_spin, addr, true);
> > > +       exec.engine_id = spin->engine;
> > > +       exec.address = addr;
> > > +       sync.handle = spin->syncobj;
> > > +       igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_EXEC, &exec),
> > > 0);
> > > +
> > > +       spin->batch = xe_spin->batch;
> > > +       spin->start = xe_spin->start;
> > > +       spin->end = xe_spin->end;
> > > +       spin->bo_size = bo_size;
> > > +       spin->address = addr;
> > > +       return spin;
> > > +
> > > +}
> > > +
> > > +void xe_spin_sync_wait(int fd, struct igt_spin *spin) {
> > > +       igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX,
> > > 0,
> > > +                               NULL)); }
> > > +
> > > +void xe_spin_free(int fd, struct igt_spin *spin) {
> > > +       xe_vm_unbind_sync(fd, spin->vm, 0, spin->address, spin-
> > > > bo_size);
> > > +       spin->end = 0;
> > > +       syncobj_destroy(fd, spin->syncobj);
> > > +       xe_engine_destroy(fd, spin->engine);
> > > +       gem_close(fd, spin->handle);
> > > +}
> > > +
> > >  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..48867eb8 100644
> > > --- a/lib/xe/xe_spin.h
> > > +++ b/lib/xe/xe_spin.h
> > > @@ -13,19 +13,26 @@
> > >  #include <stdbool.h>
> > > 
> > >  #include "xe_query.h"
> > > +#include "lib/igt_dummyload.h"
> > > 
> > >  /* Mapped GPU object */
> > > +
> > >  struct xe_spin {
> > >         uint32_t batch[16];
> > >         uint64_t pad;
> > >         uint32_t start;
> > >         uint32_t end;
> > > +
> > >  };
> > > 
> > > +igt_spin_t *
> > > +xe_spin_create(int fd, const struct igt_spin_factory *opt);
> > >  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 igt_spin *spin);
> > >  void xe_spin_wait_started(struct xe_spin *spin);
> > >  void xe_spin_end(struct xe_spin *spin);
> > > +void xe_spin_free(int fd, struct igt_spin *spin);
> > > 
> > >  struct xe_cork {
> > >         struct xe_spin *spin;
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-05-05  5:35 sai.gowtham.ch
                   ` (2 preceding siblings ...)
  2023-05-09  6:36 ` Zbigniew Kempczyński
@ 2023-06-08 14:10 ` Matthew Brost
  2023-07-17  9:19   ` Zbigniew Kempczyński
  3 siblings, 1 reply; 23+ messages in thread
From: Matthew Brost @ 2023-06-08 14:10 UTC (permalink / raw)
  To: sai.gowtham.ch; +Cc: igt-dev

On Fri, May 05, 2023 at 11:05:28AM +0530, sai.gowtham.ch@intel.com wrote:
> From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>


Who asked for this? I intentionally didn't use the existing IGT spin
stuff in Xe as that code is a mess.

Matt

> ---
>  lib/igt_dummyload.c | 28 ++++++++++++++------
>  lib/igt_dummyload.h |  8 ++++++
>  lib/xe/xe_spin.c    | 63 +++++++++++++++++++++++++++++++++++++++++++++
>  lib/xe/xe_spin.h    |  7 +++++
>  4 files changed, 98 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 740a58f3..92507819 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -46,6 +46,7 @@
>  #include "intel_reg.h"
>  #include "ioctl_wrappers.h"
>  #include "sw_sync.h"
> +#include "xe/xe_spin.h"
>  
>  /**
>   * SECTION:igt_dummyload
> @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory *opts)
>  igt_spin_t *
>  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  {
> -	return spin_create(fd, opts);
> +	if (is_xe_device(fd))
> +		return xe_spin_create(fd, opts);
> +	else
> +		return spin_create(fd, opts);
>  }
>  
>  /**
> @@ -480,8 +484,13 @@ igt_spin_factory(int fd, const struct igt_spin_factory *opts)
>  		igt_require(!gem_engine_has_cmdparser(fd, &opts->ctx->cfg,
>  						      opts->engine));
>  	}
> -
> -	spin = spin_create(fd, opts);
> +	if (is_xe_device(fd)) {
> +		spin = xe_spin_create(fd, opts);
> +		return spin;
> +	} else {
> +		spin = spin_create(fd, opts);
> +		return spin;
> +	}
>  
>  	if (!(opts->flags & IGT_SPIN_INVALID_CS)) {
>  		/*
> @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
>  	if (!spin)
>  		return;
>  
> -	pthread_mutex_lock(&list_lock);
> -	igt_list_del(&spin->link);
> -	pthread_mutex_unlock(&list_lock);
> -
> -	__igt_spin_free(fd, spin);
> +	if (is_xe_device(fd)) {
> +		xe_spin_free(fd, spin);
> +	} else {
> +		pthread_mutex_lock(&list_lock);
> +		igt_list_del(&spin->link);
> +		pthread_mutex_unlock(&list_lock);
> +		__igt_spin_free(fd, spin);
> +	}
>  }
>  
>  void igt_terminate_spins(void)
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index b247ab02..7efbb464 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -83,6 +83,14 @@ typedef struct igt_spin {
>  #define SPIN_CLFLUSH (1 << 0)
>  
>  	struct igt_spin_factory opts;
> +	union {
> +		uint64_t pad;
> +		uint32_t start;
> +		uint32_t end;
> +		uint32_t vm;
> +		uint32_t syncobj;
> +	};
> +
>  } igt_spin_t;
>  
>  
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index 856d0ba2..b79b5a0c 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -15,6 +15,7 @@
>  #include "intel_reg.h"
>  #include "xe_ioctl.h"
>  #include "xe_spin.h"
> +#include "lib/igt_dummyload.h"
>  
>  /**
>   * xe_spin_init:
> @@ -82,6 +83,68 @@ void xe_spin_end(struct xe_spin *spin)
>  	spin->end = 0;
>  }
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_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 igt_spin *spin;
> +	struct xe_spin *xe_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),
> +	};
> +
> +	igt_assert(ahnd);
> +	xe_spin = calloc(1, sizeof(struct xe_spin));
> +	igt_assert(xe_spin);
> +	spin = calloc(1, sizeof(struct igt_spin));
> +	igt_assert(spin);
> +
> +	bo = xe_bo_create(fd, eci->gt_id, vm, bo_size);
> +	spin = xe_bo_map(fd, bo, 0x1000);
> +	spin->handle = bo;
> +	spin->vm = xe_vm_create(fd, 0, 0);
> +	engine = xe_engine_create(fd, vm, eci, 0);
> +	spin->syncobj = syncobj_create(fd, 0);
> +
> +	if (ahnd)
> +		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(xe_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);
> +
> +	spin->batch = xe_spin->batch;
> +	spin->start = xe_spin->start;
> +	spin->end = xe_spin->end;
> +	return spin;
> +
> +}
> +
> +void xe_spin_sync_wait(int fd, struct igt_spin *spin)
> +{
> +	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> +				NULL));
> +}
> +
> +void xe_spin_free(int fd, struct igt_spin *spin)
> +{
> +	spin->end = 0;
> +	xe_vm_destroy(fd, spin->vm);
> +	gem_close(fd, spin->handle);
> +}
> +
>  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..48867eb8 100644
> --- a/lib/xe/xe_spin.h
> +++ b/lib/xe/xe_spin.h
> @@ -13,19 +13,26 @@
>  #include <stdbool.h>
>  
>  #include "xe_query.h"
> +#include "lib/igt_dummyload.h"
>  
>  /* Mapped GPU object */
> +
>  struct xe_spin {
>  	uint32_t batch[16];
>  	uint64_t pad;
>  	uint32_t start;
>  	uint32_t end;
> +
>  };
>  
> +igt_spin_t *
> +xe_spin_create(int fd, const struct igt_spin_factory *opt);
>  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 igt_spin *spin);
>  void xe_spin_wait_started(struct xe_spin *spin);
>  void xe_spin_end(struct xe_spin *spin);
> +void xe_spin_free(int fd, struct igt_spin *spin);
>  
>  struct xe_cork {
>  	struct xe_spin *spin;
> -- 
> 2.39.1
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe.
  2023-06-08 14:10 ` Matthew Brost
@ 2023-07-17  9:19   ` Zbigniew Kempczyński
  0 siblings, 0 replies; 23+ messages in thread
From: Zbigniew Kempczyński @ 2023-07-17  9:19 UTC (permalink / raw)
  To: Matthew Brost; +Cc: igt-dev, sai.gowtham.ch

On Thu, Jun 08, 2023 at 02:10:01PM +0000, Matthew Brost wrote:
> On Fri, May 05, 2023 at 11:05:28AM +0530, sai.gowtham.ch@intel.com wrote:
> > From: Sai Gowtham Ch <sai.gowtham.ch@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@intel.com>
> > Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch@intel.com>
> 
> 
> Who asked for this? I intentionally didn't use the existing IGT spin
> stuff in Xe as that code is a mess.

Some IGT KMS tests are written around igt_spin_factory. We try to
avoid code duplication providing wrappers to mechanisms.

And I don't think wrapper to two separately implemented spinners
is a mess. May you elaborate what's wrong with this?

--
Zbigniew

> 
> Matt
> 
> > ---
> >  lib/igt_dummyload.c | 28 ++++++++++++++------
> >  lib/igt_dummyload.h |  8 ++++++
> >  lib/xe/xe_spin.c    | 63 +++++++++++++++++++++++++++++++++++++++++++++
> >  lib/xe/xe_spin.h    |  7 +++++
> >  4 files changed, 98 insertions(+), 8 deletions(-)
> > 
> > diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> > index 740a58f3..92507819 100644
> > --- a/lib/igt_dummyload.c
> > +++ b/lib/igt_dummyload.c
> > @@ -46,6 +46,7 @@
> >  #include "intel_reg.h"
> >  #include "ioctl_wrappers.h"
> >  #include "sw_sync.h"
> > +#include "xe/xe_spin.h"
> >  
> >  /**
> >   * SECTION:igt_dummyload
> > @@ -447,7 +448,10 @@ spin_create(int fd, const struct igt_spin_factory *opts)
> >  igt_spin_t *
> >  __igt_spin_factory(int fd, const struct igt_spin_factory *opts)
> >  {
> > -	return spin_create(fd, opts);
> > +	if (is_xe_device(fd))
> > +		return xe_spin_create(fd, opts);
> > +	else
> > +		return spin_create(fd, opts);
> >  }
> >  
> >  /**
> > @@ -480,8 +484,13 @@ igt_spin_factory(int fd, const struct igt_spin_factory *opts)
> >  		igt_require(!gem_engine_has_cmdparser(fd, &opts->ctx->cfg,
> >  						      opts->engine));
> >  	}
> > -
> > -	spin = spin_create(fd, opts);
> > +	if (is_xe_device(fd)) {
> > +		spin = xe_spin_create(fd, opts);
> > +		return spin;
> > +	} else {
> > +		spin = spin_create(fd, opts);
> > +		return spin;
> > +	}
> >  
> >  	if (!(opts->flags & IGT_SPIN_INVALID_CS)) {
> >  		/*
> > @@ -647,11 +656,14 @@ void igt_spin_free(int fd, igt_spin_t *spin)
> >  	if (!spin)
> >  		return;
> >  
> > -	pthread_mutex_lock(&list_lock);
> > -	igt_list_del(&spin->link);
> > -	pthread_mutex_unlock(&list_lock);
> > -
> > -	__igt_spin_free(fd, spin);
> > +	if (is_xe_device(fd)) {
> > +		xe_spin_free(fd, spin);
> > +	} else {
> > +		pthread_mutex_lock(&list_lock);
> > +		igt_list_del(&spin->link);
> > +		pthread_mutex_unlock(&list_lock);
> > +		__igt_spin_free(fd, spin);
> > +	}
> >  }
> >  
> >  void igt_terminate_spins(void)
> > diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> > index b247ab02..7efbb464 100644
> > --- a/lib/igt_dummyload.h
> > +++ b/lib/igt_dummyload.h
> > @@ -83,6 +83,14 @@ typedef struct igt_spin {
> >  #define SPIN_CLFLUSH (1 << 0)
> >  
> >  	struct igt_spin_factory opts;
> > +	union {
> > +		uint64_t pad;
> > +		uint32_t start;
> > +		uint32_t end;
> > +		uint32_t vm;
> > +		uint32_t syncobj;
> > +	};
> > +
> >  } igt_spin_t;
> >  
> >  
> > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> > index 856d0ba2..b79b5a0c 100644
> > --- a/lib/xe/xe_spin.c
> > +++ b/lib/xe/xe_spin.c
> > @@ -15,6 +15,7 @@
> >  #include "intel_reg.h"
> >  #include "xe_ioctl.h"
> >  #include "xe_spin.h"
> > +#include "lib/igt_dummyload.h"
> >  
> >  /**
> >   * xe_spin_init:
> > @@ -82,6 +83,68 @@ void xe_spin_end(struct xe_spin *spin)
> >  	spin->end = 0;
> >  }
> >  
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_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 igt_spin *spin;
> > +	struct xe_spin *xe_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),
> > +	};
> > +
> > +	igt_assert(ahnd);
> > +	xe_spin = calloc(1, sizeof(struct xe_spin));
> > +	igt_assert(xe_spin);
> > +	spin = calloc(1, sizeof(struct igt_spin));
> > +	igt_assert(spin);
> > +
> > +	bo = xe_bo_create(fd, eci->gt_id, vm, bo_size);
> > +	spin = xe_bo_map(fd, bo, 0x1000);
> > +	spin->handle = bo;
> > +	spin->vm = xe_vm_create(fd, 0, 0);
> > +	engine = xe_engine_create(fd, vm, eci, 0);
> > +	spin->syncobj = syncobj_create(fd, 0);
> > +
> > +	if (ahnd)
> > +		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(xe_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);
> > +
> > +	spin->batch = xe_spin->batch;
> > +	spin->start = xe_spin->start;
> > +	spin->end = xe_spin->end;
> > +	return spin;
> > +
> > +}
> > +
> > +void xe_spin_sync_wait(int fd, struct igt_spin *spin)
> > +{
> > +	igt_assert(syncobj_wait(fd, &spin->syncobj, 1, INT64_MAX, 0,
> > +				NULL));
> > +}
> > +
> > +void xe_spin_free(int fd, struct igt_spin *spin)
> > +{
> > +	spin->end = 0;
> > +	xe_vm_destroy(fd, spin->vm);
> > +	gem_close(fd, spin->handle);
> > +}
> > +
> >  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..48867eb8 100644
> > --- a/lib/xe/xe_spin.h
> > +++ b/lib/xe/xe_spin.h
> > @@ -13,19 +13,26 @@
> >  #include <stdbool.h>
> >  
> >  #include "xe_query.h"
> > +#include "lib/igt_dummyload.h"
> >  
> >  /* Mapped GPU object */
> > +
> >  struct xe_spin {
> >  	uint32_t batch[16];
> >  	uint64_t pad;
> >  	uint32_t start;
> >  	uint32_t end;
> > +
> >  };
> >  
> > +igt_spin_t *
> > +xe_spin_create(int fd, const struct igt_spin_factory *opt);
> >  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 igt_spin *spin);
> >  void xe_spin_wait_started(struct xe_spin *spin);
> >  void xe_spin_end(struct xe_spin *spin);
> > +void xe_spin_free(int fd, struct igt_spin *spin);
> >  
> >  struct xe_cork {
> >  	struct xe_spin *spin;
> > -- 
> > 2.39.1
> > 

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2023-07-17  9:19 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-24 11:54 [igt-dev] [PATCH i-g-t] lib/xe/xe_spin: Integrate igt_spin_new with Xe sai.gowtham.ch
2023-04-24 12:47 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2023-04-24 15:44 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2023-04-25  3:36 ` [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
2023-04-25  7:07   ` Ch, Sai Gowtham
2023-04-25 10:48     ` Zbigniew Kempczyński
  -- strict thread matches above, loose matches on Subject: below --
2023-04-26 20:16 sai.gowtham.ch
2023-04-27  5:40 ` Modem, Bhanuprakash
2023-04-27  5:49   ` Ch, Sai Gowtham
2023-04-27  7:09     ` Zbigniew Kempczyński
2023-05-05  5:35 sai.gowtham.ch
2023-05-08 12:19 ` Hogander, Jouni
2023-05-08 12:22   ` Ch, Sai Gowtham
2023-05-08 19:55 ` Zbigniew Kempczyński
2023-05-09  6:36 ` Zbigniew Kempczyński
2023-06-08 14:10 ` Matthew Brost
2023-07-17  9:19   ` Zbigniew Kempczyński
2023-05-10  5:55 sai.gowtham.ch
2023-05-11  5:08 ` Zbigniew Kempczyński
2023-05-11 10:03   ` Ch, Sai Gowtham
2023-05-11  6:43 ` Hogander, Jouni
2023-05-11 10:02   ` Ch, Sai Gowtham
2023-05-11 10:04     ` Hogander, Jouni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox