dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/i915/uapi: fix kernel doc warnings
@ 2021-04-16 10:37 Matthew Auld
  2021-04-16 10:37 ` [PATCH 2/4] drm/doc: add section for driver uAPI Matthew Auld
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Matthew Auld @ 2021-04-16 10:37 UTC (permalink / raw)
  To: intel-gfx
  Cc: Jordan Justen, dri-devel, Kenneth Graunke, Jason Ekstrand,
	mesa-dev, Daniel Vetter

Fix the cases where it is almost already valid kernel doc, for the
others just nerf the warnings for now.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Dave Airlie <airlied@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 include/uapi/drm/i915_drm.h | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index ddc47bbf48b6..92da48e935d1 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1054,12 +1054,12 @@ struct drm_i915_gem_exec_fence {
 	__u32 flags;
 };
 
-/**
+/*
  * See drm_i915_gem_execbuffer_ext_timeline_fences.
  */
 #define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
 
-/**
+/*
  * This structure describes an array of drm_syncobj and associated points for
  * timeline variants of drm_syncobj. It is invalid to append this structure to
  * the execbuf if I915_EXEC_FENCE_ARRAY is set.
@@ -1700,7 +1700,7 @@ struct drm_i915_gem_context_param {
 	__u64 value;
 };
 
-/**
+/*
  * Context SSEU programming
  *
  * It may be necessary for either functional or performance reason to configure
@@ -2067,7 +2067,7 @@ struct drm_i915_perf_open_param {
 	__u64 properties_ptr;
 };
 
-/**
+/*
  * Enable data capture for a stream that was either opened in a disabled state
  * via I915_PERF_FLAG_DISABLED or was later disabled via
  * I915_PERF_IOCTL_DISABLE.
@@ -2081,7 +2081,7 @@ struct drm_i915_perf_open_param {
  */
 #define I915_PERF_IOCTL_ENABLE	_IO('i', 0x0)
 
-/**
+/*
  * Disable data capture for a stream.
  *
  * It is an error to try and read a stream that is disabled.
@@ -2090,7 +2090,7 @@ struct drm_i915_perf_open_param {
  */
 #define I915_PERF_IOCTL_DISABLE	_IO('i', 0x1)
 
-/**
+/*
  * Change metrics_set captured by a stream.
  *
  * If the stream is bound to a specific context, the configuration change
@@ -2103,7 +2103,7 @@ struct drm_i915_perf_open_param {
  */
 #define I915_PERF_IOCTL_CONFIG	_IO('i', 0x2)
 
-/**
+/*
  * Common to all i915 perf records
  */
 struct drm_i915_perf_record_header {
@@ -2151,7 +2151,7 @@ enum drm_i915_perf_record_type {
 	DRM_I915_PERF_RECORD_MAX /* non-ABI */
 };
 
-/**
+/*
  * Structure to upload perf dynamic configuration into the kernel.
  */
 struct drm_i915_perf_oa_config {
@@ -2292,21 +2292,21 @@ struct drm_i915_query_topology_info {
  * Describes one engine and it's capabilities as known to the driver.
  */
 struct drm_i915_engine_info {
-	/** Engine class and instance. */
+	/** @engine: Engine class and instance. */
 	struct i915_engine_class_instance engine;
 
-	/** Reserved field. */
+	/** @rsvd0: Reserved field. */
 	__u32 rsvd0;
 
-	/** Engine flags. */
+	/** @flags: Engine flags. */
 	__u64 flags;
 
-	/** Capabilities of this engine. */
+	/** @capabilities: Capabilities of this engine. */
 	__u64 capabilities;
 #define I915_VIDEO_CLASS_CAPABILITY_HEVC		(1 << 0)
 #define I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC	(1 << 1)
 
-	/** Reserved fields. */
+	/** @rsvd1: Reserved fields. */
 	__u64 rsvd1[4];
 };
 
@@ -2317,13 +2317,13 @@ struct drm_i915_engine_info {
  * an array of struct drm_i915_engine_info structures.
  */
 struct drm_i915_query_engine_info {
-	/** Number of struct drm_i915_engine_info structs following. */
+	/** @num_engines: Number of struct drm_i915_engine_info structs following. */
 	__u32 num_engines;
 
-	/** MBZ */
+	/** @rsvd: MBZ */
 	__u32 rsvd[3];
 
-	/** Marker for drm_i915_engine_info structures. */
+	/** @engines: Marker for drm_i915_engine_info structures. */
 	struct drm_i915_engine_info engines[];
 };
 
-- 
2.26.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/4] drm/doc: add section for driver uAPI
  2021-04-16 10:37 [PATCH 1/4] drm/i915/uapi: fix kernel doc warnings Matthew Auld
@ 2021-04-16 10:37 ` Matthew Auld
  2021-04-16 14:05   ` Daniel Vetter
  2021-04-16 10:37 ` [PATCH 3/4] drm/i915/uapi: convert i915_user_extension to kernel doc Matthew Auld
  2021-04-16 10:37 ` [PATCH 4/4] drm/i915/uapi: convert i915_query and friend " Matthew Auld
  2 siblings, 1 reply; 7+ messages in thread
From: Matthew Auld @ 2021-04-16 10:37 UTC (permalink / raw)
  To: intel-gfx
  Cc: Jordan Justen, dri-devel, Kenneth Graunke, Jason Ekstrand,
	mesa-dev, Daniel Vetter

Add section for drm/i915 uAPI and pull in i915_drm.h.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Dave Airlie <airlied@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
---
 Documentation/gpu/driver-uapi.rst | 8 ++++++++
 Documentation/gpu/index.rst       | 1 +
 2 files changed, 9 insertions(+)
 create mode 100644 Documentation/gpu/driver-uapi.rst

diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst
new file mode 100644
index 000000000000..4411e6919a3d
--- /dev/null
+++ b/Documentation/gpu/driver-uapi.rst
@@ -0,0 +1,8 @@
+===============
+DRM Driver uAPI
+===============
+
+drm/i915 uAPI
+=============
+
+.. kernel-doc:: include/uapi/drm/i915_drm.h
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
index ec4bc72438e4..b9c1214d8f23 100644
--- a/Documentation/gpu/index.rst
+++ b/Documentation/gpu/index.rst
@@ -10,6 +10,7 @@ Linux GPU Driver Developer's Guide
    drm-kms
    drm-kms-helpers
    drm-uapi
+   driver-uapi
    drm-client
    drivers
    backlight
-- 
2.26.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/4] drm/i915/uapi: convert i915_user_extension to kernel doc
  2021-04-16 10:37 [PATCH 1/4] drm/i915/uapi: fix kernel doc warnings Matthew Auld
  2021-04-16 10:37 ` [PATCH 2/4] drm/doc: add section for driver uAPI Matthew Auld
