[PATCH] drm: document DRM_IOCTL_MODE_GETFB2

[Daniel Vetter]

On Tue, Nov 09, 2021 at 08:56:10AM +0000, Simon Ser wrote:
> There are a few details specific to the GETFB2 IOCTL.
> 
> It's not immediately clear how user-space should check for the
> number of planes. Suggest using the pitches field.
> 
> The modifier array is filled with zeroes, ie. DRM_FORMAT_MOD_LINEAR.
> So explicitly tell user-space to not look at it unless the flag is
> set.
> 
> Signed-off-by: Simon Ser <contact at emersion.fr>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen at gmail.com>
> ---
>  include/uapi/drm/drm.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 3b810b53ba8b..9809078b0f51 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -1096,6 +1096,22 @@ extern "C" {
>  #define DRM_IOCTL_SYNCOBJ_TRANSFER	DRM_IOWR(0xCC, struct drm_syncobj_transfer)
>  #define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL	DRM_IOWR(0xCD, struct drm_syncobj_timeline_array)
>  
> +/**
> + * DRM_IOCTL_MODE_GETFB2 - Get framebuffer metadata.
> + *
> + * This queries metadata about a framebuffer. User-space fills
> + * &drm_mode_fb_cmd2.fb_id as the input, and the kernels fills the rest of the
> + * struct as the output.
> + *
> + * If the client is not DRM master and doesn't have &CAP_SYS_ADMIN,
> + * &drm_mode_fb_cmd2.handles will be zeroed. Planes are valid until one has a
> + * zero &drm_mode_fb_cmd2.pitches -- this can be used to compute the number of
> + * planes.

Maybe explain that when actually importing the buffer userspace should
look for non-zeor handles instead, since some drivers/formats leave a
silly pitch value behind. Or at least I think that can happen. Just for
additional robustness?

> + *
> + * If the framebuffer has a format modifier, &DRM_MODE_FB_MODIFIERS will be set
> + * in &drm_mode_fb_cmd2.flags. Otherwise, &drm_mode_fb_cmd2.modifier has
> + * undefined contents.

Uh is this true? We should always clear values to avoid accidental leaks
and stuff.

> + */

I think kerneldoc for drm_mode_fb_cmd2 would be neat too, so we can
document how pitch is supposed to work and all that stuff.

Anyway lgtm otherwise.
-Daniel

>  #define DRM_IOCTL_MODE_GETFB2		DRM_IOWR(0xCE, struct drm_mode_fb_cmd2)
>  
>  /*
> -- 
> 2.33.1
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch