From: Daniel Vetter <daniel@ffwll.ch>
To: Ander Conselvan de Oliveira
<ander.conselvan.de.oliveira@intel.com>,
"Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com>,
intel-gfx <intel-gfx@lists.freedesktop.org>,
Dave Airlie <airlied@gmail.com>
Subject: Re: intel_dp_detect redesign
Date: Wed, 25 Nov 2015 11:04:41 +0100 [thread overview]
Message-ID: <20151125100441.GS17050@phenom.ffwll.local> (raw)
In-Reply-To: <CAKMK7uEkWJANJKbZCGBZvfC3BmYZ8DCcVP1kh+UEpi_Pn_Wfng@mail.gmail.com>
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
>
> 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@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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-11-25 10:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-24 19:13 intel_dp_detect redesign Daniel Vetter
2015-11-25 10:04 ` Daniel Vetter [this message]
2015-11-25 11:39 ` Thulasimani, Sivakumar
2015-11-26 10:07 ` Daniel Vetter
2015-11-27 16:30 ` Daniel Stone
2015-11-30 8:19 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151125100441.GS17050@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=airlied@gmail.com \
--cc=ander.conselvan.de.oliveira@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=sivakumar.thulasimani@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.