drm: review locking for drm_fb_helper_restore_fbdev_mode

... it's required. Fix up exynos and the cma helper, and add a
corresponding WARN_ON to drm_fb_helper_restore_fbdev_mode.

Note that tegra calls the fbdev cma helper restore function also from
it's driver-load callback. Which is a bit against current practice,
since usually the call is only from ->lastclose, and initial setup is
done by drm_fb_helper_initial_config.

Also add the relevant drm DocBook entry.

v2: Add promised WARN to restore_fbdev_mode.

Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9c797f6fea75118d17104586640754db4f6ce46e..f17077307c6513ede7fc401a0742881c7b03d600 100644 (file)
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -68,9 +68,23 @@ void drm_modeset_unlock_all(struct drm_device *dev)
 
        mutex_unlock(&dev->mode_config.mutex);
 }
-
 EXPORT_SYMBOL(drm_modeset_unlock_all);
 
+/**
+ * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked
+ * @dev: device
+ */
+void drm_warn_on_modeset_not_all_locked(struct drm_device *dev)
+{
+       struct drm_crtc *crtc;
+
+       list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+               WARN_ON(!mutex_is_locked(&crtc->mutex));
+
+       WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
+}
+EXPORT_SYMBOL(drm_warn_on_modeset_not_all_locked);
+
 /* Avoid boilerplate.  I'm tired of typing. */
 #define DRM_ENUM_NAME_FN(fnname, list)                         \
        char *fnname(int val)                                   \

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 3742bc96421e8daa0ba8728b2dd4da3be0a66021..1b6ba2d4d60c02e9c7c58874d5f0ae2db5faf374 100644 (file)
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -389,8 +389,10 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini);
  */
 void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma)
 {
+       drm_modeset_lock_all(dev);
        if (fbdev_cma)
                drm_fb_helper_restore_fbdev_mode(&fbdev_cma->fb_helper);
+       drm_modeset_unlock_all(dev);
 }
 EXPORT_SYMBOL_GPL(drm_fbdev_cma_restore_mode);
 

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0c6e25e979dd7eb4b0c7005656196824e6c0c181..77378a1e907e5b78d6339f1920bf0804abe412b9 100644 (file)
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -239,10 +239,21 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 }
 EXPORT_SYMBOL(drm_fb_helper_debug_leave);
 
+/**
+ * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration
+ * @fb_helper: fbcon to restore
+ *
+ * This should be called from driver's drm->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.
+ */
 bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 {
        bool error = false;
        int i, ret;
+
+       drm_warn_on_modeset_not_all_locked(fb_helper->dev);
+
        for (i = 0; i < fb_helper->crtc_count; i++) {
                struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
                ret = drm_mode_set_config_internal(mode_set);

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 90d335cfb8c064b2bc2585e3073a9cdfaa05c387..086d0f79785f1af4c7a5168989806fd1c08a69bf 100644 (file)
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -376,5 +376,7 @@ void exynos_drm_fbdev_restore_mode(struct drm_device *dev)
        if (!private || !private->fb_helper)
                return;
 
+       drm_modeset_lock_all(dev);
        drm_fb_helper_restore_fbdev_mode(private->fb_helper);
+       drm_modeset_unlock_all(dev);
 }

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e90c8dcc028d6645753a2c03be76484191102f9e..8b7762728639ad7e3a5ec0f8ba4ab5c1bac1ea3e 100644 (file)
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -867,6 +867,7 @@ struct drm_prop_enum_list {
 
 extern void drm_modeset_lock_all(struct drm_device *dev);
 extern void drm_modeset_unlock_all(struct drm_device *dev);
+extern void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
 
 extern int drm_crtc_init(struct drm_device *dev,
                         struct drm_crtc *crtc,