On Sun, Sep 20, 2015 at 07:37:46PM +0530, Ankitprasad Sharma wrote:
> On Tue, 2015-09-15 at 10:49 +0100, Chris Wilson wrote:
> > On Tue, Sep 15, 2015 at 02:03:25PM +0530, [email protected] 
> > wrote:
> > >  i915_gem_create(struct drm_file *file,
> > >           struct drm_device *dev,
> > >           uint64_t size,
> > > +         uint32_t flags,
> > >           uint32_t *handle_p)
> > >  {
> > >   struct drm_i915_gem_object *obj;
> > > @@ -385,8 +386,31 @@ i915_gem_create(struct drm_file *file,
> > >   if (size == 0)
> > >           return -EINVAL;
> > >  
> > > + if (flags & __I915_CREATE_UNKNOWN_FLAGS)
> > > +         return -EINVAL;
> > > +
> > >   /* Allocate the new object */
> > > - obj = i915_gem_alloc_object(dev, size);
> > > + if (flags & I915_CREATE_PLACEMENT_STOLEN) {
> > > +         mutex_lock(&dev->struct_mutex);
> > > +         obj = i915_gem_object_create_stolen(dev, size);
> > > +         if (!obj) {
> > > +                 mutex_unlock(&dev->struct_mutex);
> > > +                 return -ENOMEM;
> > 
> > Note that you should change the i915_gem_object_create_stolen() to
> > report the precise error, as with the eviction support we may trigger
> > EINTR. Also ENOSPC will be preferrable for requesting a larger stolen
> > object than the available space (to help distinguish between true oom).
> > -Chris
> I would prefer to do this as a separate patch, as this might require a
> restructuring of the gem_stolen.c to some extent, something like this:

Yeah makes sense to have that split out, but I agree with Chris that we'll
need that error reporting. Follow-up patch on top of your series if fine
with me.
-Daniel

> 
> @@ -465,29 +467,29 @@ i915_gem_object_create_stolen(struct drm_device
> *dev, u64 size)
>       int ret;
>  
>       if (!drm_mm_initialized(&dev_priv->mm.stolen))
> -             return NULL;
> +             return ERR_PTR(-EINVAL);
>  
>       DRM_DEBUG_KMS("creating stolen object: size=%llx\n", size);
>       if (size == 0)
> -             return NULL;
> +             return ERR_PTR(-EINVAL);
>  
>       stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
>       if (!stolen)
> -             return NULL;
> +             return ERR_PTR(-ENOMEM);
>  
>       ret = i915_gem_stolen_insert_node(dev_priv, stolen, size, 4096);
>       if (ret) {
>               kfree(stolen);
> -             return NULL;
> +             return ERR_PTR(-ENOSPC);
>       }
>  
>       obj = _i915_gem_object_create_stolen(dev, stolen);
> -     if (obj)
> +     if (!IS_ERR(obj))
>               return obj;
>  
>       i915_gem_stolen_remove_node(dev_priv, stolen);
>       kfree(stolen);
> -     return NULL;
> +     return obj;
>  }
>  
> 
> Thanks,
> Ankit
> 
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to