On 12/10/2014 09:16 AM, Daniel Vetter wrote:
On Tue, Dec 09, 2014 at 03:23:35PM +0000, Michel Thierry wrote:
We also need a _vma_view version of i915_gem_obj_bound;
i915_gem_object_ggtt_unpin checks if the obj is ggtt_bound and it could be
that the normal view is gone but a different view is still active (it is
also used in gpu_error and debug_fs, but I don't think it's a problem
there).
Where did you see the need for the new obj_bound variant? Probably best to
reply to the patch newly with just the relevant part quoted.
It is not in the patch but in the i915_gem_object_ggtt_unpin. Which is:
i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
BUG_ON(!vma);
BUG_ON(vma->pin_count == 0);
BUG_ON(!i915_gem_obj_ggtt_bound(obj));
if (--vma->pin_count == 0)
obj->pin_mappable = false;
}
The concern is the mismatch in semantics between i915_gem_obj_to_ggtt
and i915_gem_obj_ggtt_bound. Former implies normal VMA while the latter
hasn't been touched so it returns true on _any_ GGTT bound VMA.
I don't think this BUG_ON can trigger since normal VMA exists by the
virtue of BUG_ON(!vma), but I do agree that there is a mismatch in
"documentation" (BUG_ONs). So making i915_gem_obj_ggtt_bound also imply
a normal view would be correct it seems.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
