[RFC 0/7] drm: Add generic fbdev emulation

[RFC 0/7] drm: Add generic fbdev emulation

[Noralf Trønnes]

This patchset explores the possibility of having generic fbdev emulation
in DRM for drivers that supports dumb buffers which they can export.

I was about to polish up my 'vmalloc BO helper' series, which has fbdev
support, when this idea[1] of Laurent popped into my mind:

  Ideally I'd like to remove 100% of fbdev-related code from drivers. This
  includes
  - initialization and cleanup of fbdev helpers
  - fbdev restore in last_close()
  - forwarding of hotplug events to fbdev compatibility layer

  In practice we'll likely need to keep one initialization function (or a
  drm_driver flag) to let drivers opt-in to fbdev compatibility, but the rest
  should go away. Or maybe we could even enable fbdev compatibility in all
  drivers unconditionally.

So I set out to see what it would take to accomplish that.

Noralf.

[1] https://lists.freedesktop.org/archives/dri-devel/2017-September/152612.html


David Herrmann (1):
  drm: provide management functions for drm_file

Noralf Trønnes (6):
  drm/fb-helper: Ensure driver module is pinned in fb_open()
  drm/fb-helper: Don't restore if fbdev is not in use
  drm/prime: Clear drm_gem_object->dma_buf on release
  drm: Handle fbdev emulation in core
  drm/fb-helper: Add generic fbdev emulation
  drm/vc4: Test generic fbdev emulation

 drivers/gpu/drm/drm_fb_helper.c    | 344 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_file.c         | 321 ++++++++++++++++++++--------------
 drivers/gpu/drm/drm_internal.h     |   2 +
 drivers/gpu/drm/drm_mode_config.c  |  10 ++
 drivers/gpu/drm/drm_prime.c        |   1 +
 drivers/gpu/drm/drm_probe_helper.c |   4 +
 drivers/gpu/drm/vc4/vc4_drv.c      |   3 -
 drivers/gpu/drm/vc4/vc4_kms.c      |   3 +-
 include/drm/drm_fb_helper.h        |  95 ++++++++++
 9 files changed, 645 insertions(+), 138 deletions(-)

-- 
2.14.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 1/7] drm: provide management functions for drm_file

[Noralf Trønnes]

From: David Herrmann <dh.herrmann@gmail.com>

Rather than doing drm_file allocation/destruction right in the fops, lets
provide separate helpers. This decouples drm_file management from the
still-mandatory drm-fops. It prepares for use of drm_file without the
fops, both by possible separate fops implementations and APIs (not that I
am aware of any such plans), and more importantly from in-kernel use where
no real file is available.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
[rebased]
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_file.c     | 309 +++++++++++++++++++++++------------------
 drivers/gpu/drm/drm_internal.h |   2 +
 2 files changed, 179 insertions(+), 132 deletions(-)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index b3c6e997ccdb..d208faade27e 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -101,6 +101,179 @@ DEFINE_MUTEX(drm_global_mutex);
 
 static int drm_open_helper(struct file *filp, struct drm_minor *minor);
 
