On 28.05.2015 17:38, Chris Wilson wrote: > On Thu, May 28, 2015 at 04:59:14PM +0900, Michel Dänzer wrote: >> On 27.05.2015 15:51, Chris Wilson wrote: >>> On Tue, May 26, 2015 at 02:30:32PM -0700, Keith Packard wrote: >>>> Michel Dänzer <[email protected]> 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. >> >> The patch below is an alternative fix for the problem I'm seeing, while >> preserving the window CRTC for MSC waits when possible. Your >> description sounds like it might work for you as well, Chris. Can you >> try it? >> >> >> diff --git a/present/present.c b/present/present.c >> index a634601..dc58f25 100644 >> --- a/present/present.c >> +++ b/present/present.c >> @@ -713,6 +713,7 @@ present_pixmap(WindowPtr window, >> uint64_t ust; >> uint64_t target_msc; >> uint64_t crtc_msc; >> + RRCrtcPtr new_crtc; >> int ret; >> present_vblank_ptr vblank, tmp; >> ScreenPtr screen = window->drawable.pScreen; >> @@ -734,7 +735,21 @@ present_pixmap(WindowPtr window, >> target_crtc = present_get_crtc(window); >> } >> >> - present_get_ust_msc(screen, target_crtc, &ust, &crtc_msc); >> + if (present_get_ust_msc(screen, target_crtc, &ust, &crtc_msc) != >> Success) { >> + /* Maybe we need to try a different CRTC? >> + */ >> + new_crtc = present_get_crtc(window); >> + >> + if (new_crtc != target_crtc && >> + present_get_ust_msc(screen, new_crtc, &ust, &crtc_msc) == >> Success) >> + target_crtc = new_crtc; >> + else { >> + /* Fall back to fake MSC handling >> + */ >> + target_crtc = NULL; >> + present_fake_get_ust_msc(screen, &ust, &crtc_msc); >> + } >> + } > > It survived for one more CRTC change, but still feeds passed msc into > the wait_vblank.
By how much is it off? Does it cause a hang? What's your test-case? Mine is the GNOME lock screen, i.e. basically DPMS off vs vblank waits and flips. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