@ 2021-04-16 10:37 ` Matthew Auld
  2021-04-16 18:29   ` Jason Ekstrand
  2021-04-16 10:37 ` [PATCH 4/4] drm/i915/uapi: convert i915_query and friend " Matthew Auld
  2 siblings, 1 reply; 7+ messages in thread
From: Matthew Auld @ 2021-04-16 10:37 UTC (permalink / raw)
  To: intel-gfx
  Cc: Jordan Justen, dri-devel, Daniel Vetter, Kenneth Graunke,
	Jason Ekstrand, mesa-dev, Daniel Vetter

Add some example usage for the extension chaining also, which is quite
nifty.

v2: (Daniel)
  - clarify that the name is just some integer, also document that the
    name space is not global

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Dave Airlie <airlied@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/uapi/drm/i915_drm.h | 54 ++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 92da48e935d1..d79b51c12ff2 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -62,8 +62,8 @@ extern "C" {
 #define I915_ERROR_UEVENT		"ERROR"
 #define I915_RESET_UEVENT		"RESET"
 
-/*
- * i915_user_extension: Base class for defining a chain of extensions
+/**
+ * struct i915_user_extension - Base class for defining a chain of extensions
  *
  * Many interfaces need to grow over time. In most cases we can simply
  * extend the struct and have userspace pass in more data. Another option,
@@ -76,12 +76,58 @@ extern "C" {
  * increasing complexity, and for large parts of that interface to be
  * entirely optional. The downside is more pointer chasing; chasing across
  * the __user boundary with pointers encapsulated inside u64.
+ *
+ * Example chaining:
+ *
+ * .. code-block:: C
+ *
+ *	struct i915_user_extension ext3 {
+ *		.next_extension = 0, // end
+ *		.name = ...,
+ *	};
+ *	struct i915_user_extension ext2 {
+ *		.next_extension = (uintptr_t)&ext3,
+ *		.name = ...,
+ *	};
+ *	struct i915_user_extension ext1 {
+ *		.next_extension = (uintptr_t)&ext2,
+ *		.name = ...,
+ *	};
+ *
+ * Typically the i915_user_extension would be embedded in some uAPI struct, and
+ * in this case we would feed it the head of the chain(i.e ext1), which would
+ * then apply all of the above extensions.
+ *
  */
 struct i915_user_extension {
+	/**
+	 * @next_extension:
+	 *
+	 * Pointer to the next struct i915_user_extension, or zero if the end.
+	 */
 	__u64 next_extension;
+	/**
+	 * @name: Name of the extension.
+	 *
+	 * Note that the name here is just some integer.
+	 *
+	 * Also note that the name space for this is not global for the whole
+	 * driver, but rather its scope/meaning is limited to the specific piece
+	 * of uAPI which has embedded the struct i915_user_extension.
+	 */
 	__u32 name;
-	__u32 flags; /* All undefined bits must be zero. */
-	__u32 rsvd[4]; /* Reserved for future use; must be zero. */
+	/**
+	 * @flags: MBZ
+	 *
+	 * All undefined bits must be zero.
+	 */
+	__u32 flags;
+	/**
+	 * @rsvd: MBZ
+	 *
+	 * Reserved for future use; must be zero.
+	 */
+	__u32 rsvd[4];
 };
 
 /*
-- 
2.26.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/4] drm/i915/uapi: convert i915_query and friend to kernel doc
  2021-04-16 10:37 [PATCH 1/4] drm/i915/uapi: fix kernel doc warnings Matthew Auld
  2021-04-16 10:37 ` [PATCH 2/4] drm/doc: add section for driver uAPI Matthew Auld
  2021-04-16 10:37 ` [PATCH 3/4] drm/i915/uapi: convert i915_user_extension to kernel doc Matthew Auld
@ 2021-04-16 10:37 ` Matthew Auld
  2021-04-16 18:32   ` Jason Ekstrand
  2 siblings, 1 reply; 7+ messages in thread
From: Matthew Auld @ 2021-04-16 10:37 UTC (permalink / raw)
  To: intel-gfx
  Cc: Tvrtko Ursulin, Jason Ekstrand, Jordan Justen, dri-devel,
	Daniel Vetter, Kenneth Graunke, mesa-dev, Daniel Vetter

Add a note about the two-step process.

v2(Tvrtko):
  - Also document the other method of just passing in a buffer which is
    large enough, which avoids two ioctl calls. Can make sense for
    smaller query items.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Dave Airlie <airlied@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: mesa-dev@lists.freedesktop.org
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/uapi/drm/i915_drm.h | 61 ++++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 11 deletions(-)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d79b51c12ff2..12f375c52317 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2218,14 +2218,23 @@ struct drm_i915_perf_oa_config {
 	__u64 flex_regs_ptr;
 };
 
+/**
+ * struct drm_i915_query_item - An individual query for the kernel to process.
+ *
+ * The behaviour is determined by the @query_id. Note that exactly what
+ * @data_ptr is also depends on the specific @query_id.
+ */
 struct drm_i915_query_item {
+	/** @query_id: The id for this query */
 	__u64 query_id;
 #define DRM_I915_QUERY_TOPOLOGY_INFO    1
 #define DRM_I915_QUERY_ENGINE_INFO	2
 #define DRM_I915_QUERY_PERF_CONFIG      3
 /* Must be kept compact -- no holes and well documented */
 
-	/*
+	/**
+	 * @length:
+	 *
 	 * When set to zero by userspace, this is filled with the size of the
 	 * data to be written at the data_ptr pointer. The kernel sets this
 	 * value to a negative value to signal an error on a particular query
@@ -2233,21 +2242,26 @@ struct drm_i915_query_item {
 	 */
 	__s32 length;
 
-	/*
+	/**
+	 * @flags:
+	 *
 	 * When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
 	 *
 	 * When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
-	 * following :
-	 *         - DRM_I915_QUERY_PERF_CONFIG_LIST
-	 *         - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
-	 *         - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
+	 * following:
+	 *
+	 *	- DRM_I915_QUERY_PERF_CONFIG_LIST
+	 *      - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
+	 *      - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
 	 */
 	__u32 flags;
 #define DRM_I915_QUERY_PERF_CONFIG_LIST          1
 #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
 #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID   3
 
-	/*
+	/**
+	 * @data_ptr:
+	 *
 	 * Data will be written at the location pointed by data_ptr when the
 	 * value of length matches the length of the data to be written by the
 	 * kernel.
@@ -2255,16 +2269,41 @@ struct drm_i915_query_item {
 	__u64 data_ptr;
 };
 
+/**
+ * struct drm_i915_query - Supply an array of drm_i915_query_item for the kernel
+ * to fill out.
+ *
+ * Note that this is generally a two step process for each drm_i915_query_item
+ * in the array:
+ *
+ * 1. Call the DRM_IOCTL_I915_QUERY, giving it our array of drm_i915_query_item,
+ *    with drm_i915_query_item.size set to zero. The kernel will then fill in
+ *    the size, in bytes, which tells userspace how memory it needs to allocate
+ *    for the blob(say for an array of properties).
+ *
+ * 2. Next we call DRM_IOCTL_I915_QUERY again, this time with the
+ *    drm_i915_query_item.data_ptr equal to our newly allocated blob. Note that
+ *    the i915_query_item.size should still be the same as what the kernel
+ *    previously set. At this point the kernel can fill in the blob.
+ *
+ * Note that for some query items it can make sense for userspace to just pass
+ * in a buffer/blob equal to or larger than the required size. In this case only
+ * a single ioctl call is needed. For some smaller query items this can work
+ * quite well.
+ *
+ */
 struct drm_i915_query {
+	/** @num_items: The number of elements in the @items_ptr array */
 	__u32 num_items;
 
-	/*
-	 * Unused for now. Must be cleared to zero.
+	/**
+	 * @flags: Unused for now. Must be cleared to zero.
 	 */
 	__u32 flags;
 
-	/*
-	 * This points to an array of num_items drm_i915_query_item structures.
+	/**
+	 * @items_ptr: This points to an array of num_items drm_i915_query_item
+	 * structures.
 	 */
 	__u64 items_ptr;
 };
-- 
2.26.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/4] drm/doc: add section for driver uAPI
  2021-04-16 10:37 ` [PATCH 2/4] drm/doc: add section for driver uAPI Matthew Auld
