On Fri, 18 Oct 2019 12:07:10 +0200, Janusz Krzysztofik
<janusz.krzysztofik@xxxxxxxxxxxxxxx> 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@xxxxxxxxx>
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
---
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
