[PATCH v4 06/11] drm/fb-helper: Make top-level lock more robust

[Thierry Reding <thierry.reding@gmail.com>]

From: Thierry Reding <treding@nvidia.com>

The existing drm_fb_helper_hotplug_event() function needs to take the
top-level fb-helper lock. However, the function can also be called from
code that has already taken this lock. Introduce an unlocked variant of
this function that can be used in the latter case.

This function calls drm_fb_helper_restore_fbdev_mode_unlocked(), via
drm_fb_helper_set_par(), so we also need to introduce an unlocked copy
of that to avoid recursive locking issues.

Similarly, the drm_fb_helper_initial_config() function ends up calling
drm_fb_helper_set_par(), via register_framebuffer(), and needs an
unlocked variant to avoid recursive locking.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 167 +++++++++++++++++++++++++---------------
 1 file changed, 104 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 860be51d92f6..21a90322531c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -491,18 +491,10 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 	return 0;
 }
 
-/**
- * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
- * @fb_helper: fbcon to restore
- *
- * This should be called from driver's drm &drm_driver.lastclose callback
- * when implementing an fbcon on top of kms using this helper. This ensures that
- * the user isn't greeted with a black screen when e.g. X dies.
- *
- * RETURNS:
- * Zero if everything went ok, negative error code otherwise.
- */
-int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
+static int __drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper);
+
+static int
+__drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 {
 	struct drm_device *dev = fb_helper->dev;
 	bool do_delayed;
@@ -511,7 +503,8 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	if (!drm_fbdev_emulation)
 		return -ENODEV;
 
-	mutex_lock(&fb_helper->lock);
+	WARN_ON(!mutex_is_locked(&fb_helper->lock));
+
 	drm_modeset_lock_all(dev);
 
 	ret = restore_fbdev_mode(fb_helper);
@@ -521,10 +514,31 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 		fb_helper->delayed_hotplug = false;
 
 	drm_modeset_unlock_all(dev);
-	mutex_unlock(&fb_helper->lock);
 
 	if (do_delayed)
-		drm_fb_helper_hotplug_event(fb_helper);
+		__drm_fb_helper_hotplug_event(fb_helper);
+
+	return ret;
+}
+
+/**
+ * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
+ * @fb_helper: fbcon to restore
+ *
+ * This should be called from driver's drm &drm_driver.lastclose callback
+ * when implementing an fbcon on top of kms using this helper. This ensures that
+ * the user isn't greeted with a black screen when e.g. X dies.
+ *
+ * RETURNS:
+ * Zero if everything went ok, negative error code otherwise.
+ */
+int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
+{
+	int ret;
+
+	mutex_lock(&fb_helper->lock);
+	ret = __drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
+	mutex_unlock(&fb_helper->lock);
 
 	return ret;
 }
@@ -1486,7 +1500,7 @@ int drm_fb_helper_set_par(struct fb_info *info)
 		return -EINVAL;
 	}
 
-	drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
+	__drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
 
 	return 0;
 }
@@ -2333,6 +2347,46 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	kfree(enabled);
 }
 
+static int __drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper,
+					  int bpp_sel)
+{
+	struct drm_device *dev = fb_helper->dev;
+	struct fb_info *info;
+	int ret;
+
+	if (!drm_fbdev_emulation)
+		return 0;
+
+	WARN_ON(!mutex_is_locked(&fb_helper->lock));
+
+	mutex_lock(&dev->mode_config.mutex);
+	drm_setup_crtcs(fb_helper,
+			dev->mode_config.max_width,
+			dev->mode_config.max_height);
+	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
+	mutex_unlock(&dev->mode_config.mutex);
+	if (ret)
+		return ret;
+
+	info = fb_helper->fbdev;
+	info->var.pixclock = 0;
+	ret = register_framebuffer(info);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
+		 info->node, info->fix.id);
+
+	mutex_lock(&kernel_fb_helper_lock);
+	if (list_empty(&kernel_fb_helper_list))
+		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
+
+	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
+	mutex_unlock(&kernel_fb_helper_lock);
+
+	return 0;
+}
+
 /**
  * drm_fb_helper_initial_config - setup a sane initial connector configuration
  * @fb_helper: fb_helper device struct
@@ -2377,41 +2431,50 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
  */
 int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
 {
-	struct drm_device *dev = fb_helper->dev;
-	struct fb_info *info;
 	int ret;
 
+	mutex_lock(&fb_helper->lock);
+	ret = __drm_fb_helper_initial_config(fb_helper, bpp_sel);
+	mutex_unlock(&fb_helper->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_fb_helper_initial_config);
+
+static int __drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
+{
+	struct drm_device *dev = fb_helper->dev;
+	unsigned int width, height;
+
 	if (!drm_fbdev_emulation)
 		return 0;
 
+	WARN_ON(!mutex_is_locked(&fb_helper->lock));
+
 	mutex_lock(&dev->mode_config.mutex);
-	drm_setup_crtcs(fb_helper,
-			dev->mode_config.max_width,
-			dev->mode_config.max_height);
-	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
-	mutex_unlock(&dev->mode_config.mutex);
-	if (ret)
-		return ret;
 
-	info = fb_helper->fbdev;
-	info->var.pixclock = 0;
-	ret = register_framebuffer(info);
-	if (ret < 0)
-		return ret;
+	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
+		fb_helper->delayed_hotplug = true;
+		mutex_unlock(&dev->mode_config.mutex);
+		return 0;
+	}
 
-	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
-		 info->node, info->fix.id);
+	DRM_DEBUG_KMS("\n");
 
-	mutex_lock(&kernel_fb_helper_lock);
-	if (list_empty(&kernel_fb_helper_list))
-		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
+	width = dev->mode_config.max_width;
+	height = dev->mode_config.max_height;
 
-	list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
-	mutex_unlock(&kernel_fb_helper_lock);
+	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
+		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
+
+	drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
+
+	mutex_unlock(&dev->mode_config.mutex);
+
+	drm_fb_helper_set_par(fb_helper->fbdev);
 
 	return 0;
 }
-EXPORT_SYMBOL(drm_fb_helper_initial_config);
 
 /**
  * drm_fb_helper_hotplug_event - respond to a hotplug notification by
@@ -2436,35 +2499,13 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
  */
 int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
 {
-	struct drm_device *dev = fb_helper->dev;
-	int err = 0;
-
-	if (!drm_fbdev_emulation)
-		return 0;
+	int ret;
 
 	mutex_lock(&fb_helper->lock);
-	mutex_lock(&dev->mode_config.mutex);
-
-	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
-		fb_helper->delayed_hotplug = true;
-		mutex_unlock(&dev->mode_config.mutex);
-		goto unlock;
-	}
-
-	DRM_DEBUG_KMS("\n");
-
-	drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
-
-	mutex_unlock(&dev->mode_config.mutex);
+	ret = __drm_fb_helper_hotplug_event(fb_helper);
 	mutex_unlock(&fb_helper->lock);
 
-	drm_fb_helper_set_par(fb_helper->fbdev);
-
-	return 0;
-
-unlock:
-	mutex_unlock(&fb_helper->lock);
-	return err;
+	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
 
-- 
2.12.0

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