diff mbox series

[v4,2/2] drm/doc: document drm_mode_get_plane

Message ID 20210609230039.73307-3-leandro.ribeiro@collabora.com (mailing list archive)
State New, archived
Headers show
Series Document drm_mode_get_plane | expand

Commit Message

Leandro Ribeiro June 9, 2021, 11 p.m. UTC
Add a small description and document struct fields of
drm_mode_get_plane.

Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
---
 include/uapi/drm/drm_mode.h | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

--
2.31.1

Comments

Leandro Ribeiro June 9, 2021, 11:05 p.m. UTC | #1
On 6/9/21 8:00 PM, Leandro Ribeiro wrote:
> Add a small description and document struct fields of
> drm_mode_get_plane.
> 
> Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
> ---
>  include/uapi/drm/drm_mode.h | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index a5e76aa06ad5..67bcd8e1931c 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -312,16 +312,52 @@ struct drm_mode_set_plane {
>  	__u32 src_w;
>  };
> 
> +/**
> + * struct drm_mode_get_plane - Get plane metadata.
> + *
> + * Userspace can perform a GETPLANE ioctl to retrieve information about a
> + * plane.
> + *
> + * To retrieve the number of formats supported, set @count_format_types to zero
> + * and call the ioctl. @count_format_types will be updated with the value.
> + *
> + * To retrieve these formats, allocate an array with the memory needed to store
> + * @count_format_types formats. Point @format_type_ptr to this array and call
> + * the ioctl again (with @count_format_types still set to the value returned in
> + * the first ioctl call).
> + *
> + * Between one ioctl and the other, the number of formats may change.
> + * Userspace should retry the last ioctl until this number stabilizes. The
> + * kernel won't fill any array which doesn't have the expected length.
> + */

Actually I don't know if this last paragraph applies. For connectors,
for instance, I can see this happening because of hot-plugging. But for
plane formats I have no idea. As in libdrm we have this algorithm, I've
decided to describe it here.

>  struct drm_mode_get_plane {
> +	/**
> +	 * @plane_id: Object ID of the plane whose information should be
> +	 * retrieved. Set by caller.
> +	 */
>  	__u32 plane_id;
> 
> +	/** @crtc_id: Object ID of the current CRTC. */
>  	__u32 crtc_id;
> +	/** @fb_id: Object ID of the current fb. */
>  	__u32 fb_id;
> 
> +	/**
> +	 * @possible_crtcs: Bitmask of CRTC's compatible with the plane. CRTC's
> +	 * are created and they receive an index, which corresponds to their
> +	 * position in the bitmask. Bit N corresponds to
> +	 * :ref:`CRTC index<crtc_index>` N.
> +	 */
>  	__u32 possible_crtcs;
> +	/** @gamma_size: Number of entries of the legacy gamma lookup table. */
>  	__u32 gamma_size;
> 
> +	/** @count_format_types: Number of formats. */
>  	__u32 count_format_types;
> +	/**
> +	 * @format_type_ptr: Pointer to ``__u32`` array of formats that are
> +	 * supported by the plane. These formats do not require modifiers.
> +	 */
>  	__u64 format_type_ptr;
>  };
> 
> --
> 2.31.1
> 
>
Pekka Paalanen June 10, 2021, 8:36 a.m. UTC | #2
On Wed, 9 Jun 2021 20:05:06 -0300
Leandro Ribeiro <leandro.ribeiro@collabora.com> wrote:

> On 6/9/21 8:00 PM, Leandro Ribeiro wrote:
> > Add a small description and document struct fields of
> > drm_mode_get_plane.
> > 
> > Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
> > ---
> >  include/uapi/drm/drm_mode.h | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index a5e76aa06ad5..67bcd8e1931c 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -312,16 +312,52 @@ struct drm_mode_set_plane {
> >  	__u32 src_w;
> >  };
> > 
> > +/**
> > + * struct drm_mode_get_plane - Get plane metadata.
> > + *
> > + * Userspace can perform a GETPLANE ioctl to retrieve information about a
> > + * plane.
> > + *
> > + * To retrieve the number of formats supported, set @count_format_types to zero
> > + * and call the ioctl. @count_format_types will be updated with the value.
> > + *
> > + * To retrieve these formats, allocate an array with the memory needed to store
> > + * @count_format_types formats. Point @format_type_ptr to this array and call
> > + * the ioctl again (with @count_format_types still set to the value returned in
> > + * the first ioctl call).
> > + *
> > + * Between one ioctl and the other, the number of formats may change.
> > + * Userspace should retry the last ioctl until this number stabilizes. The
> > + * kernel won't fill any array which doesn't have the expected length.
> > + */  
> 
> Actually I don't know if this last paragraph applies. For connectors,
> for instance, I can see this happening because of hot-plugging. But for
> plane formats I have no idea. As in libdrm we have this algorithm, I've
> decided to describe it here.

Hi,

I think it's fine.

However, the sentence "The kernel won't fill any array which doesn't
have the expected length." seems to be inaccurate, reading the code of
drm_mode_getplane(). It looks like it is enough for the array to have
sufficient space, it does not need to be the exact size.

So the libdrm algorithm may be slightly too pedantic, but it shouldn't
hurt.

Otherwise looks really good.


Thanks,
pq


