[RFC] drm: add unref_fb ioctl

Rob Clark robdclark at gmail.com
Tue May 9 15:36:54 UTC 2017


Similar to rmfb but does not have the side effect of shutting down any
pipes that are scanning out the fb.

Advantages compared to rmfb:
  * slightly easier userspace, it doesn't have to track fb-id's until
    they come of the screen
  * it might be desirable to keep existing layers on screen across
    process restart (for crashing or upgrading the compositor)

Disadvantages:
  * depending on userspace architecture, layers left on screen could
    be considered an information leak, ie. new incoming master process
    has access to buffers that are still being scanned out.

Signed-off-by: Rob Clark <robdclark at gmail.com>
---
 drivers/gpu/drm/drm_crtc_internal.h |  2 ++
 drivers/gpu/drm/drm_framebuffer.c   | 66 +++++++++++++++++++++++++++----------
 drivers/gpu/drm/drm_ioctl.c         |  1 +
 include/uapi/drm/drm.h              |  1 +
 include/uapi/drm/drm_mode.h         | 20 +++++++++++
 5 files changed, 72 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 8c04275..dafa17b 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -167,6 +167,8 @@ int drm_mode_addfb2(struct drm_device *dev,
 		    void *data, struct drm_file *file_priv);
 int drm_mode_rmfb(struct drm_device *dev,
 		  void *data, struct drm_file *file_priv);
+int drm_mode_unref_fb(struct drm_device *dev,
+		      void *data, struct drm_file *file_priv);
 int drm_mode_getfb(struct drm_device *dev,
 		   void *data, struct drm_file *file_priv);
 int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index e8f9c13..8f2afdf 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -356,31 +356,17 @@ static void drm_mode_rmfb_work_fn(struct work_struct *w)
 	}
 }
 
-/**
- * drm_mode_rmfb - remove an FB from the configuration
- * @dev: drm device for the ioctl
- * @data: data pointer for the ioctl
- * @file_priv: drm file for the ioctl call
- *
- * Remove the FB specified by the user.
- *
- * Called by the user via ioctl.
- *
- * Returns:
- * Zero on success, negative errno on failure.
- */
-int drm_mode_rmfb(struct drm_device *dev,
-		   void *data, struct drm_file *file_priv)
+static int __rmfb(struct drm_device *dev, struct drm_file *file_priv,
+		  uint32_t fb_id, bool rmfb)
 {
 	struct drm_framebuffer *fb = NULL;
 	struct drm_framebuffer *fbl = NULL;
-	uint32_t *id = data;
 	int found = 0;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	fb = drm_framebuffer_lookup(dev, *id);
+	fb = drm_framebuffer_lookup(dev, fb_id);
 	if (!fb)
 		return -ENOENT;
 
@@ -406,7 +392,7 @@ int drm_mode_rmfb(struct drm_device *dev,
 	 * so run this in a separate stack as there's no way to correctly
 	 * handle this after the fb is already removed from the lookup table.
 	 */
-	if (drm_framebuffer_read_refcount(fb) > 1) {
+	if (rmfb && drm_framebuffer_read_refcount(fb) > 1) {
 		struct drm_mode_rmfb_work arg;
 
 		INIT_WORK_ONSTACK(&arg.work, drm_mode_rmfb_work_fn);
@@ -427,6 +413,50 @@ int drm_mode_rmfb(struct drm_device *dev,
 }
 
 /**
+ * drm_mode_rmfb - remove an FB from the configuration
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
+ *
+ * Remove the FB specified by the user.
+ *
+ * Called by the user via ioctl.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_mode_rmfb(struct drm_device *dev,
+		  void *data, struct drm_file *file_priv)
+{
+	uint32_t *id = data;
+	return __rmfb(dev, file_priv, *id, true);
+}
+
+/**
+ * drm_mode_unref_fb - unreference an FB
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
+ *
+ * Unreference the FB specified by the user.
+ *
+ * Called by the user via ioctl.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_mode_unref_fb(struct drm_device *dev, void *data,
+		      struct drm_file *file_priv)
+{
+	struct drm_mode_unref_fb *r = data;
+
+	if (r->pad)
+		return -EINVAL;
+
+	return __rmfb(dev, file_priv, r->fb_id, false);
+}
+
+/**
  * drm_mode_getfb - get FB info
  * @dev: drm device for the ioctl
  * @data: data pointer for the ioctl
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 7d6deaa..a113972 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -642,6 +642,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATOMIC, drm_mode_atomic_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATEPROPBLOB, drm_mode_createblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROYPROPBLOB, drm_mode_destroyblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_UNREFFB, drm_mode_unref_fb, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 };
 
 #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index b2c5284..b485253 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -813,6 +813,7 @@ extern "C" {
 #define DRM_IOCTL_MODE_ATOMIC		DRM_IOWR(0xBC, struct drm_mode_atomic)
 #define DRM_IOCTL_MODE_CREATEPROPBLOB	DRM_IOWR(0xBD, struct drm_mode_create_blob)
 #define DRM_IOCTL_MODE_DESTROYPROPBLOB	DRM_IOWR(0xBE, struct drm_mode_destroy_blob)
+#define DRM_IOCTL_MODE_UNREFFB		DRM_IOR( 0xBF, struct drm_mode_unref_fb)
 
 /**
  * Device specific ioctls should only be in their respective headers
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 8c67fc0..1804b2d 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -685,6 +685,26 @@ struct drm_mode_destroy_blob {
 	__u32 blob_id;
 };
 
+/**
+ * Similar to rmfb but does not have the side effect of shutting down any
+ * pipes that are scanning out the fb.
+ *
+ * Advantages compared to rmfb:
+ *   * slightly easier userspace, it doesn't have to track fb-id's until
+ *     they come of the screen
+ *   * it might be desirable to keep existing layers on screen across
+ *     process restart (for crashing or upgrading the compositor)
+ *
+ * Disadvantages:
+ *   * depending on userspace architecture, layers left on screen could
+ *     be considered an information leak, ie. new incoming master process
+ *     has access to buffers that are still being scanned out.
+ */
+struct drm_mode_unref_fb {
+	__u32 pad;      /* must be zero for now.. */
+	__u32 fb_id;
+};
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.9.3



More information about the dri-devel mailing list