Re: [RFC PATCH] drm/i915: Restore full symmetry in i915_driver_modeset_probe/remove

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Michal Wajdeczko (2019-10-18 16:52:26)
> On Fri, 18 Oct 2019 12:07:10 +0200, Janusz Krzysztofik  
> <janusz.krzysztofik@linux.intel.com> 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@intel.com>
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > ---
> 
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> 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()

I thought you were complaining about the positioning of
intel_irq_uninstall, but you do mean the ordering of intel_bios_* vs
intel_vga_*.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx