[3/9] drm/vc4: Add create and map BO ioctls.
diff mbox

Message ID 1449002158-19156-3-git-send-email-eric@anholt.net
State New
Headers show

Commit Message

Eric Anholt Dec. 1, 2015, 8:35 p.m. UTC
While there exist dumb APIs for creating and mapping BOs, one of the
rules is that drivers doing 3D acceleration have to provide their own
APIs for buffer allocation (besides, the pitch/height parameters of
the dumb alloc don't really make sense for a lot of 3D allocations).

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_bo.c  | 41 ++++++++++++++++++++++++++
 drivers/gpu/drm/vc4/vc4_drv.c |  3 ++
 drivers/gpu/drm/vc4/vc4_drv.h |  4 +++
 include/uapi/drm/Kbuild       |  1 +
 include/uapi/drm/vc4_drm.h    | 68 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 117 insertions(+)
 create mode 100644 include/uapi/drm/vc4_drm.h

Comments

Emil Velikov Dec. 4, 2015, 2:29 p.m. UTC | #1
Hi Eric,

A couple of suggestions/requests on the UAPI header side

On 1 December 2015 at 20:35, Eric Anholt <eric@anholt.net> wrote:

> --- /dev/null
> +++ b/include/uapi/drm/vc4_drm.h

> +#include <drm/drm.h>
> +
Can we make this a "drm.h" ?


> +struct drm_vc4_create_bo {
> +       uint32_t size;
> +       uint32_t flags;
> +       /** Returned GEM handle for the BO. */
> +       uint32_t handle;
> +       uint32_t pad;
... and not use the stdint.h types but __{s,u}* through the file.

I'd rather than not nag why we want/need those, but if you prefer I
can repeat myself/others.

Thanks
Emil
Eric Anholt Dec. 8, 2015, 1:16 a.m. UTC | #2
Emil Velikov <emil.l.velikov@gmail.com> writes:

> Hi Eric,
>
> A couple of suggestions/requests on the UAPI header side
>
> On 1 December 2015 at 20:35, Eric Anholt <eric@anholt.net> wrote:
>
>> --- /dev/null
>> +++ b/include/uapi/drm/vc4_drm.h
>
>> +#include <drm/drm.h>
>> +
> Can we make this a "drm.h" ?

Nope.

include/uapi/drm/vc4_drm.h:27:17: fatal error: drm.h: No such file or directory

and none of the drivers do, either.

>> +struct drm_vc4_create_bo {
>> +       uint32_t size;
>> +       uint32_t flags;
>> +       /** Returned GEM handle for the BO. */
>> +       uint32_t handle;
>> +       uint32_t pad;
> ... and not use the stdint.h types but __{s,u}* through the file.
>
> I'd rather than not nag why we want/need those, but if you prefer I
> can repeat myself/others.

I've switched to using the special types.
Michel Dänzer Dec. 8, 2015, 2:56 a.m. UTC | #3
On 08.12.2015 10:16, Eric Anholt wrote:
> Emil Velikov <emil.l.velikov@gmail.com> writes:
> 
>> Hi Eric,
>>
>> A couple of suggestions/requests on the UAPI header side
>>
>> On 1 December 2015 at 20:35, Eric Anholt <eric@anholt.net> wrote:
>>
>>> --- /dev/null
>>> +++ b/include/uapi/drm/vc4_drm.h
>>
>>> +#include <drm/drm.h>
>>> +
>> Can we make this a "drm.h" ?
> 
> Nope.
> 
> include/uapi/drm/vc4_drm.h:27:17: fatal error: drm.h: No such file or directory

What happened to include/uapi/drm/drm.h in that tree?


> and none of the drivers do, either.

daenzer@kaveri|11:55:31> grep '#include "drm.h"' include/uapi/drm/*
include/uapi/drm/amdgpu_drm.h:#include "drm.h"
include/uapi/drm/radeon_drm.h:#include "drm.h"

Patches to convert the rest are pending.
Eric Anholt Dec. 8, 2015, 4:05 a.m. UTC | #4
Michel Dänzer <michel@daenzer.net> writes:

> On 08.12.2015 10:16, Eric Anholt wrote:
>> Emil Velikov <emil.l.velikov@gmail.com> writes:
>> 
>>> Hi Eric,
>>>
>>> A couple of suggestions/requests on the UAPI header side
>>>
>>> On 1 December 2015 at 20:35, Eric Anholt <eric@anholt.net> wrote:
>>>
>>>> --- /dev/null
>>>> +++ b/include/uapi/drm/vc4_drm.h
>>>
>>>> +#include <drm/drm.h>
>>>> +
>>> Can we make this a "drm.h" ?
>> 
>> Nope.
>> 
>> include/uapi/drm/vc4_drm.h:27:17: fatal error: drm.h: No such file or directory
>
> What happened to include/uapi/drm/drm.h in that tree?

Looks like it's just <drm.h> versus "drm.h" failure.  I've changed over
to "drm.h"

Patch
diff mbox

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 18faa5b..06cba26 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -19,6 +19,7 @@ 
  */
 
 #include "vc4_drv.h"
+#include "uapi/drm/vc4_drm.h"
 
 static void vc4_bo_stats_dump(struct vc4_dev *vc4)
 {
@@ -346,6 +347,46 @@  static void vc4_bo_cache_time_timer(unsigned long data)
 	schedule_work(&vc4->bo_cache.time_work);
 }
 
+int vc4_create_bo_ioctl(struct drm_device *dev, void *data,
+			struct drm_file *file_priv)
+{
+	struct drm_vc4_create_bo *args = data;
+	struct vc4_bo *bo = NULL;
+	int ret;
+
+	/*
+	 * We can't allocate from the BO cache, because the BOs don't
+	 * get zeroed, and that might leak data between users.
+	 */
+	bo = vc4_bo_create(dev, args->size, false);
+	if (!bo)
+		return -ENOMEM;
+
+	ret = drm_gem_handle_create(file_priv, &bo->base.base, &args->handle);
+	drm_gem_object_unreference_unlocked(&bo->base.base);
+
+	return ret;
+}
+
+int vc4_mmap_bo_ioctl(struct drm_device *dev, void *data,
+		      struct drm_file *file_priv)
+{
+	struct drm_vc4_mmap_bo *args = data;
+	struct drm_gem_object *gem_obj;
+
+	gem_obj = drm_gem_object_lookup(dev, file_priv, args->handle);
+	if (!gem_obj) {
+		DRM_ERROR("Failed to look up GEM BO %d\n", args->handle);
+		return -EINVAL;
+	}
+
+	/* The mmap offset was set up at BO allocation time. */
+	args->offset = drm_vma_node_offset_addr(&gem_obj->vma_node);
+
+	drm_gem_object_unreference_unlocked(gem_obj);
+	return 0;
+}
+
 void vc4_bo_cache_init(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index da041fa..5fa4688 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -16,6 +16,7 @@ 
 #include <linux/platform_device.h>
 #include "drm_fb_cma_helper.h"
 
+#include "uapi/drm/vc4_drm.h"
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 
@@ -73,6 +74,8 @@  static const struct file_operations vc4_drm_fops = {
 };
 
 static const struct drm_ioctl_desc vc4_drm_ioctls[] = {
+	DRM_IOCTL_DEF_DRV(VC4_CREATE_BO, vc4_create_bo_ioctl, 0),
+	DRM_IOCTL_DEF_DRV(VC4_MMAP_BO, vc4_mmap_bo_ioctl, 0),
 };
 
 static struct drm_driver vc4_drm_driver = {
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 39a1ff5..fddb0a0 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -155,6 +155,10 @@  int vc4_dumb_create(struct drm_file *file_priv,
 		    struct drm_mode_create_dumb *args);
 struct dma_buf *vc4_prime_export(struct drm_device *dev,
 				 struct drm_gem_object *obj, int flags);
+int vc4_create_bo_ioctl(struct drm_device *dev, void *data,
+			struct drm_file *file_priv);
+int vc4_mmap_bo_ioctl(struct drm_device *dev, void *data,
+		      struct drm_file *file_priv);
 void vc4_bo_cache_init(struct drm_device *dev);
 void vc4_bo_cache_destroy(struct drm_device *dev);
 int vc4_bo_stats_debugfs(struct seq_file *m, void *arg);
diff --git a/include/uapi/drm/Kbuild b/include/uapi/drm/Kbuild
index 38d4370..974fcd5 100644
--- a/include/uapi/drm/Kbuild
+++ b/include/uapi/drm/Kbuild
@@ -17,4 +17,5 @@  header-y += tegra_drm.h
 header-y += via_drm.h
 header-y += vmwgfx_drm.h
 header-y += msm_drm.h
+header-y += vc4_drm.h
 header-y += virtgpu_drm.h
diff --git a/include/uapi/drm/vc4_drm.h b/include/uapi/drm/vc4_drm.h
new file mode 100644
index 0000000..068aab9
--- /dev/null
+++ b/include/uapi/drm/vc4_drm.h
@@ -0,0 +1,68 @@ 
+/*
+ * Copyright © 2014-2015 Broadcom
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef _UAPI_VC4_DRM_H_
+#define _UAPI_VC4_DRM_H_
+
+#include <drm/drm.h>
+
+#define DRM_VC4_CREATE_BO                         0x03
+#define DRM_VC4_MMAP_BO                           0x04
+
+#define DRM_IOCTL_VC4_CREATE_BO           DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_CREATE_BO, struct drm_vc4_create_bo)
+#define DRM_IOCTL_VC4_MMAP_BO             DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_MMAP_BO, struct drm_vc4_mmap_bo)
+
+/**
+ * struct drm_vc4_create_bo - ioctl argument for creating VC4 BOs.
+ *
+ * There are currently no values for the flags argument, but it may be
+ * used in a future extension.
+ */
+struct drm_vc4_create_bo {
+	uint32_t size;
+	uint32_t flags;
+	/** Returned GEM handle for the BO. */
+	uint32_t handle;
+	uint32_t pad;
+};
+
+/**
+ * struct drm_vc4_mmap_bo - ioctl argument for mapping VC4 BOs.
+ *
+ * This doesn't actually perform an mmap.  Instead, it returns the
+ * offset you need to use in an mmap on the DRM device node.  This
+ * means that tools like valgrind end up knowing about the mapped
+ * memory.
+ *
+ * There are currently no values for the flags argument, but it may be
+ * used in a future extension.
+ */
+struct drm_vc4_mmap_bo {
+	/** Handle for the object being mapped. */
+	uint32_t handle;
+	uint32_t flags;
+	/** offset into the drm node to use for subsequent mmap call. */
+	uint64_t offset;
+};
+
+#endif /* _UAPI_VC4_DRM_H_ */