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, ankitprasad.r.sharma@xxxxxxxxx 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
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
