drm/tegra: Preparation work for destaging ABI

From: Thierry Reding <[email protected]>

A separate data structure exists for the DRM_TEGRA_CLOSE_CHANNEL IOCTL, but it is currently unused. The IOCTL was using the data structure for the DRM_TEGRA_OPEN_CHANNEL IOCTL.

Signed-off-by Thierry Reding <[email protected]>

---
 include/uapi/drm/tegra_drm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/uapi/drm/tegra_drm.h +0 -0

index d954f8c33321..99e15d82d1e9 100644

From: Thierry Reding <[email protected]>

Set the owner and name of the exported DMA-BUF in addition to the already filled-in fields.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/gem.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/gpu/drm/tegra/gem.c +0 -0

index 8b0b4ff64bb4..387ba1dfbe0d 100644

From: Thierry Reding <[email protected]>

Currently only the DRM_MODE_REFLECT_Y rotation is supported. The driver already supports reflection on the Y axis via a custom flag which is not very useful because it requires custom userspace. Replace that flag by a standard rotation property that supports 0 degree rotation and Y axis reflection.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 18 +++++++++++++++++-
 drivers/gpu/drm/tegra/drm.h   |  1 -
 drivers/gpu/drm/tegra/fb.c    | 10 ----------
 drivers/gpu/drm/tegra/plane.c |  1 +
 drivers/gpu/drm/tegra/plane.h |  2 ++
 5 files changed, 20 insertions(+), 12 deletions(-)

drivers/gpu/drm/tegra/dc.c +8 -0

