* [PATCH] drm/i915: Don't oops during modeset shutdown after lpe audio deinit
@ 2018-11-05 19:46 Ville Syrjala
2018-11-05 20:01 ` [Intel-gfx] " Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ville Syrjala @ 2018-11-05 19:46 UTC (permalink / raw)
To: intel-gfx; +Cc: stable
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.
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;
+}
/**
* intel_lpe_audio_notify() - notify lpe audio event
--
2.18.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Don't oops during modeset shutdown after lpe audio deinit
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 ` Chris Wilson
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
2 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2018-11-05 20:01 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx; +Cc: stable
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).
> 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>
-Chris
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Don't oops during modeset shutdown after lpe audio deinit
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 8:22 ` Patchwork
2018-11-06 10:39 ` ✓ Fi.CI.IGT: " Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-11-06 8:22 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Don't oops during modeset shutdown after lpe audio deinit
URL : https://patchwork.freedesktop.org/series/52046/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5089 -> Patchwork_10728 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10728 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10728, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/52046/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10728:
=== IGT changes ===
==== Warnings ====
igt@drv_selftest@live_guc:
fi-skl-iommu: SKIP -> PASS +1
== Known issues ==
Here are the changes found in Patchwork_10728 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
fi-snb-2520m: NOTRUN -> DMESG-FAIL (fdo#103713)
igt@kms_pipe_crc_basic@read-crc-pipe-a:
fi-snb-2520m: NOTRUN -> INCOMPLETE (fdo#103713)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
fi-cfl-8109u: PASS -> INCOMPLETE (fdo#106070, fdo#108126)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718)
==== Possible fixes ====
igt@drv_selftest@live_hangcheck:
fi-kbl-7560u: INCOMPLETE (fdo#108044) -> PASS
fi-skl-iommu: INCOMPLETE (fdo#108602) -> PASS
igt@kms_frontbuffer_tracking@basic:
fi-hsw-peppy: DMESG-WARN (fdo#102614) -> PASS
igt@pm_rpm@module-reload:
fi-kbl-guc: DMESG-WARN -> PASS
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#108044 https://bugs.freedesktop.org/show_bug.cgi?id=108044
fdo#108126 https://bugs.freedesktop.org/show_bug.cgi?id=108126
fdo#108602 https://bugs.freedesktop.org/show_bug.cgi?id=108602
== Participating hosts (50 -> 46) ==
Additional (1): fi-snb-2520m
Missing (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_5089 -> Patchwork_10728
CI_DRM_5089: c62e5b5e706ab5f9066ac95e4c4998326628bcd6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4709: 15dff9353621d0746b80fae534c20621e03a9f01 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10728: 0d74d4e2004aee7efb6c9c2c03e1d7643e45c962 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
0d74d4e2004a drm/i915: Don't oops during modeset shutdown after lpe audio deinit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10728/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Don't oops during modeset shutdown after lpe audio deinit
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 8:22 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-11-06 10:39 ` Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-11-06 10:39 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Don't oops during modeset shutdown after lpe audio deinit
URL : https://patchwork.freedesktop.org/series/52046/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5089_full -> Patchwork_10728_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10728_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10728_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10728_full:
=== IGT changes ===
==== Warnings ====
igt@kms_cursor_crc@cursor-128x128-offscreen:
shard-snb: PASS -> SKIP +2
== Known issues ==
Here are the changes found in Patchwork_10728_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ctx_switch@basic-all-light:
shard-kbl: PASS -> DMESG-WARN (fdo#103558, fdo#105602)
igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
shard-glk: PASS -> FAIL (fdo#108145)
igt@kms_cursor_crc@cursor-128x128-random:
shard-apl: PASS -> FAIL (fdo#103232)
igt@kms_cursor_crc@cursor-256x85-onscreen:
shard-glk: PASS -> FAIL (fdo#103232) +1
igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
shard-glk: PASS -> DMESG-WARN (fdo#106538, fdo#105763) +2
igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-ytiled:
shard-skl: PASS -> FAIL (fdo#103184)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt:
shard-skl: PASS -> FAIL (fdo#103167) +4
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
shard-apl: PASS -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
shard-glk: PASS -> FAIL (fdo#103167) +3
igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-gtt:
shard-skl: PASS -> FAIL (fdo#105682) +1
igt@kms_plane@pixel-format-pipe-c-planes:
shard-skl: NOTRUN -> DMESG-FAIL (fdo#106885, fdo#103166)
igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
shard-skl: PASS -> FAIL (fdo#107815, fdo#108145)
igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
shard-skl: PASS -> FAIL (fdo#107815)
igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
shard-apl: PASS -> FAIL (fdo#103166)
igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
shard-glk: PASS -> FAIL (fdo#103166)
igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
shard-apl: PASS -> DMESG-WARN (fdo#103558, fdo#105602) +3
igt@kms_setmode@basic:
shard-skl: NOTRUN -> FAIL (fdo#99912)
==== Possible fixes ====
igt@drv_suspend@shrink:
shard-skl: INCOMPLETE (fdo#106886) -> PASS
igt@kms_atomic@plane_primary_legacy:
shard-kbl: DMESG-WARN (fdo#103313, fdo#103558, fdo#105602) -> PASS +2
igt@kms_cursor_crc@cursor-128x128-dpms:
shard-glk: FAIL (fdo#103232) -> PASS +1
igt@kms_cursor_crc@cursor-256x256-suspend:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS
igt@kms_cursor_crc@cursor-256x85-random:
shard-apl: FAIL (fdo#103232) -> PASS +4
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
shard-apl: FAIL (fdo#103167) -> PASS
igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-mmap-gtt:
shard-glk: FAIL (fdo#103167) -> PASS +2
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-glk: FAIL (fdo#103166) -> PASS +2
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
igt@pm_rpm@system-suspend:
shard-kbl: DMESG-WARN (fdo#103313) -> PASS
==== Warnings ====
igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
shard-glk: FAIL (fdo#108145) -> DMESG-FAIL (fdo#106538, fdo#108145)
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#106885 https://bugs.freedesktop.org/show_bug.cgi?id=106885
fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (6 -> 6) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_5089 -> Patchwork_10728
CI_DRM_5089: c62e5b5e706ab5f9066ac95e4c4998326628bcd6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4709: 15dff9353621d0746b80fae534c20621e03a9f01 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10728: 0d74d4e2004aee7efb6c9c2c03e1d7643e45c962 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10728/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Don't oops during modeset shutdown after lpe audio deinit
2018-11-05 20:01 ` [Intel-gfx] " Chris Wilson
@ 2018-11-06 13:20 ` Ville Syrjälä
0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2018-11-06 13:20 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Don't oops during modeset shutdown after lpe audio deinit
@ 2018-11-06 13:20 ` Ville Syrjälä
0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2018-11-06 13:20 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-06 22:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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ä
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
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.