drm_fb_helper_is_bound() is used to check if DRM userspace is in control.
This is done by looking at the fb on the primary plane. By the time
fb-helper gets around to committing, it's possible that the facts have
changed.

Avoid this race by holding the drm_device->master_mutex lock while
committing. When DRM userspace does its first open, it will now wait
until fb-helper is done. The helper will stay away if there's a master.

Locking rule: Always take the fb-helper lock first.

Suggested-by: Daniel Vetter <daniel.vet...@ffwll.ch>
Signed-off-by: Noralf Trønnes <nor...@tronnes.org>
---
 drivers/gpu/drm/drm_auth.c      | 20 ++++++++++++++
 drivers/gpu/drm/drm_fb_helper.c | 49 ++++++++++++++++++++++++---------
 drivers/gpu/drm/drm_internal.h  |  2 ++
 3 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 1669c42c40ed..db199807b7dc 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -368,3 +368,23 @@ void drm_master_put(struct drm_master **master)
        *master = NULL;
 }
 EXPORT_SYMBOL(drm_master_put);
+
+/* Used by drm_client and drm_fb_helper */
+bool drm_master_internal_acquire(struct drm_device *dev)
+{
+       mutex_lock(&dev->master_mutex);
+       if (dev->master) {
+               mutex_unlock(&dev->master_mutex);
+               return false;
+       }
+
+       return true;
+}
+EXPORT_SYMBOL(drm_master_internal_acquire);
+
+/* Used by drm_client and drm_fb_helper */
+void drm_master_internal_release(struct drm_device *dev)
+{
+       mutex_unlock(&dev->master_mutex);
+}
+EXPORT_SYMBOL(drm_master_internal_release);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4a073cd4e423..9f253fcf3f79 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -41,6 +41,8 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_atomic.h>
 
+#include "drm_internal.h"
+
 static bool drm_fbdev_emulation = true;
 module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
 MODULE_PARM_DESC(fbdev_emulation,
@@ -235,7 +237,12 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct 
drm_fb_helper *fb_helper)
                return 0;
 
        mutex_lock(&fb_helper->lock);
-       ret = drm_client_modesets_commit(fb_helper->dev, fb_helper->modesets);
+       if (drm_master_internal_acquire(fb_helper->dev)) {
+               ret = drm_client_modesets_commit(fb_helper->dev, 
fb_helper->modesets);
+               drm_master_internal_release(fb_helper->dev);
+       } else {
+               ret = -EBUSY;
+       }
 
        do_delayed = fb_helper->delayed_hotplug;
        if (do_delayed)
@@ -332,13 +339,16 @@ static struct sysrq_key_op sysrq_drm_fb_helper_restore_op 
= { };
 static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 {
        struct drm_fb_helper *fb_helper = info->par;
+       struct drm_device *dev = fb_helper->dev;
 
        /*
         * For each CRTC in this fb, turn the connectors on/off.
         */
        mutex_lock(&fb_helper->lock);
-       if (drm_fb_helper_is_bound(fb_helper))
-               drm_client_modesets_dpms(fb_helper->dev, fb_helper->modesets, 
dpms_mode);
+       if (drm_master_internal_acquire(dev)) {
+               drm_client_modesets_dpms(dev, fb_helper->modesets, dpms_mode);
+               drm_master_internal_release(dev);
+       }
        mutex_unlock(&fb_helper->lock);
 }
 
@@ -1097,6 +1107,7 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct 
fb_info *info)
 int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 {
        struct drm_fb_helper *fb_helper = info->par;
+       struct drm_device *dev = fb_helper->dev;
        int ret;
 
        if (oops_in_progress)
@@ -1104,9 +1115,9 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
fb_info *info)
 
        mutex_lock(&fb_helper->lock);
 
-       if (!drm_fb_helper_is_bound(fb_helper)) {
+       if (!drm_master_internal_acquire(dev)) {
                ret = -EBUSY;
-               goto out;
+               goto unlock;
        }
 
        if (info->fix.visual == FB_VISUAL_TRUECOLOR)
@@ -1116,7 +1127,8 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
fb_info *info)
        else
                ret = setcmap_legacy(cmap, info);
 
-out:
+       drm_master_internal_release(dev);
+unlock:
        mutex_unlock(&fb_helper->lock);
 
        return ret;
@@ -1136,11 +1148,13 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned 
int cmd,
                        unsigned long arg)
 {
        struct drm_fb_helper *fb_helper = info->par;
+       struct drm_device *dev = fb_helper->dev;
        struct drm_crtc *crtc;
        int ret = 0;
 
        mutex_lock(&fb_helper->lock);
-       if (!drm_fb_helper_is_bound(fb_helper)) {
+
+       if (!drm_master_internal_acquire(dev)) {
                ret = -EBUSY;
                goto unlock;
        }
@@ -1177,13 +1191,15 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned 
int cmd,
                }
 
                ret = 0;
-               goto unlock;
+               break;
        default:
                ret = -ENOTTY;
        }
 
+       drm_master_internal_release(dev);
 unlock:
        mutex_unlock(&fb_helper->lock);
+
        return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_ioctl);
@@ -1426,15 +1442,19 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo 
*var,
                return -EBUSY;
 
        mutex_lock(&fb_helper->lock);
-       if (!drm_fb_helper_is_bound(fb_helper)) {
-               mutex_unlock(&fb_helper->lock);
-               return -EBUSY;
+
+       if (!drm_master_internal_acquire(dev)) {
+               ret = -EBUSY;
+               goto unlock;
        }
 
        if (drm_drv_uses_atomic_modeset(dev))
                ret = pan_display_atomic(var, info);
        else
                ret = pan_display_legacy(var, info);
+
+       drm_master_internal_release(dev);
+unlock:
        mutex_unlock(&fb_helper->lock);
 
        return ret;
@@ -1451,6 +1471,7 @@ static int drm_fb_helper_single_fb_probe(struct 
drm_fb_helper *fb_helper,
        int ret = 0;
        int crtc_count = 0;
        struct drm_connector_list_iter conn_iter;
+       struct drm_device *dev = fb_helper->dev;
        struct drm_fb_helper_surface_size sizes;
        struct drm_connector *connector;
        struct drm_mode_set *mode_set;
@@ -1593,8 +1614,10 @@ static int drm_fb_helper_single_fb_probe(struct 
drm_fb_helper *fb_helper,
                DRM_INFO("Cannot find any crtc or sizes\n");
 
                /* First time: disable all crtc's.. */
-               if (!fb_helper->deferred_setup && 
!READ_ONCE(fb_helper->dev->master))
-                       drm_client_modesets_commit(fb_helper->dev, 
fb_helper->modesets);
+               if (!fb_helper->deferred_setup && 
drm_master_internal_acquire(dev)) {
+                       drm_client_modesets_commit(dev, fb_helper->modesets);
+                       drm_master_internal_release(dev);
+               }
                return -EAGAIN;
        }
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 251d67e04c2d..6d5d1184c084 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -91,6 +91,8 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
                         struct drm_file *file_priv);
 int drm_master_open(struct drm_file *file_priv);
 void drm_master_release(struct drm_file *file_priv);
+bool drm_master_internal_acquire(struct drm_device *dev);
+void drm_master_internal_release(struct drm_device *dev);
 
 /* drm_sysfs.c */
 extern struct class *drm_class;
-- 
2.20.1

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

Reply via email to