[Intel-gfx] intel_dp_detect redesign

[Thulasimani, Sivakumar]

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)

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 at 5-15-26-95.residential.rdsnet.ro) 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