* [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