* [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" @ 2020-11-27 14:57 Chris Wilson 2020-11-27 15:25 ` Jani Nikula ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Chris Wilson @ 2020-11-27 14:57 UTC (permalink / raw) To: intel-gfx; +Cc: Lucas De Marchi, Chris Wilson We now use ilk_hpd_irq_setup for all GMCH platforms that do not have hotplug. These are early gen3 and gen2 devices that now explode on boot as they try to access non-existent registers. Fixes: 794d61a19090 ("drm/i915: re-order if/else ladder for hpd_irq_setup") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: José Roberto de Souza <jose.souza@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index dc6febc63f1c..c80eeac53952 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -4242,18 +4242,21 @@ void intel_irq_init(struct drm_i915_private *dev_priv) */ dev_priv->hotplug.hpd_short_storm_enabled = !HAS_DP_MST(dev_priv); - if (HAS_PCH_DG1(dev_priv)) - dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; - else if (INTEL_GEN(dev_priv) >= 11) - dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; - else if (IS_GEN9_LP(dev_priv)) - dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; - else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) - dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; - else if (HAS_GMCH(dev_priv) && I915_HAS_HOTPLUG(dev_priv)) - dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; - else - dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; + if (HAS_GMCH(dev_priv)) { + if (I915_HAS_HOTPLUG(dev_priv)) + dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; + } else { + if (HAS_PCH_DG1(dev_priv)) + dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; + else if (INTEL_GEN(dev_priv) >= 11) + dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; + else if (IS_GEN9_LP(dev_priv)) + dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) + dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; + else + dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; + } } /** -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" 2020-11-27 14:57 [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" Chris Wilson @ 2020-11-27 15:25 ` Jani Nikula 2020-11-27 18:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Jani Nikula @ 2020-11-27 15:25 UTC (permalink / raw) To: Chris Wilson, intel-gfx; +Cc: Lucas De Marchi, Chris Wilson On Fri, 27 Nov 2020, Chris Wilson <chris@chris-wilson.co.uk> wrote: > We now use ilk_hpd_irq_setup for all GMCH platforms that do not have > hotplug. These are early gen3 and gen2 devices that now explode on boot > as they try to access non-existent registers. > > Fixes: 794d61a19090 ("drm/i915: re-order if/else ladder for hpd_irq_setup") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: José Roberto de Souza <jose.souza@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++++++++------------ > 1 file changed, 15 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index dc6febc63f1c..c80eeac53952 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -4242,18 +4242,21 @@ void intel_irq_init(struct drm_i915_private *dev_priv) > */ > dev_priv->hotplug.hpd_short_storm_enabled = !HAS_DP_MST(dev_priv); > > - if (HAS_PCH_DG1(dev_priv)) > - dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; > - else if (INTEL_GEN(dev_priv) >= 11) > - dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; > - else if (IS_GEN9_LP(dev_priv)) > - dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; > - else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) > - dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; > - else if (HAS_GMCH(dev_priv) && I915_HAS_HOTPLUG(dev_priv)) > - dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; > - else > - dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; > + if (HAS_GMCH(dev_priv)) { > + if (I915_HAS_HOTPLUG(dev_priv)) > + dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; > + } else { > + if (HAS_PCH_DG1(dev_priv)) > + dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; > + else if (INTEL_GEN(dev_priv) >= 11) > + dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; > + else if (IS_GEN9_LP(dev_priv)) > + dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; > + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) > + dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; > + else > + dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; > + } > } > > /** -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" 2020-11-27 14:57 [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" Chris Wilson 2020-11-27 15:25 ` Jani Nikula @ 2020-11-27 18:40 ` Patchwork 2020-11-27 19:50 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2020-11-28 4:52 ` [Intel-gfx] [PATCH] " Lucas De Marchi 3 siblings, 0 replies; 10+ messages in thread From: Patchwork @ 2020-11-27 18:40 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx [-- Attachment #1.1: Type: text/plain, Size: 3787 bytes --] == Series Details == Series: Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" URL : https://patchwork.freedesktop.org/series/84352/ State : success == Summary == CI Bug Log - changes from CI_DRM_9399 -> Patchwork_19003 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/index.html New tests --------- New tests have been introduced between CI_DRM_9399 and Patchwork_19003: ### New CI tests (1) ### * boot: - Statuses : 40 pass(s) - Exec time: [0.0] s Known issues ------------ Here are the changes found in Patchwork_19003 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_mmap_gtt@basic: - fi-tgl-y: [PASS][1] -> [DMESG-WARN][2] ([i915#402]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/fi-tgl-y/igt@gem_mmap_gtt@basic.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/fi-tgl-y/igt@gem_mmap_gtt@basic.html * igt@kms_busy@basic@flip: - fi-tgl-y: [PASS][3] -> [DMESG-WARN][4] ([i915#1982]) +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/fi-tgl-y/igt@kms_busy@basic@flip.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/fi-tgl-y/igt@kms_busy@basic@flip.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - fi-byt-j1900: [PASS][5] -> [DMESG-WARN][6] ([i915#1982]) +1 similar issue [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html #### Possible fixes #### * igt@gem_flink_basic@bad-open: - fi-tgl-y: [DMESG-WARN][7] ([i915#402]) -> [PASS][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/fi-tgl-y/igt@gem_flink_basic@bad-open.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/fi-tgl-y/igt@gem_flink_basic@bad-open.html * igt@i915_selftest@live@gt_heartbeat: - fi-kbl-soraka: [DMESG-FAIL][9] ([i915#541]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html * igt@kms_busy@basic@flip: - fi-kbl-soraka: [DMESG-WARN][11] ([i915#1982]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/fi-kbl-soraka/igt@kms_busy@basic@flip.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/fi-kbl-soraka/igt@kms_busy@basic@flip.html [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402 [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541 Participating hosts (45 -> 40) ------------------------------ Missing (5): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus Build changes ------------- * Linux: CI_DRM_9399 -> Patchwork_19003 CI-20190529: 20190529 CI_DRM_9399: b3d976d7d7851bcfcd64b5d6c29672a9e2462442 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5873: b6321b58dcaa41ba1d28aced42d6b15dc3d49ca2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_19003: 46cf953013d1c2629d31da6a063523a5263fee94 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 46cf953013d1 Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/index.html [-- Attachment #1.2: Type: text/html, Size: 4778 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] 10+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" 2020-11-27 14:57 [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" Chris Wilson 2020-11-27 15:25 ` Jani Nikula 2020-11-27 18:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork @ 2020-11-27 19:50 ` Patchwork 2020-11-28 4:52 ` [Intel-gfx] [PATCH] " Lucas De Marchi 3 siblings, 0 replies; 10+ messages in thread From: Patchwork @ 2020-11-27 19:50 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx [-- Attachment #1.1: Type: text/plain, Size: 15101 bytes --] == Series Details == Series: Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" URL : https://patchwork.freedesktop.org/series/84352/ State : failure == Summary == CI Bug Log - changes from CI_DRM_9399_full -> Patchwork_19003_full ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_19003_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_19003_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_19003_full: ### IGT changes ### #### Possible regressions #### * igt@i915_pm_sseu@full-enable: - shard-skl: [PASS][1] -> [FAIL][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl5/igt@i915_pm_sseu@full-enable.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl9/igt@i915_pm_sseu@full-enable.html New tests --------- New tests have been introduced between CI_DRM_9399_full and Patchwork_19003_full: ### New CI tests (1) ### * boot: - Statuses : 199 pass(s) - Exec time: [0.0] s Known issues ------------ Here are the changes found in Patchwork_19003_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@feature_discovery@psr2: - shard-iclb: [PASS][3] -> [SKIP][4] ([i915#658]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-iclb2/igt@feature_discovery@psr2.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-iclb1/igt@feature_discovery@psr2.html * igt@gem_exec_whisper@basic-fds-forked: - shard-glk: [PASS][5] -> [DMESG-WARN][6] ([i915#118] / [i915#95]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-glk6/igt@gem_exec_whisper@basic-fds-forked.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-glk5/igt@gem_exec_whisper@basic-fds-forked.html * igt@gem_exec_whisper@basic-queues-priority-all: - shard-iclb: [PASS][7] -> [INCOMPLETE][8] ([i915#1895]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-iclb4/igt@gem_exec_whisper@basic-queues-priority-all.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-iclb6/igt@gem_exec_whisper@basic-queues-priority-all.html * igt@i915_suspend@sysfs-reader: - shard-skl: [PASS][9] -> [INCOMPLETE][10] ([i915#198]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl8/igt@i915_suspend@sysfs-reader.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl5/igt@i915_suspend@sysfs-reader.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: - shard-apl: [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) +3 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-apl8/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-apl7/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html * igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic: - shard-tglb: [PASS][13] -> [FAIL][14] ([i915#2346]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-tglb6/igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-tglb6/igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic.html * igt@kms_flip@basic-flip-vs-modeset@a-edp1: - shard-skl: [PASS][15] -> [DMESG-WARN][16] ([i915#1982]) +3 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1: - shard-skl: [PASS][17] -> [FAIL][18] ([i915#79]) +1 similar issue [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-render: - shard-tglb: [PASS][19] -> [DMESG-WARN][20] ([i915#1982]) +1 similar issue [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-render.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-render.html * igt@kms_psr@psr2_no_drrs: - shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109441]) +2 similar issues [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-iclb2/igt@kms_psr@psr2_no_drrs.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-iclb4/igt@kms_psr@psr2_no_drrs.html * igt@perf_pmu@module-unload: - shard-apl: [PASS][23] -> [DMESG-WARN][24] ([i915#1982] / [i915#262]) [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-apl2/igt@perf_pmu@module-unload.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-apl6/igt@perf_pmu@module-unload.html - shard-iclb: [PASS][25] -> [DMESG-WARN][26] ([i915#1982] / [i915#262]) [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-iclb3/igt@perf_pmu@module-unload.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-iclb3/igt@perf_pmu@module-unload.html #### Possible fixes #### * igt@gem_eio@in-flight-suspend: - shard-skl: [DMESG-WARN][27] ([i915#1037] / [i915#1982]) -> [PASS][28] [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl9/igt@gem_eio@in-flight-suspend.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl1/igt@gem_eio@in-flight-suspend.html * igt@gem_exec_gttfill@engines@rcs0: - shard-glk: [DMESG-WARN][29] ([i915#118] / [i915#95]) -> [PASS][30] [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-glk9/igt@gem_exec_gttfill@engines@rcs0.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-glk8/igt@gem_exec_gttfill@engines@rcs0.html * igt@kms_cursor_crc@pipe-b-cursor-64x21-offscreen: - shard-skl: [FAIL][31] ([i915#54]) -> [PASS][32] +3 similar issues [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl5/igt@kms_cursor_crc@pipe-b-cursor-64x21-offscreen.html [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl7/igt@kms_cursor_crc@pipe-b-cursor-64x21-offscreen.html * igt@kms_cursor_edge_walk@pipe-c-256x256-bottom-edge: - shard-tglb: [DMESG-WARN][33] ([i915#1982]) -> [PASS][34] [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-tglb8/igt@kms_cursor_edge_walk@pipe-c-256x256-bottom-edge.html [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-tglb2/igt@kms_cursor_edge_walk@pipe-c-256x256-bottom-edge.html * igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-xtiled: - shard-skl: [FAIL][35] ([i915#52] / [i915#54]) -> [PASS][36] +1 similar issue [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl7/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-xtiled.html [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl4/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-xtiled.html * igt@kms_flip@flip-vs-expired-vblank@a-edp1: - shard-tglb: [FAIL][37] ([i915#2598]) -> [PASS][38] [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-tglb8/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-tglb2/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html * igt@kms_flip@plain-flip-fb-recreate-interruptible@a-edp1: - shard-skl: [FAIL][39] ([i915#2122]) -> [PASS][40] [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl3/igt@kms_flip@plain-flip-fb-recreate-interruptible@a-edp1.html [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl3/igt@kms_flip@plain-flip-fb-recreate-interruptible@a-edp1.html * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-gtt: - shard-skl: [DMESG-WARN][41] ([i915#1982]) -> [PASS][42] [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl10/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-gtt.html [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl6/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-gtt.html * igt@kms_hdr@bpc-switch-suspend: - shard-skl: [FAIL][43] ([i915#1188]) -> [PASS][44] [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl7/igt@kms_hdr@bpc-switch-suspend.html [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl7/igt@kms_hdr@bpc-switch-suspend.html * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: - shard-skl: [FAIL][45] ([fdo#108145] / [i915#265]) -> [PASS][46] +2 similar issues [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html * igt@kms_psr2_su@frontbuffer: - shard-iclb: [SKIP][47] ([fdo#109642] / [fdo#111068]) -> [PASS][48] [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-iclb1/igt@kms_psr2_su@frontbuffer.html [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-iclb2/igt@kms_psr2_su@frontbuffer.html * igt@kms_psr@psr2_cursor_mmap_cpu: - shard-iclb: [SKIP][49] ([fdo#109441]) -> [PASS][50] +2 similar issues [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-iclb1/igt@kms_psr@psr2_cursor_mmap_cpu.html [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html #### Warnings #### * igt@i915_pm_rc6_residency@rc6-fence: - shard-iclb: [WARN][51] ([i915#1804] / [i915#2684]) -> [WARN][52] ([i915#2684]) +1 similar issue [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-iclb7/igt@i915_pm_rc6_residency@rc6-fence.html [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-iclb5/igt@i915_pm_rc6_residency@rc6-fence.html * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: - shard-glk: [INCOMPLETE][53] ([i915#2635]) -> [DMESG-WARN][54] ([i915#2635]) [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-glk2/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-glk4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html * igt@runner@aborted: - shard-glk: ([FAIL][55], [FAIL][56]) ([i915#1814] / [i915#2295] / [i915#2722] / [i915#483] / [k.org#202321]) -> ([FAIL][57], [FAIL][58]) ([i915#1814] / [i915#2295] / [i915#2722] / [k.org#202321]) [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-glk2/igt@runner@aborted.html [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-glk3/igt@runner@aborted.html [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-glk4/igt@runner@aborted.html [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-glk2/igt@runner@aborted.html - shard-skl: [FAIL][59] ([i915#2295] / [i915#2722] / [i915#483]) -> ([FAIL][60], [FAIL][61]) ([i915#2295] / [i915#2722]) [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9399/shard-skl7/igt@runner@aborted.html [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl8/igt@runner@aborted.html [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19003/shard-skl4/igt@runner@aborted.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 [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642 [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068 [i915#1037]: https://gitlab.freedesktop.org/drm/intel/issues/1037 [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118 [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188 [i915#1804]: https://gitlab.freedesktop.org/drm/intel/issues/1804 [i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814 [i915#1895]: https://gitlab.freedesktop.org/drm/intel/issues/1895 [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#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295 [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346 [i915#2369]: https://gitlab.freedesktop.org/drm/intel/issues/2369 [i915#2598]: https://gitlab.freedesktop.org/drm/intel/issues/2598 [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262 [i915#2635]: https://gitlab.freedesktop.org/drm/intel/issues/2635 [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265 [i915#2684]: https://gitlab.freedesktop.org/drm/intel/issues/2684 [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722 [i915#483]: https://gitlab.freedesktop.org/drm/intel/issues/483 [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52 [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54 [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658 [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79 [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95 [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321 Participating hosts (10 -> 10) ------------------------------ No changes in participating hosts Build changes ------------- * Linux: CI_DRM_9399 -> Patchwork_19003 CI-20190529: 20190529 CI_DRM_9399: b3d976d7d7851bcfcd64b5d6c29672a9e2462442 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5873: b6321b58dcaa41ba1d28aced42d6b15dc3d49ca2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_19003: 46cf953013d1c2629d31da6a063523a5263fee94 @ 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_19003/index.html [-- Attachment #1.2: Type: text/html, Size: 18132 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] 10+ messages in thread
* Re: [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" 2020-11-27 14:57 [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" Chris Wilson ` (2 preceding siblings ...) 2020-11-27 19:50 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork @ 2020-11-28 4:52 ` Lucas De Marchi 2020-11-30 14:19 ` Ville Syrjälä 3 siblings, 1 reply; 10+ messages in thread From: Lucas De Marchi @ 2020-11-28 4:52 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Fri, Nov 27, 2020 at 02:57:48PM +0000, Chris Wilson wrote: >We now use ilk_hpd_irq_setup for all GMCH platforms that do not have >hotplug. These are early gen3 and gen2 devices that now explode on boot >as they try to access non-existent registers. humn... true, my bad. But I don't think a revert is the right fix. It would be much better if we would not be setting up the hpd setup function at all for platforms that do not have hotplug. I think a separate early check for I915_HAS_HOTPLUG() would be deserved. Lucas De Marchi > >Fixes: 794d61a19090 ("drm/i915: re-order if/else ladder for hpd_irq_setup") >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >Cc: José Roberto de Souza <jose.souza@intel.com> >Cc: Jani Nikula <jani.nikula@linux.intel.com> >--- > drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++++++++------------ > 1 file changed, 15 insertions(+), 12 deletions(-) > >diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >index dc6febc63f1c..c80eeac53952 100644 >--- a/drivers/gpu/drm/i915/i915_irq.c >+++ b/drivers/gpu/drm/i915/i915_irq.c >@@ -4242,18 +4242,21 @@ void intel_irq_init(struct drm_i915_private *dev_priv) > */ > dev_priv->hotplug.hpd_short_storm_enabled = !HAS_DP_MST(dev_priv); > >- if (HAS_PCH_DG1(dev_priv)) >- dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; >- else if (INTEL_GEN(dev_priv) >= 11) >- dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; >- else if (IS_GEN9_LP(dev_priv)) >- dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; >- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) >- dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; >- else if (HAS_GMCH(dev_priv) && I915_HAS_HOTPLUG(dev_priv)) >- dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; >- else >- dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; >+ if (HAS_GMCH(dev_priv)) { >+ if (I915_HAS_HOTPLUG(dev_priv)) >+ dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; >+ } else { >+ if (HAS_PCH_DG1(dev_priv)) >+ dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; >+ else if (INTEL_GEN(dev_priv) >= 11) >+ dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; >+ else if (IS_GEN9_LP(dev_priv)) >+ dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; >+ else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) >+ dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; >+ else >+ dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; >+ } > } > > /** >-- >2.20.1 > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" 2020-11-28 4:52 ` [Intel-gfx] [PATCH] " Lucas De Marchi @ 2020-11-30 14:19 ` Ville Syrjälä 2020-11-30 17:31 ` Lucas De Marchi 0 siblings, 1 reply; 10+ messages in thread From: Ville Syrjälä @ 2020-11-30 14:19 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx, Chris Wilson On Fri, Nov 27, 2020 at 08:52:29PM -0800, Lucas De Marchi wrote: > On Fri, Nov 27, 2020 at 02:57:48PM +0000, Chris Wilson wrote: > >We now use ilk_hpd_irq_setup for all GMCH platforms that do not have > >hotplug. These are early gen3 and gen2 devices that now explode on boot > >as they try to access non-existent registers. > > humn... true, my bad. But I don't think a revert is the right fix. It > would be much better if we would not be setting up the hpd setup > function at all for platforms that do not have hotplug. I think a > separate early check for I915_HAS_HOTPLUG() would be deserved. I think it generally leads to much less convoluted logic when we keep gmch vs. rest separate. So I'm confused as to what we're even trying to achieve here? > > Lucas De Marchi > > > > >Fixes: 794d61a19090 ("drm/i915: re-order if/else ladder for hpd_irq_setup") > >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >Cc: Lucas De Marchi <lucas.demarchi@intel.com> > >Cc: José Roberto de Souza <jose.souza@intel.com> > >Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > >--- > > drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++++++++------------ > > 1 file changed, 15 insertions(+), 12 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > >index dc6febc63f1c..c80eeac53952 100644 > >--- a/drivers/gpu/drm/i915/i915_irq.c > >+++ b/drivers/gpu/drm/i915/i915_irq.c > >@@ -4242,18 +4242,21 @@ void intel_irq_init(struct drm_i915_private *dev_priv) > > */ > > dev_priv->hotplug.hpd_short_storm_enabled = !HAS_DP_MST(dev_priv); > > > >- if (HAS_PCH_DG1(dev_priv)) > >- dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; > >- else if (INTEL_GEN(dev_priv) >= 11) > >- dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; > >- else if (IS_GEN9_LP(dev_priv)) > >- dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; > >- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) > >- dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; > >- else if (HAS_GMCH(dev_priv) && I915_HAS_HOTPLUG(dev_priv)) > >- dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; > >- else > >- dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; > >+ if (HAS_GMCH(dev_priv)) { > >+ if (I915_HAS_HOTPLUG(dev_priv)) > >+ dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; > >+ } else { > >+ if (HAS_PCH_DG1(dev_priv)) > >+ dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; > >+ else if (INTEL_GEN(dev_priv) >= 11) > >+ dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; > >+ else if (IS_GEN9_LP(dev_priv)) > >+ dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; > >+ else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) > >+ dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; > >+ else > >+ dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; > >+ } > > } > > > > /** > >-- > >2.20.1 > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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] 10+ messages in thread
* Re: [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" 2020-11-30 14:19 ` Ville Syrjälä @ 2020-11-30 17:31 ` Lucas De Marchi 2020-11-30 17:46 ` Ville Syrjälä 0 siblings, 1 reply; 10+ messages in thread From: Lucas De Marchi @ 2020-11-30 17:31 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, Chris Wilson On Mon, Nov 30, 2020 at 04:19:54PM +0200, Ville Syrjälä wrote: >On Fri, Nov 27, 2020 at 08:52:29PM -0800, Lucas De Marchi wrote: >> On Fri, Nov 27, 2020 at 02:57:48PM +0000, Chris Wilson wrote: >> >We now use ilk_hpd_irq_setup for all GMCH platforms that do not have >> >hotplug. These are early gen3 and gen2 devices that now explode on boot >> >as they try to access non-existent registers. >> >> humn... true, my bad. But I don't think a revert is the right fix. It >> would be much better if we would not be setting up the hpd setup >> function at all for platforms that do not have hotplug. I think a >> separate early check for I915_HAS_HOTPLUG() would be deserved. > >I think it generally leads to much less convoluted logic when we keep >gmch vs. rest separate. So I'm confused as to what we're even trying >to achieve here? 1) Stop trying to setup hotplug in a platform that doesn't have hotplug was the main focus. Later it would be better to move some of these hotplug to display/ as they are clearly display related and account for a great portion of i915_irq.c. I left the I915_HAS_HOTPLUG() in the middle by mistake, it should had been an earlier call. 2) semi-related is the move of GMCH to the middle and I guess this is what you're complaining here. I find it's cumbersome to have it separate as we go and extend these checks for newer platforms. Almost everywhere we settled on having last platform first in the if/else ladders - this makes it much more clear on how/where to add a new platform. Lucas De Marchi > >> >> Lucas De Marchi >> >> > >> >Fixes: 794d61a19090 ("drm/i915: re-order if/else ladder for hpd_irq_setup") >> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> >Cc: José Roberto de Souza <jose.souza@intel.com> >> >Cc: Jani Nikula <jani.nikula@linux.intel.com> >> >> >> >--- >> > drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++++++++------------ >> > 1 file changed, 15 insertions(+), 12 deletions(-) >> > >> >diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> >index dc6febc63f1c..c80eeac53952 100644 >> >--- a/drivers/gpu/drm/i915/i915_irq.c >> >+++ b/drivers/gpu/drm/i915/i915_irq.c >> >@@ -4242,18 +4242,21 @@ void intel_irq_init(struct drm_i915_private *dev_priv) >> > */ >> > dev_priv->hotplug.hpd_short_storm_enabled = !HAS_DP_MST(dev_priv); >> > >> >- if (HAS_PCH_DG1(dev_priv)) >> >- dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; >> >- else if (INTEL_GEN(dev_priv) >= 11) >> >- dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; >> >- else if (IS_GEN9_LP(dev_priv)) >> >- dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; >> >- else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) >> >- dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; >> >- else if (HAS_GMCH(dev_priv) && I915_HAS_HOTPLUG(dev_priv)) >> >- dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; >> >- else >> >- dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; >> >+ if (HAS_GMCH(dev_priv)) { >> >+ if (I915_HAS_HOTPLUG(dev_priv)) >> >+ dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup; >> >+ } else { >> >+ if (HAS_PCH_DG1(dev_priv)) >> >+ dev_priv->display.hpd_irq_setup = dg1_hpd_irq_setup; >> >+ else if (INTEL_GEN(dev_priv) >= 11) >> >+ dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup; >> >+ else if (IS_GEN9_LP(dev_priv)) >> >+ dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup; >> >+ else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT) >> >+ dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup; >> >+ else >> >+ dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup; >> >+ } >> > } >> > >> > /** >> >-- >> >2.20.1 >> > >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >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] 10+ messages in thread
* Re: [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" 2020-11-30 17:31 ` Lucas De Marchi @ 2020-11-30 17:46 ` Ville Syrjälä 2020-12-01 5:46 ` Lucas De Marchi 0 siblings, 1 reply; 10+ messages in thread From: Ville Syrjälä @ 2020-11-30 17:46 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx, Chris Wilson On Mon, Nov 30, 2020 at 09:31:04AM -0800, Lucas De Marchi wrote: > On Mon, Nov 30, 2020 at 04:19:54PM +0200, Ville Syrjälä wrote: > >On Fri, Nov 27, 2020 at 08:52:29PM -0800, Lucas De Marchi wrote: > >> On Fri, Nov 27, 2020 at 02:57:48PM +0000, Chris Wilson wrote: > >> >We now use ilk_hpd_irq_setup for all GMCH platforms that do not have > >> >hotplug. These are early gen3 and gen2 devices that now explode on boot > >> >as they try to access non-existent registers. > >> > >> humn... true, my bad. But I don't think a revert is the right fix. It > >> would be much better if we would not be setting up the hpd setup > >> function at all for platforms that do not have hotplug. I think a > >> separate early check for I915_HAS_HOTPLUG() would be deserved. > > > >I think it generally leads to much less convoluted logic when we keep > >gmch vs. rest separate. So I'm confused as to what we're even trying > >to achieve here? > > 1) Stop trying to setup hotplug in a platform that doesn't have hotplug > was the main focus. Later it would be better to move some of these > hotplug to display/ as they are clearly display related and account for > a great portion of i915_irq.c. > > I left the I915_HAS_HOTPLUG() in the middle by > mistake, it should had been an earlier call. > > 2) semi-related is the move of GMCH to the middle and I guess this is > what you're complaining here. I find it's cumbersome to have it > separate as we go and extend these checks for newer platforms. Almost > everywhere we settled on having last platform first in the if/else > ladders - this makes it much more clear on how/where to add a new > platform. You never touch the gmch path for new platforms. What could be more clear than that? -- 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] 10+ messages in thread
* Re: [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" 2020-11-30 17:46 ` Ville Syrjälä @ 2020-12-01 5:46 ` Lucas De Marchi 2020-12-01 17:25 ` Ville Syrjälä 0 siblings, 1 reply; 10+ messages in thread From: Lucas De Marchi @ 2020-12-01 5:46 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, Chris Wilson On Mon, Nov 30, 2020 at 07:46:39PM +0200, Ville Syrjälä wrote: >On Mon, Nov 30, 2020 at 09:31:04AM -0800, Lucas De Marchi wrote: >> On Mon, Nov 30, 2020 at 04:19:54PM +0200, Ville Syrjälä wrote: >> >On Fri, Nov 27, 2020 at 08:52:29PM -0800, Lucas De Marchi wrote: >> >> On Fri, Nov 27, 2020 at 02:57:48PM +0000, Chris Wilson wrote: >> >> >We now use ilk_hpd_irq_setup for all GMCH platforms that do not have >> >> >hotplug. These are early gen3 and gen2 devices that now explode on boot >> >> >as they try to access non-existent registers. >> >> >> >> humn... true, my bad. But I don't think a revert is the right fix. It >> >> would be much better if we would not be setting up the hpd setup >> >> function at all for platforms that do not have hotplug. I think a >> >> separate early check for I915_HAS_HOTPLUG() would be deserved. >> > >> >I think it generally leads to much less convoluted logic when we keep >> >gmch vs. rest separate. So I'm confused as to what we're even trying >> >to achieve here? >> >> 1) Stop trying to setup hotplug in a platform that doesn't have hotplug >> was the main focus. Later it would be better to move some of these >> hotplug to display/ as they are clearly display related and account for >> a great portion of i915_irq.c. >> >> I left the I915_HAS_HOTPLUG() in the middle by >> mistake, it should had been an earlier call. >> >> 2) semi-related is the move of GMCH to the middle and I guess this is >> what you're complaining here. I find it's cumbersome to have it >> separate as we go and extend these checks for newer platforms. Almost >> everywhere we settled on having last platform first in the if/else >> ladders - this makes it much more clear on how/where to add a new >> platform. > >You never touch the gmch path for new platforms. What could be more >clear than that? the second level branch mixing the code path for new and old platform instead of following the convention we settled on. But I'm ok with moving it back as a HAS_* check in the middle of GEN_* check is proving controversial. Lucas De Marchi > >-- >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] 10+ messages in thread
* Re: [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" 2020-12-01 5:46 ` Lucas De Marchi @ 2020-12-01 17:25 ` Ville Syrjälä 0 siblings, 0 replies; 10+ messages in thread From: Ville Syrjälä @ 2020-12-01 17:25 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx, Chris Wilson On Mon, Nov 30, 2020 at 09:46:18PM -0800, Lucas De Marchi wrote: > On Mon, Nov 30, 2020 at 07:46:39PM +0200, Ville Syrjälä wrote: > >On Mon, Nov 30, 2020 at 09:31:04AM -0800, Lucas De Marchi wrote: > >> On Mon, Nov 30, 2020 at 04:19:54PM +0200, Ville Syrjälä wrote: > >> >On Fri, Nov 27, 2020 at 08:52:29PM -0800, Lucas De Marchi wrote: > >> >> On Fri, Nov 27, 2020 at 02:57:48PM +0000, Chris Wilson wrote: > >> >> >We now use ilk_hpd_irq_setup for all GMCH platforms that do not have > >> >> >hotplug. These are early gen3 and gen2 devices that now explode on boot > >> >> >as they try to access non-existent registers. > >> >> > >> >> humn... true, my bad. But I don't think a revert is the right fix. It > >> >> would be much better if we would not be setting up the hpd setup > >> >> function at all for platforms that do not have hotplug. I think a > >> >> separate early check for I915_HAS_HOTPLUG() would be deserved. > >> > > >> >I think it generally leads to much less convoluted logic when we keep > >> >gmch vs. rest separate. So I'm confused as to what we're even trying > >> >to achieve here? > >> > >> 1) Stop trying to setup hotplug in a platform that doesn't have hotplug > >> was the main focus. Later it would be better to move some of these > >> hotplug to display/ as they are clearly display related and account for > >> a great portion of i915_irq.c. > >> > >> I left the I915_HAS_HOTPLUG() in the middle by > >> mistake, it should had been an earlier call. > >> > >> 2) semi-related is the move of GMCH to the middle and I guess this is > >> what you're complaining here. I find it's cumbersome to have it > >> separate as we go and extend these checks for newer platforms. Almost > >> everywhere we settled on having last platform first in the if/else > >> ladders - this makes it much more clear on how/where to add a new > >> platform. > > > >You never touch the gmch path for new platforms. What could be more > >clear than that? > > the second level branch mixing the code path for new and old platform > instead of following the convention we settled on. The convention I care about most is "clear code" which in many cases is best achieved with gmch vs. not split. > But I'm ok with > moving it back as a HAS_* check in the middle of GEN_* check is proving > controversial. > > Lucas De Marchi > > > > >-- > >Ville Syrjälä > >Intel -- 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] 10+ messages in thread
end of thread, other threads:[~2020-12-01 17:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-27 14:57 [Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup" Chris Wilson 2020-11-27 15:25 ` Jani Nikula 2020-11-27 18:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork 2020-11-27 19:50 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2020-11-28 4:52 ` [Intel-gfx] [PATCH] " Lucas De Marchi 2020-11-30 14:19 ` Ville Syrjälä 2020-11-30 17:31 ` Lucas De Marchi 2020-11-30 17:46 ` Ville Syrjälä 2020-12-01 5:46 ` Lucas De Marchi 2020-12-01 17:25 ` Ville Syrjälä
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.