[Intel-gfx] [PATCH] drm/i915: WARN_ON fence pin leaks

[Chris Wilson]

On Tue, May 13, 2014 at 12:10:24PM +0200, Daniel Vetter wrote:
> On Tue, May 13, 2014 at 12:05 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> ---
> >> +bool
> >> +i915_gem_object_pin_fence(struct drm_i915_gem_object *obj)
> >> +{
> >> +     if (obj->fence_reg != I915_FENCE_REG_NONE) {
> >> +             struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> >> +             struct i915_vma *ggtt_vma = i915_gem_obj_to_ggtt(obj);
> >> +
> >> +             WARN_ON(!ggtt_vma ||
> >> +                     dev_priv->fence_regs[obj->fence_reg].pin_count >
> >> +                     ggtt_vma->pin_count);
> >
> > Just do the first check as that supersedes the second.
> 
> Yeah, will do.
> 
> > As a stricter test, pass in i915_vma from the caller so that we can
> > assert that we only ever pin a fence when binding to the GGTT. That's
> > only a marginal nuisance for intel_pin_and_fence_fb_obj().
> 
> Imo cleaning up our function interfaces to use vma more directly
> should be orthogonal work, and done with the overall picture in mind.
> So I'd like to punt on this for now. It did cross my mind, so I
> generally agree with you here.

You are introducing the vma. This function otherwise doesn't need it
(except for the implications that you should only pin a fence after
pinning to the GTT).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre