On Thu, Sep 20, 2012 at 11:17:51AM +0200, Daniel Vetter wrote: > On Thu, Sep 20, 2012 at 10:56 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote: > > We need to wait for pending operations on the CRTC to retire before we > > can modify the CRTC. For example, if userspace has queued a batch that > > uses a WAIT_FOR_EVENT associated with the current FB, we can not modify > > the pipe with that outstanding, as we may then prevent that > > WAIT_FOR_EVENT from ever completing and so hanging the GPU. (Imagine a > > scanline wait waiting for line 1024 and the pipe being adjusted to > > 600-line mode.) There is also the sequencing issue of the immediate > > update versus a pending pageflip. > > > > In both cases the function to serialise the modeset with the pending > > operations existed but was simply not being called, or called after the > > damage was already done. > > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> > > I've looked at this situation again and we do have a > wait_for_pending_flips already in per-platform crtc_disable functions, > which are called for for switching off crtcs and also only just > disabling them for a modeset. > > So I think this finish_fb call in set_base is totally unnecessary and > can be just removed. Moving it to the crtc_set_config function doesn't > help (and this patch misses the case where we disable other crtcs than > set->crtc). This whole wait for pending flips mechanism looks iffy to me. First, when we schedule a flip, we record the fact that there is a pending flip into the old bo's pending_flip atomic mask thingy. When we later want to wait for the CRTC operations to finish, we do 'intel_finish_fb(crtc->fb)'. But notice that 'crtc->fb' is now the fb we flipped _to_, so it can't actually tell us whether there's still a flip pending. -- Ville Syrj?l? Intel OTC