@ 2021-04-16 14:05   ` Daniel Vetter
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2021-04-16 14:05 UTC (permalink / raw)
  To: Matthew Auld
  Cc: Jordan Justen, intel-gfx, dri-devel, Kenneth Graunke,
	Jason Ekstrand, Mesa Dev, Daniel Vetter

On Fri, Apr 16, 2021 at 12:37 PM Matthew Auld <matthew.auld@intel.com> wrote:
>
> Add section for drm/i915 uAPI and pull in i915_drm.h.
>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: mesa-dev@lists.freedesktop.org

lgtm. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  Documentation/gpu/driver-uapi.rst | 8 ++++++++
>  Documentation/gpu/index.rst       | 1 +
>  2 files changed, 9 insertions(+)
>  create mode 100644 Documentation/gpu/driver-uapi.rst
>
> diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst
> new file mode 100644
> index 000000000000..4411e6919a3d
> --- /dev/null
> +++ b/Documentation/gpu/driver-uapi.rst
> @@ -0,0 +1,8 @@
> +===============
> +DRM Driver uAPI
> +===============
> +
> +drm/i915 uAPI
> +=============
> +
> +.. kernel-doc:: include/uapi/drm/i915_drm.h
> diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
> index ec4bc72438e4..b9c1214d8f23 100644
> --- a/Documentation/gpu/index.rst
> +++ b/Documentation/gpu/index.rst
> @@ -10,6 +10,7 @@ Linux GPU Driver Developer's Guide
>     drm-kms
>     drm-kms-helpers
>     drm-uapi
> +   driver-uapi
>     drm-client
>     drivers
>     backlight
> --
> 2.26.3
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/4] drm/i915/uapi: convert i915_user_extension to kernel doc
  2021-04-16 10:37 ` [PATCH 3/4] drm/i915/uapi: convert i915_user_extension to kernel doc Matthew Auld
