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