On Mon, Aug 24, 2015 at 05:28:14PM +0530, ankitprasad.r.sharma@xxxxxxxxx wrote:
> +static int
> +__i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
> +{
> + const struct drm_i915_gem_object_ops *ops = obj->ops;
> + int ret;
> +
> + WARN_ON(obj->pages);
> +
> + if (obj->madv != I915_MADV_WILLNEED) {
> + DRM_DEBUG("Attempting to obtain a purgeable object\n");
> + return -EFAULT;
> + }
> +
> + BUG_ON(obj->pages_pin_count);
Put the parameter checking into i915_gem_object_get_pages(). The __i915
version is only allowed from strict contexts and we can place the burden
of being correct on the caller.
> + ret = ops->get_pages(obj);
> + if (ret)
> + return ret;
> +
> + obj->get_page.sg = obj->pages->sgl;
> + obj->get_page.last = 0;
> +
> + return 0;
> +}
> +
> /* Ensure that the associated pages are gathered from the backing storage
> * and pinned into our object. i915_gem_object_get_pages() may be called
> * multiple times before they are released by a single call to
> @@ -2339,28 +2377,17 @@ int
> i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
> {
> struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> - const struct drm_i915_gem_object_ops *ops = obj->ops;
> int ret;
>
> if (obj->pages)
> return 0;
>
> - if (obj->madv != I915_MADV_WILLNEED) {
> - DRM_DEBUG("Attempting to obtain a purgeable object\n");
> - return -EFAULT;
> - }
> -
> - BUG_ON(obj->pages_pin_count);
> -
> - ret = ops->get_pages(obj);
> + ret = __i915_gem_object_get_pages(obj);
> if (ret)
> return ret;
>
> list_add_tail(&obj->global_list, &dev_priv->mm.unbound_list);
I am tempted to say this should be in a new
__i915_gem_object_get_pages__tail_locked()
so that we don't have to hunt down users if we ever need to modify the
global lists.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
