On Mon, Dec 05, 2016 at 09:39:49PM +0000, 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 I'll leave that for you to work out if something needs to be sent after the downstream disappeared. > 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); This is ok, as this is clearing the local ref which is only obtained if port != NULL. (If port == NULL, we have no local ref which we need to put.) -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
