* [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream.
@ 2020-10-20 7:45 Khaled Almahallawy
2020-10-20 8:09 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Khaled Almahallawy @ 2020-10-20 7:45 UTC (permalink / raw)
To: Intel-gfx; +Cc: seanpaul
This patch avoids failing atomic commits sent by user space by making sure CRTC/Connector added to drm_atomic_state by the driver are in valid state.
When disconnecting MST hub with two or more connected displays. The user space sends IOCTL for each MST pipe to disable.
drm_atomic_state object sent from user space contains only the state of the crtc/pipe intended to disable.
In TGL, intel_dp_mst_atomic_master_trans_check will add all other CRTC and connectors that share the MST stream to drm_atomic_state:
drm_atomic_commit
drm_atomic_helper_check_modeset
update_connector_routing
intel_dp_mst_atomic_check = funcs->atomic_check(connector, state);
intel_dp_mst_atomic_master_trans_check
intel_atomic_get_digital_connector_state
drm_atomic_get_connector_state <-- Add all Connectors
drm_atomic_get_crtc_state <-- Add all CRTCs
update_connector_routing <-- Check added Connector/CRTCs - Will fail
However the added crtc/connector pair will be in invalid state (enabled state for a removed connector)
triggering this condition in drm_atomic_helper.c/update_connector_routing:
if (!state->duplicated && drm_connector_is_unregistered(connector) &&
crtc_state->active) {
DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n",
connector->base.id, connector->name);
return -EINVAL;
}
Which will cause the drm_atomic_commit/IOCTL for disabling one of MST stream pipes (Main MST) to fail.
The problem happens when a user space (as Chrome) doesn’t retry a falling commit, leaving a disconnected MST pipe still ON,
which will result in failing reconnect of MST hub or even worse leaving TC PHY in a connected state while the MST Hub is disconnected.
Tested on Ubuntu(drm-tip) and Chrome(kernel-next 5.9 rc7)
Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index e948aacbd4ab..1ede980876ed 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -265,6 +265,9 @@ intel_dp_mst_atomic_master_trans_check(struct intel_connector *connector,
return ret;
}
crtc_state->uapi.mode_changed = true;
+
+ if (connector_iter->base.status == connector_status_disconnected)
+ crtc_state->uapi.active = false;
}
drm_connector_list_iter_end(&connector_list_iter);
--
2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-20 7:45 [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream Khaled Almahallawy @ 2020-10-20 8:09 ` Patchwork 2020-10-20 8:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Patchwork @ 2020-10-20 8:09 UTC (permalink / raw) To: Khaled Almahallawy; +Cc: intel-gfx == Series Details == Series: drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. URL : https://patchwork.freedesktop.org/series/82858/ State : warning == Summary == $ dim checkpatch origin/drm-tip 70477df4f55c drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. -:10: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #10: This patch avoids failing atomic commits sent by user space by making sure CRTC/Connector added to drm_atomic_state by the driver are in valid state. total: 0 errors, 1 warnings, 0 checks, 9 lines checked _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-20 7:45 [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream Khaled Almahallawy 2020-10-20 8:09 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork @ 2020-10-20 8:34 ` Patchwork 2020-10-20 9:41 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2020-10-20 12:41 ` [Intel-gfx] [PATCH] " Ville Syrjälä 3 siblings, 0 replies; 12+ messages in thread From: Patchwork @ 2020-10-20 8:34 UTC (permalink / raw) To: Khaled Almahallawy; +Cc: intel-gfx [-- Attachment #1.1: Type: text/plain, Size: 3212 bytes --] == Series Details == Series: drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. URL : https://patchwork.freedesktop.org/series/82858/ State : success == Summary == CI Bug Log - changes from CI_DRM_9166 -> Patchwork_18739 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/index.html Known issues ------------ Here are the changes found in Patchwork_18739 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@core_hotunplug@unbind-rebind: - fi-icl-y: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/fi-icl-y/igt@core_hotunplug@unbind-rebind.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/fi-icl-y/igt@core_hotunplug@unbind-rebind.html * igt@i915_pm_rpm@basic-pci-d3-state: - fi-byt-j1900: [PASS][3] -> [DMESG-WARN][4] ([i915#1982]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/fi-byt-j1900/igt@i915_pm_rpm@basic-pci-d3-state.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/fi-byt-j1900/igt@i915_pm_rpm@basic-pci-d3-state.html * igt@vgem_basic@unload: - fi-skl-guc: [PASS][5] -> [DMESG-WARN][6] ([i915#2203]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/fi-skl-guc/igt@vgem_basic@unload.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/fi-skl-guc/igt@vgem_basic@unload.html #### Possible fixes #### * igt@i915_pm_rpm@basic-pci-d3-state: - fi-glk-dsi: [DMESG-WARN][7] ([i915#1982]) -> [PASS][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/fi-glk-dsi/igt@i915_pm_rpm@basic-pci-d3-state.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/fi-glk-dsi/igt@i915_pm_rpm@basic-pci-d3-state.html - fi-bsw-kefka: [DMESG-WARN][9] ([i915#1982]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203 Participating hosts (45 -> 39) ------------------------------ Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus Build changes ------------- * Linux: CI_DRM_9166 -> Patchwork_18739 CI-20190529: 20190529 CI_DRM_9166: f10a69af784776f63cf892611a6baa33e8c35fca @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5822: b4bcf05cb9839037128905deda7146434155cc41 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18739: 70477df4f55c58d84f8416fa12e7b7f45e65fd2f @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 70477df4f55c drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/index.html [-- Attachment #1.2: Type: text/html, Size: 4131 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-20 7:45 [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream Khaled Almahallawy 2020-10-20 8:09 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork 2020-10-20 8:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork @ 2020-10-20 9:41 ` Patchwork 2020-10-20 12:41 ` [Intel-gfx] [PATCH] " Ville Syrjälä 3 siblings, 0 replies; 12+ messages in thread From: Patchwork @ 2020-10-20 9:41 UTC (permalink / raw) To: Khaled Almahallawy; +Cc: intel-gfx [-- Attachment #1.1: Type: text/plain, Size: 17967 bytes --] == Series Details == Series: drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. URL : https://patchwork.freedesktop.org/series/82858/ State : failure == Summary == CI Bug Log - changes from CI_DRM_9166_full -> Patchwork_18739_full ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_18739_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_18739_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_18739_full: ### IGT changes ### #### Possible regressions #### * igt@prime_vgem@coherency-blt: - shard-hsw: [PASS][1] -> [FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-hsw8/igt@prime_vgem@coherency-blt.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-hsw8/igt@prime_vgem@coherency-blt.html Known issues ------------ Here are the changes found in Patchwork_18739_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_ctx_isolation@preservation-s3@rcs0: - shard-skl: [PASS][3] -> [INCOMPLETE][4] ([i915#198]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl4/igt@gem_ctx_isolation@preservation-s3@rcs0.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl10/igt@gem_ctx_isolation@preservation-s3@rcs0.html * igt@gem_exec_reloc@basic-many-active@rcs0: - shard-snb: [PASS][5] -> [FAIL][6] ([i915#2389]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-snb4/igt@gem_exec_reloc@basic-many-active@rcs0.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-snb5/igt@gem_exec_reloc@basic-many-active@rcs0.html * igt@gem_exec_suspend@basic: - shard-glk: [PASS][7] -> [FAIL][8] ([i915#1888]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-glk3/igt@gem_exec_suspend@basic.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-glk9/igt@gem_exec_suspend@basic.html * igt@gem_exec_whisper@basic-forked-all: - shard-glk: [PASS][9] -> [DMESG-WARN][10] ([i915#118] / [i915#95]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-glk9/igt@gem_exec_whisper@basic-forked-all.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-glk9/igt@gem_exec_whisper@basic-forked-all.html * igt@gem_huc_copy@huc-copy: - shard-tglb: [PASS][11] -> [SKIP][12] ([i915#2190]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-tglb3/igt@gem_huc_copy@huc-copy.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-tglb6/igt@gem_huc_copy@huc-copy.html * igt@gem_userptr_blits@sync-unmap-cycles: - shard-skl: [PASS][13] -> [TIMEOUT][14] ([i915#2424]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl1/igt@gem_userptr_blits@sync-unmap-cycles.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl4/igt@gem_userptr_blits@sync-unmap-cycles.html * igt@kms_big_fb@x-tiled-32bpp-rotate-0: - shard-skl: [PASS][15] -> [DMESG-WARN][16] ([i915#1982]) +3 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl2/igt@kms_big_fb@x-tiled-32bpp-rotate-0.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl7/igt@kms_big_fb@x-tiled-32bpp-rotate-0.html * igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled: - shard-snb: [PASS][17] -> [FAIL][18] ([i915#54]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-snb7/igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-snb4/igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled.html * igt@kms_flip@wf_vblank-ts-check-interruptible@c-edp1: - shard-skl: [PASS][19] -> [FAIL][20] ([i915#2122]) +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl4/igt@kms_flip@wf_vblank-ts-check-interruptible@c-edp1.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl1/igt@kms_flip@wf_vblank-ts-check-interruptible@c-edp1.html * igt@kms_frontbuffer_tracking@fbc-shrfb-scaledprimary: - shard-tglb: [PASS][21] -> [DMESG-WARN][22] ([i915#1982]) +1 similar issue [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-tglb5/igt@kms_frontbuffer_tracking@fbc-shrfb-scaledprimary.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-tglb2/igt@kms_frontbuffer_tracking@fbc-shrfb-scaledprimary.html * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc: - shard-skl: [PASS][23] -> [FAIL][24] ([fdo#108145] / [i915#265]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html * igt@kms_psr@psr2_cursor_render: - shard-iclb: [PASS][25] -> [SKIP][26] ([fdo#109441]) +2 similar issues [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-iclb2/igt@kms_psr@psr2_cursor_render.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-iclb7/igt@kms_psr@psr2_cursor_render.html * igt@kms_universal_plane@universal-plane-gen9-features-pipe-a: - shard-kbl: [PASS][27] -> [DMESG-WARN][28] ([i915#1982]) [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-kbl2/igt@kms_universal_plane@universal-plane-gen9-features-pipe-a.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-kbl1/igt@kms_universal_plane@universal-plane-gen9-features-pipe-a.html * igt@kms_vblank@pipe-c-query-forked-busy-hang: - shard-apl: [PASS][29] -> [DMESG-WARN][30] ([i915#1635] / [i915#1982]) [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-apl6/igt@kms_vblank@pipe-c-query-forked-busy-hang.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-apl2/igt@kms_vblank@pipe-c-query-forked-busy-hang.html * igt@perf@polling-parameterized: - shard-iclb: [PASS][31] -> [FAIL][32] ([i915#1542]) [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-iclb3/igt@perf@polling-parameterized.html [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-iclb3/igt@perf@polling-parameterized.html #### Possible fixes #### * igt@api_intel_bb@blit-noreloc-purge-cache: - shard-snb: [FAIL][33] -> [PASS][34] [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-snb5/igt@api_intel_bb@blit-noreloc-purge-cache.html [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-snb7/igt@api_intel_bb@blit-noreloc-purge-cache.html * igt@gem_exec_parallel@fds@vecs0: - shard-iclb: [DMESG-WARN][35] ([i915#1982]) -> [PASS][36] [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-iclb2/igt@gem_exec_parallel@fds@vecs0.html [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-iclb7/igt@gem_exec_parallel@fds@vecs0.html * igt@gem_exec_reloc@basic-many-active@vecs0: - shard-glk: [FAIL][37] ([i915#2389]) -> [PASS][38] +2 similar issues [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-glk2/igt@gem_exec_reloc@basic-many-active@vecs0.html [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-glk5/igt@gem_exec_reloc@basic-many-active@vecs0.html * igt@gem_exec_whisper@basic-queues-forked: - shard-glk: [DMESG-WARN][39] ([i915#118] / [i915#95]) -> [PASS][40] [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-glk6/igt@gem_exec_whisper@basic-queues-forked.html [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-glk4/igt@gem_exec_whisper@basic-queues-forked.html * igt@i915_suspend@debugfs-reader: - shard-kbl: [INCOMPLETE][41] ([i915#155]) -> [PASS][42] [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-kbl6/igt@i915_suspend@debugfs-reader.html [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-kbl7/igt@i915_suspend@debugfs-reader.html * {igt@kms_async_flips@alternate-sync-async-flip}: - shard-skl: [FAIL][43] ([i915#2521]) -> [PASS][44] [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl9/igt@kms_async_flips@alternate-sync-async-flip.html [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl10/igt@kms_async_flips@alternate-sync-async-flip.html - shard-kbl: [FAIL][45] ([i915#2521]) -> [PASS][46] [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-kbl1/igt@kms_async_flips@alternate-sync-async-flip.html [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-kbl4/igt@kms_async_flips@alternate-sync-async-flip.html * igt@kms_big_fb@linear-32bpp-rotate-180: - shard-glk: [DMESG-FAIL][47] ([i915#118] / [i915#95]) -> [PASS][48] [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-glk4/igt@kms_big_fb@linear-32bpp-rotate-180.html [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-glk4/igt@kms_big_fb@linear-32bpp-rotate-180.html * igt@kms_big_fb@y-tiled-8bpp-rotate-180: - shard-kbl: [DMESG-WARN][49] ([i915#1982]) -> [PASS][50] +1 similar issue [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-kbl6/igt@kms_big_fb@y-tiled-8bpp-rotate-180.html [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-kbl7/igt@kms_big_fb@y-tiled-8bpp-rotate-180.html - shard-apl: [DMESG-WARN][51] ([i915#1635] / [i915#1982]) -> [PASS][52] [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-apl8/igt@kms_big_fb@y-tiled-8bpp-rotate-180.html [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-apl2/igt@kms_big_fb@y-tiled-8bpp-rotate-180.html * igt@kms_ccs@pipe-a-crc-primary-rotation-180: - shard-skl: [FAIL][53] -> [PASS][54] [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl3/igt@kms_ccs@pipe-a-crc-primary-rotation-180.html [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl8/igt@kms_ccs@pipe-a-crc-primary-rotation-180.html * igt@kms_cursor_crc@pipe-c-cursor-128x128-onscreen: - shard-skl: [FAIL][55] ([i915#54]) -> [PASS][56] [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl4/igt@kms_cursor_crc@pipe-c-cursor-128x128-onscreen.html [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl1/igt@kms_cursor_crc@pipe-c-cursor-128x128-onscreen.html * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size: - shard-skl: [DMESG-WARN][57] ([i915#1982]) -> [PASS][58] +5 similar issues [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl8/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl7/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html * igt@kms_cursor_legacy@cursor-vs-flip-varying-size: - shard-hsw: [FAIL][59] ([i915#2370]) -> [PASS][60] [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-hsw4/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html * igt@kms_draw_crc@draw-method-xrgb2101010-blt-untiled: - shard-snb: [FAIL][61] ([i915#54]) -> [PASS][62] [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-snb4/igt@kms_draw_crc@draw-method-xrgb2101010-blt-untiled.html [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-snb2/igt@kms_draw_crc@draw-method-xrgb2101010-blt-untiled.html * igt@kms_flip@2x-flip-vs-absolute-wf_vblank@ab-vga1-hdmi-a1: - shard-hsw: [DMESG-WARN][63] ([i915#1982]) -> [PASS][64] [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-hsw6/igt@kms_flip@2x-flip-vs-absolute-wf_vblank@ab-vga1-hdmi-a1.html [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-hsw4/igt@kms_flip@2x-flip-vs-absolute-wf_vblank@ab-vga1-hdmi-a1.html * igt@kms_flip@plain-flip-fb-recreate@b-edp1: - shard-skl: [FAIL][65] ([i915#2122]) -> [PASS][66] [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl2/igt@kms_flip@plain-flip-fb-recreate@b-edp1.html [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl7/igt@kms_flip@plain-flip-fb-recreate@b-edp1.html * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt: - shard-tglb: [DMESG-WARN][67] ([i915#1982]) -> [PASS][68] +1 similar issue [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt.html [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt.html * igt@kms_hdr@bpc-switch-dpms: - shard-skl: [FAIL][69] ([i915#1188]) -> [PASS][70] [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl6/igt@kms_hdr@bpc-switch-dpms.html [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: - shard-skl: [FAIL][71] ([fdo#108145] / [i915#265]) -> [PASS][72] [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html * igt@kms_psr@psr2_cursor_plane_onoff: - shard-iclb: [SKIP][73] ([fdo#109441]) -> [PASS][74] +1 similar issue [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-iclb6/igt@kms_psr@psr2_cursor_plane_onoff.html [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html * igt@kms_vblank@pipe-c-ts-continuation-suspend: - shard-skl: [INCOMPLETE][75] ([i915#198]) -> [PASS][76] [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl6/igt@kms_vblank@pipe-c-ts-continuation-suspend.html [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl3/igt@kms_vblank@pipe-c-ts-continuation-suspend.html #### Warnings #### * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min: - shard-skl: [DMESG-WARN][77] ([i915#1982]) -> [FAIL][78] ([fdo#108145] / [i915#265]) [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html * igt@kms_setmode@basic: - shard-skl: [FAIL][79] ([i915#31]) -> [DMESG-FAIL][80] ([i915#1982] / [i915#31]) [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9166/shard-skl6/igt@kms_setmode@basic.html [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18739/shard-skl9/igt@kms_setmode@basic.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145 [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441 [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118 [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188 [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542 [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155 [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635 [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888 [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198 [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122 [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190 [i915#2370]: https://gitlab.freedesktop.org/drm/intel/issues/2370 [i915#2389]: https://gitlab.freedesktop.org/drm/intel/issues/2389 [i915#2424]: https://gitlab.freedesktop.org/drm/intel/issues/2424 [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521 [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265 [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31 [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 Participating hosts (11 -> 12) ------------------------------ Additional (1): pig-snb-2600 Build changes ------------- * Linux: CI_DRM_9166 -> Patchwork_18739 CI-20190529: 20190529 CI_DRM_9166: f10a69af784776f63cf892611a6baa33e8c35fca @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5822: b4bcf05cb9839037128905deda7146434155cc41 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_18739: 70477df4f55c58d84f8416fa12e7b7f45e65fd2f @ 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_18739/index.html [-- Attachment #1.2: Type: text/html, Size: 21433 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-20 7:45 [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream Khaled Almahallawy ` (2 preceding siblings ...) 2020-10-20 9:41 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork @ 2020-10-20 12:41 ` Ville Syrjälä 2020-10-20 23:25 ` Souza, Jose 3 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2020-10-20 12:41 UTC (permalink / raw) To: Khaled Almahallawy; +Cc: Daniel Vetter, Intel-gfx, seanpaul On Tue, Oct 20, 2020 at 12:45:55AM -0700, Khaled Almahallawy wrote: > This patch avoids failing atomic commits sent by user space by making sure CRTC/Connector added to drm_atomic_state by the driver are in valid state. > > When disconnecting MST hub with two or more connected displays. The user space sends IOCTL for each MST pipe to disable. > drm_atomic_state object sent from user space contains only the state of the crtc/pipe intended to disable. > In TGL, intel_dp_mst_atomic_master_trans_check will add all other CRTC and connectors that share the MST stream to drm_atomic_state: > > drm_atomic_commit > drm_atomic_helper_check_modeset > update_connector_routing > intel_dp_mst_atomic_check = funcs->atomic_check(connector, state); > intel_dp_mst_atomic_master_trans_check > intel_atomic_get_digital_connector_state > drm_atomic_get_connector_state <-- Add all Connectors > drm_atomic_get_crtc_state <-- Add all CRTCs > update_connector_routing <-- Check added Connector/CRTCs - Will fail > > However the added crtc/connector pair will be in invalid state (enabled state for a removed connector) > triggering this condition in drm_atomic_helper.c/update_connector_routing: > > if (!state->duplicated && drm_connector_is_unregistered(connector) && > crtc_state->active) { > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n", > connector->base.id, connector->name); > return -EINVAL; > } Yeah, I think that "reject modeset on unregistered connectors" idea is a bit broken given how the uapi has worked in the past. Cc:ing danvet and lyude who IIRC were involved with that. Hmm. Maybe we could add the other stuff to the state only after the connector .atomic_check() stuff has been done? I don't quite remember why we decided to do it here. José do you recall the details? > > Which will cause the drm_atomic_commit/IOCTL for disabling one of MST stream pipes (Main MST) to fail. > > The problem happens when a user space (as Chrome) doesn’t retry a falling commit, leaving a disconnected MST pipe still ON, > which will result in failing reconnect of MST hub or even worse leaving TC PHY in a connected state while the MST Hub is disconnected. > > Tested on Ubuntu(drm-tip) and Chrome(kernel-next 5.9 rc7) > > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index e948aacbd4ab..1ede980876ed 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -265,6 +265,9 @@ intel_dp_mst_atomic_master_trans_check(struct intel_connector *connector, > return ret; > } > crtc_state->uapi.mode_changed = true; > + > + if (connector_iter->base.status == connector_status_disconnected) > + crtc_state->uapi.active = false; That will make the state userspace last set inconsistent with what's really going on. Which means suddenly page flips/vblank waits and whatnot will start to fail. Also that wil directly mutate the prop visible to user space, which is not how these things are supposed to work. I think if we did do something like this we should maybe have some kind of internal flag for it. > } > drm_connector_list_iter_end(&connector_list_iter); > > -- > 2.25.1 -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-20 12:41 ` [Intel-gfx] [PATCH] " Ville Syrjälä @ 2020-10-20 23:25 ` Souza, Jose 2020-10-21 0:29 ` Souza, Jose 2020-10-21 13:26 ` Ville Syrjälä 0 siblings, 2 replies; 12+ messages in thread From: Souza, Jose @ 2020-10-20 23:25 UTC (permalink / raw) To: ville.syrjala@linux.intel.com, Almahallawy, Khaled Cc: daniel.vetter@ffwll.ch, Intel-gfx@lists.freedesktop.org, seanpaul@chromium.org On Tue, 2020-10-20 at 15:41 +0300, Ville Syrjälä wrote: > On Tue, Oct 20, 2020 at 12:45:55AM -0700, Khaled Almahallawy wrote: > > This patch avoids failing atomic commits sent by user space by making sure CRTC/Connector added to drm_atomic_state by the driver are in valid state. > > > > When disconnecting MST hub with two or more connected displays. The user space sends IOCTL for each MST pipe to disable. > > drm_atomic_state object sent from user space contains only the state of the crtc/pipe intended to disable. > > In TGL, intel_dp_mst_atomic_master_trans_check will add all other CRTC and connectors that share the MST stream to drm_atomic_state: > > > > drm_atomic_commit > > drm_atomic_helper_check_modeset > > update_connector_routing > > intel_dp_mst_atomic_check = funcs->atomic_check(connector, state); > > intel_dp_mst_atomic_master_trans_check > > intel_atomic_get_digital_connector_state > > drm_atomic_get_connector_state <-- Add all Connectors > > drm_atomic_get_crtc_state <-- Add all CRTCs > > update_connector_routing <-- Check added Connector/CRTCs - Will fail > > > > However the added crtc/connector pair will be in invalid state (enabled state for a removed connector) > > triggering this condition in drm_atomic_helper.c/update_connector_routing: > > > > if (!state->duplicated && drm_connector_is_unregistered(connector) && > > crtc_state->active) { > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n", > > connector->base.id, connector->name); > > return -EINVAL; > > } > > Yeah, I think that "reject modeset on unregistered connectors" idea is > a bit broken given how the uapi has worked in the past. Cc:ing danvet > and lyude who IIRC were involved with that. > > Hmm. Maybe we could add the other stuff to the state only after the > connector .atomic_check() stuff has been done? I don't quite remember > why we decided to do it here. José do you recall the details? Because the connector check function runs twice in drm_atomic_helper_check_modeset(), in the first iteration it will add all connectors that share the same MST stream to state, the second one will make sure all other checks passed in all connectors of the MST stream. To me looks like the Chrome userspace is not doing the right thing, it is sending asynchronous atomic commits with conflicting state between each commit. If it had a pool that dispatch one atomic state at time waiting for completion before dispatch the next one it would not be a issue. > > > > > Which will cause the drm_atomic_commit/IOCTL for disabling one of MST stream pipes (Main MST) to fail. > > > > The problem happens when a user space (as Chrome) doesn’t retry a falling commit, leaving a disconnected MST pipe still ON, > > which will result in failing reconnect of MST hub or even worse leaving TC PHY in a connected state while the MST Hub is disconnected. > > > > Tested on Ubuntu(drm-tip) and Chrome(kernel-next 5.9 rc7) > > > > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > index e948aacbd4ab..1ede980876ed 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > @@ -265,6 +265,9 @@ intel_dp_mst_atomic_master_trans_check(struct intel_connector *connector, > > return ret; > > } > > crtc_state->uapi.mode_changed = true; > > + > > + if (connector_iter->base.status == connector_status_disconnected) > > + crtc_state->uapi.active = false; > > That will make the state userspace last set inconsistent with what's > really going on. Which means suddenly page flips/vblank waits and > whatnot will start to fail. > > Also that wil directly mutate the prop visible to user space, which > is not how these things are supposed to work. I think if we did do > something like this we should maybe have some kind of internal > flag for it. > > > } > > drm_connector_list_iter_end(&connector_list_iter); > > > > > > > > > > -- > > 2.25.1 > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-20 23:25 ` Souza, Jose @ 2020-10-21 0:29 ` Souza, Jose 2020-10-21 0:53 ` Almahallawy, Khaled 2020-10-21 13:26 ` Ville Syrjälä 1 sibling, 1 reply; 12+ messages in thread From: Souza, Jose @ 2020-10-21 0:29 UTC (permalink / raw) To: ville.syrjala@linux.intel.com, Almahallawy, Khaled Cc: daniel.vetter@ffwll.ch, Intel-gfx@lists.freedesktop.org, seanpaul@chromium.org On Tue, 2020-10-20 at 16:25 -0700, José Roberto de Souza wrote: > On Tue, 2020-10-20 at 15:41 +0300, Ville Syrjälä wrote: > > On Tue, Oct 20, 2020 at 12:45:55AM -0700, Khaled Almahallawy wrote: > > > This patch avoids failing atomic commits sent by user space by making sure CRTC/Connector added to drm_atomic_state by the driver are in valid state. > > > > > > When disconnecting MST hub with two or more connected displays. The user space sends IOCTL for each MST pipe to disable. > > > drm_atomic_state object sent from user space contains only the state of the crtc/pipe intended to disable. > > > In TGL, intel_dp_mst_atomic_master_trans_check will add all other CRTC and connectors that share the MST stream to drm_atomic_state: > > > > > > drm_atomic_commit > > > drm_atomic_helper_check_modeset > > > update_connector_routing > > > intel_dp_mst_atomic_check = funcs->atomic_check(connector, state); > > > intel_dp_mst_atomic_master_trans_check > > > intel_atomic_get_digital_connector_state > > > drm_atomic_get_connector_state <-- Add all Connectors > > > drm_atomic_get_crtc_state <-- Add all CRTCs > > > update_connector_routing <-- Check added Connector/CRTCs - Will fail > > > > > > However the added crtc/connector pair will be in invalid state (enabled state for a removed connector) > > > triggering this condition in drm_atomic_helper.c/update_connector_routing: > > > > > > if (!state->duplicated && drm_connector_is_unregistered(connector) && > > > crtc_state->active) { > > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n", > > > connector->base.id, connector->name); > > > return -EINVAL; > > > } > > > > Yeah, I think that "reject modeset on unregistered connectors" idea is > > a bit broken given how the uapi has worked in the past. Cc:ing danvet > > and lyude who IIRC were involved with that. > > > > Hmm. Maybe we could add the other stuff to the state only after the > > connector .atomic_check() stuff has been done? I don't quite remember > > why we decided to do it here. José do you recall the details? > > Because the connector check function runs twice in drm_atomic_helper_check_modeset(), in the first iteration it will add all connectors that share the > same MST stream to state, the second one will make sure all other checks passed in all connectors of the MST stream. > > To me looks like the Chrome userspace is not doing the right thing, it is sending asynchronous atomic commits with conflicting state between each > commit. Oh it do not have information about other connectors so not conflicting but anyways userspace should retry atomic commits anyways. > If it had a pool that dispatch one atomic state at time waiting for completion before dispatch the next one it would not be a issue. > > > > > > > > > Which will cause the drm_atomic_commit/IOCTL for disabling one of MST stream pipes (Main MST) to fail. > > > > > > The problem happens when a user space (as Chrome) doesn’t retry a falling commit, leaving a disconnected MST pipe still ON, > > > which will result in failing reconnect of MST hub or even worse leaving TC PHY in a connected state while the MST Hub is disconnected. > > > > > > Tested on Ubuntu(drm-tip) and Chrome(kernel-next 5.9 rc7) > > > > > > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > index e948aacbd4ab..1ede980876ed 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > @@ -265,6 +265,9 @@ intel_dp_mst_atomic_master_trans_check(struct intel_connector *connector, > > > return ret; > > > } > > > crtc_state->uapi.mode_changed = true; > > > + > > > + if (connector_iter->base.status == connector_status_disconnected) > > > + crtc_state->uapi.active = false; > > > > That will make the state userspace last set inconsistent with what's > > really going on. Which means suddenly page flips/vblank waits and > > whatnot will start to fail. > > > > Also that wil directly mutate the prop visible to user space, which > > is not how these things are supposed to work. I think if we did do > > something like this we should maybe have some kind of internal > > flag for it. > > > > > } > > > drm_connector_list_iter_end(&connector_list_iter); > > > > > > > > > > > > > > > -- > > > 2.25.1 > > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-21 0:29 ` Souza, Jose @ 2020-10-21 0:53 ` Almahallawy, Khaled 2020-10-21 15:25 ` Mark Yacoub 0 siblings, 1 reply; 12+ messages in thread From: Almahallawy, Khaled @ 2020-10-21 0:53 UTC (permalink / raw) To: ville.syrjala@linux.intel.com, Souza, Jose Cc: daniel.vetter@ffwll.ch, Intel-gfx@lists.freedesktop.org, seanpaul@chromium.org, gildekel@google.com, markyacoub@google.com, levinale@google.com On Wed, 2020-10-21 at 00:29 +0000, Souza, Jose wrote: > On Tue, 2020-10-20 at 16:25 -0700, José Roberto de Souza wrote: > > On Tue, 2020-10-20 at 15:41 +0300, Ville Syrjälä wrote: > > > On Tue, Oct 20, 2020 at 12:45:55AM -0700, Khaled Almahallawy > > > wrote: > > > > This patch avoids failing atomic commits sent by user space by > > > > making sure CRTC/Connector added to drm_atomic_state by the > > > > driver are in valid state. > > > > > > > > When disconnecting MST hub with two or more connected displays. > > > > The user space sends IOCTL for each MST pipe to disable. > > > > drm_atomic_state object sent from user space contains only the > > > > state of the crtc/pipe intended to disable. > > > > In TGL, intel_dp_mst_atomic_master_trans_check will add all > > > > other CRTC and connectors that share the MST stream to > > > > drm_atomic_state: > > > > > > > > drm_atomic_commit > > > > drm_atomic_helper_check_modeset > > > > update_connector_routing > > > > intel_dp_mst_atomic_check = funcs- > > > > >atomic_check(connector, state); > > > > intel_dp_mst_atomic_master_trans_check > > > > intel_atomic_get_digital_connector_state > > > > drm_atomic_get_connector_state <-- Add all Connectors > > > > drm_atomic_get_crtc_state <-- Add all CRTCs > > > > update_connector_routing <-- Check added Connector/CRTCs > > > > - Will fail > > > > > > > > However the added crtc/connector pair will be in invalid state > > > > (enabled state for a removed connector) > > > > triggering this condition in > > > > drm_atomic_helper.c/update_connector_routing: > > > > > > > > if (!state->duplicated && > > > > drm_connector_is_unregistered(connector) && > > > > crtc_state->active) { > > > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n", > > > > connector->base.id, connector->name); > > > > return -EINVAL; > > > > } > > > > > > Yeah, I think that "reject modeset on unregistered connectors" > > > idea is > > > a bit broken given how the uapi has worked in the past. Cc:ing > > > danvet > > > and lyude who IIRC were involved with that. > > > Actually by removing this condition everything works fine as well. However we were worried about touching this part and screwing other stuff :-) > > > Hmm. Maybe we could add the other stuff to the state only after > > > the > > > connector .atomic_check() stuff has been done? I don't quite > > > remember > > > why we decided to do it here. José do you recall the details? > > > > Because the connector check function runs twice in > > drm_atomic_helper_check_modeset(), in the first iteration it will > > add all connectors that share the > > same MST stream to state, the second one will make sure all other > > checks passed in all connectors of the MST stream. > > > > To me looks like the Chrome userspace is not doing the right thing, > > it is sending asynchronous atomic commits with conflicting state > > between each > > commit. > > Oh it do not have information about other connectors so not > conflicting but anyways userspace should retry atomic commits > anyways. > CC: Mark and Gill for Chrome userspace input Yes Chrome is sending the atomic commits with correct states one after another. Matter of fact, the failure we observe in Chrome is also happening in Ubuntu. We checked that by printing the drm_atomic_state sent by user space before atomic check: +++ b/drivers/gpu/drm/drm_atomic.c @@ -1365,9 +1365,12 @@ int drm_atomic_commit(struct drm_atomic_state *state) struct drm_mode_config *config = &state->dev->mode_config; int ret; + drm_atomic_print_state(state); ret = drm_atomic_check_only(state); Using MST with two displays and eDP connected. First MST on Pipe B and Second MST on Pipe C. Chrome send atomic commit to disable Pipe B first as below: [ 60.406455] [drm:drm_atomic_print_state] checking 00000000c0e0cc6c [ 60.406455] i915 0000:00:02.0: [drm] crtc[152]: pipe B [ 60.406456] i915 0000:00:02.0: [drm] enable=0 [ 60.406457] i915 0000:00:02.0: [drm] active=0 [ 60.406458] i915 0000:00:02.0: [drm] self_refresh_active=0 [ 60.406459] i915 0000:00:02.0: [drm] planes_changed=0 [ 60.406460] i915 0000:00:02.0: [drm] mode_changed=0 [ 60.406461] i915 0000:00:02.0: [drm] active_changed=0 [ 60.406462] i915 0000:00:02.0: [drm] connectors_changed=0 [ 60.406463] i915 0000:00:02.0: [drm] color_mgmt_changed=0 [ 60.406464] i915 0000:00:02.0: [drm] plane_mask=100 [ 60.406465] i915 0000:00:02.0: [drm] connector_mask=0 [ 60.406466] i915 0000:00:02.0: [drm] encoder_mask=2000 [ 60.406467] i915 0000:00:02.0: [drm] mode: "": 0 0 0 0 0 0 0 0 0 0 0x0 0x0 [ 60.406468] i915 0000:00:02.0: [drm] connector[353]: DP-6 [ 60.406469] i915 0000:00:02.0: [drm] crtc=(null) [ 60.406470] i915 0000:00:02.0: [drm] self_refresh_aware=0 [ 60.406471] [drm:drm_atomic_check_only] checking 00000000c0e0cc6c [ 60.406473] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] mode changed [ 60.406474] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] enable changed [ 60.406474] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] active changed ... [ 60.407674] [drm:drm_atomic_check_only] atomic driver check for 00000000c0e0cc6c failed: -22 Even in Ubuntu we get similar atomic commit with similar state for Pipe B: [ 301.136961] i915 0000:00:02.0: [drm] crtc[152]: pipe B [ 301.136964] i915 0000:00:02.0: [drm] enable=1 [ 301.136966] i915 0000:00:02.0: [drm] active=0 [ 301.136969] i915 0000:00:02.0: [drm] self_refresh_active=0 [ 301.136995] i915 0000:00:02.0: [drm] planes_changed=0 [ 301.136998] i915 0000:00:02.0: [drm] mode_changed=0 [ 301.137001] i915 0000:00:02.0: [drm] active_changed=0 [ 301.137004] i915 0000:00:02.0: [drm] connectors_changed=0 [ 301.137008] i915 0000:00:02.0: [drm] color_mgmt_changed=0 [ 301.137011] i915 0000:00:02.0: [drm] plane_mask=8100 [ 301.137014] i915 0000:00:02.0: [drm] connector_mask=40 [ 301.137017] i915 0000:00:02.0: [drm] encoder_mask=4000 [ 301.137021] i915 0000:00:02.0: [drm] mode: "": 60 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x0 0x5 [ 301.137025] i915 0000:00:02.0: [drm] connector[327]: DP-5 [ 301.137028] i915 0000:00:02.0: [drm] crtc=pipe B [ 301.137031] i915 0000:00:02.0: [drm] self_refresh_aware=0 [ 301.137035] [drm:drm_atomic_check_only] checking 00000000238dbb90 [ 301.137041] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] active changed ... [ 301.179635] [drm:drm_atomic_check_only] atomic driver check for 00000000238dbb90 failed: -22 The problem now, is that i915 added CRTC/Connector for pipe C to drm_atomic_state (not the userspace) which resulted in having a CRTC/Connector for Pipe C in a bad state (crtc_state->active = true && Connector is disconnected). This triggered the failure by taking the above condition in update_connector_routing. Chrome will not retry sending atomic commit to disable Pipe B anymore. However, Ubuntu keeps retrying sending atomic_commits for disabling Pipe B and each time it will fail on the same condition (crtc_state- >active = true and Disconnected connector for pipe C). Eventually Ubuntu will send atomic commit for Pipe C that succeed, then it send one more time atomic_commit for Pipe B and at this time crtc_state->active = false for Pipe C and the above condition in update_connctor_routing will not be taken and commit for Pipe B succeed and pipe B will get disabled. This also explains why modest disable for MST takes a little bit longer now in TGL. [ 301.285786] [drm:drm_atomic_print_state] checking 000000002c7f205f [ 301.285789] i915 0000:00:02.0: [drm] crtc[213]: pipe C [ 301.285790] i915 0000:00:02.0: [drm] enable=1 [ 301.285791] i915 0000:00:02.0: [drm] active=0 [ 301.285793] i915 0000:00:02.0: [drm] self_refresh_active=0 [ 301.285794] i915 0000:00:02.0: [drm] planes_changed=0 [ 301.285796] i915 0000:00:02.0: [drm] mode_changed=0 [ 301.285797] i915 0000:00:02.0: [drm] active_changed=0 [ 301.285799] i915 0000:00:02.0: [drm] connectors_changed=0 [ 301.285800] i915 0000:00:02.0: [drm] color_mgmt_changed=0 [ 301.285802] i915 0000:00:02.0: [drm] plane_mask=10000 [ 301.285803] i915 0000:00:02.0: [drm] connector_mask=80 [ 301.285805] i915 0000:00:02.0: [drm] encoder_mask=8000 [ 301.285807] i915 0000:00:02.0: [drm] mode: "": 60 533250 3840 3888 3920 4000 2160 2163 2168 2222 0x0 0x9 [ 301.285808] i915 0000:00:02.0: [drm] connector[333]: DP-6 [ 301.285809] i915 0000:00:02.0: [drm] crtc=pipe C [ 301.285811] i915 0000:00:02.0: [drm] self_refresh_aware=0 [ 301.285813] [drm:drm_atomic_check_only] checking 000000002c7f205f [ 301.285816] [drm:drm_atomic_helper_check_modeset] [CRTC:213:pipe C] active changed 301.285958] [drm:drm_atomic_helper_check_modeset.cold] Updating routing for [CONNECTOR:333:DP-6] [ 302.080580] [drm:drm_atomic_print_state] checking 000000009f6d6341 [ 302.080582] i915 0000:00:02.0: [drm] plane[92]: plane 1B [ 302.080584] i915 0000:00:02.0: [drm] crtc=(null) [ 302.080585] i915 0000:00:02.0: [drm] fb=0 [ 302.080587] i915 0000:00:02.0: [drm] crtc-pos=1920x1080+0+0 [ 302.080589] i915 0000:00:02.0: [drm] src- pos=1920.000000x1080.000000+1920.000000+0.000000 [ 302.080590] i915 0000:00:02.0: [drm] rotation=1 [ 302.080592] i915 0000:00:02.0: [drm] normalized-zpos=0 [ 302.080593] i915 0000:00:02.0: [drm] color-encoding=ITU-R BT.709 YCbCr [ 302.080595] i915 0000:00:02.0: [drm] color-range=YCbCr limited range [ 302.080597] i915 0000:00:02.0: [drm] crtc[152]: pipe B [ 302.080598] i915 0000:00:02.0: [drm] enable=0 [ 302.080600] i915 0000:00:02.0: [drm] active=0 [ 302.080601] i915 0000:00:02.0: [drm] self_refresh_active=0 [ 302.080602] i915 0000:00:02.0: [drm] planes_changed=0 [ 302.080604] i915 0000:00:02.0: [drm] mode_changed=0 [ 302.080605] i915 0000:00:02.0: [drm] active_changed=0 [ 302.080607] i915 0000:00:02.0: [drm] connectors_changed=0 [ 302.080608] i915 0000:00:02.0: [drm] color_mgmt_changed=0 [ 302.080609] i915 0000:00:02.0: [drm] plane_mask=0 [ 302.080611] i915 0000:00:02.0: [drm] connector_mask=0 [ 302.080612] i915 0000:00:02.0: [drm] encoder_mask=4000 [ 302.080614] i915 0000:00:02.0: [drm] mode: "": 0 0 0 0 0 0 0 0 0 0 0x0 0x0 [ 302.080616] i915 0000:00:02.0: [drm] connector[327]: DP-5 [ 302.080617] i915 0000:00:02.0: [drm] crtc=(null) [ 302.080619] i915 0000:00:02.0: [drm] self_refresh_aware=0 [ 302.080620] [drm:drm_atomic_check_only] checking 000000009f6d6341 [ 302.080623] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] mode changed [ 302.080625] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] enable changed [ 302.080627] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] active changed [ 302.080826] [drm:drm_atomic_helper_check_modeset.cold] Updating routing for [CONNECTOR:327:DP-5] [ 302.080841] [drm:drm_atomic_get_connector_state] Added [CONNECTOR:333:DP-6] 00000000aa611aad state to 000000009f6d6341 [ 302.080852] [drm:drm_atomic_get_crtc_state] Added [CRTC:213:pipe C] 0000000019b57d0d state to 000000009f6d6341 [ 302.080884] [drm:drm_atomic_helper_check_modeset.cold] Updating routing for [CONNECTOR:333:DP-6] [ 302.093785] [drm:drm_atomic_helper_check_modeset.cold] [CONNECTOR:333:DP-6] keeps [ENCODER:313:DP-MST C], now on [CRTC:213:pipe C] > > If it had a pool that dispatch one atomic state at time waiting for > > completion before dispatch the next one it would not be a issue. > > > > > > Which will cause the drm_atomic_commit/IOCTL for disabling one > > > > of MST stream pipes (Main MST) to fail. > > > > > > > > The problem happens when a user space (as Chrome) doesn’t retry > > > > a falling commit, leaving a disconnected MST pipe still ON, > > > > which will result in failing reconnect of MST hub or even worse > > > > leaving TC PHY in a connected state while the MST Hub is > > > > disconnected. > > > > > > > > Tested on Ubuntu(drm-tip) and Chrome(kernel-next 5.9 rc7) > > > > > > > > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com > > > > > > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > index e948aacbd4ab..1ede980876ed 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > @@ -265,6 +265,9 @@ > > > > intel_dp_mst_atomic_master_trans_check(struct intel_connector > > > > *connector, > > > > return ret; > > > > } > > > > crtc_state->uapi.mode_changed = true; > > > > + > > > > +if (connector_iter->base.status == > > > > connector_status_disconnected) > > > > +crtc_state->uapi.active = false; > > > > > > That will make the state userspace last set inconsistent with > > > what's > > > really going on. Which means suddenly page flips/vblank waits and > > > whatnot will start to fail. > > > > > > Also that wil directly mutate the prop visible to user space, > > > which > > > is not how these things are supposed to work. I think if we did > > > do > > > something like this we should maybe have some kind of internal > > > flag for it. > > > > > > > } > > > > drm_connector_list_iter_end(&connector_list_iter); > > > > > > > > > > > > > > > > > > > > -- > > > > 2.25.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-21 0:53 ` Almahallawy, Khaled @ 2020-10-21 15:25 ` Mark Yacoub 0 siblings, 0 replies; 12+ messages in thread From: Mark Yacoub @ 2020-10-21 15:25 UTC (permalink / raw) To: Almahallawy, Khaled Cc: daniel.vetter@ffwll.ch, Intel-gfx@lists.freedesktop.org, seanpaul@chromium.org, gildekel@google.com, levinale@google.com On Tue, Oct 20, 2020 at 8:54 PM Almahallawy, Khaled <khaled.almahallawy@intel.com> wrote: > > On Wed, 2020-10-21 at 00:29 +0000, Souza, Jose wrote: > > On Tue, 2020-10-20 at 16:25 -0700, José Roberto de Souza wrote: > > > On Tue, 2020-10-20 at 15:41 +0300, Ville Syrjälä wrote: > > > > On Tue, Oct 20, 2020 at 12:45:55AM -0700, Khaled Almahallawy > > > > wrote: > > > > > This patch avoids failing atomic commits sent by user space by > > > > > making sure CRTC/Connector added to drm_atomic_state by the > > > > > driver are in valid state. > > > > > > > > > > When disconnecting MST hub with two or more connected displays. > > > > > The user space sends IOCTL for each MST pipe to disable. > > > > > drm_atomic_state object sent from user space contains only the > > > > > state of the crtc/pipe intended to disable. > > > > > In TGL, intel_dp_mst_atomic_master_trans_check will add all > > > > > other CRTC and connectors that share the MST stream to > > > > > drm_atomic_state: > > > > > > > > > > drm_atomic_commit > > > > > drm_atomic_helper_check_modeset > > > > > update_connector_routing > > > > > intel_dp_mst_atomic_check = funcs- > > > > > >atomic_check(connector, state); > > > > > intel_dp_mst_atomic_master_trans_check > > > > > intel_atomic_get_digital_connector_state > > > > > drm_atomic_get_connector_state <-- Add all Connectors > > > > > drm_atomic_get_crtc_state <-- Add all CRTCs > > > > > update_connector_routing <-- Check added Connector/CRTCs > > > > > - Will fail > > > > > > > > > > However the added crtc/connector pair will be in invalid state > > > > > (enabled state for a removed connector) > > > > > triggering this condition in > > > > > drm_atomic_helper.c/update_connector_routing: > > > > > > > > > > if (!state->duplicated && > > > > > drm_connector_is_unregistered(connector) && > > > > > crtc_state->active) { > > > > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n", > > > > > connector->base.id, connector->name); > > > > > return -EINVAL; > > > > > } > > > > > > > > Yeah, I think that "reject modeset on unregistered connectors" > > > > idea is > > > > a bit broken given how the uapi has worked in the past. Cc:ing > > > > danvet > > > > and lyude who IIRC were involved with that. > > > > > > Actually by removing this condition everything works fine as well. > However we were worried about touching this part and screwing other > stuff :-) > > > > > Hmm. Maybe we could add the other stuff to the state only after > > > > the > > > > connector .atomic_check() stuff has been done? I don't quite > > > > remember > > > > why we decided to do it here. José do you recall the details? > > > > > > Because the connector check function runs twice in > > > drm_atomic_helper_check_modeset(), in the first iteration it will > > > add all connectors that share the > > > same MST stream to state, the second one will make sure all other > > > checks passed in all connectors of the MST stream. > > > > > > To me looks like the Chrome userspace is not doing the right thing, > > > it is sending asynchronous atomic commits with conflicting state > > > between each > > > commit. > > > > Oh it do not have information about other connectors so not > > conflicting but anyways userspace should retry atomic commits > > anyways. > > > > CC: Mark and Gill for Chrome userspace input > > Yes Chrome is sending the atomic commits with correct states one after > another. Matter of fact, the failure we observe in Chrome is also > happening in Ubuntu. > > We checked that by printing the drm_atomic_state sent by user space > before atomic check: > > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1365,9 +1365,12 @@ int drm_atomic_commit(struct drm_atomic_state > *state) > struct drm_mode_config *config = &state->dev->mode_config; > int ret; > > + drm_atomic_print_state(state); > ret = drm_atomic_check_only(state); > > > Using MST with two displays and eDP connected. First MST on Pipe B and > Second MST on Pipe C. > > Chrome send atomic commit to disable Pipe B first as below: > > [ 60.406455] [drm:drm_atomic_print_state] checking 00000000c0e0cc6c > [ 60.406455] i915 0000:00:02.0: [drm] crtc[152]: pipe B > [ 60.406456] i915 0000:00:02.0: [drm] enable=0 > [ 60.406457] i915 0000:00:02.0: [drm] active=0 > [ 60.406458] i915 0000:00:02.0: [drm] self_refresh_active=0 > [ 60.406459] i915 0000:00:02.0: [drm] planes_changed=0 > [ 60.406460] i915 0000:00:02.0: [drm] mode_changed=0 > [ 60.406461] i915 0000:00:02.0: [drm] active_changed=0 > [ 60.406462] i915 0000:00:02.0: [drm] connectors_changed=0 > [ 60.406463] i915 0000:00:02.0: [drm] color_mgmt_changed=0 > [ 60.406464] i915 0000:00:02.0: [drm] plane_mask=100 > [ 60.406465] i915 0000:00:02.0: [drm] connector_mask=0 > [ 60.406466] i915 0000:00:02.0: [drm] encoder_mask=2000 > [ 60.406467] i915 0000:00:02.0: [drm] mode: "": 0 0 0 0 0 0 0 > 0 0 0 0x0 0x0 > [ 60.406468] i915 0000:00:02.0: [drm] connector[353]: DP-6 > [ 60.406469] i915 0000:00:02.0: [drm] crtc=(null) > [ 60.406470] i915 0000:00:02.0: [drm] self_refresh_aware=0 > [ 60.406471] [drm:drm_atomic_check_only] checking 00000000c0e0cc6c > [ 60.406473] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] > mode changed > [ 60.406474] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] > enable changed > [ 60.406474] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] > active changed > ... > [ 60.407674] [drm:drm_atomic_check_only] atomic driver check for > 00000000c0e0cc6c failed: -22 > > > Even in Ubuntu we get similar atomic commit with similar state for Pipe > B: > > [ 301.136961] i915 0000:00:02.0: [drm] crtc[152]: pipe B > [ 301.136964] i915 0000:00:02.0: [drm] enable=1 > [ 301.136966] i915 0000:00:02.0: [drm] active=0 > [ 301.136969] i915 0000:00:02.0: [drm] self_refresh_active=0 > [ 301.136995] i915 0000:00:02.0: [drm] planes_changed=0 > [ 301.136998] i915 0000:00:02.0: [drm] mode_changed=0 > [ 301.137001] i915 0000:00:02.0: [drm] active_changed=0 > [ 301.137004] i915 0000:00:02.0: [drm] connectors_changed=0 > [ 301.137008] i915 0000:00:02.0: [drm] color_mgmt_changed=0 > [ 301.137011] i915 0000:00:02.0: [drm] plane_mask=8100 > [ 301.137014] i915 0000:00:02.0: [drm] connector_mask=40 > [ 301.137017] i915 0000:00:02.0: [drm] encoder_mask=4000 > [ 301.137021] i915 0000:00:02.0: [drm] mode: "": 60 148500 > 1920 2008 2052 2200 1080 1084 1089 1125 0x0 0x5 > [ 301.137025] i915 0000:00:02.0: [drm] connector[327]: DP-5 > [ 301.137028] i915 0000:00:02.0: [drm] crtc=pipe B > [ 301.137031] i915 0000:00:02.0: [drm] self_refresh_aware=0 > [ 301.137035] [drm:drm_atomic_check_only] checking 00000000238dbb90 > [ 301.137041] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] > active changed > ... > [ 301.179635] [drm:drm_atomic_check_only] atomic driver check for > 00000000238dbb90 failed: -22 > > > The problem now, is that i915 added CRTC/Connector for pipe C to > drm_atomic_state (not the userspace) which resulted in having a > CRTC/Connector for Pipe C in a bad state (crtc_state->active = true && > Connector is disconnected). This triggered the failure by taking the > above condition in update_connector_routing. > > Chrome will not retry sending atomic commit to disable Pipe B anymore. IMO, I don't think Chrome should keep on retrying, given the nature of disabling (unlike enabling, you just wanna shut off the display, there are no "different" configurations you wanna disable with). With your analysis, I think you pointed out where we should be looking. IIUC, Pipe C is not really in "bad" state, it's more in a "transient" state: connector physically disconnected but it's still active because Chrome hasn't disabled it yet. Is that right? If so, this seems like it will be a common case with MST: a number of connectors are connected -> you unplug their common cable -> the userspace disables them one by one. Because of MST, we might need to handle them in a special way. 2 ideas come to mind: either ignore connectors in "transient state" that are irrelevant to the current pipe [in our example, if you know you're on MST with Pipe C, ignore its transient state while you're disabling Pipe B], or, queue all transient disable requests until you get all displays then deactivate them together [in our example, when Pipe B wants to be disabled, add it "i want to disable these"_queue because C is still transient, and once you get a request for Pipe C, deactivate them all together] (more complex i think and kinda risky) > However, Ubuntu keeps retrying sending atomic_commits for disabling > Pipe B and each time it will fail on the same condition (crtc_state- > >active = true and Disconnected connector for pipe C). > Eventually Ubuntu will send atomic commit for Pipe C that succeed, then > it send one more time atomic_commit for Pipe B and at this time > crtc_state->active = false for Pipe C and the above condition in > update_connctor_routing will not be taken and commit for Pipe B succeed > and pipe B will get disabled. > This also explains why modest disable for MST takes a little bit longer > now in TGL. > > [ 301.285786] [drm:drm_atomic_print_state] checking 000000002c7f205f > [ 301.285789] i915 0000:00:02.0: [drm] crtc[213]: pipe C > [ 301.285790] i915 0000:00:02.0: [drm] enable=1 > [ 301.285791] i915 0000:00:02.0: [drm] active=0 > [ 301.285793] i915 0000:00:02.0: [drm] self_refresh_active=0 > [ 301.285794] i915 0000:00:02.0: [drm] planes_changed=0 > [ 301.285796] i915 0000:00:02.0: [drm] mode_changed=0 > [ 301.285797] i915 0000:00:02.0: [drm] active_changed=0 > [ 301.285799] i915 0000:00:02.0: [drm] connectors_changed=0 > [ 301.285800] i915 0000:00:02.0: [drm] color_mgmt_changed=0 > [ 301.285802] i915 0000:00:02.0: [drm] plane_mask=10000 > [ 301.285803] i915 0000:00:02.0: [drm] connector_mask=80 > [ 301.285805] i915 0000:00:02.0: [drm] encoder_mask=8000 > [ 301.285807] i915 0000:00:02.0: [drm] mode: "": 60 533250 > 3840 3888 3920 4000 2160 2163 2168 2222 0x0 0x9 > [ 301.285808] i915 0000:00:02.0: [drm] connector[333]: DP-6 > [ 301.285809] i915 0000:00:02.0: [drm] crtc=pipe C > [ 301.285811] i915 0000:00:02.0: [drm] self_refresh_aware=0 > [ 301.285813] [drm:drm_atomic_check_only] checking 000000002c7f205f > [ 301.285816] [drm:drm_atomic_helper_check_modeset] [CRTC:213:pipe C] > active changed > 301.285958] [drm:drm_atomic_helper_check_modeset.cold] Updating > routing for [CONNECTOR:333:DP-6] > > > [ 302.080580] [drm:drm_atomic_print_state] checking 000000009f6d6341 > [ 302.080582] i915 0000:00:02.0: [drm] plane[92]: plane 1B > [ 302.080584] i915 0000:00:02.0: [drm] crtc=(null) > [ 302.080585] i915 0000:00:02.0: [drm] fb=0 > [ 302.080587] i915 0000:00:02.0: [drm] crtc-pos=1920x1080+0+0 > [ 302.080589] i915 0000:00:02.0: [drm] src- > pos=1920.000000x1080.000000+1920.000000+0.000000 > [ 302.080590] i915 0000:00:02.0: [drm] rotation=1 > [ 302.080592] i915 0000:00:02.0: [drm] normalized-zpos=0 > [ 302.080593] i915 0000:00:02.0: [drm] color-encoding=ITU-R > BT.709 YCbCr > [ 302.080595] i915 0000:00:02.0: [drm] color-range=YCbCr > limited range > [ 302.080597] i915 0000:00:02.0: [drm] crtc[152]: pipe B > [ 302.080598] i915 0000:00:02.0: [drm] enable=0 > [ 302.080600] i915 0000:00:02.0: [drm] active=0 > [ 302.080601] i915 0000:00:02.0: [drm] self_refresh_active=0 > [ 302.080602] i915 0000:00:02.0: [drm] planes_changed=0 > [ 302.080604] i915 0000:00:02.0: [drm] mode_changed=0 > [ 302.080605] i915 0000:00:02.0: [drm] active_changed=0 > [ 302.080607] i915 0000:00:02.0: [drm] connectors_changed=0 > [ 302.080608] i915 0000:00:02.0: [drm] color_mgmt_changed=0 > [ 302.080609] i915 0000:00:02.0: [drm] plane_mask=0 > [ 302.080611] i915 0000:00:02.0: [drm] connector_mask=0 > [ 302.080612] i915 0000:00:02.0: [drm] encoder_mask=4000 > [ 302.080614] i915 0000:00:02.0: [drm] mode: "": 0 0 0 0 0 0 0 > 0 0 0 0x0 0x0 > [ 302.080616] i915 0000:00:02.0: [drm] connector[327]: DP-5 > [ 302.080617] i915 0000:00:02.0: [drm] crtc=(null) > [ 302.080619] i915 0000:00:02.0: [drm] self_refresh_aware=0 > [ 302.080620] [drm:drm_atomic_check_only] checking 000000009f6d6341 > [ 302.080623] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] > mode changed > [ 302.080625] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] > enable changed > [ 302.080627] [drm:drm_atomic_helper_check_modeset] [CRTC:152:pipe B] > active changed > > [ 302.080826] [drm:drm_atomic_helper_check_modeset.cold] Updating > routing for [CONNECTOR:327:DP-5] > [ 302.080841] [drm:drm_atomic_get_connector_state] Added > [CONNECTOR:333:DP-6] 00000000aa611aad state to 000000009f6d6341 > [ 302.080852] [drm:drm_atomic_get_crtc_state] Added [CRTC:213:pipe C] > 0000000019b57d0d state to 000000009f6d6341 > [ 302.080884] [drm:drm_atomic_helper_check_modeset.cold] Updating > routing for [CONNECTOR:333:DP-6] > [ 302.093785] [drm:drm_atomic_helper_check_modeset.cold] > [CONNECTOR:333:DP-6] keeps [ENCODER:313:DP-MST C], now on > [CRTC:213:pipe C] > > > > > > If it had a pool that dispatch one atomic state at time waiting for > > > completion before dispatch the next one it would not be a issue. > > > > > > > > Which will cause the drm_atomic_commit/IOCTL for disabling one > > > > > of MST stream pipes (Main MST) to fail. > > > > > > > > > > The problem happens when a user space (as Chrome) doesn’t retry > > > > > a falling commit, leaving a disconnected MST pipe still ON, > > > > > which will result in failing reconnect of MST hub or even worse > > > > > leaving TC PHY in a connected state while the MST Hub is > > > > > disconnected. > > > > > > > > > > Tested on Ubuntu(drm-tip) and Chrome(kernel-next 5.9 rc7) > > > > > > > > > > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com > > > > > > > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > > index e948aacbd4ab..1ede980876ed 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > > > @@ -265,6 +265,9 @@ > > > > > intel_dp_mst_atomic_master_trans_check(struct intel_connector > > > > > *connector, > > > > > return ret; > > > > > } > > > > > crtc_state->uapi.mode_changed = true; > > > > > + > > > > > +if (connector_iter->base.status == > > > > > connector_status_disconnected) > > > > > +crtc_state->uapi.active = false; > > > > > > > > That will make the state userspace last set inconsistent with > > > > what's > > > > really going on. Which means suddenly page flips/vblank waits and > > > > whatnot will start to fail. > > > > > > > > Also that wil directly mutate the prop visible to user space, > > > > which > > > > is not how these things are supposed to work. I think if we did > > > > do > > > > something like this we should maybe have some kind of internal > > > > flag for it. > > > > Agree with Ville, this is explicitly setting fields for the purpose of getting something working, I'd be worried of the inconsistencies it could introduce down the line. > > > > > } > > > > > drm_connector_list_iter_end(&connector_list_iter); > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > 2.25.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-20 23:25 ` Souza, Jose 2020-10-21 0:29 ` Souza, Jose @ 2020-10-21 13:26 ` Ville Syrjälä 2020-10-21 21:25 ` Lyude Paul 1 sibling, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2020-10-21 13:26 UTC (permalink / raw) To: Souza, Jose Cc: daniel.vetter@ffwll.ch, Intel-gfx@lists.freedesktop.org, seanpaul@chromium.org On Tue, Oct 20, 2020 at 11:25:53PM +0000, Souza, Jose wrote: > On Tue, 2020-10-20 at 15:41 +0300, Ville Syrjälä wrote: > > On Tue, Oct 20, 2020 at 12:45:55AM -0700, Khaled Almahallawy wrote: > > > This patch avoids failing atomic commits sent by user space by making sure CRTC/Connector added to drm_atomic_state by the driver are in valid state. > > > > > > When disconnecting MST hub with two or more connected displays. The user space sends IOCTL for each MST pipe to disable. > > > drm_atomic_state object sent from user space contains only the state of the crtc/pipe intended to disable. > > > In TGL, intel_dp_mst_atomic_master_trans_check will add all other CRTC and connectors that share the MST stream to drm_atomic_state: > > > > > > drm_atomic_commit > > > drm_atomic_helper_check_modeset > > > update_connector_routing > > > intel_dp_mst_atomic_check = funcs->atomic_check(connector, state); > > > intel_dp_mst_atomic_master_trans_check > > > intel_atomic_get_digital_connector_state > > > drm_atomic_get_connector_state <-- Add all Connectors > > > drm_atomic_get_crtc_state <-- Add all CRTCs > > > update_connector_routing <-- Check added Connector/CRTCs - Will fail > > > > > > However the added crtc/connector pair will be in invalid state (enabled state for a removed connector) > > > triggering this condition in drm_atomic_helper.c/update_connector_routing: > > > > > > if (!state->duplicated && drm_connector_is_unregistered(connector) && > > > crtc_state->active) { > > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n", > > > connector->base.id, connector->name); > > > return -EINVAL; > > > } > > > > Yeah, I think that "reject modeset on unregistered connectors" idea is > > a bit broken given how the uapi has worked in the past. Cc:ing danvet > > and lyude who IIRC were involved with that. > > > > Hmm. Maybe we could add the other stuff to the state only after the > > connector .atomic_check() stuff has been done? I don't quite remember > > why we decided to do it here. José do you recall the details? > > Because the connector check function runs twice in drm_atomic_helper_check_modeset(), in the first iteration it will add all connectors that share the > same MST stream to state, the second one will make sure all other checks passed in all connectors of the MST stream. > > To me looks like the Chrome userspace is not doing the right thing, it is sending asynchronous atomic commits with conflicting state between each > commit. > If it had a pool that dispatch one atomic state at time waiting for completion before dispatch the next one it would not be a issue. Yeah, with atomic userspace could avoid this potentially. Though it may be racy depending on whether it has noticed all the MST connectors disappearing yet or not. Either way it's still an issue for legacy uapi. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-21 13:26 ` Ville Syrjälä @ 2020-10-21 21:25 ` Lyude Paul 2020-10-22 11:07 ` Ville Syrjälä 0 siblings, 1 reply; 12+ messages in thread From: Lyude Paul @ 2020-10-21 21:25 UTC (permalink / raw) To: Ville Syrjälä, Souza, Jose Cc: daniel.vetter@ffwll.ch, Intel-gfx@lists.freedesktop.org, seanpaul@chromium.org On Wed, 2020-10-21 at 16:26 +0300, Ville Syrjälä wrote: > On Tue, Oct 20, 2020 at 11:25:53PM +0000, Souza, Jose wrote: > > On Tue, 2020-10-20 at 15:41 +0300, Ville Syrjälä wrote: > > > On Tue, Oct 20, 2020 at 12:45:55AM -0700, Khaled Almahallawy wrote: > > > > This patch avoids failing atomic commits sent by user space by making > > > > sure CRTC/Connector added to drm_atomic_state by the driver are in valid > > > > state. > > > > > > > > When disconnecting MST hub with two or more connected displays. The user > > > > space sends IOCTL for each MST pipe to disable. > > > > drm_atomic_state object sent from user space contains only the state of > > > > the crtc/pipe intended to disable. > > > > In TGL, intel_dp_mst_atomic_master_trans_check will add all other CRTC > > > > and connectors that share the MST stream to drm_atomic_state: > > > > > > > > drm_atomic_commit > > > >    drm_atomic_helper_check_modeset > > > >        update_connector_routing > > > >        intel_dp_mst_atomic_check = funcs- > > > > >atomic_check(connector, state); > > > >        intel_dp_mst_atomic_master_trans_chec > > > > k > > > > intel_atomic_get_digital_connector_state > > > > drm_atomic_get_connector_state <-- Add all > > > > Connectors > > > > drm_atomic_get_crtc_state <-- Add all CRTCs > > > >        update_connector_routing <-- Check added > > > > Connector/CRTCs - Will fail > > > > > > > > However the added crtc/connector pair will be in invalid state (enabled > > > > state for a removed connector) > > > > triggering this condition in > > > > drm_atomic_helper.c/update_connector_routing: > > > > > > > > if (!state->duplicated && > > > > drm_connector_is_unregistered(connector) && > > > > crtc_state->active) { > > > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not > > > > registered\n", > > > > connector->base.id, connector->name); > > > > return -EINVAL; > > > > } > > > > > > Yeah, I think that "reject modeset on unregistered connectors" idea is > > > a bit broken given how the uapi has worked in the past. Cc:ing danvet > > > and lyude who IIRC were involved with that. > > > > > > Hmm. Maybe we could add the other stuff to the state only after the > > > connector .atomic_check() stuff has been done? I don't quite remember > > > why we decided to do it here. José do you recall the details? > > > > Because the connector check function runs twice in > > drm_atomic_helper_check_modeset(), in the first iteration it will add all > > connectors that share the > > same MST stream to state, the second one will make sure all other checks > > passed in all connectors of the MST stream. > > > > To me looks like the Chrome userspace is not doing the right thing, it is > > sending asynchronous atomic commits with conflicting state between each > > commit. > > If it had a pool that dispatch one atomic state at time waiting for > > completion before dispatch the next one it would not be a issue. > > Yeah, with atomic userspace could avoid this potentially. Though it > may be racy depending on whether it has noticed all the MST connectors > disappearing yet or not. Either way it's still an issue for legacy > uapi. Sigh-I had hoped that we would have hooked this up such that we'd avoid this (as I've already had to fix some issues this caused with legacy modesetting) but I guess not. Have you guys considered trying to use the connector epochs whenever you receive a hotplug event to differentiate between removed ('stale') connectors and other connectors? tbh, if you can't find a connector with the same mst path and epoch you last had as your stale connector then it's safe to just assume it's gone. Also - I'm totally open to better ideas for handling this or making it more obvious when a connector has been removed, most of the reason for adding these checks was to try our best (as this is impossible to fully guarantee) to avoid situations where a host tried to enable an MST display that no longer existed and put the hardware into a weird state. At least if I remember correctly, it's been a while. > -- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've asked me a question, are waiting for a review/merge on a patch, etc. and I haven't responded in a while, please feel free to send me another email to check on my status. I don't bite! _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream. 2020-10-21 21:25 ` Lyude Paul @ 2020-10-22 11:07 ` Ville Syrjälä 0 siblings, 0 replies; 12+ messages in thread From: Ville Syrjälä @ 2020-10-22 11:07 UTC (permalink / raw) To: Lyude Paul Cc: daniel.vetter@ffwll.ch, Intel-gfx@lists.freedesktop.org, seanpaul@chromium.org On Wed, Oct 21, 2020 at 05:25:40PM -0400, Lyude Paul wrote: > On Wed, 2020-10-21 at 16:26 +0300, Ville Syrjälä wrote: > > On Tue, Oct 20, 2020 at 11:25:53PM +0000, Souza, Jose wrote: > > > On Tue, 2020-10-20 at 15:41 +0300, Ville Syrjälä wrote: > > > > On Tue, Oct 20, 2020 at 12:45:55AM -0700, Khaled Almahallawy wrote: > > > > > This patch avoids failing atomic commits sent by user space by making > > > > > sure CRTC/Connector added to drm_atomic_state by the driver are in valid > > > > > state. > > > > > > > > > > When disconnecting MST hub with two or more connected displays. The user > > > > > space sends IOCTL for each MST pipe to disable. > > > > > drm_atomic_state object sent from user space contains only the state of > > > > > the crtc/pipe intended to disable. > > > > > In TGL, intel_dp_mst_atomic_master_trans_check will add all other CRTC > > > > > and connectors that share the MST stream to drm_atomic_state: > > > > > > > > > > drm_atomic_commit > > > > >    drm_atomic_helper_check_modeset > > > > >        update_connector_routing > > > > >        intel_dp_mst_atomic_check = funcs- > > > > > >atomic_check(connector, state); > > > > >        intel_dp_mst_atomic_master_trans_chec > > > > > k > > > > > intel_atomic_get_digital_connector_state > > > > > drm_atomic_get_connector_state <-- Add all > > > > > Connectors > > > > > drm_atomic_get_crtc_state <-- Add all CRTCs > > > > >        update_connector_routing <-- Check added > > > > > Connector/CRTCs - Will fail > > > > > > > > > > However the added crtc/connector pair will be in invalid state (enabled > > > > > state for a removed connector) > > > > > triggering this condition in > > > > > drm_atomic_helper.c/update_connector_routing: > > > > > > > > > > if (!state->duplicated && > > > > > drm_connector_is_unregistered(connector) && > > > > > crtc_state->active) { > > > > > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not > > > > > registered\n", > > > > > connector->base.id, connector->name); > > > > > return -EINVAL; > > > > > } > > > > > > > > Yeah, I think that "reject modeset on unregistered connectors" idea is > > > > a bit broken given how the uapi has worked in the past. Cc:ing danvet > > > > and lyude who IIRC were involved with that. > > > > > > > > Hmm. Maybe we could add the other stuff to the state only after the > > > > connector .atomic_check() stuff has been done? I don't quite remember > > > > why we decided to do it here. José do you recall the details? > > > > > > Because the connector check function runs twice in > > > drm_atomic_helper_check_modeset(), in the first iteration it will add all > > > connectors that share the > > > same MST stream to state, the second one will make sure all other checks > > > passed in all connectors of the MST stream. > > > > > > To me looks like the Chrome userspace is not doing the right thing, it is > > > sending asynchronous atomic commits with conflicting state between each > > > commit. > > > If it had a pool that dispatch one atomic state at time waiting for > > > completion before dispatch the next one it would not be a issue. > > > > Yeah, with atomic userspace could avoid this potentially. Though it > > may be racy depending on whether it has noticed all the MST connectors > > disappearing yet or not. Either way it's still an issue for legacy > > uapi. > > Sigh-I had hoped that we would have hooked this up such that we'd avoid this (as > I've already had to fix some issues this caused with legacy modesetting) but I > guess not. Have you guys considered trying to use the connector epochs whenever > you receive a hotplug event to differentiate between removed ('stale') > connectors and other connectors? tbh, if you can't find a connector with the > same mst path and epoch you last had as your stale connector then it's safe to > just assume it's gone. > > Also - I'm totally open to better ideas for handling this or making it more > obvious when a connector has been removed, most of the reason for adding these > checks was to try our best (as this is impossible to fully guarantee) to avoid > situations where a host tried to enable an MST display that no longer existed > and put the hardware into a weird state. At least if I remember correctly, it's > been a while. It's all racy anyway is it not? Because of that I'm pretty firmly in the "just plow ahead blindly" camp. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-10-22 11:08 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-10-20 7:45 [Intel-gfx] [PATCH] drm/i915/tgl: Set drm_crtc_state.active=false for all added disconnected CRTCs sharing MST stream Khaled Almahallawy 2020-10-20 8:09 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork 2020-10-20 8:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2020-10-20 9:41 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2020-10-20 12:41 ` [Intel-gfx] [PATCH] " Ville Syrjälä 2020-10-20 23:25 ` Souza, Jose 2020-10-21 0:29 ` Souza, Jose 2020-10-21 0:53 ` Almahallawy, Khaled 2020-10-21 15:25 ` Mark Yacoub 2020-10-21 13:26 ` Ville Syrjälä 2020-10-21 21:25 ` Lyude Paul 2020-10-22 11:07 ` Ville Syrjälä
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox