On Fri, Sep 14, 2012 at 02:57:26PM +0100, Chris Wilson wrote: > On Wed, 12 Sep 2012 18:47:07 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > +static void intel_flip_finish(struct drm_flip *flip) > > +{ > > + struct intel_flip *intel_flip = > > + container_of(flip, struct intel_flip, base); > > + struct drm_device *dev = intel_flip->crtc->dev; > > + > > + if (intel_flip->old_bo) { > > + mutex_lock(&dev->struct_mutex); > > + > > + intel_finish_fb(intel_flip->old_bo); > > So if I understand correctly, this code is called after the flip is > already complete? Yes. > The intel_finish_fb() exists to flush pending batches and flips on the > current fb, prior to changing the scanout registers. (There is a > hardware dependency such that the GPU may be executing a command that > required the current modesetting.) In the case of flip completion, all > of those dependencies have already been retired and so the finish should > be a no-op. And so it should no be required, nor the changes to > intel_finish_fb (which should have included a change in the name to > indicate that is now taking the fb_obj). Actually I'm not quite sure where this intel_finish_fb() call originated. Based on the name it didn't make sense to me, but I left it there for now. Hmm. OK it came from one patch from Imre while I was on vacation. I suppose he got it from intel_pipe_set_base() which does call intel_finish_fb() on the old fb. Why does it do that? I've not really dug into the GPU synchronization side and pin/fence stuff, so it's no surprise my code is a bit fscked up in those areas. -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel