From: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Chris Wilson <chris@chris-wilson.co.uk> Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't oops during modeset shutdown after lpe audio deinit Date: Tue, 6 Nov 2018 15:20:01 +0200 [thread overview] Message-ID: <20181106132001.GQ9144@intel.com> (raw) In-Reply-To: <154144807218.24769.14242978166683753961@skylake-alporthouse-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
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Chris Wilson <chris@chris-wilson.co.uk> Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't oops during modeset shutdown after lpe audio deinit Date: Tue, 6 Nov 2018 15:20:01 +0200 [thread overview] Message-ID: <20181106132001.GQ9144@intel.com> (raw) In-Reply-To: <154144807218.24769.14242978166683753961@skylake-alporthouse-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
next prev parent reply other threads:[~2018-11-06 22:45 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-05 19:46 [PATCH] drm/i915: Don't oops during modeset shutdown after lpe audio deinit Ville Syrjala 2018-11-05 20:01 ` [Intel-gfx] " Chris Wilson 2018-11-06 13:20 ` Ville Syrjälä [this message] 2018-11-06 13:20 ` Ville Syrjälä 2018-11-06 8:22 ` ✓ Fi.CI.BAT: success for " Patchwork 2018-11-06 10:39 ` ✓ Fi.CI.IGT: " Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20181106132001.GQ9144@intel.com \ --to=ville.syrjala@linux.intel.com \ --cc=chris@chris-wilson.co.uk \ --cc=intel-gfx@lists.freedesktop.org \ --cc=stable@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.