On 18/01/2019 14:00, Chris Wilson wrote:
In preparation to making the ppGTT binding for a context explicit (to
facilitate reusing the same ppGTT between different contexts), allow the
user to create and destroy named ppGTT.
Needs some more text to explain uAPI usage, or even better kerneldoc in
i915_drm.h.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 2 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_gem_context.c | 83 +++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_gem_context.h | 5 ++
drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +
include/uapi/drm/i915_drm.h | 10 +++
6 files changed, 105 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f462a4d28af4..56bd087f1c4e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -3003,6 +3003,8 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_GEM_VM_CREATE, i915_gem_vm_create_ioctl, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_GEM_VM_DESTROY, i915_gem_vm_destroy_ioctl, DRM_RENDER_ALLOW),
};
static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 74bccb153359..cb11c23823c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -217,6 +217,9 @@ struct drm_i915_file_private {
} mm;
struct idr context_idr;
+ struct mutex vm_lock;
+ struct idr vm_idr;
+
struct intel_rps_client {
atomic_t boosts;
} rps_client;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index e8334c4bc130..7c90981704bf 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -598,19 +598,29 @@ static int context_idr_cleanup(int id, void *p, void *data)
return 0;
}
+static int vm_idr_cleanup(int id, void *p, void *data)
+{
+ i915_ppgtt_put(p);
+ return 0;
+}
+
int i915_gem_context_open(struct drm_i915_private *i915,
struct drm_file *file)
{
struct drm_i915_file_private *file_priv = file->driver_priv;
struct i915_gem_context *ctx;
+ mutex_init(&file_priv->vm_lock);
+
idr_init(&file_priv->context_idr);
+ idr_init_base(&file_priv->vm_idr, 1);
mutex_lock(&i915->drm.struct_mutex);
ctx = i915_gem_create_context(i915, file_priv);
mutex_unlock(&i915->drm.struct_mutex);
if (IS_ERR(ctx)) {
idr_destroy(&file_priv->context_idr);
+ idr_destroy(&file_priv->vm_idr);
return PTR_ERR(ctx);
}
@@ -627,6 +637,79 @@ void i915_gem_context_close(struct drm_file *file)
idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
idr_destroy(&file_priv->context_idr);
+
+ idr_for_each(&file_priv->vm_idr, vm_idr_cleanup, NULL);
+ idr_destroy(&file_priv->vm_idr);
+
+ mutex_destroy(&file_priv->vm_lock);
+}
+
+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. :)
+
+ 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?
+ 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;
+ 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?
+ 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;
?
}
static struct i915_request *
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 1e41a97b8007..9786f86b659d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -348,6 +348,11 @@ void i915_gem_context_release(struct kref *ctx_ref);
struct i915_gem_context *
i915_gem_context_create_gvt(struct drm_device *dev);
+int i915_gem_vm_create_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
+int i915_gem_vm_destroy_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
+
int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 03ade71b8d9a..61698609a62c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -396,6 +396,8 @@ struct i915_hw_ppgtt {
struct i915_page_directory_pointer pdp; /* GEN8+ */
struct i915_page_directory pd; /* GEN6-7 */
};
+
+ u32 user_handle;
};
struct gen6_hw_ppgtt {
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6ee2221838da..c3336c931a95 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -339,6 +339,8 @@ typedef struct _drm_i915_sarea {
#define DRM_I915_PERF_ADD_CONFIG 0x37
#define DRM_I915_PERF_REMOVE_CONFIG 0x38
#define DRM_I915_QUERY 0x39
+#define DRM_I915_GEM_VM_CREATE 0x3a
+#define DRM_I915_GEM_VM_DESTROY 0x3b
#define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
#define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
@@ -397,6 +399,8 @@ typedef struct _drm_i915_sarea {
#define DRM_IOCTL_I915_PERF_ADD_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config)
#define DRM_IOCTL_I915_PERF_REMOVE_CONFIG DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64)
#define DRM_IOCTL_I915_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY, struct drm_i915_query)
+#define DRM_IOCTL_I915_GEM_VM_CREATE DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_VM_CREATE, struct drm_i915_gem_vm_create)
+#define DRM_IOCTL_I915_GEM_VM_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_VM_DESTROY, uint32_t)
/* Allow drivers to submit batchbuffers directly to hardware, relying
* on the security mechanisms provided by hardware.
@@ -1442,6 +1446,12 @@ struct drm_i915_gem_context_destroy {
__u32 pad;
};
Some kernel doc overview of create and destroy usage, especially since
destroy does not have it's own data structure.
+struct drm_i915_gem_vm_create {
+ __u64 extensions;
+ __u32 flags;
+ __u32 id; /* output: id of new vm */
+};
+
struct drm_i915_reg_read {
/*
* Register offset.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
