On 23/01/2019 11:51, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-01-23 11:30:39)
+int i915_gem_vm_create_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+ struct drm_i915_private *i915 = to_i915(dev);
+ struct drm_i915_gem_vm_create *args = data;
+ struct drm_i915_file_private *file_priv = file->driver_priv;
+ struct i915_hw_ppgtt *ppgtt;
+ int err;
+
+ if (!HAS_FULL_PPGTT(i915))
+ return -ENODEV;
+
+ if (args->flags)
+ return -EINVAL;
+
+ if (args->extensions)
+ return -EINVAL;
+
+ ppgtt = i915_ppgtt_create(i915, file_priv);
+ if (IS_ERR(ppgtt))
+ return PTR_ERR(ppgtt);
+
+ err = mutex_lock_interruptible(&file_priv->vm_lock);
+ if (err)
+ goto err_put;
+
+ err = idr_alloc(&file_priv->vm_idr, ppgtt, 0, 0, GFP_KERNEL);
+ mutex_unlock(&file_priv->vm_lock);
+ if (err < 0)
+ goto err_put;
else if (GEM_WARN_ON(err == 0) {
err = -EINVAL;
goto err_put;
}
?
Or a GEM_BUG_ON(err == 0) if you must. :)
Not our bug :) We told it never to return 0.
+
+ ppgtt->user_handle = err;
+ args->id = err;
+ return 0;
+
+err_put:
+ i915_ppgtt_put(ppgtt);
+ return err;
+}
+
+int i915_gem_vm_destroy_ioctl(struct drm_device *dev, void *data,
Can we cheat and declare data as u32 * here and so avoid need for the
local, since there are only two dereferences?
Tempting, we need to cook up the macros to hide the function pointer
differences for the ioctl tables.
Probably not then, I was hoping that we could get away with function
declaration and definition not being exactly the same and that would be
all that's needed. If not never mind.
Regards,
Tvrtko
+ struct drm_file *file)
+{
+ struct drm_i915_file_private *file_priv = file->driver_priv;
+ struct i915_hw_ppgtt *ppgtt;
+ int err;
+ u32 id;
+
+ id = *(u32 *)data;
Oh crikey, did I write this?
Remember, do it right the first time as I won't remember when I was
cutting corners.
+ if (!id)
+ return -ENOENT;
+
+ err = mutex_lock_interruptible(&file_priv->vm_lock);
+ if (err)
+ return err;
+
+ ppgtt = idr_remove(&file_priv->vm_idr, id);
+ if (ppgtt)
GEM_WARN_ON(id != ppgtt->user_handle) too much paranoia?
BUG_ON then!
+ ppgtt->user_handle = 0;
+
+ mutex_unlock(&file_priv->vm_lock);
+ if (!ppgtt)
+ return -ENOENT;
+
+ i915_ppgtt_put(ppgtt);
+ return 0;
Or end with simply:
i915_ppgtt_put(ppgtt);
return ppgtt ? 0 : -ENOENT;
?
I feel a slight disappointment at the anticlimatic nature of this
function, leaving a gap, nay, a yearning, for more.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
