* [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove @ 2019-10-18 10:07 Janusz Krzysztofik 2019-10-18 11:43 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Janusz Krzysztofik @ 2019-10-18 10:07 UTC (permalink / raw) To: intel-gfx; +Cc: Jani Nikula Commit 2d6f6f359fd8 ("drm/i915: add i915_driver_modeset_remove()") claimed removal of asymmetry in probe() and remove() calls, however, it didn't take care of calling intel_irq_uninstall() on driver remove. That doesn't hurt as long as we still call it from intel_modeset_driver_remove() but in order to have full symmetry we should call it again from i915_driver_modeset_remove(). Note that it's safe to call intel_irq_uninstall() twice thanks to commit b318b82455bd ("drm/i915: Nuke drm_driver irq vfuncs"). We may only want to mention the case we are adding in a related FIXME comment provided by that commit. While being at it, update the name of function mentioned as calling it out of sequence as that name has been changed meanwhile by commit 78dae1ac35dd ("drm/i915: Propagate "_remove" function name suffix down"). Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ drivers/gpu/drm/i915/i915_irq.c | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index dd9613e45723..4ae9bfa96290 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -452,6 +452,8 @@ static void i915_driver_modeset_remove(struct drm_i915_private *i915) intel_modeset_driver_remove(i915); + intel_irq_uninstall(i915); + intel_bios_driver_remove(i915); vga_switcheroo_unregister_client(pdev); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index bc83f094065a..0160283860a6 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -4515,10 +4515,10 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) int irq = dev_priv->drm.pdev->irq; /* - * FIXME we can get called twice during driver load - * error handling due to intel_modeset_cleanup() - * calling us out of sequence. Would be nice if - * it didn't do that... + * FIXME we can get called twice during driver probe + * error handling as well as during driver remove due to + * intel_modeset_driver_remove() calling us out of sequence. + * Would be nice if it didn't do that... */ if (!dev_priv->drm.irq_enabled) return; -- 2.21.0 _______________________________________________ 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
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove 2019-10-18 10:07 [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove Janusz Krzysztofik @ 2019-10-18 11:43 ` Patchwork 2019-10-18 11:53 ` Janusz Krzysztofik 2019-10-18 12:13 ` ✗ Fi.CI.BAT: failure " Patchwork ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Patchwork @ 2019-10-18 11:43 UTC (permalink / raw) To: Janusz Krzysztofik; +Cc: intel-gfx == Series Details == Series: drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove URL : https://patchwork.freedesktop.org/series/68188/ State : warning == Summary == $ dim checkpatch origin/drm-tip 586f33cacab5 drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove -:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 78dae1ac35dd ("drm/i915: Propagate "_remove" function name suffix down")' #19: changed meanwhile by commit 78dae1ac35dd ("drm/i915: Propagate total: 1 errors, 0 warnings, 0 checks, 22 lines checked _______________________________________________ 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: ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove 2019-10-18 11:43 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork @ 2019-10-18 11:53 ` Janusz Krzysztofik 0 siblings, 0 replies; 10+ messages in thread From: Janusz Krzysztofik @ 2019-10-18 11:53 UTC (permalink / raw) To: intel-gfx; +Cc: Jani Nikula On Friday, October 18, 2019 1:43:32 PM CEST Patchwork wrote: > == Series Details == > > Series: drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove > URL : https://patchwork.freedesktop.org/series/68188/ > State : warning > > == Summary == > > $ dim checkpatch origin/drm-tip > 586f33cacab5 drm/i915: Restore full symmetry in i915_driver_modeset_probe/ remove > -:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 78dae1ac35dd ("drm/i915: Propagate "_remove" function name suffix down")' > #19: > changed meanwhile by commit 78dae1ac35dd ("drm/i915: Propagate > > total: 1 errors, 0 warnings, 0 checks, 22 lines checked That's a false positive, I believe. checkpatch has problems with quoted commit messages that contain double quotes inside, like 'Propagate "__remove" ...' in this case. Thanks, Janusz _______________________________________________ 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
* ✗ Fi.CI.BAT: failure for drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove 2019-10-18 10:07 [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove Janusz Krzysztofik 2019-10-18 11:43 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork @ 2019-10-18 12:13 ` Patchwork 2019-10-18 13:06 ` Janusz Krzysztofik 2019-10-18 15:52 ` [RFC PATCH] " Michal Wajdeczko 2019-10-21 12:10 ` Jani Nikula 3 siblings, 1 reply; 10+ messages in thread From: Patchwork @ 2019-10-18 12:13 UTC (permalink / raw) To: Janusz Krzysztofik; +Cc: intel-gfx == Series Details == Series: drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove URL : https://patchwork.freedesktop.org/series/68188/ State : failure == Summary == CI Bug Log - changes from CI_DRM_7127 -> Patchwork_14878 ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_14878 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_14878, 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_14878/index.html Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_14878: ### IGT changes ### #### Possible regressions #### * igt@i915_selftest@live_execlists: - fi-kbl-soraka: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-kbl-soraka/igt@i915_selftest@live_execlists.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-kbl-soraka/igt@i915_selftest@live_execlists.html #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@gem_ctx_create@basic: - {fi-tgl-u2}: [PASS][3] -> [INCOMPLETE][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-tgl-u2/igt@gem_ctx_create@basic.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-tgl-u2/igt@gem_ctx_create@basic.html Known issues ------------ Here are the changes found in Patchwork_14878 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_mmap@basic-small-bo: - fi-icl-u3: [PASS][5] -> [DMESG-WARN][6] ([fdo#107724]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-u3/igt@gem_mmap@basic-small-bo.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-u3/igt@gem_mmap@basic-small-bo.html * igt@i915_selftest@live_execlists: - fi-skl-6600u: [PASS][7] -> [INCOMPLETE][8] ([fdo#111934]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-skl-6600u/igt@i915_selftest@live_execlists.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-skl-6600u/igt@i915_selftest@live_execlists.html * igt@i915_selftest@live_gem_contexts: - fi-cfl-8109u: [PASS][9] -> [DMESG-FAIL][10] ([fdo#112050 ]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-cfl-8109u/igt@i915_selftest@live_gem_contexts.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-cfl-8109u/igt@i915_selftest@live_gem_contexts.html * igt@kms_chamelium@hdmi-hpd-fast: - fi-kbl-7500u: [PASS][11] -> [FAIL][12] ([fdo#111045] / [fdo#111096]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html #### Possible fixes #### * igt@gem_ringfill@basic-default-interruptible: - fi-icl-u3: [DMESG-WARN][13] ([fdo#107724]) -> [PASS][14] +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-u3/igt@gem_ringfill@basic-default-interruptible.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-u3/igt@gem_ringfill@basic-default-interruptible.html * igt@i915_selftest@live_execlists: - fi-skl-lmem: [INCOMPLETE][15] ([fdo#111934]) -> [PASS][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-skl-lmem/igt@i915_selftest@live_execlists.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-skl-lmem/igt@i915_selftest@live_execlists.html * igt@i915_selftest@live_hangcheck: - fi-icl-u2: [INCOMPLETE][17] ([fdo#107713] / [fdo#108569]) -> [PASS][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-u2/igt@i915_selftest@live_hangcheck.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-u2/igt@i915_selftest@live_hangcheck.html - {fi-icl-guc}: [INCOMPLETE][19] ([fdo#107713] / [fdo#108569]) -> [PASS][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-guc/igt@i915_selftest@live_hangcheck.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-guc/igt@i915_selftest@live_hangcheck.html - {fi-icl-u4}: [INCOMPLETE][21] ([fdo#107713] / [fdo#108569]) -> [PASS][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-u4/igt@i915_selftest@live_hangcheck.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-u4/igt@i915_selftest@live_hangcheck.html * igt@kms_busy@basic-flip-a: - fi-kbl-soraka: [DMESG-WARN][23] ([fdo#105763]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-kbl-soraka/igt@kms_busy@basic-flip-a.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-kbl-soraka/igt@kms_busy@basic-flip-a.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763 [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713 [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724 [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569 [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045 [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096 [fdo#111934]: https://bugs.freedesktop.org/show_bug.cgi?id=111934 [fdo#112050 ]: https://bugs.freedesktop.org/show_bug.cgi?id=112050 Participating hosts (52 -> 44) ------------------------------ Additional (1): fi-tgl-u Missing (9): fi-ilk-m540 fi-bxt-dsi fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-skl-6260u fi-icl-y fi-byt-clapper fi-bdw-samus Build changes ------------- * CI: CI-20190529 -> None * Linux: CI_DRM_7127 -> Patchwork_14878 CI-20190529: 20190529 CI_DRM_7127: b20b01f8392f688d8d73d4e6dce35a92e5898ad1 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5232: bb5735423eaf6fdbf6b2f94ef0b8520e74eab993 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_14878: 586f33cacab5b8b2a9c4cd4dafcaf3d6608b1648 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 586f33cacab5 drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/index.html _______________________________________________ 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: ✗ Fi.CI.BAT: failure for drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove 2019-10-18 12:13 ` ✗ Fi.CI.BAT: failure " Patchwork @ 2019-10-18 13:06 ` Janusz Krzysztofik 0 siblings, 0 replies; 10+ messages in thread From: Janusz Krzysztofik @ 2019-10-18 13:06 UTC (permalink / raw) To: intel-gfx; +Cc: Jani Nikula On Friday, October 18, 2019 2:13:18 PM CEST Patchwork wrote: > == Series Details == > > Series: drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove > URL : https://patchwork.freedesktop.org/series/68188/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_7127 -> Patchwork_14878 > ==================================================== > > Summary > ------- > > **FAILURE** > > Serious unknown changes coming with Patchwork_14878 absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_14878, 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_14878/index.html > > Possible new issues > ------------------- > > Here are the unknown changes that may have been introduced in Patchwork_14878: > > ### IGT changes ### > > #### Possible regressions #### > > * igt@i915_selftest@live_execlists: > - fi-kbl-soraka: [PASS][1] -> [INCOMPLETE][2] > [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-kbl-soraka/igt@i915_selftest@live_execlists.html > [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-kbl-soraka/igt@i915_selftest@live_execlists.html live_timeslice_queue:590 GEM_BUG_ON(execlists_active(&engine->execlists) != rq) Has nothing to do with the change, I believe. Looks like a sporadic but serious issue, false positive in this case (see below). > #### Suppressed #### > > The following results come from untrusted machines, tests, or statuses. > They do not affect the overall result. > > * igt@gem_ctx_create@basic: > - {fi-tgl-u2}: [PASS][3] -> [INCOMPLETE][4] > [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-tgl-u2/igt@gem_ctx_create@basic.html > [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-tgl-u2/igt@gem_ctx_create@basic.html > > > Known issues > ------------ > > Here are the changes found in Patchwork_14878 that come from known issues: > > ### IGT changes ### > > #### Issues hit #### > > * igt@gem_mmap@basic-small-bo: > - fi-icl-u3: [PASS][5] -> [DMESG-WARN][6] ([fdo#107724]) > [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-u3/igt@gem_mmap@basic-small-bo.html > [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-u3/igt@gem_mmap@basic-small-bo.html > > * igt@i915_selftest@live_execlists: > - fi-skl-6600u: [PASS][7] -> [INCOMPLETE][8] ([fdo#111934]) > [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-skl-6600u/igt@i915_selftest@live_execlists.html > [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-skl-6600u/igt@i915_selftest@live_execlists.html The same BUG, here recognized as a known issue. > * igt@i915_selftest@live_gem_contexts: > - fi-cfl-8109u: [PASS][9] -> [DMESG-FAIL][10] ([fdo#112050 ]) > [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-cfl-8109u/igt@i915_selftest@live_gem_contexts.html > [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-cfl-8109u/igt@i915_selftest@live_gem_contexts.html > > * igt@kms_chamelium@hdmi-hpd-fast: > - fi-kbl-7500u: [PASS][11] -> [FAIL][12] ([fdo#111045] / [fdo#111096]) > [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html > [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html > > > #### Possible fixes #### > > * igt@gem_ringfill@basic-default-interruptible: > - fi-icl-u3: [DMESG-WARN][13] ([fdo#107724]) -> [PASS][14] +1 similar issue > [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-u3/igt@gem_ringfill@basic-default-interruptible.html > [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-u3/igt@gem_ringfill@basic-default-interruptible.html > > * igt@i915_selftest@live_execlists: > - fi-skl-lmem: [INCOMPLETE][15] ([fdo#111934]) -> [PASS][16] > [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-skl-lmem/igt@i915_selftest@live_execlists.html > [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-skl-lmem/igt@i915_selftest@live_execlists.html And here, the patch recognized as a possible fix for still the same BUG. Thanks, Janusz > * igt@i915_selftest@live_hangcheck: > - fi-icl-u2: [INCOMPLETE][17] ([fdo#107713] / [fdo#108569]) -> [PASS][18] > [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-u2/igt@i915_selftest@live_hangcheck.html > [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-u2/igt@i915_selftest@live_hangcheck.html > - {fi-icl-guc}: [INCOMPLETE][19] ([fdo#107713] / [fdo#108569]) -> [PASS][20] > [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-guc/igt@i915_selftest@live_hangcheck.html > [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-guc/igt@i915_selftest@live_hangcheck.html > - {fi-icl-u4}: [INCOMPLETE][21] ([fdo#107713] / [fdo#108569]) -> [PASS][22] > [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-icl-u4/igt@i915_selftest@live_hangcheck.html > [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-icl-u4/igt@i915_selftest@live_hangcheck.html > > * igt@kms_busy@basic-flip-a: > - fi-kbl-soraka: [DMESG-WARN][23] ([fdo#105763]) -> [PASS][24] > [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7127/fi-kbl-soraka/igt@kms_busy@basic-flip-a.html > [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/fi-kbl-soraka/igt@kms_busy@basic-flip-a.html > > > {name}: This element is suppressed. This means it is ignored when computing > the status of the difference (SUCCESS, WARNING, or FAILURE). > > [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763 > [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713 > [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724 > [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569 > [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045 > [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096 > [fdo#111934]: https://bugs.freedesktop.org/show_bug.cgi?id=111934 > [fdo#112050 ]: https://bugs.freedesktop.org/show_bug.cgi?id=112050 > > > Participating hosts (52 -> 44) > ------------------------------ > > Additional (1): fi-tgl-u > Missing (9): fi-ilk-m540 fi-bxt-dsi fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-skl-6260u fi-icl-y fi-byt-clapper fi-bdw-samus > > > Build changes > ------------- > > * CI: CI-20190529 -> None > * Linux: CI_DRM_7127 -> Patchwork_14878 > > CI-20190529: 20190529 > CI_DRM_7127: b20b01f8392f688d8d73d4e6dce35a92e5898ad1 @ git://anongit.freedesktop.org/gfx-ci/linux > IGT_5232: bb5735423eaf6fdbf6b2f94ef0b8520e74eab993 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > Patchwork_14878: 586f33cacab5b8b2a9c4cd4dafcaf3d6608b1648 @ git://anongit.freedesktop.org/gfx-ci/linux > > > == Linux commits == > > 586f33cacab5 drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14878/index.html > _______________________________________________ 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: [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove 2019-10-18 10:07 [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove Janusz Krzysztofik 2019-10-18 11:43 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork 2019-10-18 12:13 ` ✗ Fi.CI.BAT: failure " Patchwork @ 2019-10-18 15:52 ` Michal Wajdeczko 2019-10-18 21:42 ` Chris Wilson 2019-10-21 12:10 ` Jani Nikula 3 siblings, 1 reply; 10+ messages in thread From: Michal Wajdeczko @ 2019-10-18 15:52 UTC (permalink / raw) To: intel-gfx, Janusz Krzysztofik; +Cc: Jani Nikula On Fri, 18 Oct 2019 12:07:10 +0200, Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote: > Commit 2d6f6f359fd8 ("drm/i915: add i915_driver_modeset_remove()") > claimed removal of asymmetry in probe() and remove() calls, however, it > didn't take care of calling intel_irq_uninstall() on driver remove. > That doesn't hurt as long as we still call it from > intel_modeset_driver_remove() but in order to have full symmetry we > should call it again from i915_driver_modeset_remove(). > > Note that it's safe to call intel_irq_uninstall() twice thanks to > commit b318b82455bd ("drm/i915: Nuke drm_driver irq vfuncs"). We may > only want to mention the case we are adding in a related FIXME comment > provided by that commit. While being at it, update the name of > function mentioned as calling it out of sequence as that name has been > changed meanwhile by commit 78dae1ac35dd ("drm/i915: Propagate > "_remove" function name suffix down"). > > Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > --- Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> but please see below > drivers/gpu/drm/i915/i915_drv.c | 2 ++ > drivers/gpu/drm/i915/i915_irq.c | 8 ++++---- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c > index dd9613e45723..4ae9bfa96290 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -452,6 +452,8 @@ static void i915_driver_modeset_remove(struct > drm_i915_private *i915) > intel_modeset_driver_remove(i915); > + intel_irq_uninstall(i915); > + > intel_bios_driver_remove(i915); I'm wondering if this is a good location for this call as in i915_driver_modeset_probe() we call it before intel_vga_register() so likely cleanup should be done after intel_vga_unregister() it also looks that we missed to call intel_bios_driver_remove() on the error path in i915_driver_modeset_probe() but let's fix it in a separate patch > vga_switcheroo_unregister_client(pdev); > diff --git a/drivers/gpu/drm/i915/i915_irq.c > b/drivers/gpu/drm/i915/i915_irq.c > index bc83f094065a..0160283860a6 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -4515,10 +4515,10 @@ void intel_irq_uninstall(struct drm_i915_private > *dev_priv) > int irq = dev_priv->drm.pdev->irq; > /* > - * FIXME we can get called twice during driver load > - * error handling due to intel_modeset_cleanup() > - * calling us out of sequence. Would be nice if > - * it didn't do that... > + * FIXME we can get called twice during driver probe > + * error handling as well as during driver remove due to > + * intel_modeset_driver_remove() calling us out of sequence. > + * Would be nice if it didn't do that... > */ > if (!dev_priv->drm.irq_enabled) > return; _______________________________________________ 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: [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove 2019-10-18 15:52 ` [RFC PATCH] " Michal Wajdeczko @ 2019-10-18 21:42 ` Chris Wilson 0 siblings, 0 replies; 10+ messages in thread From: Chris Wilson @ 2019-10-18 21:42 UTC (permalink / raw) To: Janusz Krzysztofik, Michal Wajdeczko, intel-gfx; +Cc: Jani Nikula Quoting Michal Wajdeczko (2019-10-18 16:52:26) > On Fri, 18 Oct 2019 12:07:10 +0200, Janusz Krzysztofik > <janusz.krzysztofik@linux.intel.com> wrote: > > > Commit 2d6f6f359fd8 ("drm/i915: add i915_driver_modeset_remove()") > > claimed removal of asymmetry in probe() and remove() calls, however, it > > didn't take care of calling intel_irq_uninstall() on driver remove. > > That doesn't hurt as long as we still call it from > > intel_modeset_driver_remove() but in order to have full symmetry we > > should call it again from i915_driver_modeset_remove(). > > > > Note that it's safe to call intel_irq_uninstall() twice thanks to > > commit b318b82455bd ("drm/i915: Nuke drm_driver irq vfuncs"). We may > > only want to mention the case we are adding in a related FIXME comment > > provided by that commit. While being at it, update the name of > > function mentioned as calling it out of sequence as that name has been > > changed meanwhile by commit 78dae1ac35dd ("drm/i915: Propagate > > "_remove" function name suffix down"). > > > > Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > --- > > Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > but please see below > > > drivers/gpu/drm/i915/i915_drv.c | 2 ++ > > drivers/gpu/drm/i915/i915_irq.c | 8 ++++---- > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index dd9613e45723..4ae9bfa96290 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -452,6 +452,8 @@ static void i915_driver_modeset_remove(struct > > drm_i915_private *i915) > > intel_modeset_driver_remove(i915); > > + intel_irq_uninstall(i915); > > + > > intel_bios_driver_remove(i915); > > I'm wondering if this is a good location for this call as in > i915_driver_modeset_probe() we call it before intel_vga_register() > so likely cleanup should be done after intel_vga_unregister() I thought you were complaining about the positioning of intel_irq_uninstall, but you do mean the ordering of intel_bios_* vs intel_vga_*. -Chris _______________________________________________ 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: [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove 2019-10-18 10:07 [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove Janusz Krzysztofik ` (2 preceding siblings ...) 2019-10-18 15:52 ` [RFC PATCH] " Michal Wajdeczko @ 2019-10-21 12:10 ` Jani Nikula 2019-10-21 12:20 ` Jani Nikula 3 siblings, 1 reply; 10+ messages in thread From: Jani Nikula @ 2019-10-21 12:10 UTC (permalink / raw) To: Janusz Krzysztofik, intel-gfx On Fri, 18 Oct 2019, Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote: > Commit 2d6f6f359fd8 ("drm/i915: add i915_driver_modeset_remove()") Please look at that commit and explain to me how it impacts anything about intel_irq_uninstall(). BR, Jani. > claimed removal of asymmetry in probe() and remove() calls, however, it > didn't take care of calling intel_irq_uninstall() on driver remove. > That doesn't hurt as long as we still call it from > intel_modeset_driver_remove() but in order to have full symmetry we > should call it again from i915_driver_modeset_remove(). > > Note that it's safe to call intel_irq_uninstall() twice thanks to > commit b318b82455bd ("drm/i915: Nuke drm_driver irq vfuncs"). We may > only want to mention the case we are adding in a related FIXME comment > provided by that commit. While being at it, update the name of > function mentioned as calling it out of sequence as that name has been > changed meanwhile by commit 78dae1ac35dd ("drm/i915: Propagate > "_remove" function name suffix down"). > > Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 ++ > drivers/gpu/drm/i915/i915_irq.c | 8 ++++---- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index dd9613e45723..4ae9bfa96290 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -452,6 +452,8 @@ static void i915_driver_modeset_remove(struct drm_i915_private *i915) > > intel_modeset_driver_remove(i915); > > + intel_irq_uninstall(i915); > + > intel_bios_driver_remove(i915); > > vga_switcheroo_unregister_client(pdev); > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index bc83f094065a..0160283860a6 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -4515,10 +4515,10 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > int irq = dev_priv->drm.pdev->irq; > > /* > - * FIXME we can get called twice during driver load > - * error handling due to intel_modeset_cleanup() > - * calling us out of sequence. Would be nice if > - * it didn't do that... > + * FIXME we can get called twice during driver probe > + * error handling as well as during driver remove due to > + * intel_modeset_driver_remove() calling us out of sequence. > + * Would be nice if it didn't do that... > */ > if (!dev_priv->drm.irq_enabled) > return; -- 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
* Re: [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove 2019-10-21 12:10 ` Jani Nikula @ 2019-10-21 12:20 ` Jani Nikula 2019-10-21 12:23 ` Jani Nikula 0 siblings, 1 reply; 10+ messages in thread From: Jani Nikula @ 2019-10-21 12:20 UTC (permalink / raw) To: Janusz Krzysztofik, intel-gfx On Mon, 21 Oct 2019, Jani Nikula <jani.nikula@intel.com> wrote: > On Fri, 18 Oct 2019, Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote: >> Commit 2d6f6f359fd8 ("drm/i915: add i915_driver_modeset_remove()") > > Please look at that commit and explain to me how it impacts anything > about intel_irq_uninstall(). Seriously, why was this merged already? It does not fix *anything*. It cites a commit that does not break anything. You're calling intel_irq_uninstall() *twice* for no good reason. The whole probe/remove callchains are a mess, and I'm in the process of cleaning it up properly. What good does this patch bring for the overall effort, other than another useless quirk in the call chain to take into account? Indeed, the only actual functional change it makes in the remove path is not mentioned in the commit message at all! BR, Jani. > > BR, > Jani. > > >> claimed removal of asymmetry in probe() and remove() calls, however, it >> didn't take care of calling intel_irq_uninstall() on driver remove. >> That doesn't hurt as long as we still call it from >> intel_modeset_driver_remove() but in order to have full symmetry we >> should call it again from i915_driver_modeset_remove(). >> >> Note that it's safe to call intel_irq_uninstall() twice thanks to >> commit b318b82455bd ("drm/i915: Nuke drm_driver irq vfuncs"). We may >> only want to mention the case we are adding in a related FIXME comment >> provided by that commit. While being at it, update the name of >> function mentioned as calling it out of sequence as that name has been >> changed meanwhile by commit 78dae1ac35dd ("drm/i915: Propagate >> "_remove" function name suffix down"). >> >> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 2 ++ >> drivers/gpu/drm/i915/i915_irq.c | 8 ++++---- >> 2 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >> index dd9613e45723..4ae9bfa96290 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -452,6 +452,8 @@ static void i915_driver_modeset_remove(struct drm_i915_private *i915) >> >> intel_modeset_driver_remove(i915); >> >> + intel_irq_uninstall(i915); >> + >> intel_bios_driver_remove(i915); >> >> vga_switcheroo_unregister_client(pdev); >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> index bc83f094065a..0160283860a6 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -4515,10 +4515,10 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) >> int irq = dev_priv->drm.pdev->irq; >> >> /* >> - * FIXME we can get called twice during driver load >> - * error handling due to intel_modeset_cleanup() >> - * calling us out of sequence. Would be nice if >> - * it didn't do that... >> + * FIXME we can get called twice during driver probe >> + * error handling as well as during driver remove due to >> + * intel_modeset_driver_remove() calling us out of sequence. >> + * Would be nice if it didn't do that... >> */ >> if (!dev_priv->drm.irq_enabled) >> return; -- 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
* Re: [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove 2019-10-21 12:20 ` Jani Nikula @ 2019-10-21 12:23 ` Jani Nikula 0 siblings, 0 replies; 10+ messages in thread From: Jani Nikula @ 2019-10-21 12:23 UTC (permalink / raw) To: Janusz Krzysztofik, intel-gfx On Mon, 21 Oct 2019, Jani Nikula <jani.nikula@intel.com> wrote: > On Mon, 21 Oct 2019, Jani Nikula <jani.nikula@intel.com> wrote: >> On Fri, 18 Oct 2019, Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote: >>> Commit 2d6f6f359fd8 ("drm/i915: add i915_driver_modeset_remove()") >> >> Please look at that commit and explain to me how it impacts anything >> about intel_irq_uninstall(). > > Seriously, why was this merged already? It does not fix *anything*. It > cites a commit that does not break anything. And for crying out loud, merged without passing CI results. > > You're calling intel_irq_uninstall() *twice* for no good reason. > > The whole probe/remove callchains are a mess, and I'm in the process of > cleaning it up properly. What good does this patch bring for the overall > effort, other than another useless quirk in the call chain to take into > account? Indeed, the only actual functional change it makes in the > remove path is not mentioned in the commit message at all! > > > BR, > Jani. > > >> >> BR, >> Jani. >> >> >>> claimed removal of asymmetry in probe() and remove() calls, however, it >>> didn't take care of calling intel_irq_uninstall() on driver remove. >>> That doesn't hurt as long as we still call it from >>> intel_modeset_driver_remove() but in order to have full symmetry we >>> should call it again from i915_driver_modeset_remove(). >>> >>> Note that it's safe to call intel_irq_uninstall() twice thanks to >>> commit b318b82455bd ("drm/i915: Nuke drm_driver irq vfuncs"). We may >>> only want to mention the case we are adding in a related FIXME comment >>> provided by that commit. While being at it, update the name of >>> function mentioned as calling it out of sequence as that name has been >>> changed meanwhile by commit 78dae1ac35dd ("drm/i915: Propagate >>> "_remove" function name suffix down"). >>> >>> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >>> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> >>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_drv.c | 2 ++ >>> drivers/gpu/drm/i915/i915_irq.c | 8 ++++---- >>> 2 files changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >>> index dd9613e45723..4ae9bfa96290 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.c >>> +++ b/drivers/gpu/drm/i915/i915_drv.c >>> @@ -452,6 +452,8 @@ static void i915_driver_modeset_remove(struct drm_i915_private *i915) >>> >>> intel_modeset_driver_remove(i915); >>> >>> + intel_irq_uninstall(i915); >>> + >>> intel_bios_driver_remove(i915); >>> >>> vga_switcheroo_unregister_client(pdev); >>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >>> index bc83f094065a..0160283860a6 100644 >>> --- a/drivers/gpu/drm/i915/i915_irq.c >>> +++ b/drivers/gpu/drm/i915/i915_irq.c >>> @@ -4515,10 +4515,10 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) >>> int irq = dev_priv->drm.pdev->irq; >>> >>> /* >>> - * FIXME we can get called twice during driver load >>> - * error handling due to intel_modeset_cleanup() >>> - * calling us out of sequence. Would be nice if >>> - * it didn't do that... >>> + * FIXME we can get called twice during driver probe >>> + * error handling as well as during driver remove due to >>> + * intel_modeset_driver_remove() calling us out of sequence. >>> + * Would be nice if it didn't do that... >>> */ >>> if (!dev_priv->drm.irq_enabled) >>> return; -- 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
end of thread, other threads:[~2019-10-21 12:23 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-18 10:07 [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove Janusz Krzysztofik 2019-10-18 11:43 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork 2019-10-18 11:53 ` Janusz Krzysztofik 2019-10-18 12:13 ` ✗ Fi.CI.BAT: failure " Patchwork 2019-10-18 13:06 ` Janusz Krzysztofik 2019-10-18 15:52 ` [RFC PATCH] " Michal Wajdeczko 2019-10-18 21:42 ` Chris Wilson 2019-10-21 12:10 ` Jani Nikula 2019-10-21 12:20 ` Jani Nikula 2019-10-21 12:23 ` Jani Nikula
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.