@ 2021-04-16 18:29   ` Jason Ekstrand
  0 siblings, 0 replies; 7+ messages in thread
From: Jason Ekstrand @ 2021-04-16 18:29 UTC (permalink / raw)
  To: Matthew Auld, intel-gfx
  Cc: Jordan Justen, dri-devel, Daniel Vetter, Kenneth Graunke,
	mesa-dev, Daniel Vetter


[-- Attachment #1.1: Type: text/plain, Size: 3731 bytes --]

On April 16, 2021 05:37:52 Matthew Auld <matthew.auld@intel.com> wrote:

> Add some example usage for the extension chaining also, which is quite
> nifty.
>
> v2: (Daniel)
>  - clarify that the name is just some integer, also document that the
>    name space is not global
>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: mesa-dev@lists.freedesktop.org
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> include/uapi/drm/i915_drm.h | 54 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 92da48e935d1..d79b51c12ff2 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -62,8 +62,8 @@ extern "C" {
> #define I915_ERROR_UEVENT "ERROR"
> #define I915_RESET_UEVENT "RESET"
>
> -/*
> - * i915_user_extension: Base class for defining a chain of extensions
> +/**
> + * struct i915_user_extension - Base class for defining a chain of extensions
>  *
>  * Many interfaces need to grow over time. In most cases we can simply
>  * extend the struct and have userspace pass in more data. Another option,
> @@ -76,12 +76,58 @@ extern "C" {
>  * increasing complexity, and for large parts of that interface to be
>  * entirely optional. The downside is more pointer chasing; chasing across
>  * the __user boundary with pointers encapsulated inside u64.
> + *
> + * Example chaining:
> + *
> + * .. code-block:: C
> + *
> + * struct i915_user_extension ext3 {
> + * .next_extension = 0, // end
> + * .name = ...,
> + * };
> + * struct i915_user_extension ext2 {
> + * .next_extension = (uintptr_t)&ext3,
> + * .name = ...,
> + * };
> + * struct i915_user_extension ext1 {
> + * .next_extension = (uintptr_t)&ext2,
> + * .name = ...,
> + * };
> + *
> + * Typically the i915_user_extension would be embedded in some uAPI 
> struct, and
> + * in this case we would feed it the head of the chain(i.e ext1), which would
> + * then apply all of the above extensions.
> + *
>  */
> struct i915_user_extension {
> + /**
> + * @next_extension:
> + *
> + * Pointer to the next struct i915_user_extension, or zero if the end.
> + */
>  __u64 next_extension;
> + /**
> + * @name: Name of the extension.
> + *
> + * Note that the name here is just some integer.
> + *
> + * Also note that the name space for this is not global for the whole
> + * driver, but rather its scope/meaning is limited to the specific piece
> + * of uAPI which has embedded the struct i915_user_extension.

We may want to rethink this decision. In Vulkan, we have several cases 
where we use the same extension multiple places.  Given that the only 
extensible thing currently landed is context creation, we still could make 
this global.  Then again, forcing us to go through the exercise of 
redefining every time has its advantages too.

In any case, this is a correct description of the current state of affairs, so

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>


> + */
>  __u32 name;
> - __u32 flags; /* All undefined bits must be zero. */
> - __u32 rsvd[4]; /* Reserved for future use; must be zero. */
> + /**
> + * @flags: MBZ
> + *
> + * All undefined bits must be zero.
> + */
> + __u32 flags;
> + /**
> + * @rsvd: MBZ
> + *
> + * Reserved for future use; must be zero.
> + */
> + __u32 rsvd[4];
> };
>
> /*
> --
> 2.26.3


[-- Attachment #1.2: Type: text/html, Size: 6959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4] drm/i915/uapi: convert i915_query and friend to kernel doc
  2021-04-16 10:37 ` [PATCH 4/4] drm/i915/uapi: convert i915_query and friend " Matthew Auld
