Re: [PATCH 13/16] drm/fb-helper: Avoid race with DRM userspace
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
Signed-off-by: Noralf Trønnes
>>>
>>> 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(>master_mutex);
+ if (dev->master) {
+ mutex_unlock(>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(>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
#include
+#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(_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
Re: [PATCH 13/16] drm/fb-helper: Avoid race with DRM userspace
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
> >> Signed-off-by: Noralf Trønnes
> >
> > 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(>master_mutex);
> >> + if (dev->master) {
> >> + mutex_unlock(>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(>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
> >> #include
> >>
> >> +#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(_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.
> >>
Re: [PATCH 13/16] drm/fb-helper: Avoid race with DRM userspace
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
>> Signed-off-by: Noralf Trønnes
>
> 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(>master_mutex);
>> +if (dev->master) {
>> +mutex_unlock(>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(>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
>> #include
>>
>> +#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(_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(_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(_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(_helper->lock);
>>
>> -if
Re: [PATCH 13/16] drm/fb-helper: Avoid race with DRM userspace
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
> Signed-off-by: Noralf Trønnes
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(>master_mutex);
> + if (dev->master) {
> + mutex_unlock(>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(>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
> #include
>
> +#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(_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(_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(_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(_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(_helper->lock);
>
> return ret;
> @@ -1136,11 +1148,13 @@ int drm_fb_helper_ioctl(struct fb_info *info,
> unsigned int
[PATCH 13/16] drm/fb-helper: Avoid race with DRM userspace
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
Signed-off-by: Noralf Trønnes
---
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(>master_mutex);
+ if (dev->master) {
+ mutex_unlock(>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(>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
#include
+#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(_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(_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(_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(_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(_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(_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;
