[Intel-gfx] [PATCH 45/55] drm/i915: Convert intel_overlay to request tracking

[Chris Wilson]

On Wed, Jul 27, 2016 at 11:12:32AM +0300, Joonas Lahtinen wrote:
> On ma, 2016-07-25 at 18:32 +0100, Chris Wilson wrote:
> >  
> > -static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
> > +static void intel_overlay_submit_request(struct intel_overlay *overlay,
> >  					 struct drm_i915_gem_request *req,
> > -					 void (*tail)(struct intel_overlay *))
> > +					 void (*retire)(struct i915_gem_active *,
> > +							struct drm_i915_gem_request *))
> 
> Ugh, not such a beauty. We do not do callback typedefs?

Not often that we actually pass around function pointers.

> > -static void intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
> > +static void intel_overlay_release_old_vid_tail(struct i915_gem_active *active,
> > +					       struct drm_i915_gem_request *req)
> >  {
> > +	struct intel_overlay *overlay =
> > +		container_of(active, typeof(*overlay), last_flip);
> >  	struct drm_i915_gem_object *obj = overlay->old_vid_bo;
> >  
> > +	i915_gem_track_fb(obj, NULL,
> > +			  INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
> > +
> 
> Previously this call was after the below calls and old_vid_bo was
> always NULL, does this fix a bug?

Yes.
https://bugs.freedesktop.org/show_bug.cgi?id=93730
and a recent one.

> >  	i915_gem_object_ggtt_unpin(obj);
> >  	i915_gem_object_put(obj);
> >  
> >  	overlay->old_vid_bo = NULL;
> >  }
> >  
> >
> >  /* Wait for pending overlay flip and release old frame.
> > @@ -452,13 +450,9 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
> >  						    intel_overlay_release_old_vid_tail);
> >  		if (ret)
> >  			return ret;
> > -	}
> > +	} else
> > +		intel_overlay_release_old_vid_tail(&overlay->last_flip, NULL);
> 
> Why you added else? 

Habit?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre