| Message ID | 20190326175546.18126-14-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/fb-helper: Move modesetting code to drm_client | expand |
On Tue, Mar 26, 2019 at 06:55:43PM +0100, Noralf Trønnes wrote: > 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.vetter@ffwll.ch> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> I think it'd be good to reorder this earlier in the series. And I'm wondering why you didn't replace all occurences of the _is_bound() function. With the consistent master state check we're doing with this I don't think any of the fbdev checking is still needed. Plus looking at crtc->primary->fb without any locks is racy, so would be good to ditch that hack. -Daniel > --- > 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 >
Den 28.03.2019 09.17, skrev Daniel Vetter: > On Tue, Mar 26, 2019 at 06:55:43PM +0100, Noralf Trønnes wrote: >> 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.vetter@ffwll.ch> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > I think it'd be good to reorder this earlier in the series. And I'm > wondering why you didn't replace all occurences of the _is_bound() > function. With the consistent master state check we're doing with this I > don't think any of the fbdev checking is still needed. Plus looking at > crtc->primary->fb without any locks is racy, so would be good to ditch > that hack. _is_bound() is used in drm_fb_helper_hotplug_event() to decide on delayed hotplug. The master lock won't help here. I have studied drm_fb_helper for 2 years now, and I still think it is convoluted and difficult to grasp in places. So I kept this one call site. I'm vary of making unrelated changes in this series. The reason I included this patch is because it's used with the bootsplash example support code. I can move the patch earlier in the series. Noralf. > -Daniel > >> --- >> 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 >> >
On Sat, Mar 30, 2019 at 10:07:58PM +0100, Noralf Trønnes wrote: > > > Den 28.03.2019 09.17, skrev Daniel Vetter: > > On Tue, Mar 26, 2019 at 06:55:43PM +0100, Noralf Trønnes wrote: > >> 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.vetter@ffwll.ch> > >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > > > I think it'd be good to reorder this earlier in the series. And I'm > > wondering why you didn't replace all occurences of the _is_bound() > > function. With the consistent master state check we're doing with this I > > don't think any of the fbdev checking is still needed. Plus looking at > > crtc->primary->fb without any locks is racy, so would be good to ditch > > that hack. > > _is_bound() is used in drm_fb_helper_hotplug_event() to decide on > delayed hotplug. The master lock won't help here. I have studied > drm_fb_helper for 2 years now, and I still think it is convoluted and > difficult to grasp in places. So I kept this one call site. I'm vary of > making unrelated changes in this series. Hm, why would it not work for the hotplug case? _is_bound already checks dev->master (but with races, unlike this here). And the other checks are various levels of cargo-culting. dev->master is (well, has become through retrofitting) the "who owns the display" tracking, I think fully standardizing on that makes sense. The only case where there's a difference is if: - some compositor dropped master - but didn't disable it's framebuffers We actually want that for smooth vt switching, so that the new compositor could take over in a transition. Having a special case with fbdev where the compositor has to disable all its fb or fbcon won't take over feels a bit irky. -Daniel > The reason I included this patch is because it's used with the > bootsplash example support code. I can move the patch earlier in the series. > > Noralf. > > > -Daniel > > > >> --- > >> 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 > >> > >
Den 01.04.2019 09.12, skrev Daniel Vetter: > On Sat, Mar 30, 2019 at 10:07:58PM +0100, Noralf Trønnes wrote: >> >> >> Den 28.03.2019 09.17, skrev Daniel Vetter: >>> On Tue, Mar 26, 2019 at 06:55:43PM +0100, Noralf Trønnes wrote: >>>> 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.vetter@ffwll.ch> >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >>> >>> I think it'd be good to reorder this earlier in the series. And I'm >>> wondering why you didn't replace all occurences of the _is_bound() >>> function. With the consistent master state check we're doing with this I >>> don't think any of the fbdev checking is still needed. Plus looking at >>> crtc->primary->fb without any locks is racy, so would be good to ditch >>> that hack. >> >> _is_bound() is used in drm_fb_helper_hotplug_event() to decide on >> delayed hotplug. The master lock won't help here. I have studied >> drm_fb_helper for 2 years now, and I still think it is convoluted and >> difficult to grasp in places. So I kept this one call site. I'm vary of >> making unrelated changes in this series. > > Hm, why would it not work for the hotplug case? _is_bound already checks > dev->master (but with races, unlike this here). And the other checks are > various levels of cargo-culting. dev->master is (well, has become through > retrofitting) the "who owns the display" tracking, I think fully > standardizing on that makes sense. The only case where there's a > difference is if: > - some compositor dropped master > - but didn't disable it's framebuffers > We actually want that for smooth vt switching, so that the new compositor > could take over in a transition. Having a special case with fbdev where > the compositor has to disable all its fb or fbcon won't take over feels a > bit irky. You're right ofc. I read '->deferred_setup = true' when in fact it is '->delayed_hotplug = true', so I assumed there was something that _is_bound() was protecting wrt. to setup state. I'll fix it in the next version. Noralf. > -Daniel > >> The reason I included this patch is because it's used with the >> bootsplash example support code. I can move the patch earlier in the series. >> >> Noralf. >> >>> -Daniel >>> >>>> --- >>>> 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 >>>> >>> >
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;
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.vetter@ffwll.ch> Signed-off-by: Noralf Trønnes <noralf@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(-)