Thread (13 messages) 13 messages, 5 authors, 2016-03-02

Re: [Intel-gfx] [PATCH] drm/i915: Resume DP MST before doing any kind of modesetting

From: Dave Airlie <airlied@gmail.com>
Date: 2016-02-29 21:33:14
Also in: dri-devel, intel-gfx

On 1 March 2016 at 02:12, Daniel Vetter [off-list ref] wrote:
On Wed, Feb 24, 2016 at 08:03:04AM +0530, Thulasimani, Sivakumar wrote:
quoted

On 2/24/2016 3:41 AM, Lyude wrote:
quoted
As it turns out, resuming DP MST is racey since we don't make sure MST
is ready before we start modesetting, it just usually happens to be
ready in time. This isn't the case on all systems, particularly a
ThinkPad T560 with displays connected through the dock. On these
systems, resuming the laptop while connected to the dock usually results
in blank monitors. Making sure MST is ready before doing any kind of
modesetting fixes this issue.
basic question since i haven't worked on MST much. MST should work like any
other digital panel on resume. i.e detect followed by modeset. in the
modified
commit mentioned below is it failing to detect the panel or failing at the
modeset ?
if we are depending on the intel_display_resume, how about moving the
intel_dp_mst_resume just above intel_display_resume?


Generic question to others in mail list on i915_drm_resume
we are doing a modeset and then doing the detect/hpd init.
shouldn't this be the other way round ? almost all displays
will pass a modeset even if display is not connected so we
are spending time on modeset even for displays that were
removed during the suspend state. if this is to simply
drm_state being saved and restored,
We must restore anyway, we're not really allowed to shut down a display
without userspace's consent. DP mst breaks this, and it's not fun really.
So for hotunplug the flow should always be:
1. kernel notices something has changed in the output config.
2. kernel sends out uevent
3. userspace figures out what changed, and what needs to be done
4. userspace asks the kernel to change display configuration through
setCrtc and Atomic ioctl calls.

Irrespective of hotunplug handling, the kernel also _must_ restore the
entire display configuration before userspace resumes. We can do that
asynchronously (and there's plans for that), but even then we must stall
userspace on the first KMS ioclt to keep up the illusion that a system s/r
is transparent.

In short, even if we do hpd processing before resuming the display,
nothing will be faster - we must wait for userspace to make up its mind,
and that can only happen once we've restored the display config.

And again, mst is kinda breaking this, since and mst unplug results in a
force-disable. Which can upset userspace and in general results in the
need for lots of fragile error handling all over.
quoted
quoted
We originally changed the resume order in

    commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw state")

to fix a ton of WARN_ON's after resume, but this doesn't seem to be an
issue now anyhow.

CC: stable@vger.kernel.org
Signed-off-by: Lyude <redacted>
Wrt the patch itself: I think only in 4.6 we've actually fixed up all
these mst WARN_ON. Cc: stable seems extremely risky on this one. Also, the
modeset state readout for mst is still not entirely correct (it still
relies on software state), so I'm not sure we've actually managed to shut
up all the WARNINGs. Iirc the way to repro them is to hot-unplug something
while suspended. In short the get_hw_state functions for mst should not
rely on any mst software state, but instead just look at hw registers and
dp aux registers to figure out what's going on. But not sure whether this
will result on WARNINGs on resume, since generally the bios doesn't enable
anything in that case.

Furthermore MST still does a force-modeset when processing a hotunplug.
Doing that before we've resumed the display is likely a very bad idea.
What we need to fix that part is to separate the dp mst connector
hotplug/unplugging from actually updating the modeset change. This needs
reference-counting on drm_connector (so that we can lazily free
drm_connector structs after hot-unplug), and is a major change.
I agree on the dropping stable on this, just put it in 4.6 for now, actually I'm
happy to send in for 4.5 at this stage as it fixes some laptops I have as well.

However I'm not sure how you think we can keep hw state programmed when
the hw is gone away until userspace comes along to do something about it,
the hw state is mostly on the other side of the MST link which has also gone
away. My experience with leaving the crtcs programmed was and I might not
be 100% sure was hw hangs, I even managed to get some MCEs over time.

Though I might be tempted to go investigate this a bit more.

Dave.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help