+/**
+ * drm_file_alloc - allocate file context
+ * @minor: minor to allocate on
+ *
+ * This allocates a new DRM file context. It is not linked into any context and
+ * can be used by the caller freely. Note that the context keeps a pointer to
+ * @minor, so it must be freed before @minor is.
+ *
+ * The legacy paths might require the drm_global_mutex to be held.
+ *
+ * RETURNS:
+ * Pointer to newly allocated context, ERR_PTR on failure.
+ */
+struct drm_file *drm_file_alloc(struct drm_minor *minor)
+{
+	struct drm_device *dev = minor->dev;
+	struct drm_file *file;
+	int ret;
+
+	file = kzalloc(sizeof(*file), GFP_KERNEL);
+	if (!file)
+		return ERR_PTR(-ENOMEM);
+
+	file->pid = get_pid(task_pid(current));
+	file->minor = minor;
+
+	/* for compatibility root is always authenticated */
+	file->authenticated = capable(CAP_SYS_ADMIN);
+	file->lock_count = 0;
+
+	INIT_LIST_HEAD(&file->lhead);
+	INIT_LIST_HEAD(&file->fbs);
+	mutex_init(&file->fbs_lock);
+	INIT_LIST_HEAD(&file->blobs);
+	INIT_LIST_HEAD(&file->pending_event_list);
+	INIT_LIST_HEAD(&file->event_list);
+	init_waitqueue_head(&file->event_wait);
+	file->event_space = 4096; /* set aside 4k for event buffer */
+
+	mutex_init(&file->event_read_lock);
+
+	if (drm_core_check_feature(dev, DRIVER_GEM))
+		drm_gem_open(dev, file);
+
+	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+		drm_syncobj_open(file);
+
+	if (drm_core_check_feature(dev, DRIVER_PRIME))
+		drm_prime_init_file_private(&file->prime);
+
+	if (dev->driver->open) {
+		ret = dev->driver->open(dev, file);
+		if (ret < 0)
+			goto out_prime_destroy;
+	}
+
+	if (drm_is_primary_client(file)) {
+		ret = drm_master_open(file);
+		if (ret)
+			goto out_close;
+	}
+
+	return file;
+
+out_close:
+	if (dev->driver->postclose)
+		dev->driver->postclose(dev, file);
+out_prime_destroy:
+	if (drm_core_check_feature(dev, DRIVER_PRIME))
+		drm_prime_destroy_file_private(&file->prime);
+	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+		drm_syncobj_release(file);
+	if (drm_core_check_feature(dev, DRIVER_GEM))
+		drm_gem_release(dev, file);
+	put_pid(file->pid);
+	kfree(file);
+
+	return ERR_PTR(ret);
+}
+
+static void drm_events_release(struct drm_file *file_priv)
+{
+	struct drm_device *dev = file_priv->minor->dev;
+	struct drm_pending_event *e, *et;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+
+	/* Unlink pending events */
+	list_for_each_entry_safe(e, et, &file_priv->pending_event_list,
+				 pending_link) {
+		list_del(&e->pending_link);
+		e->file_priv = NULL;
+	}
+
+	/* Remove unconsumed events */
+	list_for_each_entry_safe(e, et, &file_priv->event_list, link) {
+		list_del(&e->link);
+		kfree(e);
+	}
+
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+}
+
+/**
+ * drm_file_free - free file context
+ * @file: context to free, or NULL
+ *
+ * This destroys and deallocates a DRM file context previously allocated via
+ * drm_file_alloc(). The caller must make sure to unlink it from any contexts
+ * before calling this.
+ *
+ * The legacy paths might require the drm_global_mutex to be held.
+ *
+ * If NULL is passed, this is a no-op.
+ *
+ * RETURNS:
+ * 0 on success, or error code on failure.
+ */
+void drm_file_free(struct drm_file *file)
+{
+	struct drm_device *dev;
+
+	if (!file)
+		return;
+
+	dev = file->minor->dev;
+
+	DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n",
+		  task_pid_nr(current),
+		  (long)old_encode_dev(file->minor->kdev->devt),
+		  dev->open_count);
+
+	if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
+	    dev->driver->preclose)
+		dev->driver->preclose(dev, file);
+
+	if (drm_core_check_feature(dev, DRIVER_LEGACY))
+		drm_legacy_lock_release(dev, file->filp);
+
+	if (drm_core_check_feature(dev, DRIVER_HAVE_DMA))
+		drm_legacy_reclaim_buffers(dev, file);
+
+	drm_events_release(file);
+
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		drm_fb_release(file);
+		drm_property_destroy_user_blobs(dev, file);
+	}
+
+	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+		drm_syncobj_release(file);
+
+	if (drm_core_check_feature(dev, DRIVER_GEM))
+		drm_gem_release(dev, file);
+
+	drm_legacy_ctxbitmap_flush(dev, file);
+
+	if (drm_is_primary_client(file))
+		drm_master_release(file);
+
+	if (dev->driver->postclose)
+		dev->driver->postclose(dev, file);
+
+	if (drm_core_check_feature(dev, DRIVER_PRIME))
+		drm_prime_destroy_file_private(&file->prime);
+
+	WARN_ON(!list_empty(&file->event_list));
+
+	put_pid(file->pid);
+	kfree(file);
+}
+
 static int drm_setup(struct drm_device * dev)
 {
 	int ret;
@@ -196,7 +369,6 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 {
 	struct drm_device *dev = minor->dev;
 	struct drm_file *priv;
-	int ret;
 
 	if (filp->f_flags & O_EXCL)
 		return -EBUSY;	/* No exclusive opens */
@@ -207,50 +379,12 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 
 	DRM_DEBUG("pid = %d, minor = %d\n", task_pid_nr(current), minor->index);
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
+	priv = drm_file_alloc(minor);
+	if (IS_ERR(priv))
+		return PTR_ERR(priv);
 
 	filp->private_data = priv;
 	priv->filp = filp;
-	priv->pid = get_pid(task_pid(current));
-	priv->minor = minor;
-
-	/* for compatibility root is always authenticated */
-	priv->authenticated = capable(CAP_SYS_ADMIN);
-	priv->lock_count = 0;
-
-	INIT_LIST_HEAD(&priv->lhead);
-	INIT_LIST_HEAD(&priv->fbs);
-	mutex_init(&priv->fbs_lock);
-	INIT_LIST_HEAD(&priv->blobs);
-	INIT_LIST_HEAD(&priv->pending_event_list);
-	INIT_LIST_HEAD(&priv->event_list);
-	init_waitqueue_head(&priv->event_wait);
-	priv->event_space = 4096; /* set aside 4k for event buffer */
-
-	mutex_init(&priv->event_read_lock);
-
-	if (drm_core_check_feature(dev, DRIVER_GEM))
-		drm_gem_open(dev, priv);
-
-	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
-		drm_syncobj_open(priv);
-
-	if (drm_core_check_feature(dev, DRIVER_PRIME))
-		drm_prime_init_file_private(&priv->prime);
-
-	if (dev->driver->open) {
-		ret = dev->driver->open(dev, priv);
-		if (ret < 0)
-			goto out_prime_destroy;
-	}
-
-	if (drm_is_primary_client(priv)) {
-		ret = drm_master_open(priv);
-		if (ret)
-			goto out_close;
-	}
 
 	mutex_lock(&dev->filelist_mutex);
 	list_add(&priv->lhead, &dev->filelist);
@@ -277,45 +411,6 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 #endif
 
 	return 0;
-
-out_close:
-	if (dev->driver->postclose)
-		dev->driver->postclose(dev, priv);
-out_prime_destroy:
-	if (drm_core_check_feature(dev, DRIVER_PRIME))
-		drm_prime_destroy_file_private(&priv->prime);
-	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
-		drm_syncobj_release(priv);
-	if (drm_core_check_feature(dev, DRIVER_GEM))
-		drm_gem_release(dev, priv);
-	put_pid(priv->pid);
-	kfree(priv);
-	filp->private_data = NULL;
-	return ret;
-}
-
-static void drm_events_release(struct drm_file *file_priv)
-{
-	struct drm_device *dev = file_priv->minor->dev;
-	struct drm_pending_event *e, *et;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-
-	/* Unlink pending events */
-	list_for_each_entry_safe(e, et, &file_priv->pending_event_list,
-				 pending_link) {
-		list_del(&e->pending_link);
-		e->file_priv = NULL;
-	}
-
-	/* Remove unconsumed events */
-	list_for_each_entry_safe(e, et, &file_priv->event_list, link) {
-		list_del(&e->link);
-		kfree(e);
-	}
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
 static void drm_legacy_dev_reinit(struct drm_device *dev)
@@ -382,57 +477,7 @@ int drm_release(struct inode *inode, struct file *filp)
 	list_del(&file_priv->lhead);
 	mutex_unlock(&dev->filelist_mutex);
 
-	if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
-	    dev->driver->preclose)
-		dev->driver->preclose(dev, file_priv);
-
-	/* ========================================================
-	 * Begin inline drm_release
-	 */
-
-	DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n",
-		  task_pid_nr(current),
-		  (long)old_encode_dev(file_priv->minor->kdev->devt),
-		  dev->open_count);
-
-	if (drm_core_check_feature(dev, DRIVER_LEGACY))
-		drm_legacy_lock_release(dev, filp);
-
-	if (drm_core_check_feature(dev, DRIVER_HAVE_DMA))
-		drm_legacy_reclaim_buffers(dev, file_priv);
-
-	drm_events_release(file_priv);
-
-	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		drm_fb_release(file_priv);
-		drm_property_destroy_user_blobs(dev, file_priv);
-	}
-
-	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
-		drm_syncobj_release(file_priv);
-
-	if (drm_core_check_feature(dev, DRIVER_GEM))
-		drm_gem_release(dev, file_priv);
-
-	drm_legacy_ctxbitmap_flush(dev, file_priv);
-
-	if (drm_is_primary_client(file_priv))
-		drm_master_release(file_priv);
-
-	if (dev->driver->postclose)
-		dev->driver->postclose(dev, file_priv);
-
-	if (drm_core_check_feature(dev, DRIVER_PRIME))
-		drm_prime_destroy_file_private(&file_priv->prime);
-
-	WARN_ON(!list_empty(&file_priv->event_list));
-
-	put_pid(file_priv->pid);
-	kfree(file_priv);
-
-	/* ========================================================
-	 * End inline drm_release
-	 */
+	drm_file_free(file_priv);
 
 	if (!--dev->open_count) {
 		drm_lastclose(dev);
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index b72242e93ea4..40179c5fc6b8 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -26,6 +26,8 @@
 
 /* drm_file.c */
 extern struct mutex drm_global_mutex;
+struct drm_file *drm_file_alloc(struct drm_minor *minor);
+void drm_file_free(struct drm_file *file);
 void drm_lastclose(struct drm_device *dev);
 
 /* drm_pci.c */
-- 
2.14.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 2/7] drm/fb-helper: Ensure driver module is pinned in fb_open()

