Re: [PATCH] drm/i915: Fix pages pin counting around swizzle quirk

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx