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