[PATCH v2 0/2] DRM device registration fixes

[PATCH v2 0/2] DRM device registration fixes

[Krzysztof Niemiec]

This series contains patches pertaining the drm_dev_register() and
drm_dev_unregister() functions. Turns out drm_dev_register() doesn't
properly unwind its effects in the error path, causing confusion and
bugs.

Additionally, a related patch for i915 is bundled, as i915 mistakenly
calls drm_dev_unregister() in the error path triggered by failing
drm_dev_register(), which on one hand introduced a WARN_ON() in
drm_client_sysrq_unregister(), caused by calling it without a previous
_register() (as it was skipped in drm_dev_register()); but on the other
hand silenced yet another WARN_ON() later on in the error path in
drm_mode_config_cleanup(). With the other patch, the driver can just
handle the error from drm_dev_register() cleanly without calling
drm_dev_unregister(). This is explained in detail in both the patches.

Krzysztof Karas (1):
  drm: Unwind drm device registration upon error

Krzysztof Niemiec (1):
  drm/i915: Remove drm_dev_unregister() from the error path during
    i915_driver_register()

 drivers/gpu/drm/drm_drv.c          | 2 ++
 drivers/gpu/drm/i915/i915_driver.c | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
2.55.0

[PATCH v2 1/2] drm/i915: Remove drm_dev_unregister() from the error path during i915_driver_register()

[Krzysztof Niemiec]

During driver probe, in i915_driver_register(), if drm_dev_register()
fails, the code enters into an error path. In it, a call to
drm_dev_unregister() is made as part of cleanup in case drm_dev_register()
returns an error. However, this is unnecessary, as in case of an error
drm_dev_register() undoes its own setup in its own error path.

Calling drm_dev_unregister() after a failed drm_dev_register() also
generates a superfluous WARN_ON() from drm_client_sysrq_unregister().
drm_client_sysrq_register() is only called in drm_dev_register() if
there was no error in the latter. drm_dev_unregister() calls
drm_client_sysrq_unregister() as it expects to be called after a successful
drm_dev_register(), in which case the _sysrq_register() counterpart is
called. However, if the call to drm_dev_register() failed,
_sysrq_register() is never called; calling drm_dev_unregister() will
cause _sysrq_unregister() to be called (unconditionally) with no
corresponding _sysrq_register() call before it, which is caught in the
aforementioned WARN_ON().

Remove the redundant call to drm_dev_unregister() in case
drm_dev_register() returns an error.

