On ke, 2016-11-02 at 09:43 +0000, Chris Wilson wrote:
> @@ -2458,17 +2459,16 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
> if (err)
> return err;
>
> - if (likely(obj->mm.pages)) {
> - __i915_gem_object_pin_pages(obj);
> - goto unlock;
> - }
> -
> - GEM_BUG_ON(i915_gem_object_has_pinned_pages(obj));
> + if (unlikely(!obj->mm.pages)) {
> + GEM_BUG_ON(i915_gem_object_has_pinned_pages(obj));
> + err = ____i915_gem_object_get_pages(obj);
> + if (err)
> + goto unlock;
>
> - err = ____i915_gem_object_get_pages(obj);
> - if (!err)
> - atomic_set_release(&obj->mm.pages_pin_count, 1);
> + smp_mb__before_atomic();
This is not cool without atomic in sight. Inline wrap as
__i915_gem_object_pages_mb() or something.
> @@ -3707,6 +3707,7 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma)
> {
> int ret = 0;
>
> + GEM_BUG_ON(!i915_gem_object_has_pinned_pages(vma->obj));
Rather confusing, simple mind would think as
__i915_gem_object_pin_pages has GEM_BUG_ON(!obj->mm.pages),
the next branch would never be taken?
> if (vma->pages)
> return 0;
>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