index 9f83a65b5ea9..e2e574202ca3 100644
@@ -451,6 +451,7 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
 				    struct drm_plane_state *state)
 {
 	struct tegra_plane_state *plane_state = to_tegra_plane_state(state);
+	unsigned int rotation = DRM_MODE_ROTATE_0 | DRM_MODE_REFLECT_Y;
 	struct tegra_bo_tiling *tiling = &plane_state->tiling;
 	struct tegra_plane *tegra = to_tegra_plane(plane);
 	struct tegra_dc *dc = to_tegra_dc(state->crtc);

@@ -498,6 +499,13 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
+	rotation = drm_rotation_simplify(state->rotation, rotation);
+
+	if (rotation & DRM_MODE_REFLECT_Y)
+		plane_state->bottom_up = true;
+	else
+		plane_state->bottom_up = false;
+
 	/*
 	 * Tegra doesn't support different strides for U and V planes so we
 	 * error out if the user tries to display a framebuffer with such a

drivers/gpu/drm/tegra/dc.c +0 -0

 	/* copy from state */
 	window.zpos = plane->state->normalized_zpos;

@@ -643,6 +651,14 @@ static struct drm_plane *tegra_primary_plane_create(struct drm_device *drm,
 	if (dc->soc->supports_blending)

drivers/gpu/drm/tegra/dc.c +0 -0

 	return &plane->base;
 }
 

drivers/gpu/drm/tegra/drm.h +0 -1

index 4f41aaec8530..84930d13e28d 100644
@@ -177,7 +177,6 @@ int drm_dp_aux_train(struct drm_dp_aux *aux, struct drm_dp_link *link,
 /* from fb.c */
 struct tegra_bo *tegra_fb_get_plane(struct drm_framebuffer *framebuffer,
 				    unsigned int index);
-bool tegra_fb_is_bottom_up(struct drm_framebuffer *framebuffer);
 int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
 			struct tegra_bo_tiling *tiling);
 struct drm_framebuffer *tegra_fb_create(struct drm_device *drm,

drivers/gpu/drm/tegra/fb.c +0 -10

index e69434909a42..35827c729462 100644
@@ -38,16 +38,6 @@ struct tegra_bo *tegra_fb_get_plane(struct drm_framebuffer *framebuffer,
 	return fb->planes[index];
 }
 
-bool tegra_fb_is_bottom_up(struct drm_framebuffer *framebuffer)
-{
-	struct tegra_fb *fb = to_tegra_fb(framebuffer);
-
-	if (fb->planes[0]->flags & TEGRA_BO_BOTTOM_UP)
-		return true;
-
-	return false;
-}
-
 int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
 			struct tegra_bo_tiling *tiling)
 {

drivers/gpu/drm/tegra/plane.c +1 -0

index 176ef46c615c..271729b2c653 100644
@@ -53,6 +53,7 @@ tegra_plane_atomic_duplicate_state(struct drm_plane *plane)
 	copy->tiling = state->tiling;
 	copy->format = state->format;
 	copy->swap = state->swap;
+	copy->bottom_up = state->bottom_up;
 	copy->opaque = state->opaque;
 
 	for (i = 0; i < 3; i++)

drivers/gpu/drm/tegra/plane.h +0 -0

index 6938719e7e5d..867b9c32b1b6 100644

From: Thierry Reding <[email protected]>

Set the interface version implemented by the gr2d module. This allows userspace to pass the correct command stream when programming the gr2d module.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/gr2d.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

drivers/gpu/drm/tegra/gr2d.c +10 -0

index 9a8ea93016a9..d13752b0d915 100644
@@ -7,16 +7,23 @@
  */
 
 #include <linux/clk.h>
+#include <linux/of_device.h>
 
 #include "drm.h"
 #include "gem.h"
 #include "gr2d.h"
 
+struct gr2d_soc {
+	unsigned int version;
+};
+
 struct gr2d {
 	struct tegra_drm_client client;
 	struct host1x_channel *channel;
 	struct clk *clk;
 
+	const struct gr2d_soc *soc;
+
 	DECLARE_BITMAP(addr_regs, GR2D_NUM_REGS);
 };
 

@@ -123,9 +130,17 @@ static const struct tegra_drm_client_ops gr2d_ops = {
 	.submit = tegra_drm_submit,
 };
 
+static const struct gr2d_soc tegra20_gr2d_soc = {
+	.version = 0x20,
+};

drivers/gpu/drm/tegra/gr2d.c +2 -0

 	{ },
 };
 MODULE_DEVICE_TABLE(of, gr2d_match);

@@ -158,6 +173,8 @@ static int gr2d_probe(struct platform_device *pdev)
 	if (!gr2d)
 		return -ENOMEM;
 
+	gr2d->soc = of_device_get_match_data(dev);
+
 	syncpts = devm_kzalloc(dev, sizeof(*syncpts), GFP_KERNEL);
 	if (!syncpts)
 		return -ENOMEM;

From: Thierry Reding <[email protected]>

Set the interface version implemented by the gr3d module. This allows userspace to pass the correct command stream when programming the gr3d module.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/gr3d.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

drivers/gpu/drm/tegra/gr3d.c +24 -3

index 28c4ef63065b..0d4dce978eb2 100644
@@ -10,6 +10,7 @@
 #include <linux/clk.h>
 #include <linux/host1x.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 

@@ -19,6 +20,10 @@
 #include "gem.h"
 #include "gr3d.h"
 
+struct gr3d_soc {
+	unsigned int version;
+};
+
 struct gr3d {
 	struct tegra_drm_client client;
 	struct host1x_channel *channel;

@@ -27,6 +32,8 @@ struct gr3d {
 	struct reset_control *rst_secondary;
 	struct reset_control *rst;
 
+	const struct gr3d_soc *soc;
+
 	DECLARE_BITMAP(addr_regs, GR3D_NUM_REGS);
 };
 

@@ -125,10 +132,22 @@ static const struct tegra_drm_client_ops gr3d_ops = {
 	.submit = tegra_drm_submit,
 };
 
+static const struct gr3d_soc tegra20_gr3d_soc = {
+	.version = 0x20,
+};
+
+static const struct gr3d_soc tegra30_gr3d_soc = {
+	.version = 0x30,
+};
+
+static const struct gr3d_soc tegra114_gr3d_soc = {
+	.version = 0x35,
+};
+
 static const struct of_device_id tegra_gr3d_match[] = {
-	{ .compatible = "nvidia,tegra114-gr3d" },
-	{ .compatible = "nvidia,tegra30-gr3d" },
-	{ .compatible = "nvidia,tegra20-gr3d" },
+	{ .compatible = "nvidia,tegra114-gr3d", .data = &tegra114_gr3d_soc },
+	{ .compatible = "nvidia,tegra30-gr3d", .data = &tegra30_gr3d_soc },
+	{ .compatible = "nvidia,tegra20-gr3d", .data = &tegra20_gr3d_soc },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, tegra_gr3d_match);

@@ -250,6 +269,8 @@ static int gr3d_probe(struct platform_device *pdev)
 	if (!gr3d)
 		return -ENOMEM;
 
+	gr3d->soc = of_device_get_match_data(&pdev->dev);
+
 	syncpts = devm_kzalloc(&pdev->dev, sizeof(*syncpts), GFP_KERNEL);
 	if (!syncpts)
 		return -ENOMEM;

From: Thierry Reding <[email protected]>

Set the interface version implemented by the VIC module. This allows userspace to pass the correct command stream when programming the VIC module.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/vic.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/gpu/drm/tegra/vic.c +4 -0

index f5794dd49f3b..d739403045aa 100644
@@ -25,6 +25,7 @@
 
 struct vic_config {
 	const char *firmware;
+	unsigned int version;
 };
 
 struct vic {

@@ -264,18 +265,21 @@ static const struct tegra_drm_client_ops vic_ops = {
 
 static const struct vic_config vic_t124_config = {
 	.firmware = NVIDIA_TEGRA_124_VIC_FIRMWARE,
+	.version = 0x40,
 };
 
 #define NVIDIA_TEGRA_210_VIC_FIRMWARE "nvidia/tegra210/vic04_ucode.bin"
 
 static const struct vic_config vic_t210_config = {
 	.firmware = NVIDIA_TEGRA_210_VIC_FIRMWARE,
+	.version = 0x21,
 };
 
 #define NVIDIA_TEGRA_186_VIC_FIRMWARE "nvidia/tegra186/vic04_ucode.bin"
 
 static const struct vic_config vic_t186_config = {
 	.firmware = NVIDIA_TEGRA_186_VIC_FIRMWARE,
+	.version = 0x18,
 };
 
 static const struct of_device_id vic_match[] = {

From: Thierry Reding <[email protected]>

Document the userspace ABI with kerneldoc to provide some information on how to use it.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/gem.c  |   4 +-
 include/uapi/drm/tegra_drm.h | 480 ++++++++++++++++++++++++++++++++++-
 2 files changed, 468 insertions(+), 16 deletions(-)

drivers/gpu/drm/tegra/gem.c +2 -2

index 387ba1dfbe0d..e2987a19541d 100644
@@ -291,10 +291,10 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, size_t size,
 	if (err < 0)
 		goto release;
 
-	if (flags & DRM_TEGRA_GEM_CREATE_TILED)
+	if (flags & DRM_TEGRA_GEM_TILED)
 		bo->tiling.mode = TEGRA_BO_TILING_MODE_TILED;
 
-	if (flags & DRM_TEGRA_GEM_CREATE_BOTTOM_UP)
+	if (flags & DRM_TEGRA_GEM_BOTTOM_UP)
 		bo->flags |= TEGRA_BO_BOTTOM_UP;
 
 	return bo;

include/uapi/drm/tegra_drm.h +0 -0

index 99e15d82d1e9..86a7b1e7559d 100644

include/uapi/drm/tegra_drm.h +52 -0

+/**
+ * struct drm_tegra_gem_create - parameters for the GEM object creation IOCTL
+ */
 struct drm_tegra_gem_create {
+	/**
+	 * @size:
+	 *
+	 * The size, in bytes, of the buffer object to be created.
+	 */
 	__u64 size;
+
+	/**
+	 * @flags:
+	 *
+	 * A bitmask of flags that influence the creation of GEM objects:
+	 *
+	 * DRM_TEGRA_GEM_TILED
+	 *   Use the 16x16 tiling format for this buffer.
+	 *
+	 * DRM_TEGRA_GEM_BOTTOM_UP
+	 *   The buffer has a bottom-up layout.
+	 */
 	__u32 flags;
+
+	/**
+	 * @handle:
+	 *
+	 * Return location for the handle of the created GEM object.
+	 */
 	__u32 handle;
 };
 
+/**
+ * struct drm_tegra_gem_mmap - parameters for the GEM mmap IOCTL
+ */
 struct drm_tegra_gem_mmap {
+	/**
+	 * @handle:
+	 *
+	 * Handle of the GEM object to obtain an mmap offset for.
+	 */
 	__u32 handle;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
+
+	/**
+	 * @offset:
+	 *
+	 * Return location for the mmap offset for the given GEM object.
+	 */
 	__u64 offset;
 };
 
+/**
+ * struct drm_tegra_syncpt_read - parameters for the read syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_read {
+	/**
+	 * @id:
+	 *

include/uapi/drm/tegra_drm.h +23 -0

 	__u32 id;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_syncpt_wait - parameters for the wait syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_wait {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to wait on.
+	 */
 	__u32 id;
+
+	/**
+	 * @thresh:
+	 *
+	 * Threshold value for which to wait.
+	 */
 	__u32 thresh;
+
+	/**
+	 * @timeout:

include/uapi/drm/tegra_drm.h +38 -0

 	__u32 value;
 };
 
 #define DRM_TEGRA_NO_TIMEOUT	(0xffffffff)
 
+/**
+ * struct drm_tegra_open_channel - parameters for the open channel IOCTL
+ */
 struct drm_tegra_open_channel {
+	/**
+	 * @client:
+	 *
+	 * The client ID for this channel.
+	 */
 	__u32 client;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
+
+	/**
+	 * @context:
+	 *
+	 * Return location for the application context of this channel. This
+	 * context needs to be passed to the DRM_TEGRA_CHANNEL_CLOSE or the
+	 * DRM_TEGRA_SUBMIT IOCTLs.
+	 */
 	__u64 context;
 };
 
+/**
+ * struct drm_tegra_close_channel - parameters for the close channel IOCTL
+ */
 struct drm_tegra_close_channel {
+	/**
+	 * @context:
+	 *
+	 * The application context of this channel. This is obtained from the
+	 * DRM_TEGRA_OPEN_CHANNEL IOCTL.
+	 */
 	__u64 context;
 };
 
+/**
+ * struct drm_tegra_get_syncpt - parameters for the get syncpoint IOCTL
+ */
 struct drm_tegra_get_syncpt {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying the channel for which to obtain

include/uapi/drm/tegra_drm.h +73 -0

+
+	/**
+	 * @id:
+	 *
+	 * Return location for the ID of the given syncpoint.
+	 */
 	__u32 id;
 };>
+/**
+ * struct drm_tegra_get_syncpt_base - parameters for the get wait base IOCTL
+ */
 struct drm_tegra_get_syncpt_base {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying for which channel to obtain the
+	 * wait base.
+	 */
 	__u64 context;
+
+	/**
+	 * @syncpt:
+	 *
+	 * ID of the syncpoint for which to obtain the wait base.
+	 */
 	__u32 syncpt;
+
+	/**
+	 * @id:
+	 *
+	 * Return location for the ID of the wait base.
+	 */
 	__u32 id;
 };
 
+/**
+ * struct drm_tegra_syncpt - syncpoint increment operation
+ */
 struct drm_tegra_syncpt {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to operate on.
+	 */
 	__u32 id;
+
+	/**
+	 * @incrs:
+	 *
+	 * Number of increments to perform for the syncpoint.
+	 */
 	__u32 incrs;
 };
 
+/**
+ * struct drm_tegra_cmdbuf - structure describing a command buffer
+ */
 struct drm_tegra_cmdbuf {
+	/**
+	 * @handle:
+	 *
+	 * Handle to a GEM object containing the command buffer.
+	 */
 	__u32 handle;
+
+	/**
+	 * @offset:
+	 *
+	 * Offset, in bytes, into the GEM object identified by @handle at
+	 * which the command buffer starts.
+	 */
 	__u32 offset;
+
+	/**
+	 * @words:
+	 *
+	 * Number of 32-bit words in this command buffer.
+	 */
 	__u32 words;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_reloc - GEM object relocation structure
+ */
 struct drm_tegra_reloc {
 	struct {
+		/**
+		 * @cmdbuf.handle:

include/uapi/drm/tegra_drm.h +3 -0

+		 * relocated address.
+		 */
 		__u32 offset;
 	} cmdbuf;
 	struct {
+		/**

include/uapi/drm/tegra_drm.h +90 -0

+		 * data starts.
+		 */
 		__u32 offset;
 	} target;
+
+	/**
+	 * @shift:
+	 *
+	 * The number of bits by which to shift relocated addresses.
+	 */
 	__u32 shift;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_waitchk - wait check structure
+ */
 struct drm_tegra_waitchk {
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object containing a command stream on which to
+	 * perform the wait check.
+	 */
 	__u32 handle;
+
+	/**
+	 * @offset:
+	 *
+	 * Offset, in bytes, of the location in the command stream to perform
+	 * the wait check on.
+	 */
 	__u32 offset;
+
+	/**
+	 * @syncpt:
+	 *
+	 * ID of the syncpoint to wait check.
+	 */
 	__u32 syncpt;
+
+	/**
+	 * @thresh:
+	 *
+	 * Threshold value for which to check.
+	 */
 	__u32 thresh;
 };
 
+/**
+ * struct drm_tegra_submit - job submission structure
+ */
 struct drm_tegra_submit {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying the channel to use for the
+	 * execution of this job.
+	 */
 	__u64 context;
+
+	/**
+	 * @num_syncpts:
+	 *
+	 * The number of syncpoints operated on by this job.
+	 */
 	__u32 num_syncpts;
+
+	/**
+	 * @num_cmdbufs:
+	 *
+	 * The number of command buffers to execute as part of this job.
+	 */
 	__u32 num_cmdbufs;
+
+	/**
+	 * @num_relocs:
+	 *
+	 * The number of relocations to perform before executing this job.
+	 */
 	__u32 num_relocs;
+
+	/**
+	 * @num_waitchks:
+	 *
+	 * The number of wait checks to perform as part of this job.
+	 */
 	__u32 num_waitchks;
+
+	/**
+	 * @waitchk_mask:
+	 *
+	 * Bitmask of valid wait checks.
+	 */
 	__u32 waitchk_mask;
+
+	/**
+	 * @timeout:
+	 *
+	 * Timeout, in milliseconds, before this job is cancelled.
+	 */
 	__u32 timeout;
+
+	/**
+	 * @syncpts:

include/uapi/drm/tegra_drm.h +106 -12

 	__u64 relocs;
+
+	/**
+	 * @waitchks:
+	 *
+	 * A pointer to @num_waitchks &struct drm_tegra_waitchk structures
+	 * that specify the wait checks to be performed while executing this
+	 * job.
+	 */
 	__u64 waitchks;
-	__u32 fence;		/* Return value */
 
-	__u32 reserved[5];	/* future expansion */
+	/**
+	 * @fence:
+	 *
+	 * Return location for the threshold of the syncpoint associated with
+	 * this job. This can be used with the DRM_TEGRA_SYNCPT_WAIT IOCTL to
+	 * wait for this job to be finished.
+	 */
+	__u32 fence;
+
+	/**
+	 * @reserved:
+	 *
+	 * This field is reserved for future use. Must be 0.
+	 */
+	__u32 reserved[5];
 };
 
 #define DRM_TEGRA_GEM_TILING_MODE_PITCH 0
 #define DRM_TEGRA_GEM_TILING_MODE_TILED 1
 #define DRM_TEGRA_GEM_TILING_MODE_BLOCK 2
 
+/**
+ * struct drm_tegra_gem_set_tiling - parameters for the set tiling IOCTL
+ */
 struct drm_tegra_gem_set_tiling {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to set the tiling parameters.
+	 */
 	__u32 handle;
+
+	/**
+	 * @mode:
+	 *
+	 * The tiling mode to set. Must be one of:
+	 *
+	 * DRM_TEGRA_GEM_TILING_MODE_PITCH
+	 *   pitch linear format
+	 *
+	 * DRM_TEGRA_GEM_TILING_MODE_TILED
+	 *   16x16 tiling format
+	 *
+	 * DRM_TEGRA_GEM_TILING_MODE_BLOCK
+	 *   16Bx2 tiling format
+	 */
 	__u32 mode;
+
+	/**
+	 * @value:
+	 *
+	 * The value to set for the tiling mode parameter.
+	 */
 	__u32 value;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_gem_get_tiling - parameters for the get tiling IOCTL
+ */
 struct drm_tegra_gem_get_tiling {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to query the tiling parameters.
+	 */
 	__u32 handle;
-	/* output */
+
+	/**
+	 * @mode:
+	 *
+	 * Return location for the tiling mode.
+	 */
 	__u32 mode;
+
+	/**
+	 * @value:
+	 *
+	 * Return location for the tiling mode parameter.
+	 */
 	__u32 value;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
-#define DRM_TEGRA_GEM_BOTTOM_UP		(1 << 0)
-#define DRM_TEGRA_GEM_FLAGS		(DRM_TEGRA_GEM_BOTTOM_UP)
-
+/**
+ * struct drm_tegra_gem_set_flags - parameters for the set flags IOCTL
+ */
 struct drm_tegra_gem_set_flags {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to set the flags.
+	 */
 	__u32 handle;
-	/* output */
+
+	/**
+	 * @flags:
+	 *
+	 * The flags to set for the GEM object.
+	 */
 	__u32 flags;
 };
 
+/**
+ * struct drm_tegra_gem_get_flags - parameters for the get flags IOCTL
+ */
 struct drm_tegra_gem_get_flags {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to query the flags.
+	 */
 	__u32 handle;
-	/* output */

Quoted 36 lines · Thierry Reding

From: Thierry Reding <[email protected]>

Document the userspace ABI with kerneldoc to provide some information on how to use it.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/gem.c  |   4 +-
 include/uapi/drm/tegra_drm.h | 480 ++++++++++++++++++++++++++++++++++-
 2 files changed, 468 insertions(+), 16 deletions(-)

drivers/gpu/drm/tegra/gem.c +2 -2

index 387ba1dfbe0d..e2987a19541d 100644
@@ -291,10 +291,10 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, size_t size,
 	if (err < 0)
 		goto release;
 
-	if (flags & DRM_TEGRA_GEM_CREATE_TILED)
+	if (flags & DRM_TEGRA_GEM_TILED)
 		bo->tiling.mode = TEGRA_BO_TILING_MODE_TILED;
 
-	if (flags & DRM_TEGRA_GEM_CREATE_BOTTOM_UP)
+	if (flags & DRM_TEGRA_GEM_BOTTOM_UP)
 		bo->flags |= TEGRA_BO_BOTTOM_UP;
 
 	return bo;

include/uapi/drm/tegra_drm.h +0 -0

index 99e15d82d1e9..86a7b1e7559d 100644
@@ -29,146 +29,598 @@
 extern "C" {
 #endif
 

include/uapi/drm/tegra_drm.h line 35

Quoted 6 lines

include/uapi/drm/tegra_drm.h +4 -2

-#define DRM_TEGRA_GEM_CREATE_TILED     (1 << 0)
-#define DRM_TEGRA_GEM_CREATE_BOTTOM_UP (1 << 1)
+#define DRM_TEGRA_GEM_TILED		(1 << 0)
+#define DRM_TEGRA_GEM_BOTTOM_UP		(1 << 1)
+#define DRM_TEGRA_GEM_FLAGS		(DRM_TEGRA_GEM_TILED | \
+					 DRM_TEGRA_GEM_BOTTOM_UP)

The current userspace won't compile with the above changes, so this is the ABI breakage. Please keep the old DRM_TEGRA_GEM_CREATE_* DRM_TEGRA_GEM_* flags for now.

Quoted 7 lines

include/uapi/drm/tegra_drm.h +6 -0

+/**
+ * struct drm_tegra_syncpt_read - parameters for the read syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_read {
+	/**
+	 * @id:
+	 *

include/uapi/drm/tegra_drm.h line 112

Quoted 10 lines

include/uapi/drm/tegra_drm.h +8 -0

+	 * ID of the syncpoint to read the current value from.
+	 */
 	__u32 id;
+
+	/**
+	 * @value:
+	 *
+	 * Return location for the current syncpoint value.
+	 */>  	__u32 value;
 };

What about:

Returned value of the given syncpoint.

Could we replace "return location for the.." with just "Returned.." in other places too? My mind is stuttering while reading "location for the value", though I know that my english isn't ideal and maybe it's only me.

include/uapi/drm/tegra_drm.h line 122

Quoted 10 lines

include/uapi/drm/tegra_drm.h +8 -0

+/**
+ * struct drm_tegra_syncpt_incr - parameters for the increment syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_incr {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to read the current value from.
+	 */

Cut-n-pasted comment. Change to:

ID of the syncpoint to increment.

Quoted 30 lines

include/uapi/drm/tegra_drm.h +23 -0

 	__u32 id;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_syncpt_wait - parameters for the wait syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_wait {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to wait on.
+	 */
 	__u32 id;
+
+	/**
+	 * @thresh:
+	 *
+	 * Threshold value for which to wait.
+	 */
 	__u32 thresh;
+
+	/**
+	 * @timeout:

include/uapi/drm/tegra_drm.h line 162

Quoted 10 lines

include/uapi/drm/tegra_drm.h +9 -0

+	 *
+	 * Timeout, in milliseconds, to wait.
+	 */
 	__u32 timeout;
+
+	/**
+	 * @value:
+	 *
+	 * Return value for the current syncpoint value.
+	 */

Just:

Returned value of the syncpoint.

Quoted 54 lines

include/uapi/drm/tegra_drm.h +38 -0

 	__u32 value;
 };
 
 #define DRM_TEGRA_NO_TIMEOUT	(0xffffffff)
 
+/**
+ * struct drm_tegra_open_channel - parameters for the open channel IOCTL
+ */
 struct drm_tegra_open_channel {
+	/**
+	 * @client:
+	 *
+	 * The client ID for this channel.
+	 */
 	__u32 client;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
+
+	/**
+	 * @context:
+	 *
+	 * Return location for the application context of this channel. This
+	 * context needs to be passed to the DRM_TEGRA_CHANNEL_CLOSE or the
+	 * DRM_TEGRA_SUBMIT IOCTLs.
+	 */
 	__u64 context;
 };
 
+/**
+ * struct drm_tegra_close_channel - parameters for the close channel IOCTL
+ */
 struct drm_tegra_close_channel {
+	/**
+	 * @context:
+	 *
+	 * The application context of this channel. This is obtained from the
+	 * DRM_TEGRA_OPEN_CHANNEL IOCTL.
+	 */
 	__u64 context;
 };
 
+/**
+ * struct drm_tegra_get_syncpt - parameters for the get syncpoint IOCTL
+ */
 struct drm_tegra_get_syncpt {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying the channel for which to obtain

include/uapi/drm/tegra_drm.h line 226

Quoted 10 lines

include/uapi/drm/tegra_drm.h +8 -0

+	 * the syncpoint ID.
+	 */
 	__u64 context;
+
+	/**
+	 * @index:
+	 *
+	 * Channel-relative index of the syncpoint for which to obtain the ID.
+	 */
 	__u32 index;

Client-relative. And what about:

Clients syncpoint index for which to obtain the ID.

Quoted 99 lines

include/uapi/drm/tegra_drm.h +77 -0

+
+	/**
+	 * @id:
+	 *
+	 * Return location for the ID of the given syncpoint.
+	 */
 	__u32 id;
 };>
+/**
+ * struct drm_tegra_get_syncpt_base - parameters for the get wait base IOCTL
+ */
 struct drm_tegra_get_syncpt_base {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying for which channel to obtain the
+	 * wait base.
+	 */
 	__u64 context;
+
+	/**
+	 * @syncpt:
+	 *
+	 * ID of the syncpoint for which to obtain the wait base.
+	 */
 	__u32 syncpt;
+
+	/**
+	 * @id:
+	 *
+	 * Return location for the ID of the wait base.
+	 */
 	__u32 id;
 };
 
+/**
+ * struct drm_tegra_syncpt - syncpoint increment operation
+ */
 struct drm_tegra_syncpt {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to operate on.
+	 */
 	__u32 id;
+
+	/**
+	 * @incrs:
+	 *
+	 * Number of increments to perform for the syncpoint.
+	 */
 	__u32 incrs;
 };
 
+/**
+ * struct drm_tegra_cmdbuf - structure describing a command buffer
+ */
 struct drm_tegra_cmdbuf {
+	/**
+	 * @handle:
+	 *
+	 * Handle to a GEM object containing the command buffer.
+	 */
 	__u32 handle;
+
+	/**
+	 * @offset:
+	 *
+	 * Offset, in bytes, into the GEM object identified by @handle at
+	 * which the command buffer starts.
+	 */
 	__u32 offset;
+
+	/**
+	 * @words:
+	 *
+	 * Number of 32-bit words in this command buffer.
+	 */
 	__u32 words;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_reloc - GEM object relocation structure
+ */
 struct drm_tegra_reloc {
 	struct {
+		/**
+		 * @cmdbuf.handle:
+		 *
+		 * Handle to the GEM object containing the command buffer for
+		 * which to perform this GEM object relocation.
+		 */

include/uapi/drm/tegra_drm.h line 332

Quoted 6 lines

include/uapi/drm/tegra_drm.h +5 -0

 		__u32 handle;
+
+		/**
+		 * @cmdbuf.offset:
+		 *
+		 * Offset into the command buffer at which to insert the the

Offset, in bytes,

Quoted 10 lines

include/uapi/drm/tegra_drm.h +7 -0

+		 * relocated address.
+		 */
 		__u32 offset;
 	} cmdbuf;
 	struct {
+		/**
+		 * @target.handle:
+		 *
+		 * Handle to the GEM object to be relocated.
+		 */

include/uapi/drm/tegra_drm.h line 348

Quoted 6 lines

include/uapi/drm/tegra_drm.h +5 -0

 		__u32 handle;
+
+		/**
+		 * @target.offset:
+		 *
+		 * Offset into the target GEM object at which the relocated

Offset, in bytes,

Quoted 115 lines

include/uapi/drm/tegra_drm.h +94 -0

+		 * data starts.
+		 */
 		__u32 offset;
 	} target;
+
+	/**
+	 * @shift:
+	 *
+	 * The number of bits by which to shift relocated addresses.
+	 */
 	__u32 shift;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_waitchk - wait check structure
+ */
 struct drm_tegra_waitchk {
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object containing a command stream on which to
+	 * perform the wait check.
+	 */
 	__u32 handle;
+
+	/**
+	 * @offset:
+	 *
+	 * Offset, in bytes, of the location in the command stream to perform
+	 * the wait check on.
+	 */
 	__u32 offset;
+
+	/**
+	 * @syncpt:
+	 *
+	 * ID of the syncpoint to wait check.
+	 */
 	__u32 syncpt;
+
+	/**
+	 * @thresh:
+	 *
+	 * Threshold value for which to check.
+	 */
 	__u32 thresh;
 };
 
+/**
+ * struct drm_tegra_submit - job submission structure
+ */
 struct drm_tegra_submit {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying the channel to use for the
+	 * execution of this job.
+	 */
 	__u64 context;
+
+	/**
+	 * @num_syncpts:
+	 *
+	 * The number of syncpoints operated on by this job.
+	 */
 	__u32 num_syncpts;
+
+	/**
+	 * @num_cmdbufs:
+	 *
+	 * The number of command buffers to execute as part of this job.
+	 */
 	__u32 num_cmdbufs;
+
+	/**
+	 * @num_relocs:
+	 *
+	 * The number of relocations to perform before executing this job.
+	 */
 	__u32 num_relocs;
+
+	/**
+	 * @num_waitchks:
+	 *
+	 * The number of wait checks to perform as part of this job.
+	 */
 	__u32 num_waitchks;
+
+	/**
+	 * @waitchk_mask:
+	 *
+	 * Bitmask of valid wait checks.
+	 */
 	__u32 waitchk_mask;
+
+	/**
+	 * @timeout:
+	 *
+	 * Timeout, in milliseconds, before this job is cancelled.
+	 */
 	__u32 timeout;
+
+	/**
+	 * @syncpts:
+	 *
+	 * A pointer to @num_syncpts &struct drm_tegra_syncpt structures that
+	 * specify the syncpoint operations performed as part of this job.
+	 */

include/uapi/drm/tegra_drm.h line 469

Quoted 6 lines

include/uapi/drm/tegra_drm.h +5 -0

 	__u64 syncpts;
+
+	/**
+	 * @cmdbufs:
+	 *
+	 * A pointer to @num_cmdbufs &struct drm_tegra_cmdbuf_legacy

We don't have drm_tegra_cmdbuf_legacy yet, should be drm_tegra_cmdbuf.

Quoted 6 lines

include/uapi/drm/tegra_drm.h +5 -0

+	 * structures that define the command buffers to execute as part of
+	 * this job.
+	 */
 	__u64 cmdbufs;
+
+	/**

include/uapi/drm/tegra_drm.h line 481

Quoted 6 lines

include/uapi/drm/tegra_drm.h +6 -0

+	 * @relocs:
+	 *
+	 * A pointer to @num_relocs &struct drm_tegra_reloc_legacy structures
+	 * that specify the relocations that need to be performed before
+	 * executing this job.
+	 */

Same as for drm_tegra_cmdbuf_legacy.

Quoted 149 lines

include/uapi/drm/tegra_drm.h +107 -12

 	__u64 relocs;
+
+	/**
+	 * @waitchks:
+	 *
+	 * A pointer to @num_waitchks &struct drm_tegra_waitchk structures
+	 * that specify the wait checks to be performed while executing this
+	 * job.
+	 */
 	__u64 waitchks;
-	__u32 fence;		/* Return value */
 
-	__u32 reserved[5];	/* future expansion */
+	/**
+	 * @fence:
+	 *
+	 * Return location for the threshold of the syncpoint associated with
+	 * this job. This can be used with the DRM_TEGRA_SYNCPT_WAIT IOCTL to
+	 * wait for this job to be finished.
+	 */
+	__u32 fence;
+
+	/**
+	 * @reserved:
+	 *
+	 * This field is reserved for future use. Must be 0.
+	 */
+	__u32 reserved[5];
 };
 
 #define DRM_TEGRA_GEM_TILING_MODE_PITCH 0
 #define DRM_TEGRA_GEM_TILING_MODE_TILED 1
 #define DRM_TEGRA_GEM_TILING_MODE_BLOCK 2
 
+/**
+ * struct drm_tegra_gem_set_tiling - parameters for the set tiling IOCTL
+ */
 struct drm_tegra_gem_set_tiling {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to set the tiling parameters.
+	 */
 	__u32 handle;
+
+	/**
+	 * @mode:
+	 *
+	 * The tiling mode to set. Must be one of:
+	 *
+	 * DRM_TEGRA_GEM_TILING_MODE_PITCH
+	 *   pitch linear format
+	 *
+	 * DRM_TEGRA_GEM_TILING_MODE_TILED
+	 *   16x16 tiling format
+	 *
+	 * DRM_TEGRA_GEM_TILING_MODE_BLOCK
+	 *   16Bx2 tiling format
+	 */
 	__u32 mode;
+
+	/**
+	 * @value:
+	 *
+	 * The value to set for the tiling mode parameter.
+	 */
 	__u32 value;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_gem_get_tiling - parameters for the get tiling IOCTL
+ */
 struct drm_tegra_gem_get_tiling {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to query the tiling parameters.
+	 */
 	__u32 handle;
-	/* output */
+
+	/**
+	 * @mode:
+	 *
+	 * Return location for the tiling mode.
+	 */
 	__u32 mode;
+
+	/**
+	 * @value:
+	 *
+	 * Return location for the tiling mode parameter.
+	 */
 	__u32 value;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
-#define DRM_TEGRA_GEM_BOTTOM_UP		(1 << 0)
-#define DRM_TEGRA_GEM_FLAGS		(DRM_TEGRA_GEM_BOTTOM_UP)
-
+/**
+ * struct drm_tegra_gem_set_flags - parameters for the set flags IOCTL
+ */
 struct drm_tegra_gem_set_flags {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to set the flags.
+	 */
 	__u32 handle;
-	/* output */
+
+	/**
+	 * @flags:
+	 *
+	 * The flags to set for the GEM object.
+	 */
 	__u32 flags;
 };
 
+/**
+ * struct drm_tegra_gem_get_flags - parameters for the get flags IOCTL
+ */
 struct drm_tegra_gem_get_flags {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to query the flags.
+	 */
 	__u32 handle;
-	/* output */
+

include/uapi/drm/tegra_drm.h line 626

Quoted 8 lines

include/uapi/drm/tegra_drm.h +5 -0

+	/**
+	 * @flags:
+	 *
+	 * Return location for the flags queried from the GEM object.
+	 */
 	__u32 flags;
 };
 

Quoted 37 lines · Dmitry Osipenko

Quoted 36 lines · Thierry Reding

From: Thierry Reding <[email protected]>

Document the userspace ABI with kerneldoc to provide some information on how to use it.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/gem.c  |   4 +-
 include/uapi/drm/tegra_drm.h | 480 ++++++++++++++++++++++++++++++++++-
 2 files changed, 468 insertions(+), 16 deletions(-)

drivers/gpu/drm/tegra/gem.c +2 -2

index 387ba1dfbe0d..e2987a19541d 100644
@@ -291,10 +291,10 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, size_t size,
 	if (err < 0)
 		goto release;
 
-	if (flags & DRM_TEGRA_GEM_CREATE_TILED)
+	if (flags & DRM_TEGRA_GEM_TILED)
 		bo->tiling.mode = TEGRA_BO_TILING_MODE_TILED;
 
-	if (flags & DRM_TEGRA_GEM_CREATE_BOTTOM_UP)
+	if (flags & DRM_TEGRA_GEM_BOTTOM_UP)
 		bo->flags |= TEGRA_BO_BOTTOM_UP;
 
 	return bo;

include/uapi/drm/tegra_drm.h +0 -0

index 99e15d82d1e9..86a7b1e7559d 100644
@@ -29,146 +29,598 @@
 extern "C" {
 #endif
 

Quoted 9 lines

include/uapi/drm/tegra_drm.h line 35

Quoted 6 lines

include/uapi/drm/tegra_drm.h +4 -2

-#define DRM_TEGRA_GEM_CREATE_TILED     (1 << 0)
-#define DRM_TEGRA_GEM_CREATE_BOTTOM_UP (1 << 1)
+#define DRM_TEGRA_GEM_TILED		(1 << 0)
+#define DRM_TEGRA_GEM_BOTTOM_UP		(1 << 1)
+#define DRM_TEGRA_GEM_FLAGS		(DRM_TEGRA_GEM_TILED | \
+					 DRM_TEGRA_GEM_BOTTOM_UP)

The current userspace won't compile with the above changes, so this is the ABI breakage. Please keep the old DRM_TEGRA_GEM_CREATE_* DRM_TEGRA_GEM_* flags for now.

It looks like I fumbled this during a rebase and didn't catch it. I've left the original flags in.

[...]

Quoted 20 lines

Quoted 7 lines

include/uapi/drm/tegra_drm.h +6 -0

+/**
+ * struct drm_tegra_syncpt_read - parameters for the read syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_read {
+	/**
+	 * @id:
+	 *

include/uapi/drm/tegra_drm.h line 112

Quoted 10 lines

include/uapi/drm/tegra_drm.h +8 -0

+	 * ID of the syncpoint to read the current value from.
+	 */
 	__u32 id;
+
+	/**
+	 * @value:
+	 *
+	 * Return location for the current syncpoint value.
+	 */>  	__u32 value;
 };

What about:

Quoted 5 lines

Returned value of the given syncpoint.

Could we replace "return location for the.." with just "Returned.." in other places too? My mind is stuttering while reading "location for the value", though I know that my english isn't ideal and maybe it's only me.

How about something a little more explicit, like:

The current value of the syncpoint. Will be set by the kernel upon successful completion of the IOCTL.

?

Quoted 14 lines

include/uapi/drm/tegra_drm.h line 122

Quoted 11 lines

include/uapi/drm/tegra_drm.h +8 -0

+/**
+ * struct drm_tegra_syncpt_incr - parameters for the increment syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_incr {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to read the current value from.
+	 */

Cut-n-pasted comment. Change to:

Quoted 1 line

ID of the syncpoint to increment.

Good catch. Fixed.

Quoted 42 lines

Quoted 30 lines

include/uapi/drm/tegra_drm.h +23 -0

 	__u32 id;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_syncpt_wait - parameters for the wait syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_wait {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to wait on.
+	 */
 	__u32 id;
+
+	/**
+	 * @thresh:
+	 *
+	 * Threshold value for which to wait.
+	 */
 	__u32 thresh;
+
+	/**
+	 * @timeout:

include/uapi/drm/tegra_drm.h line 162

Quoted 10 lines

include/uapi/drm/tegra_drm.h +9 -0

+	 *
+	 * Timeout, in milliseconds, to wait.
+	 */
 	__u32 timeout;
+
+	/**
+	 * @value:
+	 *
+	 * Return value for the current syncpoint value.
+	 */

Just:

Quoted 1 line

Returned value of the syncpoint.

Will reword this similar to the above.

Quoted 67 lines

Quoted 54 lines

include/uapi/drm/tegra_drm.h +38 -0

 	__u32 value;
 };
 
 #define DRM_TEGRA_NO_TIMEOUT	(0xffffffff)
 
+/**
+ * struct drm_tegra_open_channel - parameters for the open channel IOCTL
+ */
 struct drm_tegra_open_channel {
+	/**
+	 * @client:
+	 *
+	 * The client ID for this channel.
+	 */
 	__u32 client;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
+
+	/**
+	 * @context:
+	 *
+	 * Return location for the application context of this channel. This
+	 * context needs to be passed to the DRM_TEGRA_CHANNEL_CLOSE or the
+	 * DRM_TEGRA_SUBMIT IOCTLs.
+	 */
 	__u64 context;
 };
 
+/**
+ * struct drm_tegra_close_channel - parameters for the close channel IOCTL
+ */
 struct drm_tegra_close_channel {
+	/**
+	 * @context:
+	 *
+	 * The application context of this channel. This is obtained from the
+	 * DRM_TEGRA_OPEN_CHANNEL IOCTL.
+	 */
 	__u64 context;
 };
 
+/**
+ * struct drm_tegra_get_syncpt - parameters for the get syncpoint IOCTL
+ */
 struct drm_tegra_get_syncpt {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying the channel for which to obtain

include/uapi/drm/tegra_drm.h line 226

Quoted 10 lines

include/uapi/drm/tegra_drm.h +8 -0

+	 * the syncpoint ID.
+	 */
 	__u64 context;
+
+	/**
+	 * @index:
+	 *
+	 * Channel-relative index of the syncpoint for which to obtain the ID.
+	 */
 	__u32 index;

Client-relative. And what about:

Quoted 1 line

Clients syncpoint index for which to obtain the ID.

I'll try to find some better wording for this.

Quoted 99 lines

Quoted 99 lines

include/uapi/drm/tegra_drm.h +77 -0

+
+	/**
+	 * @id:
+	 *
+	 * Return location for the ID of the given syncpoint.
+	 */
 	__u32 id;
 };>
+/**
+ * struct drm_tegra_get_syncpt_base - parameters for the get wait base IOCTL
+ */
 struct drm_tegra_get_syncpt_base {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying for which channel to obtain the
+	 * wait base.
+	 */
 	__u64 context;
+
+	/**
+	 * @syncpt:
+	 *
+	 * ID of the syncpoint for which to obtain the wait base.
+	 */
 	__u32 syncpt;
+
+	/**
+	 * @id:
+	 *
+	 * Return location for the ID of the wait base.
+	 */
 	__u32 id;
 };
 
+/**
+ * struct drm_tegra_syncpt - syncpoint increment operation
+ */
 struct drm_tegra_syncpt {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to operate on.
+	 */
 	__u32 id;
+
+	/**
+	 * @incrs:
+	 *
+	 * Number of increments to perform for the syncpoint.
+	 */
 	__u32 incrs;
 };
 
+/**
+ * struct drm_tegra_cmdbuf - structure describing a command buffer
+ */
 struct drm_tegra_cmdbuf {
+	/**
+	 * @handle:
+	 *
+	 * Handle to a GEM object containing the command buffer.
+	 */
 	__u32 handle;
+
+	/**
+	 * @offset:
+	 *
+	 * Offset, in bytes, into the GEM object identified by @handle at
+	 * which the command buffer starts.
+	 */
 	__u32 offset;
+
+	/**
+	 * @words:
+	 *
+	 * Number of 32-bit words in this command buffer.
+	 */
 	__u32 words;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_reloc - GEM object relocation structure
+ */
 struct drm_tegra_reloc {
 	struct {
+		/**
+		 * @cmdbuf.handle:
+		 *
+		 * Handle to the GEM object containing the command buffer for
+		 * which to perform this GEM object relocation.
+		 */

Quoted 8 lines

include/uapi/drm/tegra_drm.h line 332

Quoted 6 lines

include/uapi/drm/tegra_drm.h +5 -0

 		__u32 handle;
+
+		/**
+		 * @cmdbuf.offset:
+		 *
+		 * Offset into the command buffer at which to insert the the

Offset, in bytes,

Done.

Quoted 10 lines

Quoted 10 lines

include/uapi/drm/tegra_drm.h +7 -0

+		 * relocated address.
+		 */
 		__u32 offset;
 	} cmdbuf;
 	struct {
+		/**
+		 * @target.handle:
+		 *
+		 * Handle to the GEM object to be relocated.
+		 */

Quoted 8 lines

include/uapi/drm/tegra_drm.h line 348

Quoted 6 lines

include/uapi/drm/tegra_drm.h +5 -0

 		__u32 handle;
+
+		/**
+		 * @target.offset:
+		 *
+		 * Offset into the target GEM object at which the relocated

Offset, in bytes,

Done.

Quoted 115 lines

Quoted 115 lines

include/uapi/drm/tegra_drm.h +94 -0

+		 * data starts.
+		 */
 		__u32 offset;
 	} target;
+
+	/**
+	 * @shift:
+	 *
+	 * The number of bits by which to shift relocated addresses.
+	 */
 	__u32 shift;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_waitchk - wait check structure
+ */
 struct drm_tegra_waitchk {
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object containing a command stream on which to
+	 * perform the wait check.
+	 */
 	__u32 handle;
+
+	/**
+	 * @offset:
+	 *
+	 * Offset, in bytes, of the location in the command stream to perform
+	 * the wait check on.
+	 */
 	__u32 offset;
+
+	/**
+	 * @syncpt:
+	 *
+	 * ID of the syncpoint to wait check.
+	 */
 	__u32 syncpt;
+
+	/**
+	 * @thresh:
+	 *
+	 * Threshold value for which to check.
+	 */
 	__u32 thresh;
 };
 
+/**
+ * struct drm_tegra_submit - job submission structure
+ */
 struct drm_tegra_submit {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying the channel to use for the
+	 * execution of this job.
+	 */
 	__u64 context;
+
+	/**
+	 * @num_syncpts:
+	 *
+	 * The number of syncpoints operated on by this job.
+	 */
 	__u32 num_syncpts;
+
+	/**
+	 * @num_cmdbufs:
+	 *
+	 * The number of command buffers to execute as part of this job.
+	 */
 	__u32 num_cmdbufs;
+
+	/**
+	 * @num_relocs:
+	 *
+	 * The number of relocations to perform before executing this job.
+	 */
 	__u32 num_relocs;
+
+	/**
+	 * @num_waitchks:
+	 *
+	 * The number of wait checks to perform as part of this job.
+	 */
 	__u32 num_waitchks;
+
+	/**
+	 * @waitchk_mask:
+	 *
+	 * Bitmask of valid wait checks.
+	 */
 	__u32 waitchk_mask;
+
+	/**
+	 * @timeout:
+	 *
+	 * Timeout, in milliseconds, before this job is cancelled.
+	 */
 	__u32 timeout;
+
+	/**
+	 * @syncpts:
+	 *
+	 * A pointer to @num_syncpts &struct drm_tegra_syncpt structures that
+	 * specify the syncpoint operations performed as part of this job.
+	 */

Quoted 8 lines

include/uapi/drm/tegra_drm.h line 469

Quoted 6 lines

include/uapi/drm/tegra_drm.h +5 -0

 	__u64 syncpts;
+
+	/**
+	 * @cmdbufs:
+	 *
+	 * A pointer to @num_cmdbufs &struct drm_tegra_cmdbuf_legacy

We don't have drm_tegra_cmdbuf_legacy yet, should be drm_tegra_cmdbuf.

Done.

Quoted 6 lines

Quoted 6 lines

include/uapi/drm/tegra_drm.h +5 -0

+	 * structures that define the command buffers to execute as part of
+	 * this job.
+	 */
 	__u64 cmdbufs;
+
+	/**

Quoted 8 lines

include/uapi/drm/tegra_drm.h line 481

Quoted 6 lines

include/uapi/drm/tegra_drm.h +6 -0

+	 * @relocs:
+	 *
+	 * A pointer to @num_relocs &struct drm_tegra_reloc_legacy structures
+	 * that specify the relocations that need to be performed before
+	 * executing this job.
+	 */

Same as for drm_tegra_cmdbuf_legacy.

Done.

Quoted 153 lines

Quoted 153 lines

include/uapi/drm/tegra_drm.h +111 -12

 	__u64 relocs;
+
+	/**
+	 * @waitchks:
+	 *
+	 * A pointer to @num_waitchks &struct drm_tegra_waitchk structures
+	 * that specify the wait checks to be performed while executing this
+	 * job.
+	 */
 	__u64 waitchks;
-	__u32 fence;		/* Return value */
 
-	__u32 reserved[5];	/* future expansion */
+	/**
+	 * @fence:
+	 *
+	 * Return location for the threshold of the syncpoint associated with
+	 * this job. This can be used with the DRM_TEGRA_SYNCPT_WAIT IOCTL to
+	 * wait for this job to be finished.
+	 */
+	__u32 fence;
+
+	/**
+	 * @reserved:
+	 *
+	 * This field is reserved for future use. Must be 0.
+	 */
+	__u32 reserved[5];
 };
 
 #define DRM_TEGRA_GEM_TILING_MODE_PITCH 0
 #define DRM_TEGRA_GEM_TILING_MODE_TILED 1
 #define DRM_TEGRA_GEM_TILING_MODE_BLOCK 2
 
+/**
+ * struct drm_tegra_gem_set_tiling - parameters for the set tiling IOCTL
+ */
 struct drm_tegra_gem_set_tiling {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to set the tiling parameters.
+	 */
 	__u32 handle;
+
+	/**
+	 * @mode:
+	 *
+	 * The tiling mode to set. Must be one of:
+	 *
+	 * DRM_TEGRA_GEM_TILING_MODE_PITCH
+	 *   pitch linear format
+	 *
+	 * DRM_TEGRA_GEM_TILING_MODE_TILED
+	 *   16x16 tiling format
+	 *
+	 * DRM_TEGRA_GEM_TILING_MODE_BLOCK
+	 *   16Bx2 tiling format
+	 */
 	__u32 mode;
+
+	/**
+	 * @value:
+	 *
+	 * The value to set for the tiling mode parameter.
+	 */
 	__u32 value;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_gem_get_tiling - parameters for the get tiling IOCTL
+ */
 struct drm_tegra_gem_get_tiling {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to query the tiling parameters.
+	 */
 	__u32 handle;
-	/* output */
+
+	/**
+	 * @mode:
+	 *
+	 * Return location for the tiling mode.
+	 */
 	__u32 mode;
+
+	/**
+	 * @value:
+	 *
+	 * Return location for the tiling mode parameter.
+	 */
 	__u32 value;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
-#define DRM_TEGRA_GEM_BOTTOM_UP		(1 << 0)
-#define DRM_TEGRA_GEM_FLAGS		(DRM_TEGRA_GEM_BOTTOM_UP)
-
+/**
+ * struct drm_tegra_gem_set_flags - parameters for the set flags IOCTL
+ */
 struct drm_tegra_gem_set_flags {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to set the flags.
+	 */
 	__u32 handle;
-	/* output */
+
+	/**
+	 * @flags:
+	 *
+	 * The flags to set for the GEM object.
+	 */
 	__u32 flags;
 };
 
+/**
+ * struct drm_tegra_gem_get_flags - parameters for the get flags IOCTL
+ */
 struct drm_tegra_gem_get_flags {
-	/* input */
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object for which to query the flags.
+	 */
 	__u32 handle;
-	/* output */
+
+	/**
+	 * @flags:
+	 *
+	 * Return location for the flags queried from the GEM object.

Quoted 4 lines

include/uapi/drm/tegra_drm.h line 68

Quoted 4 lines

include/uapi/drm/tegra_drm.h +1 -0

+	 */
 	__u32 flags;
 };
 

Thanks for the review. I'm going to think about a better wording for some of the descriptions and send out a new version of this patch individually, for review.

Quoted 78 lines · Thierry Reding

Quoted 37 lines · Dmitry Osipenko

Quoted 36 lines · Thierry Reding

From: Thierry Reding <[email protected]>

Document the userspace ABI with kerneldoc to provide some information on how to use it.

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/gem.c  |   4 +-
 include/uapi/drm/tegra_drm.h | 480 ++++++++++++++++++++++++++++++++++-
 2 files changed, 468 insertions(+), 16 deletions(-)

drivers/gpu/drm/tegra/gem.c +2 -2

index 387ba1dfbe0d..e2987a19541d 100644
@@ -291,10 +291,10 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, size_t size,
 	if (err < 0)
 		goto release;
 
-	if (flags & DRM_TEGRA_GEM_CREATE_TILED)
+	if (flags & DRM_TEGRA_GEM_TILED)
 		bo->tiling.mode = TEGRA_BO_TILING_MODE_TILED;
 
-	if (flags & DRM_TEGRA_GEM_CREATE_BOTTOM_UP)
+	if (flags & DRM_TEGRA_GEM_BOTTOM_UP)
 		bo->flags |= TEGRA_BO_BOTTOM_UP;
 
 	return bo;

include/uapi/drm/tegra_drm.h +0 -0

index 99e15d82d1e9..86a7b1e7559d 100644
@@ -29,146 +29,598 @@
 extern "C" {
 #endif
 

Quoted 9 lines

include/uapi/drm/tegra_drm.h line 35

Quoted 6 lines

include/uapi/drm/tegra_drm.h +4 -2

-#define DRM_TEGRA_GEM_CREATE_TILED     (1 << 0)
-#define DRM_TEGRA_GEM_CREATE_BOTTOM_UP (1 << 1)
+#define DRM_TEGRA_GEM_TILED		(1 << 0)
+#define DRM_TEGRA_GEM_BOTTOM_UP		(1 << 1)
+#define DRM_TEGRA_GEM_FLAGS		(DRM_TEGRA_GEM_TILED | \
+					 DRM_TEGRA_GEM_BOTTOM_UP)

The current userspace won't compile with the above changes, so this is the ABI breakage. Please keep the old DRM_TEGRA_GEM_CREATE_* DRM_TEGRA_GEM_* flags for now.

It looks like I fumbled this during a rebase and didn't catch it. I've left the original flags in.

[...]

Quoted 20 lines

Quoted 7 lines

include/uapi/drm/tegra_drm.h +6 -0

+/**
+ * struct drm_tegra_syncpt_read - parameters for the read syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_read {
+	/**
+	 * @id:
+	 *

include/uapi/drm/tegra_drm.h line 112

Quoted 10 lines

include/uapi/drm/tegra_drm.h +8 -0

+	 * ID of the syncpoint to read the current value from.
+	 */
 	__u32 id;
+
+	/**
+	 * @value:
+	 *
+	 * Return location for the current syncpoint value.
+	 */>  	__u32 value;
 };

What about:

Quoted 5 lines

Returned value of the given syncpoint.

Could we replace "return location for the.." with just "Returned.." in other places too? My mind is stuttering while reading "location for the value", though I know that my english isn't ideal and maybe it's only me.

Quoted 6 lines

How about something a little more explicit, like:

The current value of the syncpoint. Will be set by the kernel upon successful completion of the IOCTL.

?

That's better.

Maybe we could also use format like this:

/**
* struct drm_tegra_syncpt_read - parameters for the read syncpoint IOCTL
* @id: ID of the syncpoint to read the current value from.
* @value: Return current value of the syncpoint.
*/

struct drm_tegra_syncpt_read {
	__u32 id;
	__u32 value;
};

From: Thierry Reding <[email protected]>

Document the userspace ABI with kerneldoc to provide some information on how to use it.

v2:
- keep GEM object creation flags for ABI compatibility
- fix typo in struct drm_tegra_syncpt_incr kerneldoc
- fix typos in struct drm_tegra_submit kerneldoc
- reworded some descriptions as suggested

Signed-off-by Thierry Reding <[email protected]>

---
 include/uapi/drm/tegra_drm.h | 480 ++++++++++++++++++++++++++++++++++-
 1 file changed, 471 insertions(+), 9 deletions(-)

include/uapi/drm/tegra_drm.h +329 -0

index 99e15d82d1e9..7e121c69cd9a 100644
@@ -32,143 +32,605 @@ extern "C" {
 #define DRM_TEGRA_GEM_CREATE_TILED     (1 << 0)
 #define DRM_TEGRA_GEM_CREATE_BOTTOM_UP (1 << 1)
 
+/**
+ * struct drm_tegra_gem_create - parameters for the GEM object creation IOCTL
+ */
 struct drm_tegra_gem_create {
+	/**
+	 * @size:
+	 *
+	 * The size, in bytes, of the buffer object to be created.
+	 */
 	__u64 size;
+
+	/**
+	 * @flags:
+	 *
+	 * A bitmask of flags that influence the creation of GEM objects:
+	 *
+	 * DRM_TEGRA_GEM_CREATE_TILED
+	 *   Use the 16x16 tiling format for this buffer.
+	 *
+	 * DRM_TEGRA_GEM_CREATE_BOTTOM_UP
+	 *   The buffer has a bottom-up layout.
+	 */
 	__u32 flags;
+
+	/**
+	 * @handle:
+	 *
+	 * The handle of the created GEM object. Set by the kernel upon
+	 * successful completion of the IOCTL.
+	 */
 	__u32 handle;
 };
 
+/**
+ * struct drm_tegra_gem_mmap - parameters for the GEM mmap IOCTL
+ */
 struct drm_tegra_gem_mmap {
+	/**
+	 * @handle:
+	 *
+	 * Handle of the GEM object to obtain an mmap offset for.
+	 */
 	__u32 handle;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
+
+	/**
+	 * @offset:
+	 *
+	 * The mmap offset for the given GEM object. Set by the kernel upon
+	 * successful completion of the IOCTL.
+	 */
 	__u64 offset;
 };
 
+/**
+ * struct drm_tegra_syncpt_read - parameters for the read syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_read {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to read the current value from.
+	 */
 	__u32 id;
+
+	/**
+	 * @value:
+	 *
+	 * The current syncpoint value. Set by the kernel upon successful
+	 * completion of the IOCTL.
+	 */
 	__u32 value;
 };
 
+/**
+ * struct drm_tegra_syncpt_incr - parameters for the increment syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_incr {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to increment.
+	 */
 	__u32 id;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_syncpt_wait - parameters for the wait syncpoint IOCTL
+ */
 struct drm_tegra_syncpt_wait {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to wait on.
+	 */
 	__u32 id;
+
+	/**
+	 * @thresh:
+	 *
+	 * Threshold value for which to wait.
+	 */
 	__u32 thresh;
+
+	/**
+	 * @timeout:
+	 *
+	 * Timeout, in milliseconds, to wait.
+	 */
 	__u32 timeout;
+
+	/**
+	 * @value:
+	 *
+	 * The new syncpoint value after the wait. Set by the kernel upon
+	 * successful completion of the IOCTL.
+	 */
 	__u32 value;
 };
 
 #define DRM_TEGRA_NO_TIMEOUT	(0xffffffff)
 
+/**
+ * struct drm_tegra_open_channel - parameters for the open channel IOCTL
+ */
 struct drm_tegra_open_channel {
+	/**
+	 * @client:
+	 *
+	 * The client ID for this channel.
+	 */
 	__u32 client;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
+
+	/**
+	 * @context:
+	 *
+	 * The application context of this channel. Set by the kernel upon
+	 * successful completion of the IOCTL. This context needs to be passed
+	 * to the DRM_TEGRA_CHANNEL_CLOSE or the DRM_TEGRA_SUBMIT IOCTLs.
+	 */
 	__u64 context;
 };
 
+/**
+ * struct drm_tegra_close_channel - parameters for the close channel IOCTL
+ */
 struct drm_tegra_close_channel {
+	/**
+	 * @context:
+	 *
+	 * The application context of this channel. This is obtained from the
+	 * DRM_TEGRA_OPEN_CHANNEL IOCTL.
+	 */
 	__u64 context;
 };
 
+/**
+ * struct drm_tegra_get_syncpt - parameters for the get syncpoint IOCTL
+ */
 struct drm_tegra_get_syncpt {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying the channel for which to obtain
+	 * the syncpoint ID.
+	 */
 	__u64 context;
+
+	/**
+	 * @index:
+	 *
+	 * Index of the client syncpoint for which to obtain the ID.
+	 */
 	__u32 index;
+
+	/**
+	 * @id:
+	 *
+	 * The ID of the given syncpoint. Set by the kernel upon successful
+	 * completion of the IOCTL.
+	 */
 	__u32 id;
 };
 
+/**
+ * struct drm_tegra_get_syncpt_base - parameters for the get wait base IOCTL
+ */
 struct drm_tegra_get_syncpt_base {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying for which channel to obtain the
+	 * wait base.
+	 */
 	__u64 context;
+
+	/**
+	 * @syncpt:
+	 *
+	 * ID of the syncpoint for which to obtain the wait base.
+	 */
 	__u32 syncpt;
+
+	/**
+	 * @id:
+	 *
+	 * The ID of the wait base corresponding to the client syncpoint. Set
+	 * by the kernel upon successful completion of the IOCTL.
+	 */
 	__u32 id;
 };
 
+/**
+ * struct drm_tegra_syncpt - syncpoint increment operation
+ */
 struct drm_tegra_syncpt {
+	/**
+	 * @id:
+	 *
+	 * ID of the syncpoint to operate on.
+	 */
 	__u32 id;
+
+	/**
+	 * @incrs:
+	 *
+	 * Number of increments to perform for the syncpoint.
+	 */
 	__u32 incrs;
 };
 
+/**
+ * struct drm_tegra_cmdbuf - structure describing a command buffer
+ */
 struct drm_tegra_cmdbuf {
+	/**
+	 * @handle:
+	 *
+	 * Handle to a GEM object containing the command buffer.
+	 */
 	__u32 handle;
+
+	/**
+	 * @offset:
+	 *
+	 * Offset, in bytes, into the GEM object identified by @handle at
+	 * which the command buffer starts.
+	 */
 	__u32 offset;
+
+	/**
+	 * @words:
+	 *
+	 * Number of 32-bit words in this command buffer.
+	 */
 	__u32 words;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_reloc - GEM object relocation structure
+ */
 struct drm_tegra_reloc {
 	struct {
+		/**
+		 * @cmdbuf.handle:
+		 *
+		 * Handle to the GEM object containing the command buffer for
+		 * which to perform this GEM object relocation.
+		 */
 		__u32 handle;
+
+		/**
+		 * @cmdbuf.offset:
+		 *
+		 * Offset, in bytes, into the command buffer at which to
+		 * insert the relocated address.
+		 */
 		__u32 offset;
 	} cmdbuf;
 	struct {
+		/**
+		 * @target.handle:
+		 *
+		 * Handle to the GEM object to be relocated.
+		 */
 		__u32 handle;
+
+		/**
+		 * @target.offset:
+		 *
+		 * Offset, in bytes, into the target GEM object at which the
+		 * relocated data starts.
+		 */
 		__u32 offset;
 	} target;
+
+	/**
+	 * @shift:
+	 *
+	 * The number of bits by which to shift relocated addresses.
+	 */
 	__u32 shift;
+
+	/**
+	 * @pad:
+	 *
+	 * Structure padding that may be used in the future. Must be 0.
+	 */
 	__u32 pad;
 };
 
+/**
+ * struct drm_tegra_waitchk - wait check structure
+ */
 struct drm_tegra_waitchk {
+	/**
+	 * @handle:
+	 *
+	 * Handle to the GEM object containing a command stream on which to
+	 * perform the wait check.
+	 */
 	__u32 handle;
+
+	/**
+	 * @offset:
+	 *
+	 * Offset, in bytes, of the location in the command stream to perform
+	 * the wait check on.
+	 */
 	__u32 offset;
+
+	/**
+	 * @syncpt:
+	 *
+	 * ID of the syncpoint to wait check.
+	 */
 	__u32 syncpt;
+
+	/**
+	 * @thresh:
+	 *
+	 * Threshold value for which to check.
+	 */
 	__u32 thresh;
 };
 
+/**
+ * struct drm_tegra_submit - job submission structure
+ */
 struct drm_tegra_submit {
+	/**
+	 * @context:
+	 *
+	 * The application context identifying the channel to use for the
+	 * execution of this job.
+	 */
 	__u64 context;
+
+	/**
+	 * @num_syncpts:
+	 *
+	 * The number of syncpoints operated on by this job.
+	 */
 	__u32 num_syncpts;
+
+	/**
+	 * @num_cmdbufs:
+	 *
+	 * The number of command buffers to execute as part of this job.
+	 */
 	__u32 num_cmdbufs;
+
+	/**
+	 * @num_relocs:
+	 *
+	 * The number of relocations to perform before executing this job.
+	 */
 	__u32 num_relocs;
+
+	/**
+	 * @num_waitchks:
+	 *
+	 * The number of wait checks to perform as part of this job.
+	 */
 	__u32 num_waitchks;
+
+	/**
+	 * @waitchk_mask:
+	 *
+	 * Bitmask of valid wait checks.
+	 */
 	__u32 waitchk_mask;
+
+	/**