> >  struct drm_mode_get_plane {
> > +	/**
> > +	 * @plane_id: Object ID of the plane whose information should be
> > +	 * retrieved. Set by caller.
> > +	 */
> >  	__u32 plane_id;
> > 
> > +	/** @crtc_id: Object ID of the current CRTC. */
> >  	__u32 crtc_id;
> > +	/** @fb_id: Object ID of the current fb. */
> >  	__u32 fb_id;
> > 
> > +	/**
> > +	 * @possible_crtcs: Bitmask of CRTC's compatible with the plane. CRTC's
> > +	 * are created and they receive an index, which corresponds to their
> > +	 * position in the bitmask. Bit N corresponds to
> > +	 * :ref:`CRTC index<crtc_index>` N.
> > +	 */
> >  	__u32 possible_crtcs;
> > +	/** @gamma_size: Number of entries of the legacy gamma lookup table. */
> >  	__u32 gamma_size;
> > 
> > +	/** @count_format_types: Number of formats. */
> >  	__u32 count_format_types;
> > +	/**
> > +	 * @format_type_ptr: Pointer to ``__u32`` array of formats that are
> > +	 * supported by the plane. These formats do not require modifiers.
> > +	 */
> >  	__u64 format_type_ptr;
> >  };
> > 
> > --
> > 2.31.1
> > 
> >
Ville Syrjälä June 10, 2021, 12:53 p.m. UTC | #3
On Wed, Jun 09, 2021 at 08:00:39PM -0300, Leandro Ribeiro wrote:
> Add a small description and document struct fields of
> drm_mode_get_plane.
> 
> Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
> ---
>  include/uapi/drm/drm_mode.h | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index a5e76aa06ad5..67bcd8e1931c 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -312,16 +312,52 @@ struct drm_mode_set_plane {
>  	__u32 src_w;
>  };
> 
> +/**
> + * struct drm_mode_get_plane - Get plane metadata.
> + *
> + * Userspace can perform a GETPLANE ioctl to retrieve information about a
> + * plane.
> + *
> + * To retrieve the number of formats supported, set @count_format_types to zero
> + * and call the ioctl. @count_format_types will be updated with the value.
> + *
> + * To retrieve these formats, allocate an array with the memory needed to store
> + * @count_format_types formats. Point @format_type_ptr to this array and call
> + * the ioctl again (with @count_format_types still set to the value returned in
> + * the first ioctl call).
> + *
> + * Between one ioctl and the other, the number of formats may change.

Can't happen.

> + * Userspace should retry the last ioctl until this number stabilizes. The
> + * kernel won't fill any array which doesn't have the expected length.
> + */
>  struct drm_mode_get_plane {
> +	/**
> +	 * @plane_id: Object ID of the plane whose information should be
> +	 * retrieved. Set by caller.
> +	 */
>  	__u32 plane_id;
> 
> +	/** @crtc_id: Object ID of the current CRTC. */
>  	__u32 crtc_id;
> +	/** @fb_id: Object ID of the current fb. */
>  	__u32 fb_id;
> 
> +	/**
> +	 * @possible_crtcs: Bitmask of CRTC's compatible with the plane. CRTC's
> +	 * are created and they receive an index, which corresponds to their
> +	 * position in the bitmask. Bit N corresponds to
> +	 * :ref:`CRTC index<crtc_index>` N.
> +	 */
>  	__u32 possible_crtcs;
> +	/** @gamma_size: Number of entries of the legacy gamma lookup table. */
>  	__u32 gamma_size;

Should be marked deprecated. There is no plane gamma in the current uapi.

> 
> +	/** @count_format_types: Number of formats. */
>  	__u32 count_format_types;
> +	/**
> +	 * @format_type_ptr: Pointer to ``__u32`` array of formats that are
> +	 * supported by the plane. These formats do not require modifiers.
> +	 */
>  	__u64 format_type_ptr;
>  };
> 
> --
> 2.31.1
diff mbox series

Patch

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index a5e76aa06ad5..67bcd8e1931c 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -312,16 +312,52 @@  struct drm_mode_set_plane {
 	__u32 src_w;
 };

+/**
+ * struct drm_mode_get_plane - Get plane metadata.
+ *
+ * Userspace can perform a GETPLANE ioctl to retrieve information about a
+ * plane.
+ *
+ * To retrieve the number of formats supported, set @count_format_types to zero
+ * and call the ioctl. @count_format_types will be updated with the value.
+ *
+ * To retrieve these formats, allocate an array with the memory needed to store
+ * @count_format_types formats. Point @format_type_ptr to this array and call
+ * the ioctl again (with @count_format_types still set to the value returned in
+ * the first ioctl call).
+ *
+ * Between one ioctl and the other, the number of formats may change.
+ * Userspace should retry the last ioctl until this number stabilizes. The
+ * kernel won't fill any array which doesn't have the expected length.
+ */
 struct drm_mode_get_plane {
+	/**
+	 * @plane_id: Object ID of the plane whose information should be
+	 * retrieved. Set by caller.
+	 */
 	__u32 plane_id;

+	/** @crtc_id: Object ID of the current CRTC. */
 	__u32 crtc_id;
+	/** @fb_id: Object ID of the current fb. */
 	__u32 fb_id;

+	/**
+	 * @possible_crtcs: Bitmask of CRTC's compatible with the plane. CRTC's
+	 * are created and they receive an index, which corresponds to their
+	 * position in the bitmask. Bit N corresponds to
+	 * :ref:`CRTC index<crtc_index>` N.
+	 */
 	__u32 possible_crtcs;
+	/** @gamma_size: Number of entries of the legacy gamma lookup table. */
 	__u32 gamma_size;

+	/** @count_format_types: Number of formats. */
 	__u32 count_format_types;
+	/**
+	 * @format_type_ptr: Pointer to ``__u32`` array of formats that are
+	 * supported by the plane. These formats do not require modifiers.
+	 */
 	__u64 format_type_ptr;
 };