[Noralf Trønnes]

If struct fb_ops is defined in a library like cma, fb_open() and fbcon
takes a ref on the library instead of the driver module. Use
fb_ops.fb_open/fb_release to ensure that the driver module is pinned.

Add drm_fb_helper_fb_open() and drm_fb_helper_fb_release() to
DRM_FB_HELPER_DEFAULT_OPS().

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 40 ++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_fb_helper.h     | 15 +++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 035784ddd133..2c6adf1d80c2 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1207,6 +1207,46 @@ void drm_fb_helper_cfb_imageblit(struct fb_info *info,
 }
 EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit);
 
+/**
+ * drm_fb_helper_fb_open - implementation for &fb_ops.fb_open
+ * @info: fbdev registered by the helper
+ * @user: 1=userspace, 0=fbcon
+ *
+ * If &fb_ops is wrapped in a library, pin the driver module.
+ */
+int drm_fb_helper_fb_open(struct fb_info *info, int user)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_device *dev = fb_helper->dev;
+
+	if (info->fbops->owner != dev->driver->fops->owner) {
+		if (!try_module_get(dev->driver->fops->owner))
+			return -ENODEV;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_fb_open);
+
+/**
+ * drm_fb_helper_fb_release - implementation for &fb_ops.fb_release
+ * @info: fbdev registered by the helper
+ * @user: 1=userspace, 0=fbcon
+ *
+ * If &fb_ops is wrapped in a library, unpin the driver module.
+ */
+int drm_fb_helper_fb_release(struct fb_info *info, int user)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_device *dev = fb_helper->dev;
+
+	if (info->fbops->owner != dev->driver->fops->owner)
+		module_put(dev->driver->fops->owner);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_fb_release);
+
 /**
  * drm_fb_helper_set_suspend - wrapper around fb_set_suspend
  * @fb_helper: driver-allocated fbdev helper, can be NULL
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index b069433e7fc1..6f546ca3a6a1 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -241,6 +241,8 @@ struct drm_fb_helper {
  * functions. To be used in struct fb_ops of drm drivers.
  */
 #define DRM_FB_HELPER_DEFAULT_OPS \
