[PATCH v3 2/4] drm/i915: Omit unnecessary driver unregister steps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Now that we have a flag that indicates device registration status, when
unregistering the driver, jump over reverts of driver registration steps
that were not called due to device registration failure.

Unfortunately, not all steps of i915_driver_unregister() are limited only
to reverting changes introduced by their corresponding steps of
i915_driver_register(), and some are called out of order (not in reverse
order of what was called / skipped during driver registration.

One example is intel_gt_driver_unregister() called for each GT.  For clean
driver removal, a step of that function -- intel_gsc_uc_flush_work() --
occurred required even if intel_gt_driver_register() was not called due to
device registration failure.  That call was added to
intel_gt_driver_unregister() with commit b09f9670b13038 ("drm/i915/gsc:
flush the GSC worker before wedging on unload").

Another exception is intel_pxp_fini().  It seems to be a counterpart of
intel_pxp_init(), called directly from i915_driver_probe(), rather than of
intel_pxp_debugfs_register().  However, it still needs to be called before
intel_gt_driver_unregister()->intel_gsc_uc_flush_work() for clean driver
removal.  The call to intel_pxp_fini() was added to
i915_driver_unregister() with commit f67986b0119c04 ("drm/i915/pxp:
Promote pxp subsystem to top-level of i915").

Also, drm_dev_unplug() must be called even if drm_dev_register() failed.
It not only does more than drm_dev_unregister() but also, some of its
initial steps might have been called and not unwound even if an error then
occurred, so they must be reverted on driver unregister.

Last, i915_pmu_unregister(), a counterpart of i915_pmu_register() always
called from i915_driver_register(), must be called unconditionally from
i915_driver_register().  However, since it is called a few steps before
drm_dev_unplug(), it also requires special handling.  BTW, its placement
within i915_driver_unregister() hasn't changes since the immediate return
from i915_driver_register() on device registration failure was introduced.

To avoid unexpected side effects, execution order of unregister steps is
strictly preserved, which potentially makes the code changes sub-optimal.
That will be addressed in follow-up patches.

v3: Don't limit the scope to fixing a subset of what needs to be handled
    properly (hwmon? gt? debugfs?). (Andi)
v2: Check in _unregister whether the drm_dev_register has succeeded and
    skip some of the _unregister() steps. (Andi)

Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Cc: Chris Wilson <chris.p.wilson@xxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
Cc: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx>
---
 drivers/gpu/drm/i915/gt/intel_gt.c |  9 +++++++--
 drivers/gpu/drm/i915/i915_driver.c | 17 +++++++++++++----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 77eb9305a1ff8..17e2ccd8b1a92 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -783,8 +783,10 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
 {
 	intel_wakeref_t wakeref;
 
-	if (gt->i915->do_unregister)
-		intel_gt_sysfs_unregister(gt);
+	if (!gt->i915->do_unregister)
+		goto not_registered;
+
+	intel_gt_sysfs_unregister(gt);
 	intel_rps_driver_unregister(&gt->rps);
 	intel_gsc_fini(&gt->gsc);
 
@@ -809,7 +811,10 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
 	 * will likely already be idle in the great majority of non-selftest
 	 * scenarios.
 	 */
+not_registered:
 	intel_gsc_uc_flush_work(&gt->uc.gsc);
+	if (!gt->i915->do_unregister)
+		return;
 
 	/*
 	 * Upon unregistering the device to prevent any new users, cancel
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 2caaeeab1f0f6..d865e90f54704 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -672,26 +672,35 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	struct intel_gt *gt;
 	unsigned int i;
 
+	if (!dev_priv->do_unregister)
+		goto do_pxp_gt;
+
 	i915_switcheroo_unregister(dev_priv);
 
 	intel_runtime_pm_disable(&dev_priv->runtime_pm);
-	if (dev_priv->do_unregister)
-		intel_power_domains_disable(display);
+	intel_power_domains_disable(display);
 
 	intel_display_driver_unregister(display);
 
+do_pxp_gt:
 	intel_pxp_fini(dev_priv);
 
 	for_each_gt(gt, dev_priv, i)
 		intel_gt_driver_unregister(gt);
 
+	if (!dev_priv->do_unregister)
+		goto do_pmu;
+
 	i915_hwmon_unregister(dev_priv);
 
 	i915_perf_unregister(dev_priv);
+do_pmu:
 	i915_pmu_unregister(dev_priv);
+	if (!dev_priv->do_unregister)
+		goto do_unplug;
 
-	if (dev_priv->do_unregister)
-		i915_teardown_sysfs(dev_priv);
+	i915_teardown_sysfs(dev_priv);
+do_unplug:
 	drm_dev_unplug(&dev_priv->drm);
 
 	i915_gem_driver_unregister(dev_priv);
-- 
2.48.1




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux