[PATCH] drm: document DRM_IOCTL_MODE_RMFB

[PATCH] drm: document DRM_IOCTL_MODE_RMFB

[Simon Ser]

Since there's no struct to attach the docs to, document the IOCTL
definition.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
Cc: Leandro Ribeiro <leandro.ribeiro@collabora.com>
---
 include/uapi/drm/drm.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index e1f49dd241f7..283d61d5a243 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -1050,6 +1050,16 @@ extern "C" {
 #define DRM_IOCTL_MODE_GETPROPBLOB	DRM_IOWR(0xAC, struct drm_mode_get_blob)
 #define DRM_IOCTL_MODE_GETFB		DRM_IOWR(0xAD, struct drm_mode_fb_cmd)
 #define DRM_IOCTL_MODE_ADDFB		DRM_IOWR(0xAE, struct drm_mode_fb_cmd)
+/**
+ * DRM_IOCTL_MODE_RMFB - Remove a framebuffer.
+ *
+ * This removes a framebuffer previously added via ADDFB/ADDFB2. The IOCTL
+ * argument is a framebuffer object ID.
+ *
+ * Warning: removing a framebuffer currently in-use on an enabled plane will
+ * disable that plane. If the plane is primary, the CRTC may also be disabled
+ * (depending on driver capabilities).
+ */
 #define DRM_IOCTL_MODE_RMFB		DRM_IOWR(0xAF, unsigned int)
 #define DRM_IOCTL_MODE_PAGE_FLIP	DRM_IOWR(0xB0, struct drm_mode_crtc_page_flip)
 #define DRM_IOCTL_MODE_DIRTYFB		DRM_IOWR(0xB1, struct drm_mode_fb_dirty_cmd)
-- 
2.32.0

Re: [PATCH] drm: document DRM_IOCTL_MODE_RMFB

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

On Mon, 26 Jul 2021 07:50:32 +0000
Simon Ser <contact@emersion.fr> wrote:

> Since there's no struct to attach the docs to, document the IOCTL
> definition.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> Cc: Leandro Ribeiro <leandro.ribeiro@collabora.com>
> ---
>  include/uapi/drm/drm.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index e1f49dd241f7..283d61d5a243 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -1050,6 +1050,16 @@ extern "C" {
>  #define DRM_IOCTL_MODE_GETPROPBLOB	DRM_IOWR(0xAC, struct drm_mode_get_blob)
>  #define DRM_IOCTL_MODE_GETFB		DRM_IOWR(0xAD, struct drm_mode_fb_cmd)
>  #define DRM_IOCTL_MODE_ADDFB		DRM_IOWR(0xAE, struct drm_mode_fb_cmd)
> +/**
> + * DRM_IOCTL_MODE_RMFB - Remove a framebuffer.
> + *
> + * This removes a framebuffer previously added via ADDFB/ADDFB2. The IOCTL
> + * argument is a framebuffer object ID.
> + *
> + * Warning: removing a framebuffer currently in-use on an enabled plane will
> + * disable that plane. If the plane is primary, the CRTC may also be disabled
> + * (depending on driver capabilities).

Hi,

I recall some discussions around having a CRTC running with only a
non-primary plane active. Maybe we should just drop the "If the plane
is primary," part to leave it more open.

I believe no-one should take advantage of RmFB turning a CRTC off. It's
just something that can happen, so userspace needs to be aware to avoid
it.

With or without that change:

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


Thanks,
pq

> + */
>  #define DRM_IOCTL_MODE_RMFB		DRM_IOWR(0xAF, unsigned int)
>  #define DRM_IOCTL_MODE_PAGE_FLIP	DRM_IOWR(0xB0, struct drm_mode_crtc_page_flip)
>  #define DRM_IOCTL_MODE_DIRTYFB		DRM_IOWR(0xB1, struct drm_mode_fb_dirty_cmd)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] drm: document DRM_IOCTL_MODE_RMFB

[Daniel Vetter]

On Tue, Jul 27, 2021 at 11:09:13AM +0300, Pekka Paalanen wrote:
> On Mon, 26 Jul 2021 07:50:32 +0000
> Simon Ser <contact@emersion.fr> wrote:
> 
> > Since there's no struct to attach the docs to, document the IOCTL
> > definition.
> > 
> > Signed-off-by: Simon Ser <contact@emersion.fr>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> > Cc: Leandro Ribeiro <leandro.ribeiro@collabora.com>
> > ---
> >  include/uapi/drm/drm.h | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > index e1f49dd241f7..283d61d5a243 100644
> > --- a/include/uapi/drm/drm.h
> > +++ b/include/uapi/drm/drm.h
> > @@ -1050,6 +1050,16 @@ extern "C" {
> >  #define DRM_IOCTL_MODE_GETPROPBLOB	DRM_IOWR(0xAC, struct drm_mode_get_blob)
> >  #define DRM_IOCTL_MODE_GETFB		DRM_IOWR(0xAD, struct drm_mode_fb_cmd)
> >  #define DRM_IOCTL_MODE_ADDFB		DRM_IOWR(0xAE, struct drm_mode_fb_cmd)
> > +/**
> > + * DRM_IOCTL_MODE_RMFB - Remove a framebuffer.
> > + *
> > + * This removes a framebuffer previously added via ADDFB/ADDFB2. The IOCTL
> > + * argument is a framebuffer object ID.
> > + *
> > + * Warning: removing a framebuffer currently in-use on an enabled plane will
> > + * disable that plane. If the plane is primary, the CRTC may also be disabled
> > + * (depending on driver capabilities).
> 
> Hi,
> 
> I recall some discussions around having a CRTC running with only a
> non-primary plane active. Maybe we should just drop the "If the plane
> is primary," part to leave it more open.

Yeah this wording is only correct for legacy drivers, for atomic ones we
can nuke the CRTC in any case if the plane disable fails to connect.
Legacy drivers just shrug (but there's none left with universal planes
anyway I think).

Also the atomic here isn't the userspace visible part, this is about
whether the driver is internally considered atomic.

tldr; Drop the primary plane qualifier like Pekka suggests, with that:

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

-Daniel

> 
> I believe no-one should take advantage of RmFB turning a CRTC off. It's
> just something that can happen, so userspace needs to be aware to avoid
> it.
> 
> With or without that change:
> 
> Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> 
> 
> Thanks,
> pq
> 
> > + */
> >  #define DRM_IOCTL_MODE_RMFB		DRM_IOWR(0xAF, unsigned int)
> >  #define DRM_IOCTL_MODE_PAGE_FLIP	DRM_IOWR(0xB0, struct drm_mode_crtc_page_flip)
> >  #define DRM_IOCTL_MODE_DIRTYFB		DRM_IOWR(0xB1, struct drm_mode_fb_dirty_cmd)
> 



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