Re: [RFC][PATCH 4/4] drm: i915: Atomic pageflip WIP

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Sep 14, 2012 at 02:57:26PM +0100, Chris Wilson wrote:
> On Wed, 12 Sep 2012 18:47:07 +0300, ville.syrjala@linux.intel.com 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