On Wed, Nov 25, 2015 at 05:09:02PM +0530, Thulasimani, Sivakumar wrote: > > > > On 11/25/2015 3:34 PM, Daniel Vetter wrote: > >On Tue, Nov 24, 2015 at 08:13:06PM +0100, Daniel Vetter wrote: > >>Hi Ander&Sivakumar, > >> > >>Dave&I had a short discussion about reprobing DP (and MST) state on > >>resume. I think this is something we've missed in our dp hpd handling > >>rework thus far. And at least for SST we need to take it into account > >>since it would be a regression. > >> > >>Currently it's done through ->detect callback from > >>drm_helper_hpd_irq_event called from i915_drm_resume. Also irc logs > >>below. > >Oh and there's an issue for the hdmi hpd changes that have been merged and > >reverted too: Those will run into the same problem. Plus in addition doing > >nothing in ->detect will break storm handling (since that falls back to > >the probe helper poll work). > >-Daniel > Storm handling is done in i915_hotplug_work_func before detection is called > so it should work on top of changes planned. our change is inside > intel_dp_detect > so any flow before this is called should remain intact. the expected flow > post > the changes will be > digport_work_func -> intel_dp_hpd_pulse > if (long pulse) > handle long pulse () > return IRQ_NONE > i915_hotplug_work_func -> detect > > however good to explicitly check for this, > following needs to be tested before sending in next patch/merge > 1) MST displays verification (Ander's reported on first set of patches) > 2) check behavior on sleep - resume (dave&danvet) > 3) storm handling needs to be handled as well. (i assume this should be > fine, > but good to check explicitly) (danvet) > Yeah the storm mitigation will keep on working. What I'm worried about is that polling won't work any more: When a storm happens we disable the hpd and switch all affected connectors completely to polling. Polling happens through the probe helpers in drm_probe_helper.c, and that code exclusively uses ->detect callbacks. Which means if we no longer re-probe in detect (since we assume hpd works correctly) then this will break the storm handling code. Simplest fix (but a bit a hack) would be to check whether polling is enabled at the top of intel_hdmi_detect and if so execute a full probe. And not just return the cached values. Note that storms are only a concern for HDMI, not DP (somehow DP hw is less shit). Cheers, Daniel > regards, > Sivakumar > >>Thanks, Daniel > >> > >><airlied> danvet: so probing on resume, it seems a bit inconsistent, > >>is the kernel driver meant to be doing it? > >><airlied> I think since we stopped vt switching we've stopped doing > >>it, which is making mst docking kinda suck > >><danvet> mst was after stopping vt-switching I thought > >><danvet> but yeah we should reprobe > >><danvet> and we do (at least occasionally if it's not broken again) > >><airlied> well people are just noticing it more with mst > >><danvet> but not for mst iirc > >><danvet> mst just restores and hopes I think > >><airlied> but when you suspend in the dock, and move the laptop, and > >>resume things don't work unless you xrandr > >><airlied> and vice-versa > >><danvet> I looked into it for 5 minutes when tedtso complained an ran ;-) > >><airlied> well reproving should bring up/tear down any mst > >><danvet> hm, xrandr shouldn't be enough to fix it, we need a real hpd > >>to redo the mst stuff I thought ... > >><airlied> so I don't think mst is special here > >><danvet> we reprobe through probe helpers > >><robclark> janesma, Pali, bleh sorry.. yeah, looks like it needs a > >>stdbool.h.. not sure why I didn't hit that compile error.. sorry about > >>that.. > >><danvet> all mst stuff is done directly from hpd since it needs to > >>know long vs. short > >><danvet> so it misses out > >><airlied> if we probe a DP port and the device is gone, MST will get torn down > >><airlied> danvet: not so > >><airlied> unless someone else has been hacking the driver > >>* xxmitsu (~mike@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) has joined #dri-devel > >><danvet> oh, the completely gone case > >>* danvet looks > >><airlied> oh maybe we don't handle that properly > >><airlied> oh you might be right, I wonder where we should hook that in > >><danvet> drm_helper_hpd_irq_event in i915_drm_resume should get this > >>right for non-mst > >><danvet> well non-DP maybe, anderco and rtshiva are reworking this > >><danvet> but it's not merged yet > >><airlied> I'm guessing detect should not if a port was in mst > >><airlied> and is now disconnected > >><danvet> ok, skeleton is there but not all > >><airlied> intel_dp_detect > >><danvet> drm_dp_mst_topology_mgr_resume should probably check the > >>entire hierarchy, not just a simple dpcd write > >><danvet> and if anything changes, we need to generate the uevent somewhere > >><danvet> so might be better to re-run the entire dp_detect pile > >><danvet> tricky part is that we need to lie about long vs. short > >><danvet> it should be treated like a long hpd if anything changed, > >>short otherwise > >><danvet> well we have mgr->cbs->hotplug in the mst manager already > >><danvet> so should reuse that hook > >><danvet> airlied, I guess just fix up drm_dp_mst_topology_mgr_resume > >>to do rescan the entire hierarchy > >><danvet> calling ->hotplug if anything changed > >><danvet> and only returning true if the sink isn't mst any more > >><danvet> along the lines of what intel_dp_probe_mst does > >><danvet> it's not going to be all that simple ;-) > >><danvet> at least if you care about stuff like laptopt connected to > >>dock -> screen > >><danvet> s/r > >><airlied> doesn't sounds like fun, I'll stick on my list of things to > >>be scared off > >><danvet> then laptop only connected to dock > >><danvet> airlied, done the same > >><airlied> well the use case is laptop in dock, suspend, resume with > >>laptop plugged into a monitor > >><danvet> but the fun part is that anderco/rtshiva want to rework this > >><danvet> and if they do they'll also break sst dp ;-) > >><danvet> so I think I have some victims > >><danvet> airlied, yeah that's step one > >><airlied> I'd prefer to get the fixes in before redesigning the tower > >><danvet> but that already has all the complexity on the driver side > >><danvet> only thing missing is the code in the mst helper to rescan > >>the entire tree and call ->hot_plug if needed > >><danvet> problem is that current dp hpd handling is a mess already > >><danvet> it's hard to fix anything in there atm > >><danvet> so fixing this properly is needed anyway > >><danvet> it's just that I've forgotten about the resume case for plain > >>DP myself ;-) > >> > >>-- > >>Daniel Vetter > >>Software Engineer, Intel Corporation > >>+41 (0) 79 365 57 48 - http://blog.ffwll.ch > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