+	.fb_open	= drm_fb_helper_fb_open, \
+	.fb_release	= drm_fb_helper_fb_release, \
 	.fb_check_var	= drm_fb_helper_check_var, \
 	.fb_set_par	= drm_fb_helper_set_par, \
 	.fb_setcmap	= drm_fb_helper_setcmap, \
@@ -297,6 +299,9 @@ void drm_fb_helper_cfb_copyarea(struct fb_info *info,
 void drm_fb_helper_cfb_imageblit(struct fb_info *info,
 				 const struct fb_image *image);
 
+int drm_fb_helper_fb_open(struct fb_info *info, int user);
+int drm_fb_helper_fb_release(struct fb_info *info, int user);
+
 void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, bool suspend);
 void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
 					bool suspend);
@@ -473,6 +478,16 @@ static inline void drm_fb_helper_cfb_imageblit(struct fb_info *info,
 {
 }
 
+static inline int drm_fb_helper_fb_open(struct fb_info *info, int user)
+{
+	return -ENODEV;
+}
+
+static inline int drm_fb_helper_fb_release(struct fb_info *info, int user)
+{
+	return -ENODEV;
+}
+
 static inline void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper,
 					     bool suspend)
 {
-- 
2.14.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 3/7] drm/fb-helper: Don't restore if fbdev is not in use

[Noralf Trønnes]

Keep track of fbdev users and only restore fbdev in
drm_fb_helper_restore_fbdev_mode_unlocked() when in use. This avoids
fbdev being restored in drm_driver.last_close when nothing uses it.
Additionally fbdev is turned off when the last user is closing.
fbcon is a user in this context.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 15 +++++++++++++++
 include/drm/drm_fb_helper.h     | 14 ++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 2c6adf1d80c2..f9dcc7a5761f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -522,6 +522,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	if (READ_ONCE(fb_helper->deferred_setup))
 		return 0;
 
+	if (!atomic_read(&fb_helper->open_count))
+		return 0;
+
 	mutex_lock(&fb_helper->lock);
 	ret = restore_fbdev_mode(fb_helper);
 
@@ -781,6 +784,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
 	INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker);
 	INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work);
 	helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
+	atomic_set(&helper->open_count, 1);
 	mutex_init(&helper->lock);
 	helper->funcs = funcs;
 	helper->dev = dev;
@@ -1212,6 +1216,7 @@ EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit);
  * @info: fbdev registered by the helper
  * @user: 1=userspace, 0=fbcon
  *
+ * Increase fbdev use count.
  * If &fb_ops is wrapped in a library, pin the driver module.
  */
 int drm_fb_helper_fb_open(struct fb_info *info, int user)
@@ -1224,6 +1229,8 @@ int drm_fb_helper_fb_open(struct fb_info *info, int user)
 			return -ENODEV;
 	}
 
+	atomic_inc(&fb_helper->open_count);
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_fb_helper_fb_open);
@@ -1233,6 +1240,7 @@ EXPORT_SYMBOL(drm_fb_helper_fb_open);
  * @info: fbdev registered by the helper
  * @user: 1=userspace, 0=fbcon
  *
+ * Decrease fbdev use count and turn off if there are no users left.
  * If &fb_ops is wrapped in a library, unpin the driver module.
  */
 int drm_fb_helper_fb_release(struct fb_info *info, int user)
@@ -1240,6 +1248,10 @@ int drm_fb_helper_fb_release(struct fb_info *info, int user)
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_device *dev = fb_helper->dev;
 
+	if (atomic_dec_and_test(&fb_helper->open_count) &&
+	    !drm_dev_is_unplugged(fb_helper->dev))
+		drm_fb_helper_blank(FB_BLANK_POWERDOWN, info);
+
 	if (info->fbops->owner != dev->driver->fops->owner)
 		module_put(dev->driver->fops->owner);
 
@@ -1936,6 +1948,9 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	if (ret < 0)
 		return ret;
 
+	if (fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open)
+		atomic_set(&fb_helper->open_count, 0);
+
 	strcpy(fb_helper->fb->comm, "[fbcon]");
 	return 0;
 }
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 6f546ca3a6a1..5d4ccc3f28aa 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -232,6 +232,20 @@ struct drm_fb_helper {
 	 * See also: @deferred_setup
 	 */
 	int preferred_bpp;
+
+	/**
+	 * @open_count:
+	 *
+	 * Keeps track of fbdev use to know when to not restore fbdev.
+	 *
+	 * Drivers that use DRM_FB_HELPER_DEFAULT_OPS and don't override
+	 * \.fb_open will get an initial value of 0 and get restore based on
+	 * actual use. Others will get an initial value of 1 which means that
+	 * fbdev will always be restored. Drivers that call
+	 * drm_fb_helper_fb_open() in their \.fb_open, thus needs to set the
+	 * initial value to 0 themselves.
+	 */
+	atomic_t open_count;
 };
 
 /**
-- 
2.14.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 4/7] drm/prime: Clear drm_gem_object->dma_buf on release

[Noralf Trønnes]

Clear the pointer so the buffer can be re-exported. Otherwise use
after free happens in the next call to drm_gem_prime_handle_to_fd().

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_prime.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 9a17725b0f7a..3214c0eb7466 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -343,6 +343,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
 
 	/* drop the reference on the export fd holds */
 	drm_gem_object_put_unlocked(obj);
+	obj->dma_buf = NULL;
 
 	drm_dev_put(dev);
 }