Signed-off-by: Krzysztof Niemiec <krzysztof.niemiec@intel.com>
---
 drivers/gpu/drm/i915/i915_driver.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index ce6d20958320..da9be352fc90 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -617,7 +617,6 @@ static int i915_driver_register(struct drm_i915_private *dev_priv)
 	if (ret) {
 		i915_probe_error(dev_priv,
 				 "Failed to register driver for userspace access!\n");
-		drm_dev_unregister(&dev_priv->drm);
 		i915_pmu_unregister(dev_priv);
 		i915_gem_driver_unregister(dev_priv);
 		return ret;
-- 
2.55.0

[PATCH v2 2/2] drm: Unwind drm device registration upon error

[Krzysztof Niemiec]

From: Krzysztof Karas <krzysztof.karas@intel.com>

The drm device registration is done via drm_dev_register().
This function attempts to undo some of the initiatlization steps
under err_unload and err_minors labels, but this process is
incomplete - debugfs entries remain and the dev->registered flag
is still set to true.

On the other hand, drm_dev_unregister() tears down everything
that was initialized during registration step. This is confusing
when considering a failure in drm_dev_register(), because some
setup steps will be undone and some settings will remain, so it
is unclear whether a call to drm_dev_unregister() is a
requirement or not.

This may trigger bugs in the error paths of drivers which rely on
functions that check for the value of dev->registered flag.

What is more, commit 6915190a50e8 ("drm/client: Support
emergency restore via sysrq for all clients") introduced a new
WARN_ON and added drm_client_sysrq_register/unregister() calls
to the drm device register/unregister paths. Client sysrqs are
registered only when drm_dev_register() is sure to complete
without failures, but drm_client_sysrq_unregister() gets called
every time during drm_dev_unregister() and prints warning
messages whenever the caller tries to clean up the mess with
immediate unregistration.

Amend the problem by removing debugfs entries and marking
registered flag as false upon error in drm_dev_register().

Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
Co-developed-by: Krzysztof Niemiec <krzysztof.niemiec@intel.com>
Signed-off-by: Krzysztof Niemiec <krzysztof.niemiec@intel.com>
---
Changelog:

RFC -> v1:
 * addedd title of commit 6915190a50e8 to commit message;
 * rebased;

v1 -> v2:
 * updated commit log slightly to mention possible bugs;
 * updated authorship - the bug was found and fixed independently by
   Krzysztof K. and Krzysztof N.; We decided to exchange the findings
   and our comments and send the patch as co-developed-by.

An instance of a bug caused by this was uncovered in i915 with fault
injection. During driver probe in the error path, after failing
drm_dev_register() there is a reasonable expectation of a full proper
teardown of whatever drm_dev_register() has done. This is, however, not
the case, as shown in the calls below:

	i915_driver_probe()

	i915_driver_register()
		calls drm_dev_register()

	drm_dev_register()

	...

	connector .late_register() callback
		called from drm_dev_register(), and fails, leaving
		dev->registered as true, propagates the error back up

	...

	drm_dev_register()
		enters error path, but doesn't fully unwind

	i915_driver_register()
		enters error path

	i915_driver_probe()
		enters error path

	intel_display_driver_remove_noirq()
		called in _probe()'s error path

	intel_mode_config_cleanup()

	drm_mode_config_cleanup()
		this triggers a WARN_ON(), dev->registered should be false

This is a bug on drm_dev_register()'s side.

 drivers/gpu/drm/drm_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index e51ed959da89..b98e8af6f1c1 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1140,6 +1140,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	drm_minor_unregister(dev, DRM_MINOR_ACCEL);
 	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
 	drm_minor_unregister(dev, DRM_MINOR_RENDER);
+	drm_debugfs_dev_fini(dev);
+	dev->registered = false;
 out_unlock:
 	if (drm_dev_needs_global_mutex(dev))
 		mutex_unlock(&drm_global_mutex);
-- 
2.55.0

✗ i915.CI.BAT: failure for DRM device registration fixes

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2708 bytes --]

== Series Details ==

Series: DRM device registration fixes
URL   : https://patchwork.freedesktop.org/series/170568/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_18835 -> Patchwork_170568v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_170568v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_170568v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) 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_170568v1/index.html

Participating hosts (42 -> 39)
------------------------------

  Missing    (3): bat-dg2-13 bat-mtlp-9 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_170568v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live:
    - bat-apl-1:          [PASS][1] -> [ABORT][2] +1 other test abort
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18835/bat-apl-1/igt@i915_selftest@live.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_170568v1/bat-apl-1/igt@i915_selftest@live.html

  
Known issues
------------

  Here are the changes found in Patchwork_170568v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_debugfs@read-all-entries:
    - bat-adlp-6:         [PASS][3] -> [DMESG-WARN][4] ([i915#15673])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18835/bat-adlp-6/igt@core_debugfs@read-all-entries.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_170568v1/bat-adlp-6/igt@core_debugfs@read-all-entries.html

  
#### Possible fixes ####

  * igt@core_auth@basic-auth:
    - bat-adlp-6:         [DMESG-WARN][5] ([i915#15673]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18835/bat-adlp-6/igt@core_auth@basic-auth.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_170568v1/bat-adlp-6/igt@core_auth@basic-auth.html

  
  [i915#15673]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15673


Build changes
-------------

  * Linux: CI_DRM_18835 -> Patchwork_170568v1

  CI-20190529: 20190529
  CI_DRM_18835: fcaba3bc017906059b910ece3f83185e93010581 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9007: 9007
  Patchwork_170568v1: fcaba3bc017906059b910ece3f83185e93010581 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_170568v1/index.html

[-- Attachment #2: Type: text/html, Size: 3437 bytes --]

Re: [PATCH v2 0/2] DRM device registration fixes

[Sebastian Brzezinka]

Hi Krzysztof,

On Thu Jul 16, 2026 at 2:00 PM CEST, Krzysztof Niemiec wrote:
> This series contains patches pertaining the drm_dev_register() and
> drm_dev_unregister() functions. Turns out drm_dev_register() doesn't
> properly unwind its effects in the error path, causing confusion and
> bugs.
>
> Additionally, a related patch for i915 is bundled, as i915 mistakenly
> calls drm_dev_unregister() in the error path triggered by failing
> drm_dev_register(), which on one hand introduced a WARN_ON() in
> drm_client_sysrq_unregister(), caused by calling it without a previous
> _register() (as it was skipped in drm_dev_register()); but on the other
> hand silenced yet another WARN_ON() later on in the error path in
> drm_mode_config_cleanup(). With the other patch, the driver can just
> handle the error from drm_dev_register() cleanly without calling
> drm_dev_unregister(). This is explained in detail in both the patches.
As has been said, you could reorder the patches. Other than that LGTM.
Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>

-- 
Best regards,
Sebastian