| Message ID | 20200811200457.134743-1-lyude@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from i915 | expand |
Can someone take a look at this? All of the CI related links here just give a permission denied when you try to view them, and I'm positive this test failure has nothing to do with this series On Tue, 2020-08-11 at 22:13 +0000, Patchwork wrote: > > Patch Details > > Series:drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from > i915 (rev2) > URL:https://patchwork.freedesktop.org/series/80542/ > State:failure > > Details: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18347/index.html > > > > > CI Bug Log - changes from CI_DRM_8872 -> Patchwork_18347 > Summary > FAILURE > Serious unknown changes coming with Patchwork_18347 absolutely need to be > > verified manually. > If you think the reported changes have nothing to do with the changes > > introduced in Patchwork_18347, please notify your bug team to allow them > > to document this new failure mode, which will reduce false positives in > CI. > External URL: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18347/index.html > Possible new issues > Here are the unknown changes that may have been introduced in > Patchwork_18347: > IGT changes > Possible regressions > > igt@runner@aborted: > fi-skl-6700k2: NOTRUN -> FAIL > > > > Known issues > Here are the changes found in Patchwork_18347 that come from known issues: > IGT changes > Issues hit > > > igt@i915_module_load@reload: > > fi-byt-j1900: PASS -> DMESG-WARN (i915#1982) > > > > igt@i915_selftest@live@execlists: > > fi-icl-y: PASS -> INCOMPLETE (i915#2276) > > > > igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1: > > fi-icl-u2: PASS -> DMESG-WARN (i915#1982) +1 similar issue > > > > Possible fixes > > > igt@i915_module_load@reload: > > > fi-apl-guc: DMESG-WARN (i915#1635 / i915#1982) -> PASS > > > fi-cml-s: DMESG-WARN (i915#1982) -> PASS > > > fi-kbl-x1275: DMESG-WARN (i915#62 / i915#92) -> PASS > > > > > igt@kms_busy@basic@flip: > > > {fi-tgl-dsi}: DMESG-WARN (i915#1982) -> PASS > > > fi-kbl-x1275: DMESG-WARN (i915#62 / i915#92 / i915#95) -> PASS > > > > > Warnings > > > igt@i915_pm_rpm@module-reload: > > fi-kbl-x1275: SKIP (fdo#109271) -> DMESG-FAIL (i915#62) > > > > igt@kms_force_connector_basic@force-edid: > > fi-kbl-x1275: DMESG-WARN (i915#62 / i915#92) -> DMESG-WARN (i915#62 / > i915#92 / i915#95) +1 similar issue > > > > {name}: This element is suppressed. This means it is ignored when computing > > the status of the difference (SUCCESS, WARNING, or FAILURE). > Participating hosts (42 -> 37) > Additional (1): fi-kbl-soraka > > 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_8872 -> Patchwork_18347 > > CI-20190529: 20190529 > > CI_DRM_8872: 494f4611d8ee77b49fec39886b8b97c14f291f18 @ > git://anongit.freedesktop.org/gfx-ci/linux > > IGT_5767: 39e9aa1032a4e60f776f34b3ccf4fb728abbfe5c @ > git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > > Patchwork_18347: ed9c2cbd0615944411c1dcfa8383a8174b1f215f @ > git://anongit.freedesktop.org/gfx-ci/linux > == Linux commits == > ed9c2cbd0615 drm/nouveau/kms: Start using drm_dp_read_dpcd_caps() > > dbc6ce6444f9 drm/i915/dp: Extract drm_dp_read_dpcd_caps() > > b2b88eaa8158 drm/nouveau/kms: Don't change EDID when it hasn't actually > changed > > c726b1b06424 drm/nouveau/kms/nv50-: Add support for DP_SINK_COUNT > > dd59fa4b47de drm/i915/dp: Extract drm_dp_get_sink_count() > > e308120f0a75 drm/i915/dp: Extract drm_dp_has_sink_count() > > 7ebbddd8fe5f drm/nouveau/kms/nv50-: Use downstream DP clock limits for mode > validation > > 43964c882505 drm/i915/dp: Extract drm_dp_downstream_read_info() > > 00e33908c535 drm/nouveau/kms: Only use hpd_work for reprobing in HPD paths > > 67b2ac39f22b drm/nouveau/kms: Move drm_dp_cec_unset_edid() into > nouveau_connector_detect() > > 1616b919e4b2 drm/nouveau/kms: Use new drm_dp_has_mst() helper for checking > MST caps > > 8b6671700abf drm/i915/dp: Extract drm_dp_has_mst() > > 2aa54c3a5de3 drm/nouveau/kms/nv50-: Refactor and cleanup DP HPD handling > > b197d002e32f drm/nouveau/kms/nv50-: Use drm_dp_dpcd_(readb|writeb)() in > nv50_sor_disable() > > 7cb0374b4558 drm/nouveau/kms: Search for encoders' connectors properly > > eb430d7e3dc4 drm/nouveau/kms: Don't clear DP_MST_CTRL DPCD in > nv50_mstm_new() > > bd4724757ea9 drm/nouveau/kms/nv50-: Use macros for DP registers in > nouveau_dp.c > > 478aa516d10c drm/nouveau/kms/nv50-: Just use drm_dp_dpcd_read() in > nouveau_dp.c > > d6d94517b732 drm/nouveau/kms/nv50-: Remove open-coded drm_dp_read_desc() > > 7429fbed7536 drm/nouveau/kms: Fix some indenting in nouveau_dp_detect() > > >
Ah-nevermind, Ickle pointed out to me that it just takes a little bit for the results to migrate over to AWS, things seem to be viewable now. On Tue, 2020-08-11 at 18:17 -0400, Lyude Paul wrote: > Can someone take a look at this? All of the CI related links here just give > a permission denied when you try to view them, and I'm positive this test > failure has nothing to do with this series > On Tue, 2020-08-11 at 22:13 +0000, Patchwork wrote: > > > > Patch Details > > > > Series:drm/dp, i915, nouveau: Cleanup nouveau HPD and add DP features from > > i915 (rev2) > > URL:https://patchwork.freedesktop.org/series/80542/ > > State:failure > > > > Details: > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18347/index.html > > > > > > > > > > CI Bug Log - changes from CI_DRM_8872 -> Patchwork_18347 > > Summary > > FAILURE > > Serious unknown changes coming with Patchwork_18347 absolutely need to be > > > > verified manually. > > If you think the reported changes have nothing to do with the changes > > > > introduced in Patchwork_18347, please notify your bug team to allow them > > > > to document this new failure mode, which will reduce false positives in > > CI. > > External URL: > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18347/index.html > > Possible new issues > > Here are the unknown changes that may have been introduced in > > Patchwork_18347: > > IGT changes > > Possible regressions > > > > igt@runner@aborted: > > fi-skl-6700k2: NOTRUN -> FAIL > > > > > > > > Known issues > > Here are the changes found in Patchwork_18347 that come from known issues: > > IGT changes > > Issues hit > > > > > > igt@i915_module_load@reload: > > > > fi-byt-j1900: PASS -> DMESG-WARN (i915#1982) > > > > > > > > igt@i915_selftest@live@execlists: > > > > fi-icl-y: PASS -> INCOMPLETE (i915#2276) > > > > > > > > igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1: > > > > fi-icl-u2: PASS -> DMESG-WARN (i915#1982) +1 similar issue > > > > > > > > Possible fixes > > > > > > igt@i915_module_load@reload: > > > > > > fi-apl-guc: DMESG-WARN (i915#1635 / i915#1982) -> PASS > > > > > > fi-cml-s: DMESG-WARN (i915#1982) -> PASS > > > > > > fi-kbl-x1275: DMESG-WARN (i915#62 / i915#92) -> PASS > > > > > > > > > > igt@kms_busy@basic@flip: > > > > > > {fi-tgl-dsi}: DMESG-WARN (i915#1982) -> PASS > > > > > > fi-kbl-x1275: DMESG-WARN (i915#62 / i915#92 / i915#95) -> PASS > > > > > > > > > > Warnings > > > > > > igt@i915_pm_rpm@module-reload: > > > > fi-kbl-x1275: SKIP (fdo#109271) -> DMESG-FAIL (i915#62) > > > > > > > > igt@kms_force_connector_basic@force-edid: > > > > fi-kbl-x1275: DMESG-WARN (i915#62 / i915#92) -> DMESG-WARN (i915#62 > > / i915#92 / i915#95) +1 similar issue > > > > > > > > {name}: This element is suppressed. This means it is ignored when > > computing > > > > the status of the difference (SUCCESS, WARNING, or FAILURE). > > Participating hosts (42 -> 37) > > Additional (1): fi-kbl-soraka > > > > 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_8872 -> Patchwork_18347 > > > > CI-20190529: 20190529 > > > > CI_DRM_8872: 494f4611d8ee77b49fec39886b8b97c14f291f18 @ > > git://anongit.freedesktop.org/gfx-ci/linux > > > > IGT_5767: 39e9aa1032a4e60f776f34b3ccf4fb728abbfe5c @ > > git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > > > > Patchwork_18347: ed9c2cbd0615944411c1dcfa8383a8174b1f215f @ > > git://anongit.freedesktop.org/gfx-ci/linux > > == Linux commits == > > ed9c2cbd0615 drm/nouveau/kms: Start using drm_dp_read_dpcd_caps() > > > > dbc6ce6444f9 drm/i915/dp: Extract drm_dp_read_dpcd_caps() > > > > b2b88eaa8158 drm/nouveau/kms: Don't change EDID when it hasn't actually > > changed > > > > c726b1b06424 drm/nouveau/kms/nv50-: Add support for DP_SINK_COUNT > > > > dd59fa4b47de drm/i915/dp: Extract drm_dp_get_sink_count() > > > > e308120f0a75 drm/i915/dp: Extract drm_dp_has_sink_count() > > > > 7ebbddd8fe5f drm/nouveau/kms/nv50-: Use downstream DP clock limits for > > mode validation > > > > 43964c882505 drm/i915/dp: Extract drm_dp_downstream_read_info() > > > > 00e33908c535 drm/nouveau/kms: Only use hpd_work for reprobing in HPD paths > > > > 67b2ac39f22b drm/nouveau/kms: Move drm_dp_cec_unset_edid() into > > nouveau_connector_detect() > > > > 1616b919e4b2 drm/nouveau/kms: Use new drm_dp_has_mst() helper for checking > > MST caps > > > > 8b6671700abf drm/i915/dp: Extract drm_dp_has_mst() > > > > 2aa54c3a5de3 drm/nouveau/kms/nv50-: Refactor and cleanup DP HPD handling > > > > b197d002e32f drm/nouveau/kms/nv50-: Use drm_dp_dpcd_(readb|writeb)() in > > nv50_sor_disable() > > > > 7cb0374b4558 drm/nouveau/kms: Search for encoders' connectors properly > > > > eb430d7e3dc4 drm/nouveau/kms: Don't clear DP_MST_CTRL DPCD in > > nv50_mstm_new() > > > > bd4724757ea9 drm/nouveau/kms/nv50-: Use macros for DP registers in > > nouveau_dp.c > > > > 478aa516d10c drm/nouveau/kms/nv50-: Just use drm_dp_dpcd_read() in > > nouveau_dp.c > > > > d6d94517b732 drm/nouveau/kms/nv50-: Remove open-coded drm_dp_read_desc() > > > > 7429fbed7536 drm/nouveau/kms: Fix some indenting in nouveau_dp_detect() > > > > > > > -- > Cheers, > Lyude Paul (she/her) > Software Engineer at Red Hat
To start off: this patch series is less work to review then it looks - most (but not all) of the nouveau related work has already been reviewed elsewhere. Most of the reason I'm asking for an RFC here is because this code pulls a lot of code out of i915 and into shared DP helpers. Anyway-nouveau's HPD related code has been collecting dust for a while. Other then the occasional runtime PM related and MST related fixes, we're missing a lot of nice things that have been added to DRM since this was originally written. Additionally, the code is just really unoptimized in general: * We handle connector probing in the same context that we handle short IRQs in for DP, which means connector probing could potentially block ESI handling for MST * When we receive a hotplug event from a connector, we reprobe every single connector instead of just the connector that was hotplugged * Additionally because of the above reason, combined with the fact I had the bad idea of reusing some of the MST locks when I last rewrote nouveau's DP MST detection, trying to handle any other events that require short HPD IRQs is a bit awkward to actually implement. * We don't actually properly check whether EDIDs change or not when reprobing, which means we basically send out a hotplug event every single time we receive one even if nothing has changed Additionally, the code for handling DP that we have in nouveau is also quite unoptimized in general, doesn't use a lot of helpers that have been added since it was written, and is also missing quite a number of features: * Downstream port capability probing * Extended DPRX cap parsing * SINK_COUNT handling for hpd on dongles Luckily for us - all of these are implemented in i915 already. Since there's no reason for us to reinvent the wheel, and having more shared code is always nice, I decided to take the opportunity to extract the code for all of these features from i915 into a set of core DP helpers, which both i915 and nouveau (and hopefully other drivers in the future) can use. As well, this patch series also addesses the other general connector probing related issues I mentioned above, along with rewriting how we handle MST probing so we don't hit any surprise locking design issues in the future. As a note - most of this work is motivated by the fact that I'm planning on adding max_bpc/output_bpc prop support, DSC support (for both MST and SST, along with proper helpers for handling bandwidth limitations and DSC), and fallback link retraining. I figured I might as clean this code up and implement missing DP features like the ones mentioned here before moving on to those tasks. Also, I'm intending for this patch series to get merged through drm-misc-next. Once that happens, upstream maintainers will probably have an easier time with merging if they pull the pending patches for nouveau from Ben's tree before merging drm-misc-next. Lyude Paul (20): drm/nouveau/kms: Fix some indenting in nouveau_dp_detect() drm/nouveau/kms/nv50-: Remove open-coded drm_dp_read_desc() drm/nouveau/kms/nv50-: Just use drm_dp_dpcd_read() in nouveau_dp.c drm/nouveau/kms/nv50-: Use macros for DP registers in nouveau_dp.c drm/nouveau/kms: Don't clear DP_MST_CTRL DPCD in nv50_mstm_new() drm/nouveau/kms: Search for encoders' connectors properly drm/nouveau/kms/nv50-: Use drm_dp_dpcd_(readb|writeb)() in nv50_sor_disable() drm/nouveau/kms/nv50-: Refactor and cleanup DP HPD handling drm/i915/dp: Extract drm_dp_has_mst() drm/nouveau/kms: Use new drm_dp_has_mst() helper for checking MST caps drm/nouveau/kms: Move drm_dp_cec_unset_edid() into nouveau_connector_detect() drm/nouveau/kms: Only use hpd_work for reprobing in HPD paths drm/i915/dp: Extract drm_dp_downstream_read_info() drm/nouveau/kms/nv50-: Use downstream DP clock limits for mode validation drm/i915/dp: Extract drm_dp_has_sink_count() drm/i915/dp: Extract drm_dp_get_sink_count() drm/nouveau/kms/nv50-: Add support for DP_SINK_COUNT drm/nouveau/kms: Don't change EDID when it hasn't actually changed drm/i915/dp: Extract drm_dp_read_dpcd_caps() drm/nouveau/kms: Start using drm_dp_read_dpcd_caps() drivers/gpu/drm/drm_dp_helper.c | 150 ++++++++++ drivers/gpu/drm/i915/display/intel_dp.c | 130 ++------- drivers/gpu/drm/i915/display/intel_dp.h | 1 - drivers/gpu/drm/i915/display/intel_lspcon.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/dac.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/dfp.c | 7 +- drivers/gpu/drm/nouveau/dispnv04/disp.c | 24 +- drivers/gpu/drm/nouveau/dispnv04/disp.h | 4 + drivers/gpu/drm/nouveau/dispnv04/tvnv04.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 308 +++++++++++--------- drivers/gpu/drm/nouveau/nouveau_connector.c | 132 ++++----- drivers/gpu/drm/nouveau/nouveau_connector.h | 1 + drivers/gpu/drm/nouveau/nouveau_display.c | 72 ++++- drivers/gpu/drm/nouveau/nouveau_display.h | 3 +- drivers/gpu/drm/nouveau/nouveau_dp.c | 201 ++++++++++--- drivers/gpu/drm/nouveau/nouveau_drm.c | 4 +- drivers/gpu/drm/nouveau/nouveau_drv.h | 2 + drivers/gpu/drm/nouveau/nouveau_encoder.h | 48 ++- include/drm/drm_dp_helper.h | 15 +- include/drm/drm_dp_mst_helper.h | 22 ++ 21 files changed, 741 insertions(+), 391 deletions(-)