-- 
2.14.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 5/7] drm: Handle fbdev emulation in core

[Noralf Trønnes]

Prepare for generic fbdev emulation by letting DRM core work directly
with the fbdev compatibility layer. This is done by adding new fbdev
helper vtable callbacks for restore, hotplug_event, unregister and
release.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_file.c         | 12 +++++++++++-
 drivers/gpu/drm/drm_mode_config.c  | 10 ++++++++++
 drivers/gpu/drm/drm_probe_helper.c |  4 ++++
 include/drm/drm_fb_helper.h        | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index d208faade27e..67f0309e22ff 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -35,6 +35,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_file.h>
 #include <drm/drmP.h>
 
@@ -439,10 +440,19 @@ static void drm_legacy_dev_reinit(struct drm_device *dev)
 
 void drm_lastclose(struct drm_device * dev)
 {
+	struct drm_fb_helper *fb_helper = dev->fb_helper;
+	int ret;
+
 	DRM_DEBUG("\n");
 
-	if (dev->driver->lastclose)
+	if (dev->driver->lastclose) {
 		dev->driver->lastclose(dev);
+	} else if (fb_helper && fb_helper->funcs && fb_helper->funcs->restore) {
+		ret = fb_helper->funcs->restore(fb_helper);
+		if (ret)
+			DRM_ERROR("Failed to restore fbdev: %d\n", ret);
+	}
+
 	DRM_DEBUG("driver lastclose completed\n");
 
 	if (drm_core_check_feature(dev, DRIVER_LEGACY))
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index bc5c46306b3d..260eb1730244 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -21,6 +21,7 @@
  */
 
 #include <drm/drm_encoder.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_mode_config.h>
 #include <drm/drmP.h>
 
@@ -61,6 +62,11 @@ int drm_modeset_register_all(struct drm_device *dev)
 
 void drm_modeset_unregister_all(struct drm_device *dev)
 {
+	struct drm_fb_helper *fb_helper = dev->fb_helper;
+
+	if (fb_helper && fb_helper->funcs && fb_helper->funcs->unregister)
+		fb_helper->funcs->unregister(fb_helper);
+
 	drm_connector_unregister_all(dev);
 	drm_encoder_unregister_all(dev);
 	drm_crtc_unregister_all(dev);
@@ -408,6 +414,7 @@ EXPORT_SYMBOL(drm_mode_config_init);
  */
 void drm_mode_config_cleanup(struct drm_device *dev)
 {
+	struct drm_fb_helper *fb_helper = dev->fb_helper;
 	struct drm_connector *connector;
 	struct drm_connector_list_iter conn_iter;
 	struct drm_crtc *crtc, *ct;
@@ -417,6 +424,9 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 	struct drm_property_blob *blob, *bt;
 	struct drm_plane *plane, *plt;
 
+	if (fb_helper && fb_helper->funcs && fb_helper->funcs->release)
+		fb_helper->funcs->release(fb_helper);
+
 	list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
 				 head) {
 		encoder->funcs->destroy(encoder);
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 555fbe54d6e2..9d8b0ba54173 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -559,10 +559,14 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
  */
 void drm_kms_helper_hotplug_event(struct drm_device *dev)
 {
+	struct drm_fb_helper *fb_helper = dev->fb_helper;
+
 	/* send a uevent + call fbdev */
 	drm_sysfs_hotplug_event(dev);
 	if (dev->mode_config.funcs->output_poll_changed)
 		dev->mode_config.funcs->output_poll_changed(dev);
+	else if (fb_helper && fb_helper->funcs && fb_helper->funcs->hotplug_event)
+		fb_helper->funcs->hotplug_event(fb_helper);
 }
 EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
 
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 5d4ccc3f28aa..f5ea79bbb831 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -125,6 +125,39 @@ struct drm_fb_helper_funcs {
 			       struct drm_display_mode **modes,
 			       struct drm_fb_offset *offsets,
 			       bool *enabled, int width, int height);
+
+	/**
+	 * @restore:
+	 *
+	 * Optional callback for restoring fbdev emulation.
+	 * Called by drm_lastclose() if &drm_driver->lastclose is not set.
+	 */
+	int (*restore)(struct drm_fb_helper *fb_helper);
+
+	/**
+	 * @hotplug_event:
+	 *
+	 * Optional callback for hotplug events.
+	 * Called by drm_kms_helper_hotplug_event() if
+	 * &drm_mode_config_funcs->output_poll_changed  is not set.
+	 */
+	int (*hotplug_event)(struct drm_fb_helper *fb_helper);
+
+	/**
+	 * @unregister:
+	 *
+	 * Optional callback for unregistrering fbdev emulation.
+	 * Called by drm_dev_unregister().
+	 */
+	void (*unregister)(struct drm_fb_helper *fb_helper);
+
+	/**
+	 * @release:
+	 *
+	 * Optional callback for releasing fbdev emulation resources.
+	 * Called by drm_mode_config_cleanup().
+	 */
+	void (*release)(struct drm_fb_helper *fb_helper);
 };
 
 struct drm_fb_helper_connector {
-- 
2.14.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 6/7] drm/fb-helper: Add generic fbdev emulation

[Noralf Trønnes]

Add generic fbdev emulation which uses a drm_file to get a dumb_buffer
and drm_framebuffer. The buffer is exported and vmap/mmap called on
the dma-buf.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 289 ++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_fb_helper.h     |  33 +++++
 2 files changed, 322 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f9dcc7a5761f..d51bd5b1ecf1 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -30,6 +30,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/console.h>
+#include <linux/dma-buf.h>
 #include <linux/kernel.h>
 #include <linux/sysrq.h>
 #include <linux/slab.h>
@@ -43,6 +44,7 @@
 
 #include "drm_crtc_internal.h"
 #include "drm_crtc_helper_internal.h"
+#include "drm_internal.h"
 
 static bool drm_fbdev_emulation = true;
 module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
@@ -2975,6 +2977,293 @@ void drm_fb_helper_output_poll_changed(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
 
+static int drm_fb_helper_generic_vmap(struct drm_fb_helper *fb_helper)
+{
+	struct drm_prime_handle prime_args = {
+		.handle = fb_helper->dumb_handle,
+	};
+	struct dma_buf *dma_buf;
+	void *vaddr;
+	int ret;
+
+	ret = drm_prime_handle_to_fd_ioctl(fb_helper->dev, &prime_args,
+					   fb_helper->file);
+	if (ret)
+		return ret;
+
+	dma_buf = dma_buf_get(prime_args.fd);
+	if (WARN_ON(IS_ERR(dma_buf)))
+		return PTR_ERR(dma_buf);
+
+	/* drop the ref we picked up in handle_to_fd */
+	dma_buf_put(dma_buf);
+
+	vaddr = dma_buf_vmap(dma_buf);
+	if (!vaddr) {
+		ret = -ENOMEM;
+		goto err_remove_handle;
+	}
+
+	if (fb_helper->fbdev->fbdefio)
+		fb_deferred_io_init(fb_helper->fbdev);
+
+	fb_helper->fbdev->screen_buffer = vaddr;
+	fb_helper->dma_buf = dma_buf;
+
+	return 0;
+
+err_remove_handle:
+	drm_prime_remove_buf_handle_locked(&fb_helper->file->prime,
+					   fb_helper->dma_buf);
+	dma_buf_put(dma_buf);
+
+	return ret;
+}
+
+static void drm_fb_helper_generic_vunmap(struct drm_fb_helper *fb_helper)
+{
+	if (fb_helper->fbdev->fbdefio) {
+		cancel_delayed_work_sync(&fb_helper->fbdev->deferred_work);
+		cancel_work_sync(&fb_helper->dirty_work);
+		fb_deferred_io_cleanup(fb_helper->fbdev);
+	}
+
+	dma_buf_vunmap(fb_helper->dma_buf, fb_helper->fbdev->screen_buffer);
+	fb_helper->fbdev->screen_buffer = NULL;
+
+	drm_prime_remove_buf_handle_locked(&fb_helper->file->prime,
+					   fb_helper->dma_buf);
+	dma_buf_put(fb_helper->dma_buf);
+	fb_helper->dma_buf = NULL;
+}
+
+static int drm_fb_helper_generic_fb_open(struct fb_info *info, int user)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	int ret;
+
+	ret = drm_fb_helper_fb_open(info, user);
+	if (ret)
+		return ret;
+
+	if (!fb_helper->fbdev->screen_buffer) {
+		/*
+		 * Exporting a buffer to get a virtual address results in
+		 * dma-buf pinning the driver module. This means that we have
+		 * to vmap/vunmap in open/close to be able to unload the driver
+		 * module.
+		 */
+		ret = drm_fb_helper_generic_vmap(fb_helper);
+		if (ret) {
+			DRM_ERROR("fbdev: Failed to vmap buffer: %d\n", ret);
+			drm_fb_helper_fb_release(info, user);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int drm_fb_helper_generic_fb_release(struct fb_info *info, int user)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+
+	drm_fb_helper_fb_release(info, user);
+
+	if (!atomic_read(&fb_helper->open_count))
+		drm_fb_helper_generic_vunmap(fb_helper);
+
+	return 0;
+}
+
+static int drm_fb_helper_generic_fb_mmap(struct fb_info *info,
+					 struct vm_area_struct *vma)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+
+	return dma_buf_mmap(fb_helper->dma_buf, vma, 0);
+}
+
+static struct fb_ops drm_fb_helper_generic_fbdev_ops = {
+	.owner		= THIS_MODULE,
+	DRM_FB_HELPER_DEFAULT_OPS,
+	.fb_open	= drm_fb_helper_generic_fb_open,
+	.fb_release	= drm_fb_helper_generic_fb_release,
+	.fb_mmap	= drm_fb_helper_generic_fb_mmap,
+	.fb_read	= drm_fb_helper_sys_read,
+	.fb_write	= drm_fb_helper_sys_write,
+	.fb_fillrect	= drm_fb_helper_sys_fillrect,
+	.fb_copyarea	= drm_fb_helper_sys_copyarea,
+	.fb_imageblit	= drm_fb_helper_sys_imageblit,
+};
+
+static struct fb_deferred_io drm_fb_helper_generic_defio = {
+	.delay		= HZ / 20,
+	.deferred_io	= drm_fb_helper_deferred_io,
+};
+
+static int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
+				       struct drm_fb_helper_surface_size *sizes)
+{
+	struct drm_mode_create_dumb dumb_args = { 0 };
+	struct drm_mode_fb_cmd2 fb_args = { 0 };
+	struct drm_device *dev = fb_helper->dev;
+	struct drm_framebuffer *fb;
+	struct drm_file *file;
+	struct fb_info *fbi;
+	int ret;
+
+	DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
+		      sizes->surface_width, sizes->surface_height,
+		      sizes->surface_bpp);
+
+	file = drm_file_alloc(dev->primary);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	drm_dropmaster_ioctl(dev, NULL, file);
+
+	dumb_args.width = sizes->surface_width;
+	dumb_args.height = sizes->surface_height;
+	dumb_args.bpp = sizes->surface_bpp;
+
+	ret = drm_mode_create_dumb_ioctl(dev, &dumb_args, file);
+	if (ret)
+		goto err_free_file;
+
+	fb_args.width = dumb_args.width;
+	fb_args.height = dumb_args.height;
+	fb_args.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
+							 sizes->surface_depth);
+	fb_args.handles[0] = dumb_args.handle;
+	fb_args.pitches[0] = dumb_args.pitch;
+
+	ret = drm_mode_addfb2(dev, &fb_args, file);
+	if (ret)
+		goto err_free_file;
+
+	fb = drm_framebuffer_lookup(dev, file, fb_args.fb_id);
+	if (!fb) {
+		ret = -ENOENT;
+		goto err_free_file;
+	}
+
+	/* drop the reference we picked up in framebuffer lookup */
+	drm_framebuffer_put(fb);
+
+	fb_helper->dumb_handle = dumb_args.handle;
+	fb_helper->file = file;
+	fb_helper->fb = fb;
+
+	fbi = drm_fb_helper_alloc_fbi(fb_helper);
+	if (IS_ERR(fbi)) {
+		ret = PTR_ERR(fbi);
+		goto err_free_file;
+	}
+
+	fbi->par = fb_helper;
+	fbi->fbops = &drm_fb_helper_generic_fbdev_ops;
+	fbi->screen_size = fb->height * fb->pitches[0];
+	fbi->fix.smem_len = fbi->screen_size;
+	strcpy(fbi->fix.id, "generic");
+
+	drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
+	drm_fb_helper_fill_var(fbi, fb_helper, sizes->fb_width, sizes->fb_height);
+
+	if (fb->funcs->dirty) {
+		struct fb_ops *fbops;
+
+		/*
+		 * fb_deferred_io_cleanup() clears &fbops->fb_mmap so a per
+		 * instance version is necessary.
+		 */
+		fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
+		if (!fbops) {
+			ret = -ENOMEM;
+			goto err_fb_info_destroy;
+		}
+
+		*fbops = *fbi->fbops;
+		fbi->fbops = fbops;
+		fbi->fbdefio = &drm_fb_helper_generic_defio;
+	}
+
+	atomic_set(&fb_helper->open_count, 0);
+
+	return 0;
+
+err_fb_info_destroy:
+	drm_fb_helper_fini(fb_helper);
+err_free_file:
+	drm_file_free(file);
+
+	return ret;
+}
+
+static void drm_fb_helper_generic_release(struct drm_fb_helper *fb_helper)
+{
+	struct fb_ops *fbops = NULL;
+
+	if (fb_helper->fbdev->fbdefio)
+		fbops = fb_helper->fbdev->fbops;
+
+	drm_fb_helper_fini(fb_helper);
+	drm_file_free(fb_helper->file);
+
+	kfree(fb_helper);
+	kfree(fbops);
+}
+
+static const struct drm_fb_helper_funcs drm_fb_helper_generic_funcs = {
+	.fb_probe	= drm_fb_helper_generic_probe,
+	.restore	= drm_fb_helper_restore_fbdev_mode_unlocked,
+	.hotplug_event	= drm_fb_helper_hotplug_event,
+	.unregister	= drm_fb_helper_unregister_fbi,
+	.release	= drm_fb_helper_generic_release,
+};
+
+/**
+ * drm_fb_helper_generic_fbdev_setup() - Setup generic fbdev emulation
+ * @dev: DRM device
+ * @preferred_bpp: Preferred bits per pixel for the device.
+ *                 @dev->mode_config.preferred_depth is used if this is zero.
+ * @max_conn_count: Maximum number of connectors.
+ *                  @dev->mode_config.num_connector is used if this is zero.
+ *
+ * This function sets up generic fbdev emulation for drivers that supports
+ * dumb buffers and provides exported buffers with a virtual address.
+ * The driver doesn't have to anything else than to call this function,
+ * restore, hotplug events and teardown are all taken care of.
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_fb_helper_generic_fbdev_setup(struct drm_device *dev,
+				      unsigned int preferred_bpp,
+				      unsigned int max_conn_count)
+{
+	struct drm_fb_helper *fb_helper;
+	int ret;
+
+	if (!drm_fbdev_emulation)
+		return 0;
+
+	fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
+	if (!fb_helper)
+		return -ENOMEM;
+
+	ret = drm_fb_helper_fbdev_setup(dev, fb_helper,
+					&drm_fb_helper_generic_funcs,
+					preferred_bpp, max_conn_count);
+	if (ret) {
+		kfree(fb_helper);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_generic_fbdev_setup);
+
 /* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
  * but the module doesn't depend on any fb console symbols.  At least
  * attempt to load fbcon to avoid leaving the system without a usable console.
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index f5ea79bbb831..38738a1abad7 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -279,6 +279,28 @@ struct drm_fb_helper {
 	 * initial value to 0 themselves.
 	 */
 	atomic_t open_count;
