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 <mic...@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. > > 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. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel