* [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly
@ 2023-06-08 9:32 Arun R Murthy
2023-06-08 11:31 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) Patchwork
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Arun R Murthy @ 2023-06-08 9:32 UTC (permalink / raw)
To: intel-gfx
At the begining of the aux transfer a check for aux control busy bit is
done. Then as per the spec on aux transfer timeout, need to retry
freshly for 3 times with a delay which is taken care by the control
register.
On each of these 3 trials a check for busy has to be done so as to start
freshly.
v2: updated the commit message
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 +++++++++------------
1 file changed, 22 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
index 197c6e81db14..25090542dd9f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
@@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
* it using the same AUX CH simultaneously
*/
- /* Try to wait for any previous AUX channel activity */
- for (try = 0; try < 3; try++) {
- status = intel_de_read_notrace(i915, ch_ctl);
- if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0)
- break;
- msleep(1);
- }
- /* just trace the final value */
- trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
-
- if (try == 3) {
- const u32 status = intel_de_read(i915, ch_ctl);
-
- if (status != intel_dp->aux_busy_last_status) {
- drm_WARN(&i915->drm, 1,
- "%s: not started (status 0x%08x)\n",
- intel_dp->aux.name, status);
- intel_dp->aux_busy_last_status = status;
- }
-
- ret = -EBUSY;
- goto out;
- }
-
/* Only 5 data registers! */
if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) {
ret = -E2BIG;
@@ -304,14 +280,31 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
}
while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
- u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
+ /* Must try at least 3 times according to DP spec */
+ for (try = 0; try < 5; try++) {
+ u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
send_bytes,
aux_clock_divider);
- send_ctl |= aux_send_ctl_flags;
+ send_ctl |= aux_send_ctl_flags;
+
+ /* Try to wait for any previous AUX channel activity */
+ status = intel_dp_aux_wait_done(intel_dp);
+ /* just trace the final value */
+ trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
+
+ if (status & DP_AUX_CH_CTL_SEND_BUSY) {
+ drm_WARN(&i915->drm, 1,
+ "%s: not started, previous Tx still in process (status 0x%08x)\n",
+ intel_dp->aux.name, status);
+ intel_dp->aux_busy_last_status = status;
+ if (try > 3) {
+ ret = -EBUSY;
+ goto out;
+ } else
+ continue;
+ }
- /* Must try at least 3 times according to DP spec */
- for (try = 0; try < 5; try++) {
/* Load the send data into the aux channel data registers */
for (i = 0; i < send_bytes; i += 4)
intel_de_write(i915, ch_data[i >> 2],
@@ -321,6 +314,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
/* Send the command and wait for it to complete */
intel_de_write(i915, ch_ctl, send_ctl);
+ /* TODO: if typeC then 4.2ms else 800us. For DG2 add 1.5ms for both cases */
status = intel_dp_aux_wait_done(intel_dp);
/* Clear done status and any errors */
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) 2023-06-08 9:32 [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly Arun R Murthy @ 2023-06-08 11:31 ` Patchwork 2023-06-08 11:55 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Patchwork @ 2023-06-08 11:31 UTC (permalink / raw) To: Arun R Murthy; +Cc: intel-gfx == Series Details == Series: drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) URL : https://patchwork.freedesktop.org/series/119055/ State : warning == Summary == Error: dim checkpatch failed 2a1723da181b drm/i915/display/dp: On AUX xfer timeout restart freshly -:6: WARNING:TYPO_SPELLING: 'begining' may be misspelled - perhaps 'beginning'? #6: At the begining of the aux transfer a check for aux control busy bit is ^^^^^^^^ -:79: CHECK:BRACES: Unbalanced braces around else statement #79: FILE: drivers/gpu/drm/i915/display/intel_dp_aux.c:304: + } else total: 0 errors, 1 warnings, 1 checks, 72 lines checked ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) 2023-06-08 9:32 [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly Arun R Murthy 2023-06-08 11:31 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) Patchwork @ 2023-06-08 11:55 ` Patchwork 2023-06-09 11:56 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork 2023-06-12 11:36 ` [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly Imre Deak 3 siblings, 0 replies; 8+ messages in thread From: Patchwork @ 2023-06-08 11:55 UTC (permalink / raw) To: Arun R Murthy; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 5480 bytes --] == Series Details == Series: drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) URL : https://patchwork.freedesktop.org/series/119055/ State : success == Summary == CI Bug Log - changes from CI_DRM_13249 -> Patchwork_119055v2 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/index.html Participating hosts (34 -> 34) ------------------------------ No changes in participating hosts Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_119055v2: ### IGT changes ### #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@kms_force_connector_basic@prune-stale-modes: - {bat-adlp-11}: [SKIP][1] ([i915#4093]) -> [FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/bat-adlp-11/igt@kms_force_connector_basic@prune-stale-modes.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/bat-adlp-11/igt@kms_force_connector_basic@prune-stale-modes.html Known issues ------------ Here are the changes found in Patchwork_119055v2 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@i915_suspend@basic-s3-without-i915: - bat-rpls-1: NOTRUN -> [ABORT][3] ([i915#6687] / [i915#7978]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/bat-rpls-1/igt@i915_suspend@basic-s3-without-i915.html * igt@kms_chamelium_hpd@common-hpd-after-suspend: - bat-dg1-7: NOTRUN -> [SKIP][4] ([i915#7828]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/bat-dg1-7/igt@kms_chamelium_hpd@common-hpd-after-suspend.html * igt@kms_pipe_crc_basic@suspend-read-crc: - bat-dg1-7: NOTRUN -> [SKIP][5] ([i915#1845] / [i915#4078]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/bat-dg1-7/igt@kms_pipe_crc_basic@suspend-read-crc.html #### Possible fixes #### * igt@i915_selftest@live@hangcheck: - bat-dg1-7: [ABORT][6] ([i915#4983]) -> [PASS][7] [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/bat-dg1-7/igt@i915_selftest@live@hangcheck.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/bat-dg1-7/igt@i915_selftest@live@hangcheck.html * igt@i915_selftest@live@reset: - bat-rpls-1: [ABORT][8] ([i915#4983] / [i915#7461] / [i915#8347] / [i915#8384]) -> [PASS][9] [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/bat-rpls-1/igt@i915_selftest@live@reset.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/bat-rpls-1/igt@i915_selftest@live@reset.html * {igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-c-dp-5}: - {bat-adlp-11}: [ABORT][10] ([i915#4423]) -> [PASS][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-c-dp-5.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-c-dp-5.html * {igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-d-dp-5}: - {bat-adlp-11}: [DMESG-WARN][12] ([i915#4309] / [i915#4423]) -> [PASS][13] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-d-dp-5.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-d-dp-5.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845 [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637 [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078 [i915#4093]: https://gitlab.freedesktop.org/drm/intel/issues/4093 [i915#4309]: https://gitlab.freedesktop.org/drm/intel/issues/4309 [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423 [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983 [i915#6121]: https://gitlab.freedesktop.org/drm/intel/issues/6121 [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687 [i915#6868]: https://gitlab.freedesktop.org/drm/intel/issues/6868 [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461 [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828 [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978 [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347 [i915#8384]: https://gitlab.freedesktop.org/drm/intel/issues/8384 Build changes ------------- * Linux: CI_DRM_13249 -> Patchwork_119055v2 CI-20190529: 20190529 CI_DRM_13249: e5065ee5a0b23e3f970315def269ac363bcb212f @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7322: 2dd77d6d827a308caae49ce3eba759c2bab394ed @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_119055v2: e5065ee5a0b23e3f970315def269ac363bcb212f @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits b4e7033c717d drm/i915/display/dp: On AUX xfer timeout restart freshly == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/index.html [-- Attachment #2: Type: text/html, Size: 6205 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) 2023-06-08 9:32 [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly Arun R Murthy 2023-06-08 11:31 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) Patchwork 2023-06-08 11:55 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork @ 2023-06-09 11:56 ` Patchwork 2023-06-12 11:36 ` [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly Imre Deak 3 siblings, 0 replies; 8+ messages in thread From: Patchwork @ 2023-06-09 11:56 UTC (permalink / raw) To: Arun R Murthy; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 15104 bytes --] == Series Details == Series: drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) URL : https://patchwork.freedesktop.org/series/119055/ State : success == Summary == CI Bug Log - changes from CI_DRM_13249_full -> Patchwork_119055v2_full ==================================================== Summary ------- **SUCCESS** No regressions found. Participating hosts (7 -> 7) ------------------------------ No changes in participating hosts Known issues ------------ Here are the changes found in Patchwork_119055v2_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_barrier_race@remote-request@rcs0: - shard-apl: [PASS][1] -> [ABORT][2] ([i915#7461] / [i915#8190]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-apl6/igt@gem_barrier_race@remote-request@rcs0.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-apl4/igt@gem_barrier_race@remote-request@rcs0.html * igt@gem_exec_balancer@parallel-ordering: - shard-glk: NOTRUN -> [SKIP][3] ([fdo#109271]) +18 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk7/igt@gem_exec_balancer@parallel-ordering.html * igt@gem_lmem_swapping@parallel-random: - shard-apl: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-apl3/igt@gem_lmem_swapping@parallel-random.html * igt@gem_lmem_swapping@verify-random-ccs: - shard-glk: NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +1 similar issue [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk7/igt@gem_lmem_swapping@verify-random-ccs.html * igt@gem_userptr_blits@vma-merge: - shard-glk: NOTRUN -> [FAIL][6] ([i915#3318]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk7/igt@gem_userptr_blits@vma-merge.html * igt@gen9_exec_parse@allowed-single: - shard-glk: [PASS][7] -> [ABORT][8] ([i915#5566]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-glk5/igt@gen9_exec_parse@allowed-single.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk9/igt@gen9_exec_parse@allowed-single.html * igt@kms_ccs@pipe-b-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc: - shard-apl: NOTRUN -> [SKIP][9] ([fdo#109271] / [i915#3886]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-apl3/igt@kms_ccs@pipe-b-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html * igt@kms_ccs@pipe-c-crc-primary-basic-y_tiled_gen12_mc_ccs: - shard-glk: NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#3886]) +1 similar issue [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk7/igt@kms_ccs@pipe-c-crc-primary-basic-y_tiled_gen12_mc_ccs.html * igt@kms_cursor_crc@cursor-rapid-movement-32x10: - shard-glk: NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#4579]) +1 similar issue [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk7/igt@kms_cursor_crc@cursor-rapid-movement-32x10.html * igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-64bpp-yftile-downscaling@pipe-a-valid-mode: - shard-apl: NOTRUN -> [SKIP][12] ([fdo#109271] / [i915#4579]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-apl3/igt@kms_flip_scaled_crc@flip-32bpp-yftile-to-64bpp-yftile-downscaling@pipe-a-valid-mode.html * igt@kms_force_connector_basic@force-connector-state: - shard-apl: NOTRUN -> [SKIP][13] ([fdo#109271]) +26 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-apl3/igt@kms_force_connector_basic@force-connector-state.html * igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-a-hdmi-a-1: - shard-glk: NOTRUN -> [FAIL][14] ([i915#4573]) +1 similar issue [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk7/igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-a-hdmi-a-1.html * igt@kms_plane_scaling@plane-upscale-with-modifiers-factor-0-25@pipe-b-hdmi-a-1: - shard-snb: NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#4579]) +5 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-snb1/igt@kms_plane_scaling@plane-upscale-with-modifiers-factor-0-25@pipe-b-hdmi-a-1.html * igt@kms_plane_scaling@planes-downscale-factor-0-75-unity-scaling@pipe-a-vga-1: - shard-snb: NOTRUN -> [SKIP][16] ([fdo#109271]) +12 similar issues [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-snb5/igt@kms_plane_scaling@planes-downscale-factor-0-75-unity-scaling@pipe-a-vga-1.html * igt@kms_psr2_sf@plane-move-sf-dmg-area: - shard-apl: NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#658]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-apl3/igt@kms_psr2_sf@plane-move-sf-dmg-area.html * igt@kms_psr2_su@page_flip-xrgb8888: - shard-glk: NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#658]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk7/igt@kms_psr2_su@page_flip-xrgb8888.html #### Possible fixes #### * igt@drm_fdinfo@most-busy-check-all@rcs0: - {shard-rkl}: [FAIL][19] ([i915#7742]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-rkl-4/igt@drm_fdinfo@most-busy-check-all@rcs0.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-rkl-7/igt@drm_fdinfo@most-busy-check-all@rcs0.html * igt@gem_barrier_race@remote-request@rcs0: - {shard-tglu}: [ABORT][21] ([i915#8211] / [i915#8234]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-tglu-4/igt@gem_barrier_race@remote-request@rcs0.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-tglu-8/igt@gem_barrier_race@remote-request@rcs0.html * igt@gem_exec_fair@basic-pace-share@rcs0: - {shard-rkl}: [FAIL][23] ([i915#2842]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-rkl-7/igt@gem_exec_fair@basic-pace-share@rcs0.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-rkl-6/igt@gem_exec_fair@basic-pace-share@rcs0.html * igt@gem_exec_reloc@basic-write-wc: - shard-glk: [TIMEOUT][25] -> [PASS][26] +2 similar issues [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-glk3/igt@gem_exec_reloc@basic-write-wc.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk7/igt@gem_exec_reloc@basic-write-wc.html * igt@gem_exec_suspend@basic-s3@smem: - shard-apl: [ABORT][27] ([i915#180] / [i915#8213]) -> [PASS][28] [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-apl2/igt@gem_exec_suspend@basic-s3@smem.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-apl3/igt@gem_exec_suspend@basic-s3@smem.html * igt@i915_pm_rpm@dpms-lpsp: - {shard-dg1}: [SKIP][29] ([i915#1397]) -> [PASS][30] [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-dg1-18/igt@i915_pm_rpm@dpms-lpsp.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-dg1-19/igt@i915_pm_rpm@dpms-lpsp.html * igt@i915_suspend@basic-s3-without-i915: - {shard-rkl}: [FAIL][31] ([fdo#103375]) -> [PASS][32] [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-rkl-6/igt@i915_suspend@basic-s3-without-i915.html [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-rkl-3/igt@i915_suspend@basic-s3-without-i915.html * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions: - shard-glk: [FAIL][33] ([i915#2346]) -> [PASS][34] +1 similar issue [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-glk3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk8/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html * igt@kms_cursor_legacy@forked-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_13249/shard-rkl-7/igt@kms_cursor_legacy@forked-move@pipe-b.html [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-rkl-6/igt@kms_cursor_legacy@forked-move@pipe-b.html - {shard-dg1}: [INCOMPLETE][37] ([i915#8011] / [i915#8347]) -> [PASS][38] +1 similar issue [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-dg1-19/igt@kms_cursor_legacy@forked-move@pipe-b.html [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-dg1-18/igt@kms_cursor_legacy@forked-move@pipe-b.html * igt@kms_frontbuffer_tracking@fbc-2p-shrfb-fliptrack-mmap-gtt: - shard-snb: [SKIP][39] ([fdo#109271]) -> [PASS][40] [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-snb7/igt@kms_frontbuffer_tracking@fbc-2p-shrfb-fliptrack-mmap-gtt.html [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-snb1/igt@kms_frontbuffer_tracking@fbc-2p-shrfb-fliptrack-mmap-gtt.html * igt@perf@enable-disable@0-rcs0: - shard-glk: [TIMEOUT][41] ([i915#8170]) -> [PASS][42] [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13249/shard-glk3/igt@perf@enable-disable@0-rcs0.html [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/shard-glk7/igt@perf@enable-disable@0-rcs0.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 [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#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289 [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295 [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#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615 [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072 [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397 [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180 [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346 [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#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842 [i915#3318]: https://gitlab.freedesktop.org/drm/intel/issues/3318 [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#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689 [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742 [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886 [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955 [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070 [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078 [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270 [i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281 [i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391 [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423 [i915#4573]: https://gitlab.freedesktop.org/drm/intel/issues/4573 [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#4816]: https://gitlab.freedesktop.org/drm/intel/issues/4816 [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833 [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176 [i915#5234]: https://gitlab.freedesktop.org/drm/intel/issues/5234 [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235 [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354 [i915#5493]: https://gitlab.freedesktop.org/drm/intel/issues/5493 [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566 [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784 [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095 [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268 [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433 [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658 [i915#6590]: https://gitlab.freedesktop.org/drm/intel/issues/6590 [i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944 [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953 [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116 [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118 [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461 [i915#7707]: https://gitlab.freedesktop.org/drm/intel/issues/7707 [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742 [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#8170]: https://gitlab.freedesktop.org/drm/intel/issues/8170 [i915#8190]: https://gitlab.freedesktop.org/drm/intel/issues/8190 [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211 [i915#8213]: https://gitlab.freedesktop.org/drm/intel/issues/8213 [i915#8228]: https://gitlab.freedesktop.org/drm/intel/issues/8228 [i915#8234]: https://gitlab.freedesktop.org/drm/intel/issues/8234 [i915#8311]: https://gitlab.freedesktop.org/drm/intel/issues/8311 [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347 [i915#8398]: https://gitlab.freedesktop.org/drm/intel/issues/8398 [i915#8502]: https://gitlab.freedesktop.org/drm/intel/issues/8502 Build changes ------------- * Linux: CI_DRM_13249 -> Patchwork_119055v2 CI-20190529: 20190529 CI_DRM_13249: e5065ee5a0b23e3f970315def269ac363bcb212f @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7322: 2dd77d6d827a308caae49ce3eba759c2bab394ed @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_119055v2: e5065ee5a0b23e3f970315def269ac363bcb212f @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119055v2/index.html [-- Attachment #2: Type: text/html, Size: 14389 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly 2023-06-08 9:32 [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly Arun R Murthy ` (2 preceding siblings ...) 2023-06-09 11:56 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork @ 2023-06-12 11:36 ` Imre Deak 2023-06-12 11:47 ` Murthy, Arun R 3 siblings, 1 reply; 8+ messages in thread From: Imre Deak @ 2023-06-12 11:36 UTC (permalink / raw) To: Arun R Murthy; +Cc: intel-gfx On Thu, Jun 08, 2023 at 03:02:18PM +0530, Arun R Murthy wrote: > At the begining of the aux transfer a check for aux control busy bit is > done. Then as per the spec on aux transfer timeout, need to retry > freshly for 3 times with a delay which is taken care by the control > register. > On each of these 3 trials a check for busy has to be done so as to start > freshly. > > v2: updated the commit message > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 +++++++++------------ > 1 file changed, 22 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c > index 197c6e81db14..25090542dd9f 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > @@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > * it using the same AUX CH simultaneously > */ > > - /* Try to wait for any previous AUX channel activity */ > - for (try = 0; try < 3; try++) { > - status = intel_de_read_notrace(i915, ch_ctl); > - if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0) > - break; > - msleep(1); > - } > - /* just trace the final value */ > - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > - > - if (try == 3) { > - const u32 status = intel_de_read(i915, ch_ctl); > - > - if (status != intel_dp->aux_busy_last_status) { > - drm_WARN(&i915->drm, 1, > - "%s: not started (status 0x%08x)\n", > - intel_dp->aux.name, status); > - intel_dp->aux_busy_last_status = status; > - } > - > - ret = -EBUSY; > - goto out; > - } > - > /* Only 5 data registers! */ > if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) { > ret = -E2BIG; > @@ -304,14 +280,31 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > } > > while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) { > - u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > + /* Must try at least 3 times according to DP spec */ > + for (try = 0; try < 5; try++) { > + u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > send_bytes, > aux_clock_divider); > > - send_ctl |= aux_send_ctl_flags; > + send_ctl |= aux_send_ctl_flags; Why is send_ctl recomputed in each iteration? > + > + /* Try to wait for any previous AUX channel activity */ > + status = intel_dp_aux_wait_done(intel_dp); How does it help to wait here for the same condition that was already waited for at the end of the loop? > + /* just trace the final value */ > + trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > + > + if (status & DP_AUX_CH_CTL_SEND_BUSY) { > + drm_WARN(&i915->drm, 1, > + "%s: not started, previous Tx still in process (status 0x%08x)\n", > + intel_dp->aux.name, status); > + intel_dp->aux_busy_last_status = status; > + if (try > 3) { > + ret = -EBUSY; > + goto out; > + } else > + continue; > + } > > - /* Must try at least 3 times according to DP spec */ > - for (try = 0; try < 5; try++) { > /* Load the send data into the aux channel data registers */ > for (i = 0; i < send_bytes; i += 4) > intel_de_write(i915, ch_data[i >> 2], > @@ -321,6 +314,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > /* Send the command and wait for it to complete */ > intel_de_write(i915, ch_ctl, send_ctl); > > + /* TODO: if typeC then 4.2ms else 800us. For DG2 add 1.5ms for both cases */ > status = intel_dp_aux_wait_done(intel_dp); > > /* Clear done status and any errors */ > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly 2023-06-12 11:36 ` [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly Imre Deak @ 2023-06-12 11:47 ` Murthy, Arun R 2023-06-12 12:57 ` Imre Deak 0 siblings, 1 reply; 8+ messages in thread From: Murthy, Arun R @ 2023-06-12 11:47 UTC (permalink / raw) To: Deak, Imre; +Cc: intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Deak, Imre <imre.deak@intel.com> > Sent: Monday, June 12, 2023 5:07 PM > To: Murthy, Arun R <arun.r.murthy@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout > restart freshly > > On Thu, Jun 08, 2023 at 03:02:18PM +0530, Arun R Murthy wrote: > > At the begining of the aux transfer a check for aux control busy bit > > is done. Then as per the spec on aux transfer timeout, need to retry > > freshly for 3 times with a delay which is taken care by the control > > register. > > On each of these 3 trials a check for busy has to be done so as to > > start freshly. > > > > v2: updated the commit message > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 > > +++++++++------------ > > 1 file changed, 22 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > index 197c6e81db14..25090542dd9f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > @@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > > * it using the same AUX CH simultaneously > > */ > > > > - /* Try to wait for any previous AUX channel activity */ > > - for (try = 0; try < 3; try++) { > > - status = intel_de_read_notrace(i915, ch_ctl); > > - if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0) > > - break; > > - msleep(1); > > - } > > - /* just trace the final value */ > > - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > > - > > - if (try == 3) { > > - const u32 status = intel_de_read(i915, ch_ctl); > > - > > - if (status != intel_dp->aux_busy_last_status) { > > - drm_WARN(&i915->drm, 1, > > - "%s: not started (status 0x%08x)\n", > > - intel_dp->aux.name, status); > > - intel_dp->aux_busy_last_status = status; > > - } > > - > > - ret = -EBUSY; > > - goto out; > > - } > > - > > /* Only 5 data registers! */ > > if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) { > > ret = -E2BIG; > > @@ -304,14 +280,31 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > > } > > > > while ((aux_clock_divider = intel_dp- > >get_aux_clock_divider(intel_dp, clock++))) { > > - u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > > + /* Must try at least 3 times according to DP spec */ > > + for (try = 0; try < 5; try++) { > > + u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > > send_bytes, > > aux_clock_divider); > > > > - send_ctl |= aux_send_ctl_flags; > > + send_ctl |= aux_send_ctl_flags; > > Why is send_ctl recomputed in each iteration? This can be moved outside the loop, since the value doesn't tend to change. > > > + > > + /* Try to wait for any previous AUX channel activity > */ > > + status = intel_dp_aux_wait_done(intel_dp); > > How does it help to wait here for the same condition that was already waited > for at the end of the loop? Here we are checking for busy bit so as to ensure that the previous transfer is complete and only then the new transfer is initiated. In the latter case, we sent the data and then wait to get the acknowledgement(busy/error/timeout). Check for busy is to be done before sending the data. Here it was done before this loop. The spec limits the trials for sending data to 3 in case of failure and in each of this iteration it has to be started freshly. So we need to ensure that the previous transfer is completed before sending the new data. Thanks and Regards, Arun R Murthy -------------------- > > > + /* just trace the final value */ > > + trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), > true); > > + > > + if (status & DP_AUX_CH_CTL_SEND_BUSY) { > > + drm_WARN(&i915->drm, 1, > > + "%s: not started, previous Tx still in > process (status 0x%08x)\n", > > + intel_dp->aux.name, status); > > + intel_dp->aux_busy_last_status = status; > > + if (try > 3) { > > + ret = -EBUSY; > > + goto out; > > + } else > > + continue; > > + } > > > > - /* Must try at least 3 times according to DP spec */ > > - for (try = 0; try < 5; try++) { > > /* Load the send data into the aux channel data > registers */ > > for (i = 0; i < send_bytes; i += 4) > > intel_de_write(i915, ch_data[i >> 2], @@ - > 321,6 +314,7 @@ > > intel_dp_aux_xfer(struct intel_dp *intel_dp, > > /* Send the command and wait for it to complete */ > > intel_de_write(i915, ch_ctl, send_ctl); > > > > + /* TODO: if typeC then 4.2ms else 800us. For DG2 > add 1.5ms for > > +both cases */ > > status = intel_dp_aux_wait_done(intel_dp); > > > > /* Clear done status and any errors */ > > -- > > 2.25.1 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly 2023-06-12 11:47 ` Murthy, Arun R @ 2023-06-12 12:57 ` Imre Deak 2023-06-12 14:15 ` Murthy, Arun R 0 siblings, 1 reply; 8+ messages in thread From: Imre Deak @ 2023-06-12 12:57 UTC (permalink / raw) To: Murthy, Arun R; +Cc: intel-gfx@lists.freedesktop.org On Mon, Jun 12, 2023 at 02:47:37PM +0300, Murthy, Arun R wrote: > > -----Original Message----- > > From: Deak, Imre <imre.deak@intel.com> > > Sent: Monday, June 12, 2023 5:07 PM > > To: Murthy, Arun R <arun.r.murthy@intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout > > restart freshly > > > > On Thu, Jun 08, 2023 at 03:02:18PM +0530, Arun R Murthy wrote: > > > At the begining of the aux transfer a check for aux control busy bit > > > is done. Then as per the spec on aux transfer timeout, need to retry > > > freshly for 3 times with a delay which is taken care by the control > > > register. > > > On each of these 3 trials a check for busy has to be done so as to > > > start freshly. > > > > > > v2: updated the commit message > > > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 > > > +++++++++------------ > > > 1 file changed, 22 insertions(+), 28 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > index 197c6e81db14..25090542dd9f 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > @@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > * it using the same AUX CH simultaneously > > > */ > > > > > > - /* Try to wait for any previous AUX channel activity */ > > > - for (try = 0; try < 3; try++) { > > > - status = intel_de_read_notrace(i915, ch_ctl); > > > - if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0) > > > - break; > > > - msleep(1); > > > - } > > > - /* just trace the final value */ > > > - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > > > - > > > - if (try == 3) { > > > - const u32 status = intel_de_read(i915, ch_ctl); > > > - > > > - if (status != intel_dp->aux_busy_last_status) { > > > - drm_WARN(&i915->drm, 1, > > > - "%s: not started (status 0x%08x)\n", > > > - intel_dp->aux.name, status); > > > - intel_dp->aux_busy_last_status = status; > > > - } > > > - > > > - ret = -EBUSY; > > > - goto out; > > > - } > > > - > > > /* Only 5 data registers! */ > > > if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) { > > > ret = -E2BIG; > > > @@ -304,14 +280,31 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > } > > > > > > while ((aux_clock_divider = intel_dp- > > >get_aux_clock_divider(intel_dp, clock++))) { > > > - u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > > > + /* Must try at least 3 times according to DP spec */ > > > + for (try = 0; try < 5; try++) { > > > + u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > > > send_bytes, > > > aux_clock_divider); > > > > > > - send_ctl |= aux_send_ctl_flags; > > > + send_ctl |= aux_send_ctl_flags; > > > > Why is send_ctl recomputed in each iteration? > > This can be moved outside the loop, since the value doesn't tend to change. > > > > > > + > > > + /* Try to wait for any previous AUX channel activity > > */ > > > + status = intel_dp_aux_wait_done(intel_dp); > > > > How does it help to wait here for the same condition that was already waited > > for at the end of the loop? > > Here we are checking for busy bit so as to ensure that the previous > transfer is complete and only then the new transfer is initiated. > > In the latter case, we sent the data and then wait to get the > acknowledgement(busy/error/timeout). > > Check for busy is to be done before sending the data. Here it was done > before this loop. The spec limits the trials for sending data to 3 in > case of failure and in each of this iteration it has to be started > freshly. So we need to ensure that the previous transfer is completed > before sending the new data. Not sure what you mean by "freshly". One potential problem in the current code I can see is that if BUSY is still set after intel_dp_aux_wait_done() returns none of the control register fields should be changed, so I think at that point the transfer should be aborted. Also since a while back intel_dp_aux_wait_done() was changed to poll for the transfer completion instead of waiting for an interrupt, the corresponding interrupt should not be enabled either in the control register. > Thanks and Regards, > Arun R Murthy > -------------------- > > > > > + /* just trace the final value */ > > > + trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), > > true); > > > + > > > + if (status & DP_AUX_CH_CTL_SEND_BUSY) { > > > + drm_WARN(&i915->drm, 1, > > > + "%s: not started, previous Tx still in > > process (status 0x%08x)\n", > > > + intel_dp->aux.name, status); > > > + intel_dp->aux_busy_last_status = status; > > > + if (try > 3) { > > > + ret = -EBUSY; > > > + goto out; > > > + } else > > > + continue; > > > + } > > > > > > - /* Must try at least 3 times according to DP spec */ > > > - for (try = 0; try < 5; try++) { > > > /* Load the send data into the aux channel data > > registers */ > > > for (i = 0; i < send_bytes; i += 4) > > > intel_de_write(i915, ch_data[i >> 2], @@ - > > 321,6 +314,7 @@ > > > intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > /* Send the command and wait for it to complete */ > > > intel_de_write(i915, ch_ctl, send_ctl); > > > > > > + /* TODO: if typeC then 4.2ms else 800us. For DG2 > > add 1.5ms for > > > +both cases */ > > > status = intel_dp_aux_wait_done(intel_dp); > > > > > > /* Clear done status and any errors */ > > > -- > > > 2.25.1 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly 2023-06-12 12:57 ` Imre Deak @ 2023-06-12 14:15 ` Murthy, Arun R 0 siblings, 0 replies; 8+ messages in thread From: Murthy, Arun R @ 2023-06-12 14:15 UTC (permalink / raw) To: Deak, Imre; +Cc: intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Deak, Imre <imre.deak@intel.com> > Sent: Monday, June 12, 2023 6:28 PM > To: Murthy, Arun R <arun.r.murthy@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout > restart freshly > > On Mon, Jun 12, 2023 at 02:47:37PM +0300, Murthy, Arun R wrote: > > > -----Original Message----- > > > From: Deak, Imre <imre.deak@intel.com> > > > Sent: Monday, June 12, 2023 5:07 PM > > > To: Murthy, Arun R <arun.r.murthy@intel.com> > > > Cc: intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer > > > timeout restart freshly > > > > > > On Thu, Jun 08, 2023 at 03:02:18PM +0530, Arun R Murthy wrote: > > > > At the begining of the aux transfer a check for aux control busy > > > > bit is done. Then as per the spec on aux transfer timeout, need to > > > > retry freshly for 3 times with a delay which is taken care by the > > > > control register. > > > > On each of these 3 trials a check for busy has to be done so as to > > > > start freshly. > > > > > > > > v2: updated the commit message > > > > > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 > > > > +++++++++------------ > > > > 1 file changed, 22 insertions(+), 28 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > > index 197c6e81db14..25090542dd9f 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > > @@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > > * it using the same AUX CH simultaneously > > > > */ > > > > > > > > - /* Try to wait for any previous AUX channel activity */ > > > > - for (try = 0; try < 3; try++) { > > > > - status = intel_de_read_notrace(i915, ch_ctl); > > > > - if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0) > > > > - break; > > > > - msleep(1); > > > > - } > > > > - /* just trace the final value */ > > > > - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > > > > - > > > > - if (try == 3) { > > > > - const u32 status = intel_de_read(i915, ch_ctl); > > > > - > > > > - if (status != intel_dp->aux_busy_last_status) { > > > > - drm_WARN(&i915->drm, 1, > > > > - "%s: not started (status 0x%08x)\n", > > > > - intel_dp->aux.name, status); > > > > - intel_dp->aux_busy_last_status = status; > > > > - } > > > > - > > > > - ret = -EBUSY; > > > > - goto out; > > > > - } > > > > - > > > > /* Only 5 data registers! */ > > > > if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) { > > > > ret = -E2BIG; > > > > @@ -304,14 +280,31 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > > } > > > > > > > > while ((aux_clock_divider = intel_dp- > > > >get_aux_clock_divider(intel_dp, clock++))) { > > > > - u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > > > > + /* Must try at least 3 times according to DP spec */ > > > > + for (try = 0; try < 5; try++) { > > > > + u32 send_ctl = > > > > + intel_dp->get_aux_send_ctl(intel_dp, > > > > send_bytes, > > > > > > > > aux_clock_divider); > > > > > > > > - send_ctl |= aux_send_ctl_flags; > > > > + send_ctl |= aux_send_ctl_flags; > > > > > > Why is send_ctl recomputed in each iteration? > > > > This can be moved outside the loop, since the value doesn't tend to > change. > > > > > > > > > + > > > > + /* Try to wait for any previous AUX channel > > > > + activity > > > */ > > > > + status = intel_dp_aux_wait_done(intel_dp); > > > > > > How does it help to wait here for the same condition that was > > > already waited for at the end of the loop? > > > > Here we are checking for busy bit so as to ensure that the previous > > transfer is complete and only then the new transfer is initiated. > > > > In the latter case, we sent the data and then wait to get the > > acknowledgement(busy/error/timeout). > > > > Check for busy is to be done before sending the data. Here it was done > > before this loop. The spec limits the trials for sending data to 3 in > > case of failure and in each of this iteration it has to be started > > freshly. So we need to ensure that the previous transfer is completed > > before sending the new data. > > Not sure what you mean by "freshly". One potential problem in the current > code I can see is that if BUSY is still set after > intel_dp_aux_wait_done() returns none of the control register fields should > be changed, so I think at that point the transfer should be aborted. Good Catch! I missed that. Will surely take care of this in the next version. > > Also since a while back intel_dp_aux_wait_done() was changed to poll for > the transfer completion instead of waiting for an interrupt, the > corresponding interrupt should not be enabled either in the control register. Yes this is in my TODO list, most likely in my next patch. Thanks and Regards, Arun R Murthy -------------------- > > > Thanks and Regards, > > Arun R Murthy > > -------------------- > > > > > > > + /* just trace the final value */ > > > > + trace_i915_reg_rw(false, ch_ctl, status, > > > > + sizeof(status), > > > true); > > > > + > > > > + if (status & DP_AUX_CH_CTL_SEND_BUSY) { > > > > + drm_WARN(&i915->drm, 1, > > > > + "%s: not started, previous Tx > > > > + still in > > > process (status 0x%08x)\n", > > > > + intel_dp->aux.name, status); > > > > + intel_dp->aux_busy_last_status = status; > > > > + if (try > 3) { > > > > + ret = -EBUSY; > > > > + goto out; > > > > + } else > > > > + continue; > > > > + } > > > > > > > > - /* Must try at least 3 times according to DP spec */ > > > > - for (try = 0; try < 5; try++) { > > > > /* Load the send data into the aux channel > > > > data > > > registers */ > > > > for (i = 0; i < send_bytes; i += 4) > > > > intel_de_write(i915, ch_data[i >> 2], > > > > @@ - > > > 321,6 +314,7 @@ > > > > intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > > /* Send the command and wait for it to complete */ > > > > intel_de_write(i915, ch_ctl, send_ctl); > > > > > > > > + /* TODO: if typeC then 4.2ms else 800us. For > > > > + DG2 > > > add 1.5ms for > > > > +both cases */ > > > > status = intel_dp_aux_wait_done(intel_dp); > > > > > > > > /* Clear done status and any errors */ > > > > -- > > > > 2.25.1 > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-06-12 14:16 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-08 9:32 [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly Arun R Murthy 2023-06-08 11:31 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display/dp: On AUX xfer timeout restart freshly (rev2) Patchwork 2023-06-08 11:55 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2023-06-09 11:56 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork 2023-06-12 11:36 ` [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly Imre Deak 2023-06-12 11:47 ` Murthy, Arun R 2023-06-12 12:57 ` Imre Deak 2023-06-12 14:15 ` Murthy, Arun R
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox