We return immediately from i915_driver_register() if drm_dev_register() fails, skipping remaining registration steps. However, the _unregister() counterpart called at device remove knows nothing about that skip and executes reverts for all those steps. For that to work correctly, those revert functions must be resistant to being called even on uninitialized objects, or we must not skip their initialization. In case of intel_power_domains_enable() being skipped, then a reference to runtime PM dedicated to power domains not put, its _disable() counterpart, which expects that dedicated PM reference not held, emits a warning that taints the kernel: <3> [525.823143] i915 0000:00:02.0: [drm] *ERROR* Failed to register driver for userspace access! ... <4> [525.831069] ------------[ cut here ]------------ <4> [525.831071] i915 0000:00:02.0: [drm] drm_WARN_ON(power_domains->init_wakeref) <4> [525.831095] WARNING: CPU: 6 PID: 3440 at drivers/gpu/drm/i915/display/intel_display_power.c:2074 intel_power_domains_disable+0xc2/0xd0 [i915] ... <4> [525.831328] CPU: 6 UID: 0 PID: 3440 Comm: i915_module_loa Tainted: G U 6.14.0-rc1-CI_DRM_16076-g7a632b6798b6+ #1 ... <4> [525.831334] RIP: 0010:intel_power_domains_disable+0xc2/0xd0 [i915] ... <4> [525.831483] Call Trace: <4> [525.831484] <TASK> ... <4> [525.831943] i915_driver_remove+0x4b/0x140 [i915] <4> [525.832028] i915_pci_remove+0x1e/0x40 [i915] <4> [525.832099] pci_device_remove+0x3e/0xb0 <4> [525.832103] device_remove+0x40/0x80 <4> [525.832107] device_release_driver_internal+0x215/0x280 ... Moreover, that unexpected PM reference is left untouched (not released) but overwritten, then that triggers another kernel warning at driver release phase: <4> [526.685700] ------------[ cut here ]------------ <4> [526.685706] i915 0000:00:02.0: [drm] i915 raw-wakerefs=1 wakelocks=1 on cleanup <4> [526.685734] WARNING: CPU: 1 PID: 3440 at drivers/gpu/drm/i915/intel_runtime_pm.c:443 intel_runtime_pm_driver_release+0x75/0x90 [i915] ... <4> [526.686090] RIP: 0010:intel_runtime_pm_driver_release+0x75/0x90 [i915] ... <4> [526.686294] Call Trace: <4> [526.686296] <TASK> ... <4> [526.687025] i915_driver_release+0x7e/0xb0 [i915] <4> [526.687243] drm_dev_put.part.0+0x47/0x90 <4> [526.687250] devm_drm_dev_init_release+0x13/0x30 <4> [526.687255] devm_action_release+0x12/0x30 <4> [526.687261] release_nodes+0x3a/0x120 <4> [526.687268] devres_release_all+0x97/0xe0 <4> [526.687277] device_unbind_cleanup+0x12/0x80 <4> [526.687282] device_release_driver_internal+0x23a/0x280 ... It seems safe to enable runtime management of power domains (and put the dedicated PM reference) even if a device registration step fails. Go for it. Move calls to intel_power_domains_disable/enable() one level up, out of i915_driver_register/unregister() where they hardly seem to belong. Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10047 Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9820 Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10131 Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10887 Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12817 Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/i915_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index 91a7748f44926..91191125fa8e6 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -654,7 +654,6 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) intel_display_driver_register(display); - intel_power_domains_enable(display); intel_runtime_pm_enable(&dev_priv->runtime_pm); intel_register_dsm_handler(); @@ -678,7 +677,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv) intel_unregister_dsm_handler(); intel_runtime_pm_disable(&dev_priv->runtime_pm); - intel_power_domains_disable(display); intel_display_driver_unregister(display); @@ -839,6 +837,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) i915_driver_register(i915); + intel_power_domains_enable(display); + enable_rpm_wakeref_asserts(&i915->runtime_pm); i915_welcome_messages(i915); @@ -885,6 +885,8 @@ void i915_driver_remove(struct drm_i915_private *i915) wakeref = intel_runtime_pm_get(&i915->runtime_pm); + intel_power_domains_disable(display); + i915_driver_unregister(i915); /* Flush any external code that still may be under the RCU lock */ -- 2.47.1
