* [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor
@ 2016-12-05 21:49 Pierre-Louis Bossart
2016-12-05 22:23 ` ✗ Fi.CI.BAT: failure for drm/dp/mst: fix kernel oops when turning off secondary monitor (rev2) Patchwork
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2016-12-05 21:49 UTC (permalink / raw)
To: intel-gfx; +Cc: daniel.vetter, Dave Airlie
100% reproducible issue found on SKL SkullCanyon NUC with two external
DP daisy-chained monitors in DP/MST mode. When turning off or changing
the input of the second monitor the machine stops with a kernel
oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
This issue is traced to an inconsistent control flow in
drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
the same time as 'req_payload.num_slots' is set to zero, but the pointer
is dereferenced even when req_payload.num_slot is zero.
The problematic dereference was introduced in commit dfda0df34
("drm/mst: rework payload table allocation to conform better")
and may impact all versions since v3.18
The fix suggested by Chris Wilson removes the kernel oops and was found to
work well after 10mn of monkey-testing with the second monitor power and
input buttons
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index aa64448..f59771d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
mgr->payloads[i].vcpi = req_payload.vcpi;
} else if (mgr->payloads[i].num_slots) {
mgr->payloads[i].num_slots = 0;
- drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
+ drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);
req_payload.payload_state = mgr->payloads[i].payload_state;
mgr->payloads[i].start_slot = 0;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/dp/mst: fix kernel oops when turning off secondary monitor (rev2)
2016-12-05 21:49 [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor Pierre-Louis Bossart
@ 2016-12-05 22:23 ` Patchwork
2017-01-26 0:12 ` [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor Nathan Ciobanu
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-12-05 22:23 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: intel-gfx
== Series Details ==
Series: drm/dp/mst: fix kernel oops when turning off secondary monitor (rev2)
URL : https://patchwork.freedesktop.org/series/16337/
State : failure
== Summary ==
Series 16337v2 drm/dp/mst: fix kernel oops when turning off secondary monitor
https://patchwork.freedesktop.org/api/1.0/series/16337/revisions/2/mbox/
Test drv_module_reload:
Subgroup basic-reload-inject:
pass -> INCOMPLETE (fi-hsw-4770)
fi-bdw-5557u total:247 pass:232 dwarn:0 dfail:0 fail:0 skip:15
fi-bsw-n3050 total:247 pass:207 dwarn:0 dfail:0 fail:0 skip:40
fi-bxt-t5700 total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-j1900 total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-n2820 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32
fi-hsw-4770 total:7 pass:6 dwarn:0 dfail:0 fail:0 skip:0
fi-hsw-4770r total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20
fi-ilk-650 total:247 pass:194 dwarn:0 dfail:0 fail:0 skip:53
fi-ivb-3770 total:247 pass:225 dwarn:0 dfail:0 fail:0 skip:22
fi-kbl-7500u total:247 pass:225 dwarn:0 dfail:0 fail:0 skip:22
fi-skl-6260u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6700k total:247 pass:223 dwarn:3 dfail:0 fail:0 skip:21
fi-skl-6770hq total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14
fi-snb-2520m total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32
fi-snb-2600 total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33
fi-ivb-3520m failed to connect after reboot
c3ccb720019c23b0c1fff182214a7ad26af519b8 drm-tip: 2016y-12m-05d-20h-50m-01s UTC integration manifest
471c1ae drm/dp/mst: fix kernel oops when turning off secondary monitor
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3198/
_______________________________________________
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/dp/mst: fix kernel oops when turning off secondary monitor
2016-12-05 21:49 [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor Pierre-Louis Bossart
2016-12-05 22:23 ` ✗ Fi.CI.BAT: failure for drm/dp/mst: fix kernel oops when turning off secondary monitor (rev2) Patchwork
@ 2017-01-26 0:12 ` Nathan Ciobanu
2017-01-26 0:25 ` Nathan Ciobanu
2017-02-20 22:34 ` Daniel Vetter
3 siblings, 0 replies; 6+ messages in thread
From: Nathan Ciobanu @ 2017-01-26 0:12 UTC (permalink / raw)
To: Pierre-Louis Bossart, intel-gfx
Cc: Dave Airlie, daniel.vetter@ffwll.ch; jani.nikula
I tested this on a KBL using 01-25-2017 dinq and it fixed the bug.
Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
On 12/05/2016 01:49 PM, Pierre-Louis Bossart wrote:
> 100% reproducible issue found on SKL SkullCanyon NUC with two external
> DP daisy-chained monitors in DP/MST mode. When turning off or changing
> the input of the second monitor the machine stops with a kernel
> oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
>
> This issue is traced to an inconsistent control flow in
> drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
> the same time as 'req_payload.num_slots' is set to zero, but the pointer
> is dereferenced even when req_payload.num_slot is zero.
>
> The problematic dereference was introduced in commit dfda0df34
> ("drm/mst: rework payload table allocation to conform better")
> and may impact all versions since v3.18
>
> The fix suggested by Chris Wilson removes the kernel oops and was found to
> work well after 10mn of monkey-testing with the second monitor power and
> input buttons
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index aa64448..f59771d 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
> mgr->payloads[i].vcpi = req_payload.vcpi;
> } else if (mgr->payloads[i].num_slots) {
> mgr->payloads[i].num_slots = 0;
> - drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
> + drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);
> req_payload.payload_state = mgr->payloads[i].payload_state;
> mgr->payloads[i].start_slot = 0;
> }
_______________________________________________
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/dp/mst: fix kernel oops when turning off secondary monitor
2016-12-05 21:49 [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor Pierre-Louis Bossart
2016-12-05 22:23 ` ✗ Fi.CI.BAT: failure for drm/dp/mst: fix kernel oops when turning off secondary monitor (rev2) Patchwork
2017-01-26 0:12 ` [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor Nathan Ciobanu
@ 2017-01-26 0:25 ` Nathan Ciobanu
2017-02-14 0:36 ` Pandiyan, Dhinakaran
2017-02-20 22:34 ` Daniel Vetter
3 siblings, 1 reply; 6+ messages in thread
From: Nathan Ciobanu @ 2017-01-26 0:25 UTC (permalink / raw)
To: Pierre-Louis Bossart, intel-gfx; +Cc: daniel.vetter, Dave Airlie
I tested this patch in dinq on a KBL system and it fixed the bug. The
system doesn't crash on disconnecting or powering off the second monitor
in the DP-MST chain. I also replied to the Bugzilla issue.
Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
On 12/05/2016 01:49 PM, Pierre-Louis Bossart wrote:
> 100% reproducible issue found on SKL SkullCanyon NUC with two external
> DP daisy-chained monitors in DP/MST mode. When turning off or changing
> the input of the second monitor the machine stops with a kernel
> oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
>
> This issue is traced to an inconsistent control flow in
> drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
> the same time as 'req_payload.num_slots' is set to zero, but the pointer
> is dereferenced even when req_payload.num_slot is zero.
>
> The problematic dereference was introduced in commit dfda0df34
> ("drm/mst: rework payload table allocation to conform better")
> and may impact all versions since v3.18
>
> The fix suggested by Chris Wilson removes the kernel oops and was found to
> work well after 10mn of monkey-testing with the second monitor power and
> input buttons
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index aa64448..f59771d 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
> mgr->payloads[i].vcpi = req_payload.vcpi;
> } else if (mgr->payloads[i].num_slots) {
> mgr->payloads[i].num_slots = 0;
> - drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
> + drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);
> req_payload.payload_state = mgr->payloads[i].payload_state;
> mgr->payloads[i].start_slot = 0;
> }
_______________________________________________
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/dp/mst: fix kernel oops when turning off secondary monitor
2017-01-26 0:25 ` Nathan Ciobanu
@ 2017-02-14 0:36 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 6+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-02-14 0:36 UTC (permalink / raw)
To: nathan.d.ciobanu; +Cc: airlied, intel-gfx, daniel.vetter
Looks like the right fix to me.
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
On Wed, 2017-01-25 at 16:25 -0800, Nathan Ciobanu wrote:
> I tested this patch in dinq on a KBL system and it fixed the bug. The
> system doesn't crash on disconnecting or powering off the second monitor
> in the DP-MST chain. I also replied to the Bugzilla issue.
>
> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
>
> On 12/05/2016 01:49 PM, Pierre-Louis Bossart wrote:
> > 100% reproducible issue found on SKL SkullCanyon NUC with two external
> > DP daisy-chained monitors in DP/MST mode. When turning off or changing
> > the input of the second monitor the machine stops with a kernel
> > oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
> >
> > This issue is traced to an inconsistent control flow in
> > drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
> > the same time as 'req_payload.num_slots' is set to zero, but the pointer
> > is dereferenced even when req_payload.num_slot is zero.
> >
> > The problematic dereference was introduced in commit dfda0df34
> > ("drm/mst: rework payload table allocation to conform better")
> > and may impact all versions since v3.18
> >
> > The fix suggested by Chris Wilson removes the kernel oops and was found to
> > work well after 10mn of monkey-testing with the second monitor power and
> > input buttons
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
> > Cc: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index aa64448..f59771d 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
> > mgr->payloads[i].vcpi = req_payload.vcpi;
> > } else if (mgr->payloads[i].num_slots) {
> > mgr->payloads[i].num_slots = 0;
> > - drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
> > + drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);
> > req_payload.payload_state = mgr->payloads[i].payload_state;
> > mgr->payloads[i].start_slot = 0;
> > }
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
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/dp/mst: fix kernel oops when turning off secondary monitor
2016-12-05 21:49 [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor Pierre-Louis Bossart
` (2 preceding siblings ...)
2017-01-26 0:25 ` Nathan Ciobanu
@ 2017-02-20 22:34 ` Daniel Vetter
3 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2017-02-20 22:34 UTC (permalink / raw)
To: Pierre-Louis Bossart, dri-devel; +Cc: Dave Airlie, intel-gfx
For next time around, pls cc dri-devel (it's in MAINTAINERS,
get_maintainers.pl gets it right) too.
-Daniel
On Mon, Dec 5, 2016 at 10:49 PM, Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
> 100% reproducible issue found on SKL SkullCanyon NUC with two external
> DP daisy-chained monitors in DP/MST mode. When turning off or changing
> the input of the second monitor the machine stops with a kernel
> oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
>
> This issue is traced to an inconsistent control flow in
> drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at
> the same time as 'req_payload.num_slots' is set to zero, but the pointer
> is dereferenced even when req_payload.num_slot is zero.
>
> The problematic dereference was introduced in commit dfda0df34
> ("drm/mst: rework payload table allocation to conform better")
> and may impact all versions since v3.18
>
> The fix suggested by Chris Wilson removes the kernel oops and was found to
> work well after 10mn of monkey-testing with the second monitor power and
> input buttons
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index aa64448..f59771d 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
> mgr->payloads[i].vcpi = req_payload.vcpi;
> } else if (mgr->payloads[i].num_slots) {
> mgr->payloads[i].num_slots = 0;
> - drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
> + drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);
> req_payload.payload_state = mgr->payloads[i].payload_state;
> mgr->payloads[i].start_slot = 0;
> }
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-20 22:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-05 21:49 [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor Pierre-Louis Bossart
2016-12-05 22:23 ` ✗ Fi.CI.BAT: failure for drm/dp/mst: fix kernel oops when turning off secondary monitor (rev2) Patchwork
2017-01-26 0:12 ` [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor Nathan Ciobanu
2017-01-26 0:25 ` Nathan Ciobanu
2017-02-14 0:36 ` Pandiyan, Dhinakaran
2017-02-20 22:34 ` Daniel Vetter
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.