* [PATCH v2] drm/i915: Discard previous atomic state on resume if connectors change
@ 2016-05-06 21:39 Lyude
2016-05-09 6:42 ` Daniel Vetter
2016-05-09 14:52 ` ✗ Fi.CI.BAT: failure for drm/i915: Discard previous atomic state on resume if connectors change (rev2) Patchwork
0 siblings, 2 replies; 6+ messages in thread
From: Lyude @ 2016-05-06 21:39 UTC (permalink / raw)
To: intel-gfx
Cc: stable,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list,
Daniel Vetter, Lyude
If an MST device is disconnected while the machine is suspended, the
number of connectors will change as well after we call
intel_dp_mst_resume(). This means that any previous atomic state we had
before suspending is no longer valid, since it'll still be pointing to
missing connectors. We need to check for this before committing the
state, otherwise we'll kernel panic on resume whenever if any MST
display was disconnected before we started resuming:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
IP: [<ffffffffa01588ef>] drm_atomic_helper_check_modeset+0x29f/0xb40 [drm_kms_helper]
Call Trace:
[<ffffffffa02354f4>] intel_atomic_check+0x34/0x1180 [i915]
[<ffffffff810e6c3f>] ? mark_held_locks+0x6f/0xa0
[<ffffffff810e6d99>] ? trace_hardirqs_on_caller+0x129/0x1b0
[<ffffffffa00ff1d2>] drm_atomic_check_only+0x192/0x620 [drm]
[<ffffffff813ee001>] ? pci_pm_thaw+0x21/0x90
[<ffffffffa00ff677>] drm_atomic_commit+0x17/0x60 [drm]
[<ffffffffa023e0ad>] intel_display_resume+0xbd/0x160 [i915]
[<ffffffff813ee070>] ? pci_pm_thaw+0x90/0x90
[<ffffffffa01b60d8>] i915_drm_resume+0xd8/0x160 [i915]
[<ffffffffa01b6185>] i915_pm_resume+0x25/0x30 [i915]
[<ffffffff813ee0d4>] pci_pm_resume+0x64/0xa0
[<ffffffff814d9ea0>] dpm_run_callback+0x90/0x190
[<ffffffff814da455>] device_resume+0xd5/0x1f0
[<ffffffff814da58d>] async_resume+0x1d/0x50
[<ffffffff810b6718>] async_run_entry_fn+0x48/0x150
[<ffffffff810acc19>] process_one_work+0x1e9/0x5c0
[<ffffffff810acb96>] ? process_one_work+0x166/0x5c0
[<ffffffff810ad038>] worker_thread+0x48/0x4e0
[<ffffffff810acff0>] ? process_one_work+0x5c0/0x5c0
[<ffffffff810b3794>] kthread+0xe4/0x100
[<ffffffff81742672>] ret_from_fork+0x22/0x50
[<ffffffff810b36b0>] ? kthread_create_on_node+0x200/0x200
Changes since v1:
- Move drm_atomic_state_free() call down so we're holding the
appropriate locks when destroying the atomic state
Cc: stable@vger.kernel.org
Signed-off-by: Lyude <cpaul@redhat.com>
---
drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0f29ef6..d68efc7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15934,6 +15934,18 @@ void intel_display_resume(struct drm_device *dev)
retry:
ret = drm_modeset_lock_all_ctx(dev, &ctx);
+ /*
+ * With MST, the number of connectors can change between suspend and
+ * resume, which means that the state we want to restore might now be
+ * impossible to use since it'll be pointing to non-existant
+ * connectors.
+ */
+ if (ret == 0 &&
+ state->num_connector != dev->mode_config.num_connector) {
+ drm_atomic_state_free(state);
+ state = NULL;
+ }
+
if (ret == 0 && !setup) {
setup = true;
--
2.5.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915: Discard previous atomic state on resume if connectors change
2016-05-06 21:39 [PATCH v2] drm/i915: Discard previous atomic state on resume if connectors change Lyude
@ 2016-05-09 6:42 ` Daniel Vetter
2016-05-09 14:07 ` Lyude Paul
2016-05-09 14:52 ` ✗ Fi.CI.BAT: failure for drm/i915: Discard previous atomic state on resume if connectors change (rev2) Patchwork
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2016-05-09 6:42 UTC (permalink / raw)
To: Lyude
Cc: intel-gfx, stable,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list,
Daniel Vetter
On Fri, May 06, 2016 at 05:39:56PM -0400, Lyude wrote:
> If an MST device is disconnected while the machine is suspended, the
> number of connectors will change as well after we call
> intel_dp_mst_resume(). This means that any previous atomic state we had
> before suspending is no longer valid, since it'll still be pointing to
> missing connectors. We need to check for this before committing the
> state, otherwise we'll kernel panic on resume whenever if any MST
> display was disconnected before we started resuming:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
> IP: [<ffffffffa01588ef>] drm_atomic_helper_check_modeset+0x29f/0xb40 [drm_kms_helper]
> Call Trace:
> [<ffffffffa02354f4>] intel_atomic_check+0x34/0x1180 [i915]
> [<ffffffff810e6c3f>] ? mark_held_locks+0x6f/0xa0
> [<ffffffff810e6d99>] ? trace_hardirqs_on_caller+0x129/0x1b0
> [<ffffffffa00ff1d2>] drm_atomic_check_only+0x192/0x620 [drm]
> [<ffffffff813ee001>] ? pci_pm_thaw+0x21/0x90
> [<ffffffffa00ff677>] drm_atomic_commit+0x17/0x60 [drm]
> [<ffffffffa023e0ad>] intel_display_resume+0xbd/0x160 [i915]
> [<ffffffff813ee070>] ? pci_pm_thaw+0x90/0x90
> [<ffffffffa01b60d8>] i915_drm_resume+0xd8/0x160 [i915]
> [<ffffffffa01b6185>] i915_pm_resume+0x25/0x30 [i915]
> [<ffffffff813ee0d4>] pci_pm_resume+0x64/0xa0
> [<ffffffff814d9ea0>] dpm_run_callback+0x90/0x190
> [<ffffffff814da455>] device_resume+0xd5/0x1f0
> [<ffffffff814da58d>] async_resume+0x1d/0x50
> [<ffffffff810b6718>] async_run_entry_fn+0x48/0x150
> [<ffffffff810acc19>] process_one_work+0x1e9/0x5c0
> [<ffffffff810acb96>] ? process_one_work+0x166/0x5c0
> [<ffffffff810ad038>] worker_thread+0x48/0x4e0
> [<ffffffff810acff0>] ? process_one_work+0x5c0/0x5c0
> [<ffffffff810b3794>] kthread+0xe4/0x100
> [<ffffffff81742672>] ret_from_fork+0x22/0x50
> [<ffffffff810b36b0>] ? kthread_create_on_node+0x200/0x200
>
> Changes since v1:
> - Move drm_atomic_state_free() call down so we're holding the
> appropriate locks when destroying the atomic state
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Lyude <cpaul@redhat.com>
Is this still an issue on -nightly with the connector refcounting fixed?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0f29ef6..d68efc7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15934,6 +15934,18 @@ void intel_display_resume(struct drm_device *dev)
> retry:
> ret = drm_modeset_lock_all_ctx(dev, &ctx);
>
> + /*
> + * With MST, the number of connectors can change between suspend and
> + * resume, which means that the state we want to restore might now be
> + * impossible to use since it'll be pointing to non-existant
> + * connectors.
> + */
> + if (ret == 0 &&
> + state->num_connector != dev->mode_config.num_connector) {
> + drm_atomic_state_free(state);
> + state = NULL;
> + }
> +
> if (ret == 0 && !setup) {
> setup = true;
>
> --
> 2.5.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915: Discard previous atomic state on resume if connectors change
2016-05-09 6:42 ` Daniel Vetter
@ 2016-05-09 14:07 ` Lyude Paul
2016-05-09 14:53 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Lyude Paul @ 2016-05-09 14:07 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, intel-gfx,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list,
stable
Yep, Dave's patches fix the issue on their own so this is only going to be
needed for 4.6.
On Mon, 2016-05-09 at 08:42 +0200, Daniel Vetter wrote:
> On Fri, May 06, 2016 at 05:39:56PM -0400, Lyude wrote:
> >
> > If an MST device is disconnected while the machine is suspended, the
> > number of connectors will change as well after we call
> > intel_dp_mst_resume(). This means that any previous atomic state we had
> > before suspending is no longer valid, since it'll still be pointing to
> > missing connectors. We need to check for this before committing the
> > state, otherwise we'll kernel panic on resume whenever if any MST
> > display was disconnected before we started resuming:
> >
> > BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
> > IP: [<ffffffffa01588ef>] drm_atomic_helper_check_modeset+0x29f/0xb40
> > [drm_kms_helper]
> > Call Trace:
> > [<ffffffffa02354f4>] intel_atomic_check+0x34/0x1180 [i915]
> > [<ffffffff810e6c3f>] ? mark_held_locks+0x6f/0xa0
> > [<ffffffff810e6d99>] ? trace_hardirqs_on_caller+0x129/0x1b0
> > [<ffffffffa00ff1d2>] drm_atomic_check_only+0x192/0x620 [drm]
> > [<ffffffff813ee001>] ? pci_pm_thaw+0x21/0x90
> > [<ffffffffa00ff677>] drm_atomic_commit+0x17/0x60 [drm]
> > [<ffffffffa023e0ad>] intel_display_resume+0xbd/0x160 [i915]
> > [<ffffffff813ee070>] ? pci_pm_thaw+0x90/0x90
> > [<ffffffffa01b60d8>] i915_drm_resume+0xd8/0x160 [i915]
> > [<ffffffffa01b6185>] i915_pm_resume+0x25/0x30 [i915]
> > [<ffffffff813ee0d4>] pci_pm_resume+0x64/0xa0
> > [<ffffffff814d9ea0>] dpm_run_callback+0x90/0x190
> > [<ffffffff814da455>] device_resume+0xd5/0x1f0
> > [<ffffffff814da58d>] async_resume+0x1d/0x50
> > [<ffffffff810b6718>] async_run_entry_fn+0x48/0x150
> > [<ffffffff810acc19>] process_one_work+0x1e9/0x5c0
> > [<ffffffff810acb96>] ? process_one_work+0x166/0x5c0
> > [<ffffffff810ad038>] worker_thread+0x48/0x4e0
> > [<ffffffff810acff0>] ? process_one_work+0x5c0/0x5c0
> > [<ffffffff810b3794>] kthread+0xe4/0x100
> > [<ffffffff81742672>] ret_from_fork+0x22/0x50
> > [<ffffffff810b36b0>] ? kthread_create_on_node+0x200/0x200
> >
> > Changes since v1:
> > - Move drm_atomic_state_free() call down so we're holding the
> > appropriate locks when destroying the atomic state
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lyude <cpaul@redhat.com>
> Is this still an issue on -nightly with the connector refcounting fixed?
> -Daniel
>
> >
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 0f29ef6..d68efc7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15934,6 +15934,18 @@ void intel_display_resume(struct drm_device *dev)
> > retry:
> > ret = drm_modeset_lock_all_ctx(dev, &ctx);
> >
> > + /*
> > + * With MST, the number of connectors can change between suspend
> > and
> > + * resume, which means that the state we want to restore might now
> > be
> > + * impossible to use since it'll be pointing to non-existant
> > + * connectors.
> > + */
> > + if (ret == 0 &&
> > + state->num_connector != dev->mode_config.num_connector) {
> > + drm_atomic_state_free(state);
> > + state = NULL;
> > + }
> > +
> > if (ret == 0 && !setup) {
> > setup = true;
> >
> > --
> > 2.5.5
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Cheers,
Lyude
_______________________________________________
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.BAT: failure for drm/i915: Discard previous atomic state on resume if connectors change (rev2)
2016-05-06 21:39 [PATCH v2] drm/i915: Discard previous atomic state on resume if connectors change Lyude
2016-05-09 6:42 ` Daniel Vetter
@ 2016-05-09 14:52 ` Patchwork
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-05-09 14:52 UTC (permalink / raw)
To: cpaul; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Discard previous atomic state on resume if connectors change (rev2)
URL : https://patchwork.freedesktop.org/series/6673/
State : failure
== Summary ==
Series 6673v2 drm/i915: Discard previous atomic state on resume if connectors change
http://patchwork.freedesktop.org/api/1.0/series/6673/revisions/2/mbox/
Test drv_hangman:
Subgroup error-state-basic:
incomplete -> PASS (snb-dellxps)
Test kms_flip:
Subgroup basic-plain-flip:
pass -> DMESG-WARN (hsw-gt2)
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-b:
skip -> PASS (skl-nuci5)
Subgroup nonblocking-crc-pipe-b-frame-sequence:
pass -> FAIL (skl-nuci5)
Subgroup nonblocking-crc-pipe-c-frame-sequence:
pass -> SKIP (skl-nuci5)
Subgroup suspend-read-crc-pipe-c:
skip -> PASS (hsw-brixbox)
bdw-nuci7-2 total:219 pass:206 dwarn:0 dfail:0 fail:0 skip:13
bdw-ultra total:219 pass:193 dwarn:0 dfail:0 fail:0 skip:26
bsw-nuc-2 total:218 pass:174 dwarn:0 dfail:0 fail:2 skip:42
byt-nuc total:218 pass:174 dwarn:0 dfail:0 fail:3 skip:41
hsw-brixbox total:219 pass:193 dwarn:0 dfail:0 fail:0 skip:26
hsw-gt2 total:219 pass:196 dwarn:1 dfail:0 fail:1 skip:21
ilk-hp8440p total:219 pass:155 dwarn:0 dfail:0 fail:1 skip:63
ivb-t430s total:219 pass:188 dwarn:0 dfail:0 fail:0 skip:31
skl-i7k-2 total:219 pass:191 dwarn:0 dfail:0 fail:0 skip:28
skl-nuci5 total:219 pass:205 dwarn:0 dfail:0 fail:1 skip:13
snb-dellxps total:195 pass:159 dwarn:0 dfail:0 fail:0 skip:35
snb-x220t total:219 pass:176 dwarn:0 dfail:0 fail:1 skip:42
Results at /archive/results/CI_IGT_test/Patchwork_2155/
bcc6a843e7e4a3f4794b90dbefb00174171365bd drm-intel-nightly: 2016y-05m-09d-12h-47m-46s UTC integration manifest
432912d drm/i915: Discard previous atomic state on resume if connectors change
_______________________________________________
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: [PATCH v2] drm/i915: Discard previous atomic state on resume if connectors change
2016-05-09 14:07 ` Lyude Paul
@ 2016-05-09 14:53 ` Daniel Vetter
2016-05-09 15:31 ` Lyude Paul
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2016-05-09 14:53 UTC (permalink / raw)
To: Lyude Paul
Cc: Daniel Vetter, intel-gfx, stable,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list,
Daniel Vetter
On Mon, May 09, 2016 at 10:07:08AM -0400, Lyude Paul wrote:
> Yep, Dave's patches fix the issue on their own so this is only going to be
> needed for 4.6.
Ok, so not the first one that only needs to be applied to stable kernels.
For that to work you need to explain that this is the most minimal fix for
stabel kernels, and add a reference to the final commit in upstream that
fixes the issue for real. Otherwise Greg KH won't take it. It still needs
my ack.
Probably best to have an entire dp mst series for all of those, but maybe
ping me first on irc so I can check them quickly before you hit send.
-Daniel
>
> On Mon, 2016-05-09 at 08:42 +0200, Daniel Vetter wrote:
> > On Fri, May 06, 2016 at 05:39:56PM -0400, Lyude wrote:
> > >
> > > If an MST device is disconnected while the machine is suspended, the
> > > number of connectors will change as well after we call
> > > intel_dp_mst_resume(). This means that any previous atomic state we had
> > > before suspending is no longer valid, since it'll still be pointing to
> > > missing connectors. We need to check for this before committing the
> > > state, otherwise we'll kernel panic on resume whenever if any MST
> > > display was disconnected before we started resuming:
> > >
> > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
> > > IP: [<ffffffffa01588ef>] drm_atomic_helper_check_modeset+0x29f/0xb40
> > > [drm_kms_helper]
> > > Call Trace:
> > > [<ffffffffa02354f4>] intel_atomic_check+0x34/0x1180 [i915]
> > > [<ffffffff810e6c3f>] ? mark_held_locks+0x6f/0xa0
> > > [<ffffffff810e6d99>] ? trace_hardirqs_on_caller+0x129/0x1b0
> > > [<ffffffffa00ff1d2>] drm_atomic_check_only+0x192/0x620 [drm]
> > > [<ffffffff813ee001>] ? pci_pm_thaw+0x21/0x90
> > > [<ffffffffa00ff677>] drm_atomic_commit+0x17/0x60 [drm]
> > > [<ffffffffa023e0ad>] intel_display_resume+0xbd/0x160 [i915]
> > > [<ffffffff813ee070>] ? pci_pm_thaw+0x90/0x90
> > > [<ffffffffa01b60d8>] i915_drm_resume+0xd8/0x160 [i915]
> > > [<ffffffffa01b6185>] i915_pm_resume+0x25/0x30 [i915]
> > > [<ffffffff813ee0d4>] pci_pm_resume+0x64/0xa0
> > > [<ffffffff814d9ea0>] dpm_run_callback+0x90/0x190
> > > [<ffffffff814da455>] device_resume+0xd5/0x1f0
> > > [<ffffffff814da58d>] async_resume+0x1d/0x50
> > > [<ffffffff810b6718>] async_run_entry_fn+0x48/0x150
> > > [<ffffffff810acc19>] process_one_work+0x1e9/0x5c0
> > > [<ffffffff810acb96>] ? process_one_work+0x166/0x5c0
> > > [<ffffffff810ad038>] worker_thread+0x48/0x4e0
> > > [<ffffffff810acff0>] ? process_one_work+0x5c0/0x5c0
> > > [<ffffffff810b3794>] kthread+0xe4/0x100
> > > [<ffffffff81742672>] ret_from_fork+0x22/0x50
> > > [<ffffffff810b36b0>] ? kthread_create_on_node+0x200/0x200
> > >
> > > Changes since v1:
> > > - Move drm_atomic_state_free() call down so we're holding the
> > > appropriate locks when destroying the atomic state
> > >
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Lyude <cpaul@redhat.com>
> > Is this still an issue on -nightly with the connector refcounting fixed?
> > -Daniel
> >
> > >
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 0f29ef6..d68efc7 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -15934,6 +15934,18 @@ void intel_display_resume(struct drm_device *dev)
> > > retry:
> > > ret = drm_modeset_lock_all_ctx(dev, &ctx);
> > >
> > > + /*
> > > + * With MST, the number of connectors can change between suspend
> > > and
> > > + * resume, which means that the state we want to restore might now
> > > be
> > > + * impossible to use since it'll be pointing to non-existant
> > > + * connectors.
> > > + */
> > > + if (ret == 0 &&
> > > + state->num_connector != dev->mode_config.num_connector) {
> > > + drm_atomic_state_free(state);
> > > + state = NULL;
> > > + }
> > > +
> > > if (ret == 0 && !setup) {
> > > setup = true;
> > >
> > > --
> > > 2.5.5
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> --
> Cheers,
> Lyude
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915: Discard previous atomic state on resume if connectors change
2016-05-09 14:53 ` Daniel Vetter
@ 2016-05-09 15:31 ` Lyude Paul
0 siblings, 0 replies; 6+ messages in thread
From: Lyude Paul @ 2016-05-09 15:31 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, intel-gfx,
open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list,
stable
no problem, I'll let you know when they're ready
On Mon, 2016-05-09 at 16:53 +0200, Daniel Vetter wrote:
> On Mon, May 09, 2016 at 10:07:08AM -0400, Lyude Paul wrote:
> >
> > Yep, Dave's patches fix the issue on their own so this is only going to be
> > needed for 4.6.
> Ok, so not the first one that only needs to be applied to stable kernels.
> For that to work you need to explain that this is the most minimal fix for
> stabel kernels, and add a reference to the final commit in upstream that
> fixes the issue for real. Otherwise Greg KH won't take it. It still needs
> my ack.
>
> Probably best to have an entire dp mst series for all of those, but maybe
> ping me first on irc so I can check them quickly before you hit send.
> -Daniel
>
> >
> >
> > On Mon, 2016-05-09 at 08:42 +0200, Daniel Vetter wrote:
> > >
> > > On Fri, May 06, 2016 at 05:39:56PM -0400, Lyude wrote:
> > > >
> > > >
> > > > If an MST device is disconnected while the machine is suspended, the
> > > > number of connectors will change as well after we call
> > > > intel_dp_mst_resume(). This means that any previous atomic state we had
> > > > before suspending is no longer valid, since it'll still be pointing to
> > > > missing connectors. We need to check for this before committing the
> > > > state, otherwise we'll kernel panic on resume whenever if any MST
> > > > display was disconnected before we started resuming:
> > > >
> > > > BUG: unable to handle kernel NULL pointer dereference at
> > > > 0000000000000008
> > > > IP: [<ffffffffa01588ef>] drm_atomic_helper_check_modeset+0x29f/0xb40
> > > > [drm_kms_helper]
> > > > Call Trace:
> > > > [<ffffffffa02354f4>] intel_atomic_check+0x34/0x1180 [i915]
> > > > [<ffffffff810e6c3f>] ? mark_held_locks+0x6f/0xa0
> > > > [<ffffffff810e6d99>] ? trace_hardirqs_on_caller+0x129/0x1b0
> > > > [<ffffffffa00ff1d2>] drm_atomic_check_only+0x192/0x620 [drm]
> > > > [<ffffffff813ee001>] ? pci_pm_thaw+0x21/0x90
> > > > [<ffffffffa00ff677>] drm_atomic_commit+0x17/0x60 [drm]
> > > > [<ffffffffa023e0ad>] intel_display_resume+0xbd/0x160 [i915]
> > > > [<ffffffff813ee070>] ? pci_pm_thaw+0x90/0x90
> > > > [<ffffffffa01b60d8>] i915_drm_resume+0xd8/0x160 [i915]
> > > > [<ffffffffa01b6185>] i915_pm_resume+0x25/0x30 [i915]
> > > > [<ffffffff813ee0d4>] pci_pm_resume+0x64/0xa0
> > > > [<ffffffff814d9ea0>] dpm_run_callback+0x90/0x190
> > > > [<ffffffff814da455>] device_resume+0xd5/0x1f0
> > > > [<ffffffff814da58d>] async_resume+0x1d/0x50
> > > > [<ffffffff810b6718>] async_run_entry_fn+0x48/0x150
> > > > [<ffffffff810acc19>] process_one_work+0x1e9/0x5c0
> > > > [<ffffffff810acb96>] ? process_one_work+0x166/0x5c0
> > > > [<ffffffff810ad038>] worker_thread+0x48/0x4e0
> > > > [<ffffffff810acff0>] ? process_one_work+0x5c0/0x5c0
> > > > [<ffffffff810b3794>] kthread+0xe4/0x100
> > > > [<ffffffff81742672>] ret_from_fork+0x22/0x50
> > > > [<ffffffff810b36b0>] ? kthread_create_on_node+0x200/0x200
> > > >
> > > > Changes since v1:
> > > > - Move drm_atomic_state_free() call down so we're holding the
> > > > appropriate locks when destroying the atomic state
> > > >
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Lyude <cpaul@redhat.com>
> > > Is this still an issue on -nightly with the connector refcounting fixed?
> > > -Daniel
> > >
> > > >
> > > >
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
> > > > 1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index 0f29ef6..d68efc7 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -15934,6 +15934,18 @@ void intel_display_resume(struct drm_device
> > > > *dev)
> > > > retry:
> > > > ret = drm_modeset_lock_all_ctx(dev, &ctx);
> > > >
> > > > + /*
> > > > + * With MST, the number of connectors can change between
> > > > suspend
> > > > and
> > > > + * resume, which means that the state we want to restore might
> > > > now
> > > > be
> > > > + * impossible to use since it'll be pointing to non-existant
> > > > + * connectors.
> > > > + */
> > > > + if (ret == 0 &&
> > > > + state->num_connector != dev->mode_config.num_connector) {
> > > > + drm_atomic_state_free(state);
> > > > + state = NULL;
> > > > + }
> > > > +
> > > > if (ret == 0 && !setup) {
> > > > setup = true;
> > > >
> > > > --
> > > > 2.5.5
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > --
> > Cheers,
> > Lyude
> >
_______________________________________________
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
end of thread, other threads:[~2016-05-09 15:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-06 21:39 [PATCH v2] drm/i915: Discard previous atomic state on resume if connectors change Lyude
2016-05-09 6:42 ` Daniel Vetter
2016-05-09 14:07 ` Lyude Paul
2016-05-09 14:53 ` Daniel Vetter
2016-05-09 15:31 ` Lyude Paul
2016-05-09 14:52 ` ✗ Fi.CI.BAT: failure for drm/i915: Discard previous atomic state on resume if connectors change (rev2) Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox