* [Intel-xe] [PATCH] drm/xe: Remove unnecessary memory barriers related to wait queue
@ 2023-07-27 18:34 Matthew Brost
2023-07-27 20:40 ` [Intel-xe] ✓ CI.Patch_applied: success for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Matthew Brost @ 2023-07-27 18:34 UTC (permalink / raw)
To: intel-xe
These not needed per the wait queue doc, remove them.
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_guc_ct.c | 3 ---
drivers/gpu/drm/xe/xe_guc_submit.c | 9 +--------
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
index cb75db30800c..3f58902b6be3 100644
--- a/drivers/gpu/drm/xe/xe_guc_ct.c
+++ b/drivers/gpu/drm/xe/xe_guc_ct.c
@@ -308,7 +308,6 @@ int xe_guc_ct_enable(struct xe_guc_ct *ct)
spin_unlock_irq(&ct->fast_lock);
mutex_unlock(&ct->lock);
- smp_mb();
wake_up_all(&ct->wq);
drm_dbg(&xe->drm, "GuC CT communication channel enabled\n");
@@ -820,8 +819,6 @@ static int parse_g2h_response(struct xe_guc_ct *ct, u32 *msg, u32 len)
g2h_release_space(ct, GUC_CTB_HXG_MSG_MAX_LEN);
g2h_fence->done = true;
- smp_mb();
-
wake_up_all(&ct->g2h_fence_wq);
return 0;
diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
index 6cb64a097297..d623cd9f6cc4 100644
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -705,7 +705,6 @@ static void disable_scheduling_deregister(struct xe_guc *guc,
int ret;
set_min_preemption_timeout(guc, e);
- smp_rmb();
ret = wait_event_timeout(guc->ct.wq, !engine_pending_enable(e) ||
guc_read_stopped(guc), HZ * 5);
if (!ret) {
@@ -888,7 +887,6 @@ guc_engine_timedout_job(struct drm_sched_job *drm_job)
* error) messages which can cause the schedule disable to get
* lost. If this occurs, trigger a GT reset to recover.
*/
- smp_rmb();
ret = wait_event_timeout(guc->ct.wq,
!engine_pending_disable(e) ||
guc_read_stopped(guc), HZ * 5);
@@ -1008,7 +1006,6 @@ static void suspend_fence_signal(struct xe_engine *e)
XE_BUG_ON(!e->guc->suspend_pending);
e->guc->suspend_pending = false;
- smp_wmb();
wake_up(&e->guc->suspend_wait);
}
@@ -1392,7 +1389,6 @@ int xe_guc_submit_reset_prepare(struct xe_guc *guc)
* and releasing any TDRs waiting on guc->submission_state.stopped.
*/
ret = atomic_fetch_or(1, &guc->submission_state.stopped);
- smp_wmb();
wake_up_all(&guc->ct.wq);
return ret;
@@ -1521,17 +1517,14 @@ int xe_guc_sched_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
if (engine_pending_enable(e)) {
e->guc->resume_time = ktime_get();
clear_engine_pending_enable(e);
- smp_wmb();
wake_up_all(&guc->ct.wq);
} else {
clear_engine_pending_disable(e);
if (e->guc->suspend_pending) {
suspend_fence_signal(e);
} else {
- if (engine_banned(e)) {
- smp_wmb();
+ if (engine_banned(e))
wake_up_all(&guc->ct.wq);
- }
deregister_engine(guc, e);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Intel-xe] ✓ CI.Patch_applied: success for drm/xe: Remove unnecessary memory barriers related to wait queue
2023-07-27 18:34 [Intel-xe] [PATCH] drm/xe: Remove unnecessary memory barriers related to wait queue Matthew Brost
@ 2023-07-27 20:40 ` Patchwork
2023-07-27 20:41 ` [Intel-xe] ✓ CI.checkpatch: " Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2023-07-27 20:40 UTC (permalink / raw)
To: Matthew Brost; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Remove unnecessary memory barriers related to wait queue
URL : https://patchwork.freedesktop.org/series/121467/
State : success
== Summary ==
=== Applying kernel patches on branch 'drm-xe-next' with base: ===
Base commit: 565e4919b drm/xe: Fix an invalid locking wait context bug
=== git am output follows ===
Applying: drm/xe: Remove unnecessary memory barriers related to wait queue
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-xe] ✓ CI.checkpatch: success for drm/xe: Remove unnecessary memory barriers related to wait queue
2023-07-27 18:34 [Intel-xe] [PATCH] drm/xe: Remove unnecessary memory barriers related to wait queue Matthew Brost
2023-07-27 20:40 ` [Intel-xe] ✓ CI.Patch_applied: success for " Patchwork
@ 2023-07-27 20:41 ` Patchwork
2023-07-27 20:42 ` [Intel-xe] ✓ CI.KUnit: " Patchwork
2023-07-28 12:13 ` [Intel-xe] [PATCH] " Matthew Auld
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2023-07-27 20:41 UTC (permalink / raw)
To: Matthew Brost; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Remove unnecessary memory barriers related to wait queue
URL : https://patchwork.freedesktop.org/series/121467/
State : success
== Summary ==
+ KERNEL=/kernel
+ git clone https://gitlab.freedesktop.org/drm/maintainer-tools mt
Cloning into 'mt'...
warning: redirecting to https://gitlab.freedesktop.org/drm/maintainer-tools.git/
+ git -C mt rev-list -n1 origin/master
c7d32770e3cd31d9fc134ce41f329b10aa33ee15
+ cd /kernel
+ git config --global --add safe.directory /kernel
+ git log -n1
commit 4b8543ca08a0fadc4d85099f3490b1e4c6a59dfe
Author: Matthew Brost <matthew.brost@intel.com>
Date: Thu Jul 27 11:34:23 2023 -0700
drm/xe: Remove unnecessary memory barriers related to wait queue
These not needed per the wait queue doc, remove them.
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
+ /mt/dim checkpatch 565e4919bd41c5a542b1f1bf29eced075968dc19 drm-intel
4b8543ca0 drm/xe: Remove unnecessary memory barriers related to wait queue
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-xe] ✓ CI.KUnit: success for drm/xe: Remove unnecessary memory barriers related to wait queue
2023-07-27 18:34 [Intel-xe] [PATCH] drm/xe: Remove unnecessary memory barriers related to wait queue Matthew Brost
2023-07-27 20:40 ` [Intel-xe] ✓ CI.Patch_applied: success for " Patchwork
2023-07-27 20:41 ` [Intel-xe] ✓ CI.checkpatch: " Patchwork
@ 2023-07-27 20:42 ` Patchwork
2023-07-28 12:13 ` [Intel-xe] [PATCH] " Matthew Auld
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2023-07-27 20:42 UTC (permalink / raw)
To: Matthew Brost; +Cc: intel-xe
== Series Details ==
Series: drm/xe: Remove unnecessary memory barriers related to wait queue
URL : https://patchwork.freedesktop.org/series/121467/
State : success
== Summary ==
+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
stty: 'standard input': Inappropriate ioctl for device
[20:41:09] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[20:41:13] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make ARCH=um O=.kunit --jobs=48
[20:41:32] Starting KUnit Kernel (1/1)...
[20:41:32] ============================================================
[20:41:32] ==================== xe_bo (2 subtests) ====================
[20:41:32] [SKIPPED] xe_ccs_migrate_kunit
[20:41:32] [SKIPPED] xe_bo_evict_kunit
[20:41:32] ===================== [SKIPPED] xe_bo ======================
[20:41:32] ================== xe_dma_buf (1 subtest) ==================
[20:41:32] [SKIPPED] xe_dma_buf_kunit
[20:41:32] =================== [SKIPPED] xe_dma_buf ===================
[20:41:32] ================== xe_migrate (1 subtest) ==================
[20:41:32] [SKIPPED] xe_migrate_sanity_kunit
[20:41:32] =================== [SKIPPED] xe_migrate ===================
[20:41:32] =================== xe_pci (2 subtests) ====================
[20:41:32] [PASSED] xe_gmdid_graphics_ip
[20:41:32] [PASSED] xe_gmdid_media_ip
[20:41:32] ===================== [PASSED] xe_pci ======================
[20:41:32] ==================== xe_rtp (1 subtest) ====================
[20:41:32] ================== xe_rtp_process_tests ===================
[20:41:32] [PASSED] coalesce-same-reg
[20:41:32] [PASSED] no-match-no-add
[20:41:32] [PASSED] no-match-no-add-multiple-rules
[20:41:32] [PASSED] two-regs-two-entries
[20:41:32] [PASSED] clr-one-set-other
[20:41:32] [PASSED] set-field
[20:41:32] [PASSED] conflict-duplicate
[20:41:32] [PASSED] conflict-not-disjoint
[20:41:32] [PASSED] conflict-reg-type
[20:41:32] ============== [PASSED] xe_rtp_process_tests ===============
[20:41:32] ===================== [PASSED] xe_rtp ======================
[20:41:32] ==================== xe_wa (1 subtest) =====================
[20:41:32] ======================== xe_wa_gt =========================
[20:41:32] [PASSED] TIGERLAKE (B0)
[20:41:32] [PASSED] DG1 (A0)
[20:41:32] [PASSED] DG1 (B0)
[20:41:32] [PASSED] ALDERLAKE_S (A0)
[20:41:32] [PASSED] ALDERLAKE_S (B0)
[20:41:32] [PASSED] ALDERLAKE_S (C0)
[20:41:32] [PASSED] ALDERLAKE_S (D0)
[20:41:32] [PASSED] ALDERLAKE_P (A0)
[20:41:32] [PASSED] ALDERLAKE_P (B0)
[20:41:32] [PASSED] ALDERLAKE_P (C0)
[20:41:32] [PASSED] DG2_G10 (A0)
[20:41:32] [PASSED] DG2_G10 (A1)
[20:41:32] [PASSED] DG2_G10 (B0)
[20:41:32] [PASSED] DG2_G10 (C0)
[20:41:32] [PASSED] DG2_G11 (A0)
[20:41:32] [PASSED] DG2_G11 (B0)
[20:41:32] [PASSED] DG2_G11 (B1)
[20:41:32] [PASSED] DG2_G12 (A0)
[20:41:32] [PASSED] DG2_G12 (A1)
[20:41:32] [PASSED] PVC (B0)
[20:41:32] [PASSED] PVC (B1)
[20:41:32] [PASSED] PVC (C0)
[20:41:32] ==================== [PASSED] xe_wa_gt =====================
[20:41:32] ====================== [PASSED] xe_wa ======================
[20:41:32] ============================================================
[20:41:32] Testing complete. Ran 37 tests: passed: 33, skipped: 4
[20:41:33] Elapsed time: 23.622s total, 4.227s configuring, 19.225s building, 0.137s running
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/tests/.kunitconfig
[20:41:33] Configuring KUnit Kernel ...
Regenerating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[20:41:34] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make ARCH=um O=.kunit --jobs=48
[20:41:53] Starting KUnit Kernel (1/1)...
[20:41:53] ============================================================
[20:41:53] ============ drm_test_pick_cmdline (2 subtests) ============
[20:41:53] [PASSED] drm_test_pick_cmdline_res_1920_1080_60
[20:41:53] =============== drm_test_pick_cmdline_named ===============
[20:41:53] [PASSED] NTSC
[20:41:53] [PASSED] NTSC-J
[20:41:53] [PASSED] PAL
[20:41:53] [PASSED] PAL-M
[20:41:53] =========== [PASSED] drm_test_pick_cmdline_named ===========
[20:41:53] ============== [PASSED] drm_test_pick_cmdline ==============
[20:41:53] ================== drm_buddy (6 subtests) ==================
[20:41:53] [PASSED] drm_test_buddy_alloc_limit
[20:41:53] [PASSED] drm_test_buddy_alloc_range
[20:41:53] [PASSED] drm_test_buddy_alloc_optimistic
[20:41:53] [PASSED] drm_test_buddy_alloc_pessimistic
[20:41:53] [PASSED] drm_test_buddy_alloc_smoke
[20:41:53] [PASSED] drm_test_buddy_alloc_pathological
[20:41:53] ==================== [PASSED] drm_buddy ====================
[20:41:53] ============= drm_cmdline_parser (40 subtests) =============
[20:41:53] [PASSED] drm_test_cmdline_force_d_only
[20:41:53] [PASSED] drm_test_cmdline_force_D_only_dvi
[20:41:53] [PASSED] drm_test_cmdline_force_D_only_hdmi
[20:41:53] [PASSED] drm_test_cmdline_force_D_only_not_digital
[20:41:53] [PASSED] drm_test_cmdline_force_e_only
[20:41:53] [PASSED] drm_test_cmdline_res
[20:41:53] [PASSED] drm_test_cmdline_res_vesa
[20:41:53] [PASSED] drm_test_cmdline_res_vesa_rblank
[20:41:53] [PASSED] drm_test_cmdline_res_rblank
[20:41:53] [PASSED] drm_test_cmdline_res_bpp
[20:41:53] [PASSED] drm_test_cmdline_res_refresh
[20:41:53] [PASSED] drm_test_cmdline_res_bpp_refresh
[20:41:53] [PASSED] drm_test_cmdline_res_bpp_refresh_interlaced
[20:41:53] [PASSED] drm_test_cmdline_res_bpp_refresh_margins
[20:41:53] [PASSED] drm_test_cmdline_res_bpp_refresh_force_off
[20:41:53] [PASSED] drm_test_cmdline_res_bpp_refresh_force_on
[20:41:53] [PASSED] drm_test_cmdline_res_bpp_refresh_force_on_analog
[20:41:53] [PASSED] drm_test_cmdline_res_bpp_refresh_force_on_digital
[20:41:53] [PASSED] drm_test_cmdline_res_bpp_refresh_interlaced_margins_force_on
[20:41:53] [PASSED] drm_test_cmdline_res_margins_force_on
[20:41:53] [PASSED] drm_test_cmdline_res_vesa_margins
[20:41:53] [PASSED] drm_test_cmdline_name
[20:41:53] [PASSED] drm_test_cmdline_name_bpp
[20:41:53] [PASSED] drm_test_cmdline_name_option
[20:41:53] [PASSED] drm_test_cmdline_name_bpp_option
[20:41:53] [PASSED] drm_test_cmdline_rotate_0
[20:41:53] [PASSED] drm_test_cmdline_rotate_90
[20:41:53] [PASSED] drm_test_cmdline_rotate_180
[20:41:53] [PASSED] drm_test_cmdline_rotate_270
[20:41:53] [PASSED] drm_test_cmdline_hmirror
[20:41:53] [PASSED] drm_test_cmdline_vmirror
[20:41:53] [PASSED] drm_test_cmdline_margin_options
[20:41:53] [PASSED] drm_test_cmdline_multiple_options
[20:41:53] [PASSED] drm_test_cmdline_bpp_extra_and_option
[20:41:53] [PASSED] drm_test_cmdline_extra_and_option
[20:41:53] [PASSED] drm_test_cmdline_freestanding_options
[20:41:53] [PASSED] drm_test_cmdline_freestanding_force_e_and_options
[20:41:53] [PASSED] drm_test_cmdline_panel_orientation
[20:41:53] ================ drm_test_cmdline_invalid =================
[20:41:53] [PASSED] margin_only
[20:41:53] [PASSED] interlace_only
[20:41:53] [PASSED] res_missing_x
[20:41:53] [PASSED] res_missing_y
[20:41:53] [PASSED] res_bad_y
[20:41:53] [PASSED] res_missing_y_bpp
[20:41:53] [PASSED] res_bad_bpp
[20:41:53] [PASSED] res_bad_refresh
[20:41:53] [PASSED] res_bpp_refresh_force_on_off
[20:41:53] [PASSED] res_invalid_mode
[20:41:53] [PASSED] res_bpp_wrong_place_mode
[20:41:53] [PASSED] name_bpp_refresh
[20:41:53] [PASSED] name_refresh
[20:41:53] [PASSED] name_refresh_wrong_mode
[20:41:53] [PASSED] name_refresh_invalid_mode
[20:41:53] [PASSED] rotate_multiple
[20:41:53] [PASSED] rotate_invalid_val
[20:41:53] [PASSED] rotate_truncated
[20:41:53] [PASSED] invalid_option
[20:41:53] [PASSED] invalid_tv_option
[20:41:53] [PASSED] truncated_tv_option
[20:41:53] ============ [PASSED] drm_test_cmdline_invalid =============
[20:41:53] =============== drm_test_cmdline_tv_options ===============
[20:41:53] [PASSED] NTSC
[20:41:53] [PASSED] NTSC_443
[20:41:53] [PASSED] NTSC_J
[20:41:53] [PASSED] PAL
[20:41:53] [PASSED] PAL_M
[20:41:53] [PASSED] PAL_N
[20:41:53] [PASSED] SECAM
[20:41:53] =========== [PASSED] drm_test_cmdline_tv_options ===========
[20:41:53] =============== [PASSED] drm_cmdline_parser ================
[20:41:53] ========== drm_get_tv_mode_from_name (2 subtests) ==========
[20:41:53] ========== drm_test_get_tv_mode_from_name_valid ===========
[20:41:53] [PASSED] NTSC
[20:41:53] [PASSED] NTSC-443
[20:41:53] [PASSED] NTSC-J
[20:41:53] [PASSED] PAL
[20:41:53] [PASSED] PAL-M
[20:41:53] [PASSED] PAL-N
[20:41:53] [PASSED] SECAM
[20:41:53] ====== [PASSED] drm_test_get_tv_mode_from_name_valid =======
[20:41:53] [PASSED] drm_test_get_tv_mode_from_name_truncated
[20:41:53] ============ [PASSED] drm_get_tv_mode_from_name ============
[20:41:53] ============= drm_damage_helper (21 subtests) ==============
[20:41:53] [PASSED] drm_test_damage_iter_no_damage
[20:41:53] [PASSED] drm_test_damage_iter_no_damage_fractional_src
[20:41:53] [PASSED] drm_test_damage_iter_no_damage_src_moved
[20:41:53] [PASSED] drm_test_damage_iter_no_damage_fractional_src_moved
[20:41:53] [PASSED] drm_test_damage_iter_no_damage_not_visible
[20:41:53] [PASSED] drm_test_damage_iter_no_damage_no_crtc
[20:41:53] [PASSED] drm_test_damage_iter_no_damage_no_fb
[20:41:53] [PASSED] drm_test_damage_iter_simple_damage
[20:41:53] [PASSED] drm_test_damage_iter_single_damage
[20:41:53] [PASSED] drm_test_damage_iter_single_damage_intersect_src
[20:41:53] [PASSED] drm_test_damage_iter_single_damage_outside_src
[20:41:53] [PASSED] drm_test_damage_iter_single_damage_fractional_src
[20:41:53] [PASSED] drm_test_damage_iter_single_damage_intersect_fractional_src
[20:41:53] [PASSED] drm_test_damage_iter_single_damage_outside_fractional_src
[20:41:53] [PASSED] drm_test_damage_iter_single_damage_src_moved
[20:41:53] [PASSED] drm_test_damage_iter_single_damage_fractional_src_moved
[20:41:53] [PASSED] drm_test_damage_iter_damage
[20:41:53] [PASSED] drm_test_damage_iter_damage_one_intersect
[20:41:53] [PASSED] drm_test_damage_iter_damage_one_outside
[20:41:53] [PASSED] drm_test_damage_iter_damage_src_moved
[20:41:53] [PASSED] drm_test_damage_iter_damage_not_visible
[20:41:53] ================ [PASSED] drm_damage_helper ================
[20:41:53] ============== drm_dp_mst_helper (2 subtests) ==============
[20:41:53] ============== drm_test_dp_mst_calc_pbn_mode ==============
[20:41:53] [PASSED] Clock 154000 BPP 30 DSC disabled
[20:41:53] [PASSED] Clock 234000 BPP 30 DSC disabled
[20:41:53] [PASSED] Clock 297000 BPP 24 DSC disabled
[20:41:53] [PASSED] Clock 332880 BPP 24 DSC enabled
[20:41:53] [PASSED] Clock 324540 BPP 24 DSC enabled
[20:41:53] ========== [PASSED] drm_test_dp_mst_calc_pbn_mode ==========
[20:41:53] ========= drm_test_dp_mst_sideband_msg_req_decode =========
[20:41:53] [PASSED] DP_ENUM_PATH_RESOURCES with port number
[20:41:53] [PASSED] DP_POWER_UP_PHY with port number
[20:41:53] [PASSED] DP_POWER_DOWN_PHY with port number
[20:41:53] [PASSED] DP_ALLOCATE_PAYLOAD with SDP stream sinks
[20:41:53] [PASSED] DP_ALLOCATE_PAYLOAD with port number
[20:41:53] [PASSED] DP_ALLOCATE_PAYLOAD with VCPI
[20:41:53] [PASSED] DP_ALLOCATE_PAYLOAD with PBN
[20:41:53] [PASSED] DP_QUERY_PAYLOAD with port number
[20:41:53] [PASSED] DP_QUERY_PAYLOAD with VCPI
[20:41:53] [PASSED] DP_REMOTE_DPCD_READ with port number
[20:41:53] [PASSED] DP_REMOTE_DPCD_READ with DPCD address
[20:41:53] [PASSED] DP_REMOTE_DPCD_READ with max number of bytes
[20:41:53] [PASSED] DP_REMOTE_DPCD_WRITE with port number
[20:41:53] [PASSED] DP_REMOTE_DPCD_WRITE with DPCD address
[20:41:53] [PASSED] DP_REMOTE_DPCD_WRITE with data array
[20:41:53] [PASSED] DP_REMOTE_I2C_READ with port number
[20:41:53] [PASSED] DP_REMOTE_I2C_READ with I2C device ID
[20:41:53] [PASSED] DP_REMOTE_I2C_READ with transactions array
[20:41:53] [PASSED] DP_REMOTE_I2C_WRITE with port number
[20:41:53] [PASSED] DP_REMOTE_I2C_WRITE with I2C device ID
[20:41:53] [PASSED] DP_REMOTE_I2C_WRITE with data array
[20:41:53] [PASSED] DP_QUERY_STREAM_ENC_STATUS with stream ID
[20:41:53] [PASSED] DP_QUERY_STREAM_ENC_STATUS with client ID
[20:41:53] [PASSED] DP_QUERY_STREAM_ENC_STATUS with stream event
[20:41:53] [PASSED] DP_QUERY_STREAM_ENC_STATUS with valid stream event
[20:41:53] [PASSED] DP_QUERY_STREAM_ENC_STATUS with stream behavior
[20:41:53] [PASSED] DP_QUERY_STREAM_ENC_STATUS with a valid stream behavior
[20:41:53] ===== [PASSED] drm_test_dp_mst_sideband_msg_req_decode =====
[20:41:53] ================ [PASSED] drm_dp_mst_helper ================
[20:41:53] =========== drm_format_helper_test (11 subtests) ===========
[20:41:53] ============== drm_test_fb_xrgb8888_to_gray8 ==============
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ========== [PASSED] drm_test_fb_xrgb8888_to_gray8 ==========
[20:41:53] ============= drm_test_fb_xrgb8888_to_rgb332 ==============
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ========= [PASSED] drm_test_fb_xrgb8888_to_rgb332 ==========
[20:41:53] ============= drm_test_fb_xrgb8888_to_rgb565 ==============
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ========= [PASSED] drm_test_fb_xrgb8888_to_rgb565 ==========
[20:41:53] ============ drm_test_fb_xrgb8888_to_xrgb1555 =============
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ======== [PASSED] drm_test_fb_xrgb8888_to_xrgb1555 =========
[20:41:53] ============ drm_test_fb_xrgb8888_to_argb1555 =============
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ======== [PASSED] drm_test_fb_xrgb8888_to_argb1555 =========
[20:41:53] ============ drm_test_fb_xrgb8888_to_rgba5551 =============
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ======== [PASSED] drm_test_fb_xrgb8888_to_rgba5551 =========
[20:41:53] ============= drm_test_fb_xrgb8888_to_rgb888 ==============
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ========= [PASSED] drm_test_fb_xrgb8888_to_rgb888 ==========
[20:41:53] ============ drm_test_fb_xrgb8888_to_argb8888 =============
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ======== [PASSED] drm_test_fb_xrgb8888_to_argb8888 =========
[20:41:53] =========== drm_test_fb_xrgb8888_to_xrgb2101010 ===========
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ======= [PASSED] drm_test_fb_xrgb8888_to_xrgb2101010 =======
[20:41:53] =========== drm_test_fb_xrgb8888_to_argb2101010 ===========
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ======= [PASSED] drm_test_fb_xrgb8888_to_argb2101010 =======
[20:41:53] ============== drm_test_fb_xrgb8888_to_mono ===============
[20:41:53] [PASSED] single_pixel_source_buffer
[20:41:53] [PASSED] single_pixel_clip_rectangle
[20:41:53] [PASSED] well_known_colors
[20:41:53] [PASSED] destination_pitch
[20:41:53] ========== [PASSED] drm_test_fb_xrgb8888_to_mono ===========
[20:41:53] ============= [PASSED] drm_format_helper_test ==============
[20:41:53] ================= drm_format (18 subtests) =================
[20:41:53] [PASSED] drm_test_format_block_width_invalid
[20:41:53] [PASSED] drm_test_format_block_width_one_plane
[20:41:53] [PASSED] drm_test_format_block_width_two_plane
[20:41:53] [PASSED] drm_test_format_block_width_three_plane
[20:41:53] [PASSED] drm_test_format_block_width_tiled
[20:41:53] [PASSED] drm_test_format_block_height_invalid
[20:41:53] [PASSED] drm_test_format_block_height_one_plane
[20:41:53] [PASSED] drm_test_format_block_height_two_plane
[20:41:53] [PASSED] drm_test_format_block_height_three_plane
[20:41:53] [PASSED] drm_test_format_block_height_tiled
[20:41:53] [PASSED] drm_test_format_min_pitch_invalid
[20:41:53] [PASSED] drm_test_format_min_pitch_one_plane_8bpp
[20:41:53] [PASSED] drm_test_format_min_pitch_one_plane_16bpp
[20:41:53] [PASSED] drm_test_format_min_pitch_one_plane_24bpp
[20:41:53] [PASSED] drm_test_format_min_pitch_one_plane_32bpp
[20:41:53] [PASSED] drm_test_format_min_pitch_two_plane
[20:41:53] [PASSED] drm_test_format_min_pitch_three_plane_8bpp
[20:41:53] [PASSED] drm_test_format_min_pitch_tiled
[20:41:53] =================== [PASSED] drm_format ====================
[20:41:53] =============== drm_framebuffer (1 subtest) ================
[20:41:53] =============== drm_test_framebuffer_create ===============
[20:41:53] [PASSED] ABGR8888 normal sizes
[20:41:53] [PASSED] ABGR8888 max sizes
[20:41:53] [PASSED] ABGR8888 pitch greater than min required
[20:41:53] [PASSED] ABGR8888 pitch less than min required
[20:41:53] [PASSED] ABGR8888 Invalid width
[20:41:53] [PASSED] ABGR8888 Invalid buffer handle
[20:41:53] [PASSED] No pixel format
[20:41:53] [PASSED] ABGR8888 Width 0
[20:41:53] [PASSED] ABGR8888 Height 0
[20:41:53] [PASSED] ABGR8888 Out of bound height * pitch combination
[20:41:53] [PASSED] ABGR8888 Large buffer offset
[20:41:53] [PASSED] ABGR8888 Set DRM_MODE_FB_MODIFIERS without modifiers
[20:41:53] [PASSED] ABGR8888 Valid buffer modifier
[20:41:53] [PASSED] ABGR8888 Invalid buffer modifier(DRM_FORMAT_MOD_SAMSUNG_64_32_TILE)
[20:41:53] [PASSED] ABGR8888 Extra pitches without DRM_MODE_FB_MODIFIERS
[20:41:53] [PASSED] ABGR8888 Extra pitches with DRM_MODE_FB_MODIFIERS
[20:41:53] [PASSED] NV12 Normal sizes
[20:41:53] [PASSED] NV12 Max sizes
[20:41:53] [PASSED] NV12 Invalid pitch
[20:41:53] [PASSED] NV12 Invalid modifier/missing DRM_MODE_FB_MODIFIERS flag
[20:41:53] [PASSED] NV12 different modifier per-plane
[20:41:53] [PASSED] NV12 with DRM_FORMAT_MOD_SAMSUNG_64_32_TILE
[20:41:53] [PASSED] NV12 Valid modifiers without DRM_MODE_FB_MODIFIERS
[20:41:53] [PASSED] NV12 Modifier for inexistent plane
[20:41:53] [PASSED] NV12 Handle for inexistent plane
[20:41:53] [PASSED] NV12 Handle for inexistent plane without DRM_MODE_FB_MODIFIERS
[20:41:53] [PASSED] YVU420 DRM_MODE_FB_MODIFIERS set without modifier
[20:41:53] [PASSED] YVU420 Normal sizes
[20:41:53] [PASSED] YVU420 Max sizes
[20:41:53] [PASSED] YVU420 Invalid pitch
[20:41:53] [PASSED] YVU420 Different pitches
[20:41:53] [PASSED] YVU420 Different buffer offsets/pitches
[20:41:53] [PASSED] YVU420 Modifier set just for plane 0, without DRM_MODE_FB_MODIFIERS
[20:41:53] [PASSED] YVU420 Modifier set just for planes 0, 1, without DRM_MODE_FB_MODIFIERS
[20:41:53] [PASSED] YVU420 Modifier set just for plane 0, 1, with DRM_MODE_FB_MODIFIERS
[20:41:53] [PASSED] YVU420 Valid modifier
[20:41:53] [PASSED] YVU420 Different modifiers per plane
[20:41:53] [PASSED] YVU420 Modifier for inexistent plane
[20:41:53] [PASSED] X0L2 Normal sizes
[20:41:53] [PASSED] X0L2 Max sizes
[20:41:53] [PASSED] X0L2 Invalid pitch
[20:41:53] [PASSED] X0L2 Pitch greater than minimum required
stty: 'standard input': Inappropriate ioctl for device
[20:41:53] [PASSED] X0L2 Handle for inexistent plane
[20:41:53] [PASSED] X0L2 Offset for inexistent plane, without DRM_MODE_FB_MODIFIERS set
[20:41:53] [PASSED] X0L2 Modifier without DRM_MODE_FB_MODIFIERS set
[20:41:53] [PASSED] X0L2 Valid modifier
[20:41:53] [PASSED] X0L2 Modifier for inexistent plane
[20:41:53] =========== [PASSED] drm_test_framebuffer_create ===========
[20:41:53] ================= [PASSED] drm_framebuffer =================
[20:41:53] =============== drm-test-managed (1 subtest) ===============
[20:41:53] [PASSED] drm_test_managed_run_action
[20:41:53] ================ [PASSED] drm-test-managed =================
[20:41:53] =================== drm_mm (19 subtests) ===================
[20:41:53] [PASSED] drm_test_mm_init
[20:41:53] [PASSED] drm_test_mm_debug
[20:42:03] [PASSED] drm_test_mm_reserve
[20:42:13] [PASSED] drm_test_mm_insert
[20:42:13] [PASSED] drm_test_mm_replace
[20:42:13] [PASSED] drm_test_mm_insert_range
[20:42:13] [PASSED] drm_test_mm_frag
[20:42:13] [PASSED] drm_test_mm_align
[20:42:13] [PASSED] drm_test_mm_align32
[20:42:14] [PASSED] drm_test_mm_align64
[20:42:14] [PASSED] drm_test_mm_evict
[20:42:14] [PASSED] drm_test_mm_evict_range
[20:42:14] [PASSED] drm_test_mm_topdown
[20:42:14] [PASSED] drm_test_mm_bottomup
[20:42:14] [PASSED] drm_test_mm_lowest
[20:42:14] [PASSED] drm_test_mm_highest
[20:42:15] [PASSED] drm_test_mm_color
[20:42:15] [PASSED] drm_test_mm_color_evict
[20:42:15] [PASSED] drm_test_mm_color_evict_range
[20:42:15] ===================== [PASSED] drm_mm ======================
[20:42:15] ============= drm_modes_analog_tv (4 subtests) =============
[20:42:15] [PASSED] drm_test_modes_analog_tv_ntsc_480i
[20:42:15] [PASSED] drm_test_modes_analog_tv_ntsc_480i_inlined
[20:42:15] [PASSED] drm_test_modes_analog_tv_pal_576i
[20:42:15] [PASSED] drm_test_modes_analog_tv_pal_576i_inlined
[20:42:15] =============== [PASSED] drm_modes_analog_tv ===============
[20:42:15] ============== drm_plane_helper (2 subtests) ===============
[20:42:15] =============== drm_test_check_plane_state ================
[20:42:15] [PASSED] clipping_simple
[20:42:15] [PASSED] clipping_rotate_reflect
[20:42:15] [PASSED] positioning_simple
[20:42:15] [PASSED] upscaling
[20:42:15] [PASSED] downscaling
[20:42:15] [PASSED] rounding1
[20:42:15] [PASSED] rounding2
[20:42:15] [PASSED] rounding3
[20:42:15] [PASSED] rounding4
[20:42:15] =========== [PASSED] drm_test_check_plane_state ============
[20:42:15] =========== drm_test_check_invalid_plane_state ============
[20:42:15] [PASSED] positioning_invalid
[20:42:15] [PASSED] upscaling_invalid
[20:42:15] [PASSED] downscaling_invalid
[20:42:15] ======= [PASSED] drm_test_check_invalid_plane_state ========
[20:42:15] ================ [PASSED] drm_plane_helper =================
[20:42:15] ====== drm_connector_helper_tv_get_modes (1 subtest) =======
[20:42:15] ====== drm_test_connector_helper_tv_get_modes_check =======
[20:42:15] [PASSED] None
[20:42:15] [PASSED] PAL
[20:42:15] [PASSED] NTSC
[20:42:15] [PASSED] Both, NTSC Default
[20:42:15] [PASSED] Both, PAL Default
[20:42:15] [PASSED] Both, NTSC Default, with PAL on command-line
[20:42:15] [PASSED] Both, PAL Default, with NTSC on command-line
[20:42:15] == [PASSED] drm_test_connector_helper_tv_get_modes_check ===
[20:42:15] ======== [PASSED] drm_connector_helper_tv_get_modes ========
[20:42:15] ================== drm_rect (9 subtests) ===================
[20:42:15] [PASSED] drm_test_rect_clip_scaled_div_by_zero
[20:42:15] [PASSED] drm_test_rect_clip_scaled_not_clipped
[20:42:15] [PASSED] drm_test_rect_clip_scaled_clipped
[20:42:15] [PASSED] drm_test_rect_clip_scaled_signed_vs_unsigned
[20:42:15] ================= drm_test_rect_intersect =================
[20:42:15] [PASSED] top-left x bottom-right: 2x2+1+1 x 2x2+0+0
[20:42:15] [PASSED] top-right x bottom-left: 2x2+0+0 x 2x2+1-1
[20:42:15] [PASSED] bottom-left x top-right: 2x2+1-1 x 2x2+0+0
[20:42:15] [PASSED] bottom-right x top-left: 2x2+0+0 x 2x2+1+1
[20:42:15] [PASSED] right x left: 2x1+0+0 x 3x1+1+0
[20:42:15] [PASSED] left x right: 3x1+1+0 x 2x1+0+0
[20:42:15] [PASSED] up x bottom: 1x2+0+0 x 1x3+0-1
[20:42:15] [PASSED] bottom x up: 1x3+0-1 x 1x2+0+0
[20:42:15] [PASSED] touching corner: 1x1+0+0 x 2x2+1+1
[20:42:15] [PASSED] touching side: 1x1+0+0 x 1x1+1+0
[20:42:15] [PASSED] equal rects: 2x2+0+0 x 2x2+0+0
[20:42:15] [PASSED] inside another: 2x2+0+0 x 1x1+1+1
[20:42:15] [PASSED] far away: 1x1+0+0 x 1x1+3+6
[20:42:15] [PASSED] points intersecting: 0x0+5+10 x 0x0+5+10
[20:42:15] [PASSED] points not intersecting: 0x0+0+0 x 0x0+5+10
[20:42:15] ============= [PASSED] drm_test_rect_intersect =============
[20:42:15] ================ drm_test_rect_calc_hscale ================
[20:42:15] [PASSED] normal use
[20:42:15] [PASSED] out of max range
[20:42:15] [PASSED] out of min range
[20:42:15] [PASSED] zero dst
[20:42:15] [PASSED] negative src
[20:42:15] [PASSED] negative dst
[20:42:15] ============ [PASSED] drm_test_rect_calc_hscale ============
[20:42:15] ================ drm_test_rect_calc_vscale ================
[20:42:15] [PASSED] normal use
[20:42:15] [PASSED] out of max range
[20:42:15] [PASSED] out of min range
[20:42:15] [PASSED] zero dst
[20:42:15] [PASSED] negative src
[20:42:15] [PASSED] negative dst
[20:42:15] ============ [PASSED] drm_test_rect_calc_vscale ============
[20:42:15] ================== drm_test_rect_rotate ===================
[20:42:15] [PASSED] reflect-x
[20:42:15] [PASSED] reflect-y
[20:42:15] [PASSED] rotate-0
[20:42:15] [PASSED] rotate-90
[20:42:15] [PASSED] rotate-180
[20:42:15] [PASSED] rotate-270
[20:42:15] ============== [PASSED] drm_test_rect_rotate ===============
[20:42:15] ================ drm_test_rect_rotate_inv =================
[20:42:15] [PASSED] reflect-x
[20:42:15] [PASSED] reflect-y
[20:42:15] [PASSED] rotate-0
[20:42:15] [PASSED] rotate-90
[20:42:15] [PASSED] rotate-180
[20:42:15] [PASSED] rotate-270
[20:42:15] ============ [PASSED] drm_test_rect_rotate_inv =============
[20:42:15] ==================== [PASSED] drm_rect =====================
[20:42:15] ============================================================
[20:42:15] Testing complete. Ran 333 tests: passed: 333
[20:42:16] Elapsed time: 42.932s total, 1.694s configuring, 18.444s building, 22.768s running
+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-xe] [PATCH] drm/xe: Remove unnecessary memory barriers related to wait queue
2023-07-27 18:34 [Intel-xe] [PATCH] drm/xe: Remove unnecessary memory barriers related to wait queue Matthew Brost
` (2 preceding siblings ...)
2023-07-27 20:42 ` [Intel-xe] ✓ CI.KUnit: " Patchwork
@ 2023-07-28 12:13 ` Matthew Auld
2023-07-28 14:52 ` Matthew Brost
3 siblings, 1 reply; 6+ messages in thread
From: Matthew Auld @ 2023-07-28 12:13 UTC (permalink / raw)
To: Matthew Brost; +Cc: intel-xe
On Thu, 27 Jul 2023 at 19:34, Matthew Brost <matthew.brost@intel.com> wrote:
>
> These not needed per the wait queue doc, remove them.
>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/xe/xe_guc_ct.c | 3 ---
> drivers/gpu/drm/xe/xe_guc_submit.c | 9 +--------
> 2 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index cb75db30800c..3f58902b6be3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -308,7 +308,6 @@ int xe_guc_ct_enable(struct xe_guc_ct *ct)
> spin_unlock_irq(&ct->fast_lock);
> mutex_unlock(&ct->lock);
>
> - smp_mb();
> wake_up_all(&ct->wq);
> drm_dbg(&xe->drm, "GuC CT communication channel enabled\n");
>
> @@ -820,8 +819,6 @@ static int parse_g2h_response(struct xe_guc_ct *ct, u32 *msg, u32 len)
> g2h_release_space(ct, GUC_CTB_HXG_MSG_MAX_LEN);
>
> g2h_fence->done = true;
> - smp_mb();
Hmm, pretty sure we still need barriers such that done = 1 is seen to
happen after the stores for fail, error, hint, retry etc, and then ofc
a matching barrier on the reader side. The barriers in wake_up_all()
vs wait_event() won't ensure that. So something like:
parse_g2h_response():
/*
* Pairs with guc_ct_send_recv().
*
* The ordering of g2h_fence.done vs wake_up_all() and
* wait_event_timeout() is already handled by that api. However here we
* also have the added dependency of ordering the g2h_fence.done after
* the previous stores, like with g2h_fence.retry, g2h_fence.fail etc.
* which we need to handle ourselves.
*/
smp_wmb();
WRITE_ONCE(g2h_fence->done, true);
wake_up_all(&ct->g2h_fence_wq);
guc_ct_send_recv():
ret = wait_event_timeout(ct->g2h_fence_wq, READ_ONCE(g2h_fence.done), HZ);
if (!ret) {
}
/* Pairs with parse_g2h_response(). */
smp_rmb();
if (READ_ONCE(g2h_fence.retry)) {
}
if (READ_ONCE(g2h_fence.fail)) {
}
But AFAICT there also seems to be scary lifetime issues in here. The
g2h_fence is allocated on the stack, and goes out of scope when
returning from guc_ct_send_recv(), however if the wait_event timedout
we don't know if parse_g2h_response() will still run before we remove
the fence from the xarray. I think we need to grab the ct->mutex to
serialize the operations. Also if we have to acquire ct->mutex anyway
it might be worth seeing if we can drop the scary shared memory access
outside locks stuff here.
> -
> wake_up_all(&ct->g2h_fence_wq);
>
> return 0;
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 6cb64a097297..d623cd9f6cc4 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -705,7 +705,6 @@ static void disable_scheduling_deregister(struct xe_guc *guc,
> int ret;
>
> set_min_preemption_timeout(guc, e);
> - smp_rmb();
> ret = wait_event_timeout(guc->ct.wq, !engine_pending_enable(e) ||
> guc_read_stopped(guc), HZ * 5);
> if (!ret) {
> @@ -888,7 +887,6 @@ guc_engine_timedout_job(struct drm_sched_job *drm_job)
> * error) messages which can cause the schedule disable to get
> * lost. If this occurs, trigger a GT reset to recover.
> */
> - smp_rmb();
> ret = wait_event_timeout(guc->ct.wq,
> !engine_pending_disable(e) ||
> guc_read_stopped(guc), HZ * 5);
> @@ -1008,7 +1006,6 @@ static void suspend_fence_signal(struct xe_engine *e)
> XE_BUG_ON(!e->guc->suspend_pending);
>
> e->guc->suspend_pending = false;
Probably a good idea to use WRITE_ONCE/READ_ONCE. Even if it is not
strictly needed from the compiler pov, it at the very least documents
that we are accessing shared memory outside of proper locking.
> - smp_wmb();
> wake_up(&e->guc->suspend_wait);
> }
>
> @@ -1392,7 +1389,6 @@ int xe_guc_submit_reset_prepare(struct xe_guc *guc)
> * and releasing any TDRs waiting on guc->submission_state.stopped.
> */
> ret = atomic_fetch_or(1, &guc->submission_state.stopped);
> - smp_wmb();
> wake_up_all(&guc->ct.wq);
>
> return ret;
> @@ -1521,17 +1517,14 @@ int xe_guc_sched_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
> if (engine_pending_enable(e)) {
> e->guc->resume_time = ktime_get();
> clear_engine_pending_enable(e);
> - smp_wmb();
> wake_up_all(&guc->ct.wq);
> } else {
> clear_engine_pending_disable(e);
> if (e->guc->suspend_pending) {
> suspend_fence_signal(e);
> } else {
> - if (engine_banned(e)) {
> - smp_wmb();
> + if (engine_banned(e))
> wake_up_all(&guc->ct.wq);
> - }
> deregister_engine(guc, e);
> }
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-xe] [PATCH] drm/xe: Remove unnecessary memory barriers related to wait queue
2023-07-28 12:13 ` [Intel-xe] [PATCH] " Matthew Auld
@ 2023-07-28 14:52 ` Matthew Brost
0 siblings, 0 replies; 6+ messages in thread
From: Matthew Brost @ 2023-07-28 14:52 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-xe
On Fri, Jul 28, 2023 at 01:13:04PM +0100, Matthew Auld wrote:
> On Thu, 27 Jul 2023 at 19:34, Matthew Brost <matthew.brost@intel.com> wrote:
> >
> > These not needed per the wait queue doc, remove them.
> >
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_guc_ct.c | 3 ---
> > drivers/gpu/drm/xe/xe_guc_submit.c | 9 +--------
> > 2 files changed, 1 insertion(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> > index cb75db30800c..3f58902b6be3 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > @@ -308,7 +308,6 @@ int xe_guc_ct_enable(struct xe_guc_ct *ct)
> > spin_unlock_irq(&ct->fast_lock);
> > mutex_unlock(&ct->lock);
> >
> > - smp_mb();
> > wake_up_all(&ct->wq);
> > drm_dbg(&xe->drm, "GuC CT communication channel enabled\n");
> >
> > @@ -820,8 +819,6 @@ static int parse_g2h_response(struct xe_guc_ct *ct, u32 *msg, u32 len)
> > g2h_release_space(ct, GUC_CTB_HXG_MSG_MAX_LEN);
> >
> > g2h_fence->done = true;
> > - smp_mb();
>
> Hmm, pretty sure we still need barriers such that done = 1 is seen to
> happen after the stores for fail, error, hint, retry etc, and then ofc
> a matching barrier on the reader side. The barriers in wake_up_all()
> vs wait_event() won't ensure that. So something like:
>
> parse_g2h_response():
> /*
> * Pairs with guc_ct_send_recv().
> *
> * The ordering of g2h_fence.done vs wake_up_all() and
> * wait_event_timeout() is already handled by that api. However here we
> * also have the added dependency of ordering the g2h_fence.done after
> * the previous stores, like with g2h_fence.retry, g2h_fence.fail etc.
> * which we need to handle ourselves.
> */
> smp_wmb();
> WRITE_ONCE(g2h_fence->done, true);
> wake_up_all(&ct->g2h_fence_wq);
>
>
> guc_ct_send_recv():
> ret = wait_event_timeout(ct->g2h_fence_wq, READ_ONCE(g2h_fence.done), HZ);
> if (!ret) {
> }
>
> /* Pairs with parse_g2h_response(). */
> smp_rmb();
> if (READ_ONCE(g2h_fence.retry)) {
> }
>
> if (READ_ONCE(g2h_fence.fail)) {
> }
>
> But AFAICT there also seems to be scary lifetime issues in here. The
> g2h_fence is allocated on the stack, and goes out of scope when
> returning from guc_ct_send_recv(), however if the wait_event timedout
> we don't know if parse_g2h_response() will still run before we remove
> the fence from the xarray. I think we need to grab the ct->mutex to
> serialize the operations. Also if we have to acquire ct->mutex anyway
> it might be worth seeing if we can drop the scary shared memory access
> outside locks stuff here.
>
Yea I think all of this is correct once I think about it.
> > -
> > wake_up_all(&ct->g2h_fence_wq);
> >
> > return 0;
> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> > index 6cb64a097297..d623cd9f6cc4 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -705,7 +705,6 @@ static void disable_scheduling_deregister(struct xe_guc *guc,
> > int ret;
> >
> > set_min_preemption_timeout(guc, e);
> > - smp_rmb();
> > ret = wait_event_timeout(guc->ct.wq, !engine_pending_enable(e) ||
> > guc_read_stopped(guc), HZ * 5);
> > if (!ret) {
> > @@ -888,7 +887,6 @@ guc_engine_timedout_job(struct drm_sched_job *drm_job)
> > * error) messages which can cause the schedule disable to get
> > * lost. If this occurs, trigger a GT reset to recover.
> > */
> > - smp_rmb();
> > ret = wait_event_timeout(guc->ct.wq,
> > !engine_pending_disable(e) ||
> > guc_read_stopped(guc), HZ * 5);
> > @@ -1008,7 +1006,6 @@ static void suspend_fence_signal(struct xe_engine *e)
> > XE_BUG_ON(!e->guc->suspend_pending);
> >
> > e->guc->suspend_pending = false;
>
> Probably a good idea to use WRITE_ONCE/READ_ONCE. Even if it is not
> strictly needed from the compiler pov, it at the very least documents
> that we are accessing shared memory outside of proper locking.
>
Sure.
Matt
>
>
>
> > - smp_wmb();
> > wake_up(&e->guc->suspend_wait);
> > }
> >
> > @@ -1392,7 +1389,6 @@ int xe_guc_submit_reset_prepare(struct xe_guc *guc)
> > * and releasing any TDRs waiting on guc->submission_state.stopped.
> > */
> > ret = atomic_fetch_or(1, &guc->submission_state.stopped);
> > - smp_wmb();
> > wake_up_all(&guc->ct.wq);
> >
> > return ret;
> > @@ -1521,17 +1517,14 @@ int xe_guc_sched_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
> > if (engine_pending_enable(e)) {
> > e->guc->resume_time = ktime_get();
> > clear_engine_pending_enable(e);
> > - smp_wmb();
> > wake_up_all(&guc->ct.wq);
> > } else {
> > clear_engine_pending_disable(e);
> > if (e->guc->suspend_pending) {
> > suspend_fence_signal(e);
> > } else {
> > - if (engine_banned(e)) {
> > - smp_wmb();
> > + if (engine_banned(e))
> > wake_up_all(&guc->ct.wq);
> > - }
> > deregister_engine(guc, e);
> > }
> > }
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-28 14:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-27 18:34 [Intel-xe] [PATCH] drm/xe: Remove unnecessary memory barriers related to wait queue Matthew Brost
2023-07-27 20:40 ` [Intel-xe] ✓ CI.Patch_applied: success for " Patchwork
2023-07-27 20:41 ` [Intel-xe] ✓ CI.checkpatch: " Patchwork
2023-07-27 20:42 ` [Intel-xe] ✓ CI.KUnit: " Patchwork
2023-07-28 12:13 ` [Intel-xe] [PATCH] " Matthew Auld
2023-07-28 14:52 ` Matthew Brost
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox