[PATCH 2/3] present: Query the Window's CRTC every time

[Chris Wilson]

On Tue, May 26, 2015 at 02:30:32PM -0700, Keith Packard wrote:
> Michel Dänzer <michel at daenzer.net> writes:
> 
> > The old code also called present_get_crtc() unless pixmap == NULL, so
> > the problem couldn't affect flips but only MSC waits.
> 
> The original code was trying to let the client control which CRTC to
> track for each window. For PresentPixmap requests, there's an explicit
> CRTC argument, which allows the client to select the right one. For
> PresentNotifyMSC, there's no such argument.
> 
> So, the code was trying to respect the client's wishes by using
> whichever CRTC was last associated with the window -- either implicitly
> through the last call to present_get_crtc or explicitly from the last
> crtc passed to PresentPixmap for the same window.
> 
> Probably the right thing to do is to add an explicit CRTC parameter to
> PresentNotifyMSC, and then have both requests call present_get_crtc when
> an explicit CRTC is not provided.
> 
> That doesn't solve the problem when an explicitly requested CRTC is
> disabled though, so this fix doesn't address the root cause, only one of
> the symptoms.
> 
> > The problem I was hitting was that this code was running for an MSC wait
> > when the CRTC referenced by window_priv->crtc was already disabled for
> > DPMS off. This caused hangs at the GNOME lock screen. This patch seems
> > to fix that problem.
> 
> Why isn't your queue_vblank function bailing when asked to queue a
> request for a CRTC which is disabled? If it simply fails,
> present_execute will get called immediately and the client would
> continue happily along.

Oh, but it does fail gracefully. The problem is not that but that it
sends a garbage msc to a valid CRTC.

> > So, I vote for applying this patch (possibly with a better commit log)
> > to master ASAP and backporting it to stable branches.
> 
> It looks like this will only solve the problem of how to deal with new
> PresentNotifyMSC requests; any PresentPixmap or PresentNotifyMSC
> requests which are pending when the CRTC is disabled may end up getting
> wedged too?

Queued events are flushed when the CRTC is disabled.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre