drm/fb-helper: Add top-level lock

Introduce a new top-level lock for the FB helper code. This will allow
better locking granularity and avoid the need to abuse modeset locking
for this purpose instead.

This patch just adds the new lock everywhere we currently grab
mode_config->mutex (explicitly, or through drm_modeset_lock_all).
Follow-up patches will push the kms locking down into only the places
that need it.

v2:
- use lockdep_assert_held
- use drm_fb_helper_for_each_connector where possible
- use the new top-level lock consistently, i.e. in all the places
  we're currently acquiring mode_config.mutex.
- small polish to the kerneldoc

Tested-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Thierry Reding <treding@nvidia.com> (v1)
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20170704151833.17304-4-daniel.vetter@ffwll.ch

1 files changed, 43 insertions, 4 deletions

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 7d0d50e404ee..30188ec809b5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -119,7 +119,8 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
 	if (!drm_fbdev_emulation)
 		return 0;
 
-	WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
+	lockdep_assert_held(&fb_helper->lock);
+	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
 
 	count = fb_helper->connector_count + 1;
 
@@ -150,11 +151,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper,
 {
 	int err;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&fb_helper->dev->mode_config.mutex);
 
 	err = __drm_fb_helper_add_one_connector(fb_helper, connector);
 
 	mutex_unlock(&fb_helper->dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 
 	return err;
 }
@@ -184,6 +187,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
 	if (!drm_fbdev_emulation)
 		return 0;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&dev->mode_config.mutex);
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
@@ -207,6 +211,7 @@ fail:
 out:
 	drm_connector_list_iter_end(&conn_iter);
 	mutex_unlock(&dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 
 	return ret;
 }
@@ -221,9 +226,9 @@ static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 	if (!drm_fbdev_emulation)
 		return 0;
 
-	WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
+	lockdep_assert_held(&fb_helper->lock);
 
-	for (i = 0; i < fb_helper->connector_count; i++) {
+	drm_fb_helper_for_each_connector(fb_helper, i) {
 		if (fb_helper->connector_info[i]->connector == connector)
 			break;
 	}
@@ -247,11 +252,13 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 {
 	int err;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&fb_helper->dev->mode_config.mutex);
 
 	err = __drm_fb_helper_remove_one_connector(fb_helper, connector);
 
 	mutex_unlock(&fb_helper->dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 
 	return err;
 }
@@ -503,16 +510,21 @@ 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);
 	drm_modeset_lock_all(dev);
+
 	ret = restore_fbdev_mode(fb_helper);
 
 	do_delayed = fb_helper->delayed_hotplug;
 	if (do_delayed)
 		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);
+
 	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
@@ -562,11 +574,13 @@ static bool drm_fb_helper_force_kernel_mode(void)
 		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 			continue;
 
+		mutex_lock(&helper->lock);
 		drm_modeset_lock_all(dev);
 		ret = restore_fbdev_mode(helper);
 		if (ret)
 			error = true;
 		drm_modeset_unlock_all(dev);
+		mutex_unlock(&helper->lock);
 	}
 	return error;
 }
@@ -606,9 +620,11 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 	/*
 	 * For each CRTC in this fb, turn the connectors on/off.
 	 */
+	mutex_lock(&fb_helper->lock);
 	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		drm_modeset_unlock_all(dev);
+		mutex_unlock(&fb_helper->lock);
 		return;
 	}
 
@@ -627,6 +643,7 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 		}
 	}
 	drm_modeset_unlock_all(dev);
+	mutex_unlock(&fb_helper->lock);
 }
 
 /**
@@ -748,6 +765,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;
+	mutex_init(&helper->lock);
 	helper->funcs = funcs;
 	helper->dev = dev;
 }
@@ -913,6 +931,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 	}
 	mutex_unlock(&kernel_fb_helper_lock);
 
+	mutex_destroy(&fb_helper->lock);
 	drm_fb_helper_crtc_free(fb_helper);
 
 }
@@ -1243,9 +1262,11 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 	if (oops_in_progress)
 		return -EBUSY;
 
+	mutex_lock(&fb_helper->lock);
 	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		drm_modeset_unlock_all(dev);
+		mutex_unlock(&fb_helper->lock);
 		return -EBUSY;
 	}
 
@@ -1278,6 +1299,7 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 	}
  out:
 	drm_modeset_unlock_all(dev);
+	mutex_unlock(&fb_helper->lock);
 	return rc;
 }
 EXPORT_SYMBOL(drm_fb_helper_setcmap);
@@ -1300,6 +1322,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 	struct drm_crtc *crtc;
 	int ret = 0;
 
+	mutex_lock(&fb_helper->lock);
 	mutex_lock(&dev->mode_config.mutex);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		ret = -EBUSY;
@@ -1346,6 +1369,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 
 unlock:
 	mutex_unlock(&dev->mode_config.mutex);
+	mutex_unlock(&fb_helper->lock);
 	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_ioctl);
@@ -1575,9 +1599,11 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 	if (oops_in_progress)
 		return -EBUSY;
 
+	mutex_lock(&fb_helper->lock);
 	drm_modeset_lock_all(dev);
 	if (!drm_fb_helper_is_bound(fb_helper)) {
 		drm_modeset_unlock_all(dev);
+		mutex_unlock(&fb_helper->lock);
 		return -EBUSY;
 	}
 
@@ -1586,6 +1612,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 	else
 		ret = pan_display_legacy(var, info);
 	drm_modeset_unlock_all(dev);
+	mutex_unlock(&fb_helper->lock);
 
 	return ret;
 }
@@ -2252,6 +2279,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
 
 	/* prevent concurrent modification of connector_count by hotplug */
+	lockdep_assert_held(&fb_helper->lock);
 	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
 
 	crtcs = kcalloc(fb_helper->connector_count,
@@ -2378,12 +2406,14 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
 	if (!drm_fbdev_emulation)
 		return 0;
 
+	mutex_lock(&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);
+	mutex_unlock(&fb_helper->lock);
 	if (ret)
 		return ret;
 
@@ -2431,25 +2461,34 @@ 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;
 
+	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);
-		return 0;
+		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);
+	mutex_unlock(&fb_helper->lock);
 
 	drm_fb_helper_set_par(fb_helper->fbdev);
 
 	return 0;
+
+unlock:
+	mutex_unlock(&fb_helper->lock);
+	return err;
 }
 EXPORT_SYMBOL(drm_fb_helper_hotplug_event);