Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
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

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox