On Thu, Jun 15, 2017 at 09:25:08AM +0100, Chris Wilson wrote:
> Actually transferring from shmemfs to the physically contiguous set of
> pages should be wholly guarded by its obj->mm.lock!
> 
> v2: Remember to free the old pages.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 44 
> +++++++++++++++++++++++++++--------------
>  1 file changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 31cbe78171a9..7f3be5deeb5e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -574,7 +574,8 @@ int
>  i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
>                           int align)
>  {
> -     int ret;
> +     struct sg_table *pages;
> +     int err;
>  
>       if (align > obj->base.size)
>               return -EINVAL;
> @@ -582,32 +583,45 @@ i915_gem_object_attach_phys(struct drm_i915_gem_object 
> *obj,
>       if (obj->ops == &i915_gem_phys_ops)
>               return 0;
>  
> -     if (obj->mm.madv != I915_MADV_WILLNEED)
> -             return -EFAULT;
> -
> -     if (obj->base.filp == NULL)
> +     if (obj->ops != &i915_gem_object_ops)
>               return -EINVAL;
>  
> -     ret = i915_gem_object_unbind(obj);
> -     if (ret)
> -             return ret;
> +     err = i915_gem_object_unbind(obj);
> +     if (err)
> +             return err;
> +
> +     mutex_lock(&obj->mm.lock);
> +
> +     if (obj->mm.quirked) {
> +             err = -EFAULT;
> +             goto err_unlock;
> +     }
>  
> -     __i915_gem_object_put_pages(obj, I915_MM_NORMAL);
> -     if (obj->mm.pages)
> -             return -EBUSY;
> +     if (obj->mm.mapping) {
> +             err = -EBUSY;
> +             goto err_unlock;
> +     }
>  
> -     GEM_BUG_ON(obj->ops != &i915_gem_object_ops);
> +     pages = obj->mm.pages;
>       obj->ops = &i915_gem_phys_ops;
>  
> -     ret = i915_gem_object_pin_pages(obj);
> -     if (ret)
> +     err = __i915_gem_object_get_pages(obj);
> +     if (err)
>               goto err_xfer;
>  
> +     /* Perma-pin (until release) the physical set of pages */
> +     __i915_gem_object_pin_pages(obj);
> +
> +     i915_gem_object_ops.put_pages(obj, pages);

Leak sorted, so this lgtm.

Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

> +     mutex_unlock(&obj->mm.lock);
>       return 0;
>  
>  err_xfer:
>       obj->ops = &i915_gem_object_ops;
> -     return ret;
> +     obj->mm.pages = pages;
> +err_unlock:
> +     mutex_unlock(&obj->mm.lock);
> +     return err;
>  }
>  
>  static int
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to