On Mon, 2015-08-24 at 13:35 +0100, Chris Wilson wrote:
> On Mon, Aug 24, 2015 at 05:28:14PM +0530, ankitprasad.r.sha...@intel.com
> 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.
Could not get you here.
is it just to add list_add_tail in a separate function
__i915_gem_object_get_pages__tail_locked(), or a new variant of
__i915_gem_object_get_pages() that will also do the link list insertion
Thanks,
Ankit
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