+
+	/**
+	 * @file:
+	 *
+	 * Optional DRM file. Used by the generic fbdev code.
+	 */
+	struct drm_file *file;
+
+	/**
+	 * @dumb_handle:
+	 *
+	 * Optional dumb buffer handle. Used by the generic fbdev code.
+	 */
+	u32 dumb_handle;
+
+	/**
+	 * @dma_buf:
+	 *
+	 * Optional pointer to a DMA buffer object.
+	 * Used by the generic fbdev code.
+	 */
+	struct dma_buf *dma_buf;
 };
 
 /**
@@ -382,6 +404,10 @@ void drm_fb_helper_fbdev_teardown(struct drm_device *dev);
 
 void drm_fb_helper_lastclose(struct drm_device *dev);
 void drm_fb_helper_output_poll_changed(struct drm_device *dev);
+
+int drm_fb_helper_generic_fbdev_setup(struct drm_device *dev,
+				      unsigned int preferred_bpp,
+				      unsigned int max_conn_count);
 #else
 static inline void drm_fb_helper_prepare(struct drm_device *dev,
 					struct drm_fb_helper *helper,
@@ -626,6 +652,13 @@ static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev)
 {
 }
 
+static inline int
+drm_fb_helper_generic_fbdev_setup(struct drm_device *dev,
+				  unsigned int preferred_bpp,
+				  unsigned int max_conn_count)
+{
+	return 0;
+}
 #endif
 
 static inline int
-- 
2.14.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 7/7] drm/vc4: Test generic fbdev emulation

[Noralf Trønnes]

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 3 ---
 drivers/gpu/drm/vc4/vc4_kms.c | 3 +--
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index ceb385fd69c5..ef8a2d3a6d1f 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -152,7 +152,6 @@ static struct drm_driver vc4_drm_driver = {
 			    DRIVER_HAVE_IRQ |
 			    DRIVER_RENDER |
 			    DRIVER_PRIME),
-	.lastclose = drm_fb_helper_lastclose,
 	.irq_handler = vc4_irq,
 	.irq_preinstall = vc4_irq_preinstall,
 	.irq_postinstall = vc4_irq_postinstall,
@@ -297,8 +296,6 @@ static void vc4_drm_unbind(struct device *dev)
 
 	drm_dev_unregister(drm);
 
-	drm_fb_cma_fbdev_fini(drm);
-
 	drm_mode_config_cleanup(drm);
 
 	drm_dev_unref(drm);
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 4256f294c346..671c62f1b4d3 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -188,7 +188,6 @@ static struct drm_framebuffer *vc4_fb_create(struct drm_device *dev,
 }
 
 static const struct drm_mode_config_funcs vc4_mode_funcs = {
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = vc4_atomic_commit,
 	.fb_create = vc4_fb_create,
@@ -219,7 +218,7 @@ int vc4_kms_load(struct drm_device *dev)
 	drm_mode_config_reset(dev);
 
 	if (dev->mode_config.num_connector)
-		drm_fb_cma_fbdev_init(dev, 32, 0);
+		drm_fb_helper_generic_fbdev_setup(dev, 32, 0);
 
 	drm_kms_helper_poll_init(dev);
 
-- 
2.14.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 4/7] drm/prime: Clear drm_gem_object->dma_buf on release

[Chris Wilson]

Quoting Noralf Trønnes (2017-12-31 13:58:40)
> Clear the pointer so the buffer can be re-exported. Otherwise use
> after free happens in the next call to drm_gem_prime_handle_to_fd().
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/drm_prime.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 9a17725b0f7a..3214c0eb7466 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -343,6 +343,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
>  
>         /* drop the reference on the export fd holds */
>         drm_gem_object_put_unlocked(obj);
> +       obj->dma_buf = NULL;

