Re: [Intel-gfx] [PATCH] drm/i915: Don't oops during modeset shutdown after lpe audio deinit

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Nov 05, 2018 at 08:01:17PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-11-05 19:46:04)
> > From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > 
> > We deinit the lpe audio device before we call
> > drm_atomic_helper_shutdown(), which means the platform device
> > may already be gone when it comes time to shut down the crtc.
> 
> Doesn't this mean that we fail to notify the audio codec of it being
> turned off? I'm wondering if we shouldn't do the display/gt idling first
> (like a i915_driver_unload_prepare).

Yeah, would probably be better to do the display off earlier.
Not too keen on diving into that rabbit hole at this time however.

> 
> > As we don't know when the last reference to the platform
> > device gets dropped by the audio driver we can't assume that
> > the device and its data are still around when turning off the
> > crtc. Mark the platform device as gone as soon as we do the
> > audio deinit.
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_lpe_audio.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > index cdf19553ffac..5d5336fbe7b0 100644
> > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > @@ -297,8 +297,10 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
> >         lpe_audio_platdev_destroy(dev_priv);
> >  
> >         irq_free_desc(dev_priv->lpe_audio.irq);
> > -}
> >  
> > +       dev_priv->lpe_audio.irq = -1;
> > +       dev_priv->lpe_audio.platdev = NULL;
> > +}
> 
> This thanks to HAS_LPE_AUDIO() (confusing macro of the day) will prevent
> a use-after-free during crtc shutdown.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks. Pushed.

-- 
Ville Syrj�l�
Intel