diff mbox series

[v2] drm: document struct drm_mode_fb_cmd2

Message ID 20220201094101.92472-1-contact@emersion.fr (mailing list archive)
State New, archived
Headers show
Series [v2] drm: document struct drm_mode_fb_cmd2 | expand

Commit Message

Simon Ser Feb. 1, 2022, 9:41 a.m. UTC
Follow-up for the DRM_IOCTL_MODE_GETFB2 docs.

v2: (Daniel Stone)
- Replace fourcc.org with drm_fourcc.h because this is the
  authoritative source and the website may have mismatches.
- Drop assumption that offsets will generally be 0.
- Mention that unused entries must be zero'ed out.

Signed-off-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
Cc: Daniel Stone <daniels@collabora.com>
---
 include/uapi/drm/drm_mode.h | 88 +++++++++++++++++++++++++------------
 1 file changed, 60 insertions(+), 28 deletions(-)

Comments

Pekka Paalanen Feb. 2, 2022, 8:36 a.m. UTC | #1
On Tue, 01 Feb 2022 09:41:20 +0000
Simon Ser <contact@emersion.fr> wrote:

> Follow-up for the DRM_IOCTL_MODE_GETFB2 docs.
> 
> v2: (Daniel Stone)
> - Replace fourcc.org with drm_fourcc.h because this is the
>   authoritative source and the website may have mismatches.
> - Drop assumption that offsets will generally be 0.
> - Mention that unused entries must be zero'ed out.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> Cc: Daniel Stone <daniels@collabora.com>
> ---
>  include/uapi/drm/drm_mode.h | 88 +++++++++++++++++++++++++------------
>  1 file changed, 60 insertions(+), 28 deletions(-)
> 
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index e1e351682872..a345404dd315 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -663,41 +663,73 @@ struct drm_mode_fb_cmd {
>  #define DRM_MODE_FB_INTERLACED	(1<<0) /* for interlaced framebuffers */
>  #define DRM_MODE_FB_MODIFIERS	(1<<1) /* enables ->modifer[] */
>  
> +/**
> + * struct drm_mode_fb_cmd2 - Frame-buffer metadata.
> + *
> + * This struct holds frame-buffer metadata. There are two ways to use it:
> + *
> + * - User-space can fill this struct and perform a &DRM_IOCTL_MODE_ADDFB2
> + *   ioctl to register a new frame-buffer. The new frame-buffer object ID will
> + *   be set by the kernel in @fb_id.
> + * - User-space can set @fb_id and perform a &DRM_IOCTL_MODE_GETFB2 ioctl to
> + *   fetch metadata about an existing frame-buffer.
> + *
> + * In case of planar formats, this struct allows up to 4 buffer objects with
> + * offsets and pitches per plane. The pitch and offset order is dictated by the
> + * format FourCC as defined by ``drm_fourcc.h``, e.g. NV12 is described as:
> + *
> + *     YUV 4:2:0 image with a plane of 8 bit Y samples followed by an
> + *     interleaved U/V plane containing 8 bit 2x2 subsampled colour difference
> + *     samples.
> + *
> + * So it would consist of a Y plane at ``offsets[0]`` and a UV plane at
> + * ``offsets[1]``.
> + *
> + * To accommodate tiled, compressed, etc formats, a modifier can be specified.
> + * For more information see the "Format Modifiers" section. Note that even
> + * though it looks like we have a modifier per-plane, we in fact do not. The
> + * modifier for each plane must be identical. Thus all combinations of
> + * different data layouts for multi-plane formats must be enumerated as
> + * separate modifiers.
> + *
> + * All of the entries in @handles, @pitches, @offsets and @modifier must be
> + * zero when unused. Warning, for @offsets and @modifier zero can't be used to
> + * figure out whether the entry is used or not since it's a valid value (a zero
> + * offset is common, and a zero modifier is &DRM_FORMAT_MOD_LINEAR).
> + */
>  struct drm_mode_fb_cmd2 {
> +	/** @fb_id: Object ID of the frame-buffer. */
>  	__u32 fb_id;
> +	/** @width: Width of the frame-buffer. */
>  	__u32 width;
> +	/** @height: Height of the frame-buffer. */
>  	__u32 height;
> -	__u32 pixel_format; /* fourcc code from drm_fourcc.h */
> -	__u32 flags; /* see above flags */
> +	/**
> +	 * @pixel_format: FourCC format code, see ``DRM_FORMAT_*`` constants in
> +	 * ``drm_fourcc.h``.
> +	 */
> +	__u32 pixel_format;
> +	/**
> +	 * @flags: Frame-buffer flags (see &DRM_MODE_FB_INTERLACED and
> +	 * &DRM_MODE_FB_MODIFIERS).
> +	 */
> +	__u32 flags;
>  
> -	/*
> -	 * In case of planar formats, this ioctl allows up to 4
> -	 * buffer objects with offsets and pitches per plane.
> -	 * The pitch and offset order is dictated by the fourcc,
> -	 * e.g. NV12 (https://fourcc.org/yuv.php#NV12) is described as:
> -	 *
> -	 *   YUV 4:2:0 image with a plane of 8 bit Y samples
> -	 *   followed by an interleaved U/V plane containing
> -	 *   8 bit 2x2 subsampled colour difference samples.
> -	 *
> -	 * So it would consist of Y as offsets[0] and UV as
> -	 * offsets[1].  Note that offsets[0] will generally
> -	 * be 0 (but this is not required).
> -	 *
> -	 * To accommodate tiled, compressed, etc formats, a
> -	 * modifier can be specified.  The default value of zero
> -	 * indicates "native" format as specified by the fourcc.
> -	 * Vendor specific modifier token.  Note that even though
> -	 * it looks like we have a modifier per-plane, we in fact
> -	 * do not. The modifier for each plane must be identical.
> -	 * Thus all combinations of different data layouts for
> -	 * multi plane formats must be enumerated as separate
> -	 * modifiers.
> +	/**
> +	 * @handles: GEM buffer handle, one per plane. Set to 0 if the plane is
> +	 * unused.

Hi,

should there be a note that the handles may not be unique? I.e. one can
use the same GEM buffer for more than one plane.

>  	 */
>  	__u32 handles[4];
> -	__u32 pitches[4]; /* pitch for each plane */
> -	__u32 offsets[4]; /* offset of each plane */
> -	__u64 modifier[4]; /* ie, tiling, compress */
> +	/** @pitches: Pitch (aka. stride), one per plane. */
> +	__u32 pitches[4];
> +	/** @offsets: Offset into the buffer, one per plane. */
> +	__u32 offsets[4];

Would be nice to mention the units for pitches and offsets.

> +	/**
> +	 * @modifier: Format modifier, one per plane. See ``DRM_FORMAT_MOD_*``
> +	 * constants in ``drm_fourcc.h``. All planes must use the same
> +	 * modifier. Ignored unless &DRM_MODE_FB_MODIFIERS is set in @flags.
> +	 */
> +	__u64 modifier[4];
>  };
>  
>  #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01

LGTM!

Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>


Thanks,
pq
diff mbox series

Patch

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index e1e351682872..a345404dd315 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -663,41 +663,73 @@  struct drm_mode_fb_cmd {
 #define DRM_MODE_FB_INTERLACED	(1<<0) /* for interlaced framebuffers */
 #define DRM_MODE_FB_MODIFIERS	(1<<1) /* enables ->modifer[] */
 
+/**
+ * struct drm_mode_fb_cmd2 - Frame-buffer metadata.
+ *
+ * This struct holds frame-buffer metadata. There are two ways to use it:
+ *
+ * - User-space can fill this struct and perform a &DRM_IOCTL_MODE_ADDFB2
+ *   ioctl to register a new frame-buffer. The new frame-buffer object ID will
+ *   be set by the kernel in @fb_id.
+ * - User-space can set @fb_id and perform a &DRM_IOCTL_MODE_GETFB2 ioctl to
+ *   fetch metadata about an existing frame-buffer.
+ *
+ * In case of planar formats, this struct allows up to 4 buffer objects with
+ * offsets and pitches per plane. The pitch and offset order is dictated by the
+ * format FourCC as defined by ``drm_fourcc.h``, e.g. NV12 is described as:
+ *
+ *     YUV 4:2:0 image with a plane of 8 bit Y samples followed by an
+ *     interleaved U/V plane containing 8 bit 2x2 subsampled colour difference
+ *     samples.
+ *
+ * So it would consist of a Y plane at ``offsets[0]`` and a UV plane at
+ * ``offsets[1]``.
+ *
+ * To accommodate tiled, compressed, etc formats, a modifier can be specified.
+ * For more information see the "Format Modifiers" section. Note that even
+ * though it looks like we have a modifier per-plane, we in fact do not. The
+ * modifier for each plane must be identical. Thus all combinations of
+ * different data layouts for multi-plane formats must be enumerated as
+ * separate modifiers.
+ *
+ * All of the entries in @handles, @pitches, @offsets and @modifier must be
+ * zero when unused. Warning, for @offsets and @modifier zero can't be used to
+ * figure out whether the entry is used or not since it's a valid value (a zero
+ * offset is common, and a zero modifier is &DRM_FORMAT_MOD_LINEAR).
+ */
 struct drm_mode_fb_cmd2 {
+	/** @fb_id: Object ID of the frame-buffer. */
 	__u32 fb_id;
+	/** @width: Width of the frame-buffer. */
 	__u32 width;
+	/** @height: Height of the frame-buffer. */
 	__u32 height;
-	__u32 pixel_format; /* fourcc code from drm_fourcc.h */
-	__u32 flags; /* see above flags */
+	/**
+	 * @pixel_format: FourCC format code, see ``DRM_FORMAT_*`` constants in
+	 * ``drm_fourcc.h``.
+	 */
+	__u32 pixel_format;
+	/**
+	 * @flags: Frame-buffer flags (see &DRM_MODE_FB_INTERLACED and
+	 * &DRM_MODE_FB_MODIFIERS).
+	 */
+	__u32 flags;
 
-	/*
-	 * In case of planar formats, this ioctl allows up to 4
-	 * buffer objects with offsets and pitches per plane.
-	 * The pitch and offset order is dictated by the fourcc,
-	 * e.g. NV12 (https://fourcc.org/yuv.php#NV12) is described as:
-	 *
-	 *   YUV 4:2:0 image with a plane of 8 bit Y samples
-	 *   followed by an interleaved U/V plane containing
-	 *   8 bit 2x2 subsampled colour difference samples.
-	 *
-	 * So it would consist of Y as offsets[0] and UV as
-	 * offsets[1].  Note that offsets[0] will generally
-	 * be 0 (but this is not required).
-	 *
-	 * To accommodate tiled, compressed, etc formats, a
-	 * modifier can be specified.  The default value of zero
-	 * indicates "native" format as specified by the fourcc.
-	 * Vendor specific modifier token.  Note that even though
-	 * it looks like we have a modifier per-plane, we in fact
-	 * do not. The modifier for each plane must be identical.
-	 * Thus all combinations of different data layouts for
-	 * multi plane formats must be enumerated as separate
-	 * modifiers.
+	/**
+	 * @handles: GEM buffer handle, one per plane. Set to 0 if the plane is
+	 * unused.
 	 */
 	__u32 handles[4];
-	__u32 pitches[4]; /* pitch for each plane */
-	__u32 offsets[4]; /* offset of each plane */
-	__u64 modifier[4]; /* ie, tiling, compress */
+	/** @pitches: Pitch (aka. stride), one per plane. */
+	__u32 pitches[4];
+	/** @offsets: Offset into the buffer, one per plane. */
+	__u32 offsets[4];
+	/**
+	 * @modifier: Format modifier, one per plane. See ``DRM_FORMAT_MOD_*``
+	 * constants in ``drm_fourcc.h``. All planes must use the same
+	 * modifier. Ignored unless &DRM_MODE_FB_MODIFIERS is set in @flags.
+	 */
+	__u64 modifier[4];
 };
 
 #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01