@ 2021-04-16 18:32   ` Jason Ekstrand
  0 siblings, 0 replies; 7+ messages in thread
From: Jason Ekstrand @ 2021-04-16 18:32 UTC (permalink / raw)
  To: Matthew Auld, intel-gfx
  Cc: Tvrtko Ursulin, Jordan Justen, dri-devel, Daniel Vetter,
	Kenneth Graunke, mesa-dev, Daniel Vetter


[-- Attachment #1.1: Type: text/plain, Size: 4801 bytes --]

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

On April 16, 2021 05:37:55 Matthew Auld <matthew.auld@intel.com> wrote:

> Add a note about the two-step process.
>
> v2(Tvrtko):
>  - Also document the other method of just passing in a buffer which is
>    large enough, which avoids two ioctl calls. Can make sense for
>    smaller query items.
>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: mesa-dev@lists.freedesktop.org
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> include/uapi/drm/i915_drm.h | 61 ++++++++++++++++++++++++++++++-------
> 1 file changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index d79b51c12ff2..12f375c52317 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -2218,14 +2218,23 @@ struct drm_i915_perf_oa_config {
> 	__u64 flex_regs_ptr;
> };
>
> +/**
> + * struct drm_i915_query_item - An individual query for the kernel to process.
> + *
> + * The behaviour is determined by the @query_id. Note that exactly what
> + * @data_ptr is also depends on the specific @query_id.
> + */
> struct drm_i915_query_item {
> +	/** @query_id: The id for this query */
> 	__u64 query_id;
> #define DRM_I915_QUERY_TOPOLOGY_INFO    1
> #define DRM_I915_QUERY_ENGINE_INFO	2
> #define DRM_I915_QUERY_PERF_CONFIG      3
> /* Must be kept compact -- no holes and well documented */
>
> -	/*
> +	/**
> +	 * @length:
> +	 *
> 	 * When set to zero by userspace, this is filled with the size of the
> 	 * data to be written at the data_ptr pointer. The kernel sets this
> 	 * value to a negative value to signal an error on a particular query
> @@ -2233,21 +2242,26 @@ struct drm_i915_query_item {
> 	 */
> 	__s32 length;
>
> -	/*
> +	/**
> +	 * @flags:
> +	 *
> 	 * When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
> 	 *
> 	 * When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
> -	 * following :
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_LIST
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> -	 *         - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> +	 * following:
> +	 *
> +	 *	- DRM_I915_QUERY_PERF_CONFIG_LIST
> +	 *      - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> +	 *      - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> 	 */
> 	__u32 flags;
> #define DRM_I915_QUERY_PERF_CONFIG_LIST          1
> #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
> #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID   3
>
> -	/*
> +	/**
> +	 * @data_ptr:
> +	 *
> 	 * Data will be written at the location pointed by data_ptr when the
> 	 * value of length matches the length of the data to be written by the
> 	 * kernel.
> @@ -2255,16 +2269,41 @@ struct drm_i915_query_item {
> 	__u64 data_ptr;
> };
>
> +/**
> + * struct drm_i915_query - Supply an array of drm_i915_query_item for the 
> kernel
> + * to fill out.
> + *
> + * Note that this is generally a two step process for each drm_i915_query_item
> + * in the array:
> + *
> + * 1. Call the DRM_IOCTL_I915_QUERY, giving it our array of 
> drm_i915_query_item,
> + *    with drm_i915_query_item.size set to zero. The kernel will then fill in
> + *    the size, in bytes, which tells userspace how memory it needs to 
> allocate
> + *    for the blob(say for an array of properties).
> + *
> + * 2. Next we call DRM_IOCTL_I915_QUERY again, this time with the
> + *    drm_i915_query_item.data_ptr equal to our newly allocated blob. Note 
> that
> + *    the i915_query_item.size should still be the same as what the kernel
> + *    previously set. At this point the kernel can fill in the blob.
> + *
> + * Note that for some query items it can make sense for userspace to just pass
> + * in a buffer/blob equal to or larger than the required size. In this 
> case only
> + * a single ioctl call is needed. For some smaller query items this can work
> + * quite well.
> + *
> + */
> struct drm_i915_query {
> +	/** @num_items: The number of elements in the @items_ptr array */
> 	__u32 num_items;
>
> -	/*
> -	 * Unused for now. Must be cleared to zero.
> +	/**
> +	 * @flags: Unused for now. Must be cleared to zero.
> 	 */
> 	__u32 flags;
>
> -	/*
> -	 * This points to an array of num_items drm_i915_query_item structures.
> +	/**
> +	 * @items_ptr: This points to an array of num_items drm_i915_query_item
> +	 * structures.
> 	 */
> 	__u64 items_ptr;
> };
> --
> 2.26.3


[-- Attachment #1.2: Type: text/html, Size: 8638 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-04-16 18:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16 10:37 [PATCH 1/4] drm/i915/uapi: fix kernel doc warnings Matthew Auld
2021-04-16 10:37 ` [PATCH 2/4] drm/doc: add section for driver uAPI Matthew Auld
2021-04-16 14:05   ` Daniel Vetter
2021-04-16 10:37 ` [PATCH 3/4] drm/i915/uapi: convert i915_user_extension to kernel doc Matthew Auld
2021-04-16 18:29   ` Jason Ekstrand
2021-04-16 10:37 ` [PATCH 4/4] drm/i915/uapi: convert i915_query and friend " Matthew Auld
2021-04-16 18:32   ` Jason Ekstrand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).