On 12/5/16 3:39 PM, Pandiyan, Dhinakaran wrote:
On Mon, 2016-12-05 at 08:02 +0000, Chris Wilson wrote:
On Sun, Dec 04, 2016 at 07:31:18PM -0600, 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.
Fix by adding test condition to make sure both variables
are used consistently. This removes the kernel oops.
There are still annoying cases where the primary display goes black
when the secondary display is turned off but it can be recovered from
by playing with the monitor inputs and power buttons.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
---
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..5481fde 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1815,7 +1815,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
drm_dp_create_payload_step1(mgr, mgr->proposed_vcpis[i]->vcpi, &req_payload);
mgr->payloads[i].num_slots = req_payload.num_slots;
mgr->payloads[i].vcpi = req_payload.vcpi;
- } else if (mgr->payloads[i].num_slots) {
+ } else if (mgr->payloads[i].num_slots && port != NULL) {
mgr->payloads[i].num_slots = 0;
drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
s/port->vcpi.vcpi/mgr->payloads[i].vcpi/ here looks to be the correct
fix.
-Chris
Hmm, not sure if that is the correct fix either. With port = NULL,
doesn't look like we send drm_dp_payload_send_msg(..., pbn = 0).
Although, we do update the payload table via DPCD
Also, if port is set to NULL, I wonder if we are messing up the
reference counting. Because, this is done below.
...
if (port)
drm_dp_put_port(port);
...
Chris' suggested fix works better than my initial proposal, I just sent
the update as a V2. It may not be correct or complete but then someone
smarter than me needs to take over. I am just an audio guy...
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