obj->dma_buf holds a reference to the dma_buf, so to get to the dma_buf
release we must have already called dma_buf_put(obj->dma_buf). See
drm_gem_object_exported_dma_buf_free(). (Note you would do the
obj->dma_buf = NULL before dropping the potentially last ref to obj.)
A BUG_ON(obj->dma_buf) may help clarify the cache was already released.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 4/7] drm/prime: Clear drm_gem_object->dma_buf on release

[Noralf Trønnes]

Den 31.12.2017 16.41, skrev Chris Wilson:
> Quoting Noralf Trønnes (2017-12-31 13:58:40)
>> Clear the pointer so the buffer can be re-exported. Otherwise use
>> after free happens in the next call to drm_gem_prime_handle_to_fd().
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/drm_prime.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>> index 9a17725b0f7a..3214c0eb7466 100644
>> --- a/drivers/gpu/drm/drm_prime.c
>> +++ b/drivers/gpu/drm/drm_prime.c
>> @@ -343,6 +343,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
>>   
>>          /* drop the reference on the export fd holds */
>>          drm_gem_object_put_unlocked(obj);
>> +       obj->dma_buf = NULL;
> obj->dma_buf holds a reference to the dma_buf, so to get to the dma_buf
> release we must have already called dma_buf_put(obj->dma_buf). See
> drm_gem_object_exported_dma_buf_free(). (Note you would do the
> obj->dma_buf = NULL before dropping the potentially last ref to obj.)
> A BUG_ON(obj->dma_buf) may help clarify the cache was already released.

Hmm, okay it was a shot in the dark.
Maybe I can defer dumb_buffer and drm_framebuffer creation until fb_open
and then free it all in fb_close. That would align fbdev emulation more
with how DRM userspace operates. Let's see what assumptions the code has
about drm_fb_helper->fb being set on probe...

Noralf.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx