* [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client
@ 2019-04-25 8:31 Noralf Trønnes
2019-04-25 8:31 ` [PATCH v4 1/1] drm/fb-helper: Avoid race with DRM userspace Noralf Trønnes
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Noralf Trønnes @ 2019-04-25 8:31 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Noralf Trønnes
The Intel CI [1] was not happy with the previous version and I don't
know which part it didn't like. So I'll split up the series and feed it
piece by piece until I know where the problem is.
Noralf.
[1] https://patchwork.freedesktop.org/series/58597/
Noralf Trønnes (1):
drm/fb-helper: Avoid race with DRM userspace
drivers/gpu/drm/drm_auth.c | 20 ++++++++
drivers/gpu/drm/drm_fb_helper.c | 90 ++++++++++++++++-----------------
drivers/gpu/drm/drm_internal.h | 2 +
3 files changed, 67 insertions(+), 45 deletions(-)
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 1/1] drm/fb-helper: Avoid race with DRM userspace
2019-04-25 8:31 [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client Noralf Trønnes
@ 2019-04-25 8:31 ` Noralf Trønnes
2019-05-04 12:34 ` Noralf Trønnes
2019-04-25 9:10 ` ✓ Fi.CI.BAT: success for drm/fb-helper: Move modesetting code to drm_client (rev5) Patchwork
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Noralf Trønnes @ 2019-04-25 8:31 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx
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.
v2:
- Remove drm_fb_helper_is_bound() (Daniel Vetter)
- No need to check fb_helper->dev->master in
drm_fb_helper_single_fb_probe(), restore_fbdev_mode() has the check.
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_auth.c | 20 ++++++++
drivers/gpu/drm/drm_fb_helper.c | 90 ++++++++++++++++-----------------
drivers/gpu/drm/drm_internal.h | 2 +
3 files changed, 67 insertions(+), 45 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 2339f0f8f5a8..578428461391 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -44,6 +44,7 @@
#include "drm_crtc_internal.h"
#include "drm_crtc_helper_internal.h"
+#include "drm_internal.h"
static bool drm_fbdev_emulation = true;
module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
@@ -509,7 +510,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
return ret;
}
-static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
+static int restore_fbdev_mode_force(struct drm_fb_helper *fb_helper)
{
struct drm_device *dev = fb_helper->dev;
@@ -519,6 +520,21 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
return restore_fbdev_mode_legacy(fb_helper);
}
+static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
+{
+ struct drm_device *dev = fb_helper->dev;
+ int ret;
+
+ if (!drm_master_internal_acquire(dev))
+ return -EBUSY;
+
+ ret = restore_fbdev_mode_force(fb_helper);
+
+ drm_master_internal_release(dev);
+
+ return ret;
+}
+
/**
* drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
* @fb_helper: driver-allocated fbdev helper, can be NULL
@@ -556,34 +572,6 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
}
EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
-static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
-{
- struct drm_device *dev = fb_helper->dev;
- struct drm_crtc *crtc;
- int bound = 0, crtcs_bound = 0;
-
- /*
- * Sometimes user space wants everything disabled, so don't steal the
- * display if there's a master.
- */
- if (READ_ONCE(dev->master))
- return false;
-
- drm_for_each_crtc(crtc, dev) {
- drm_modeset_lock(&crtc->mutex, NULL);
- if (crtc->primary->fb)
- crtcs_bound++;
- if (crtc->primary->fb == fb_helper->fb)
- bound++;
- drm_modeset_unlock(&crtc->mutex);
- }
-
- if (bound < crtcs_bound)
- return false;
-
- return true;
-}
-
#ifdef CONFIG_MAGIC_SYSRQ
/*
* restore fbcon display for all kms driver's using this helper, used for sysrq
@@ -604,7 +592,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
continue;
mutex_lock(&helper->lock);
- ret = restore_fbdev_mode(helper);
+ ret = restore_fbdev_mode_force(helper);
if (ret)
error = true;
mutex_unlock(&helper->lock);
@@ -663,20 +651,22 @@ static void dpms_legacy(struct drm_fb_helper *fb_helper, int dpms_mode)
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)) {
- mutex_unlock(&fb_helper->lock);
- return;
- }
+ if (!drm_master_internal_acquire(dev))
+ goto unlock;
- if (drm_drv_uses_atomic_modeset(fb_helper->dev))
+ if (drm_drv_uses_atomic_modeset(dev))
restore_fbdev_mode_atomic(fb_helper, dpms_mode == DRM_MODE_DPMS_ON);
else
dpms_legacy(fb_helper, dpms_mode);
+
+ drm_master_internal_release(dev);
+unlock:
mutex_unlock(&fb_helper->lock);
}
@@ -1509,6 +1499,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)
@@ -1516,9 +1507,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)
@@ -1528,7 +1519,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;
@@ -1548,12 +1540,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_mode_set *mode_set;
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;
}
@@ -1591,11 +1584,12 @@ 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;
@@ -1847,15 +1841,18 @@ 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;
@@ -2014,7 +2011,7 @@ 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))
+ if (!fb_helper->deferred_setup)
restore_fbdev_mode(fb_helper);
return -EAGAIN;
}
@@ -3028,6 +3025,7 @@ 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 || !fb_helper)
@@ -3040,12 +3038,14 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
return err;
}
- if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
+ if (!fb_helper->fb || !drm_master_internal_acquire(dev)) {
fb_helper->delayed_hotplug = true;
mutex_unlock(&fb_helper->lock);
return err;
}
+ drm_master_internal_release(dev);
+
DRM_DEBUG_KMS("\n");
drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index d9a483a5fce0..3ee97c9998a2 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for drm/fb-helper: Move modesetting code to drm_client (rev5)
2019-04-25 8:31 [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client Noralf Trønnes
2019-04-25 8:31 ` [PATCH v4 1/1] drm/fb-helper: Avoid race with DRM userspace Noralf Trønnes
@ 2019-04-25 9:10 ` Patchwork
2019-04-25 9:25 ` [Intel-gfx] [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client Daniel Vetter
2019-04-25 15:40 ` ✗ Fi.CI.IGT: failure for drm/fb-helper: Move modesetting code to drm_client (rev5) Patchwork
3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-04-25 9:10 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx
== Series Details ==
Series: drm/fb-helper: Move modesetting code to drm_client (rev5)
URL : https://patchwork.freedesktop.org/series/58597/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5996 -> Patchwork_12870
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/58597/revisions/5/mbox/
Known issues
------------
Here are the changes found in Patchwork_12870 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_contexts:
- fi-skl-gvtdvm: [PASS][1] -> [DMESG-FAIL][2] ([fdo#110235 ])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-byt-clapper: [PASS][3] -> [FAIL][4] ([fdo#103191])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/fi-byt-clapper/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/fi-byt-clapper/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
Participating hosts (40 -> 42)
------------------------------
Additional (6): fi-bxt-dsi fi-bsw-n3050 fi-byt-j1900 fi-hsw-peppy fi-icl-y fi-bsw-kefka
Missing (4): fi-kbl-soraka fi-ctg-p8600 fi-ilk-m540 fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_5996 -> Patchwork_12870
CI_DRM_5996: 526cadba413ae78043a74d85f5bc9b6e0a430c38 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4963: 11e10bc575516c56978640fcc697c27f277c660a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12870: fa2af64690cbb9403e4fc947b3ac7b224859071e @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
fa2af64690cb drm/fb-helper: Avoid race with DRM userspace
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client
2019-04-25 8:31 [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client Noralf Trønnes
2019-04-25 8:31 ` [PATCH v4 1/1] drm/fb-helper: Avoid race with DRM userspace Noralf Trønnes
2019-04-25 9:10 ` ✓ Fi.CI.BAT: success for drm/fb-helper: Move modesetting code to drm_client (rev5) Patchwork
@ 2019-04-25 9:25 ` Daniel Vetter
2019-04-25 11:56 ` Noralf Trønnes
2019-04-25 15:40 ` ✗ Fi.CI.IGT: failure for drm/fb-helper: Move modesetting code to drm_client (rev5) Patchwork
3 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2019-04-25 9:25 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx, dri-devel
On Thu, Apr 25, 2019 at 10:31 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>
> The Intel CI [1] was not happy with the previous version and I don't
> know which part it didn't like. So I'll split up the series and feed it
> piece by piece until I know where the problem is.
You can also send stuff to intel-gfx-trybot@lists.fd.o for test runs
that you don't want the world to see (it's still public, but no one
looks really).
-Daniel
>
> Noralf.
>
> [1] https://patchwork.freedesktop.org/series/58597/
>
> Noralf Trønnes (1):
> drm/fb-helper: Avoid race with DRM userspace
>
> drivers/gpu/drm/drm_auth.c | 20 ++++++++
> drivers/gpu/drm/drm_fb_helper.c | 90 ++++++++++++++++-----------------
> drivers/gpu/drm/drm_internal.h | 2 +
> 3 files changed, 67 insertions(+), 45 deletions(-)
>
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client
2019-04-25 9:25 ` [Intel-gfx] [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client Daniel Vetter
@ 2019-04-25 11:56 ` Noralf Trønnes
0 siblings, 0 replies; 9+ messages in thread
From: Noralf Trønnes @ 2019-04-25 11:56 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
Den 25.04.2019 11.25, skrev Daniel Vetter:
> On Thu, Apr 25, 2019 at 10:31 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>> The Intel CI [1] was not happy with the previous version and I don't
>> know which part it didn't like. So I'll split up the series and feed it
>> piece by piece until I know where the problem is.
>
> You can also send stuff to intel-gfx-trybot@lists.fd.o for test runs
> that you don't want the world to see (it's still public, but no one
> looks really).
Thanks, that's a better solution.
Noralf.
> -Daniel
>
>>
>> Noralf.
>>
>> [1] https://patchwork.freedesktop.org/series/58597/
>>
>> Noralf Trønnes (1):
>> drm/fb-helper: Avoid race with DRM userspace
>>
>> drivers/gpu/drm/drm_auth.c | 20 ++++++++
>> drivers/gpu/drm/drm_fb_helper.c | 90 ++++++++++++++++-----------------
>> drivers/gpu/drm/drm_internal.h | 2 +
>> 3 files changed, 67 insertions(+), 45 deletions(-)
>>
>> --
>> 2.20.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/fb-helper: Move modesetting code to drm_client (rev5)
2019-04-25 8:31 [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client Noralf Trønnes
` (2 preceding siblings ...)
2019-04-25 9:25 ` [Intel-gfx] [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client Daniel Vetter
@ 2019-04-25 15:40 ` Patchwork
3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-04-25 15:40 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx
== Series Details ==
Series: drm/fb-helper: Move modesetting code to drm_client (rev5)
URL : https://patchwork.freedesktop.org/series/58597/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5996_full -> Patchwork_12870_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_12870_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_12870_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12870_full:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@mock_requests:
- shard-skl: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl2/igt@i915_selftest@mock_requests.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl5/igt@i915_selftest@mock_requests.html
* igt@kms_fbcon_fbt@psr-suspend:
- shard-skl: [PASS][3] -> [FAIL][4] +1 similar issue
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl9/igt@kms_fbcon_fbt@psr-suspend.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl4/igt@kms_fbcon_fbt@psr-suspend.html
Known issues
------------
Here are the changes found in Patchwork_12870_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_eio@unwedge-stress:
- shard-snb: [PASS][5] -> [FAIL][6] ([fdo#109661])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-snb4/igt@gem_eio@unwedge-stress.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-snb2/igt@gem_eio@unwedge-stress.html
* igt@i915_pm_rpm@i2c:
- shard-iclb: [PASS][7] -> [DMESG-WARN][8] ([fdo#109982])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-iclb3/igt@i915_pm_rpm@i2c.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-iclb2/igt@i915_pm_rpm@i2c.html
* igt@i915_pm_rpm@universal-planes-dpms:
- shard-skl: [PASS][9] -> [INCOMPLETE][10] ([fdo#107807])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl4/igt@i915_pm_rpm@universal-planes-dpms.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl9/igt@i915_pm_rpm@universal-planes-dpms.html
* igt@i915_selftest@mock_requests:
- shard-snb: [PASS][11] -> [INCOMPLETE][12] ([fdo#105411])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-snb1/igt@i915_selftest@mock_requests.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-snb2/igt@i915_selftest@mock_requests.html
* igt@i915_suspend@fence-restore-untiled:
- shard-apl: [PASS][13] -> [DMESG-WARN][14] ([fdo#108566]) +1 similar issue
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-apl3/igt@i915_suspend@fence-restore-untiled.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-apl5/igt@i915_suspend@fence-restore-untiled.html
* igt@kms_atomic_transition@plane-all-transition-nonblocking:
- shard-iclb: [PASS][15] -> [INCOMPLETE][16] ([fdo#107713])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-iclb1/igt@kms_atomic_transition@plane-all-transition-nonblocking.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-iclb7/igt@kms_atomic_transition@plane-all-transition-nonblocking.html
* igt@kms_cursor_crc@cursor-256x256-suspend:
- shard-kbl: [PASS][17] -> [DMESG-WARN][18] ([fdo#108566]) +4 similar issues
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-kbl3/igt@kms_cursor_crc@cursor-256x256-suspend.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-kbl7/igt@kms_cursor_crc@cursor-256x256-suspend.html
* igt@kms_flip@flip-vs-expired-vblank:
- shard-skl: [PASS][19] -> [FAIL][20] ([fdo#105363])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl3/igt@kms_flip@flip-vs-expired-vblank.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt:
- shard-skl: [PASS][21] -> [FAIL][22] ([fdo#103167])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl10/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
- shard-iclb: [PASS][23] -> [FAIL][24] ([fdo#103167]) +2 similar issues
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
* igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
- shard-glk: [PASS][25] -> [SKIP][26] ([fdo#109271])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-glk9/igt@kms_plane@pixel-format-pipe-b-planes-source-clamping.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-glk5/igt@kms_plane@pixel-format-pipe-b-planes-source-clamping.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
- shard-skl: [PASS][27] -> [FAIL][28] ([fdo#103166])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl10/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: [PASS][29] -> [FAIL][30] ([fdo#108145])
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl10/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
* igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format:
- shard-glk: [PASS][31] -> [SKIP][32] ([fdo#109271] / [fdo#109278])
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-glk9/igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-glk5/igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format.html
* igt@kms_psr@psr2_primary_mmap_gtt:
- shard-iclb: [PASS][33] -> [SKIP][34] ([fdo#109441]) +1 similar issue
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-iclb2/igt@kms_psr@psr2_primary_mmap_gtt.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-iclb1/igt@kms_psr@psr2_primary_mmap_gtt.html
* igt@kms_rotation_crc@multiplane-rotation:
- shard-kbl: [PASS][35] -> [INCOMPLETE][36] ([fdo#103665])
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-kbl1/igt@kms_rotation_crc@multiplane-rotation.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-kbl1/igt@kms_rotation_crc@multiplane-rotation.html
* igt@kms_setmode@basic:
- shard-hsw: [PASS][37] -> [FAIL][38] ([fdo#99912])
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-hsw8/igt@kms_setmode@basic.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-hsw8/igt@kms_setmode@basic.html
* igt@tools_test@tools_test:
- shard-snb: [PASS][39] -> [SKIP][40] ([fdo#109271])
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-snb5/igt@tools_test@tools_test.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-snb4/igt@tools_test@tools_test.html
- shard-skl: [PASS][41] -> [SKIP][42] ([fdo#109271])
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl10/igt@tools_test@tools_test.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl5/igt@tools_test@tools_test.html
#### Possible fixes ####
* igt@gem_ctx_isolation@vcs0-s3:
- shard-skl: [INCOMPLETE][43] ([fdo#104108] / [fdo#107773]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl4/igt@gem_ctx_isolation@vcs0-s3.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl1/igt@gem_ctx_isolation@vcs0-s3.html
* igt@gem_exec_schedule@out-order-render:
- shard-apl: [INCOMPLETE][45] ([fdo#103927]) -> [PASS][46] +1 similar issue
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-apl4/igt@gem_exec_schedule@out-order-render.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-apl6/igt@gem_exec_schedule@out-order-render.html
* igt@gem_tiled_swapping@non-threaded:
- shard-iclb: [FAIL][47] ([fdo#108686]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-iclb2/igt@gem_tiled_swapping@non-threaded.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-iclb1/igt@gem_tiled_swapping@non-threaded.html
* igt@i915_pm_rpm@cursor-dpms:
- shard-skl: [INCOMPLETE][49] ([fdo#107807]) -> [PASS][50]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl6/igt@i915_pm_rpm@cursor-dpms.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl4/igt@i915_pm_rpm@cursor-dpms.html
* igt@i915_suspend@sysfs-reader:
- shard-apl: [DMESG-WARN][51] ([fdo#108566]) -> [PASS][52] +4 similar issues
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-apl6/igt@i915_suspend@sysfs-reader.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-apl8/igt@i915_suspend@sysfs-reader.html
* igt@kms_flip@flip-vs-suspend:
- shard-skl: [INCOMPLETE][53] ([fdo#109507]) -> [PASS][54]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-skl1/igt@kms_flip@flip-vs-suspend.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-skl6/igt@kms_flip@flip-vs-suspend.html
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-kbl: [INCOMPLETE][55] ([fdo#103665]) -> [PASS][56]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-suspend.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-kbl1/igt@kms_frontbuffer_tracking@fbc-suspend.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
- shard-iclb: [FAIL][57] ([fdo#103167]) -> [PASS][58] +3 similar issues
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
* igt@kms_plane@pixel-format-pipe-b-planes:
- shard-glk: [SKIP][59] ([fdo#109271]) -> [PASS][60]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-glk3/igt@kms_plane@pixel-format-pipe-b-planes.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-glk9/igt@kms_plane@pixel-format-pipe-b-planes.html
* igt@kms_psr2_su@frontbuffer:
- shard-iclb: [SKIP][61] ([fdo#109642]) -> [PASS][62]
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-iclb4/igt@kms_psr2_su@frontbuffer.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
* igt@kms_psr@psr2_sprite_plane_move:
- shard-iclb: [SKIP][63] ([fdo#109441]) -> [PASS][64] +1 similar issue
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-iclb8/igt@kms_psr@psr2_sprite_plane_move.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
* igt@kms_setmode@basic:
- shard-kbl: [FAIL][65] ([fdo#99912]) -> [PASS][66]
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-kbl3/igt@kms_setmode@basic.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-kbl6/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-c-ts-continuation-suspend:
- shard-kbl: [DMESG-WARN][67] ([fdo#108566]) -> [PASS][68] +2 similar issues
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5996/shard-kbl6/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/shard-kbl4/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
[fdo#109982]: https://bugs.freedesktop.org/show_bug.cgi?id=109982
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_5996 -> Patchwork_12870
CI_DRM_5996: 526cadba413ae78043a74d85f5bc9b6e0a430c38 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4963: 11e10bc575516c56978640fcc697c27f277c660a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12870: fa2af64690cbb9403e4fc947b3ac7b224859071e @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12870/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/1] drm/fb-helper: Avoid race with DRM userspace
2019-04-25 8:31 ` [PATCH v4 1/1] drm/fb-helper: Avoid race with DRM userspace Noralf Trønnes
@ 2019-05-04 12:34 ` Noralf Trønnes
2019-05-05 9:16 ` Noralf Trønnes
0 siblings, 1 reply; 9+ messages in thread
From: Noralf Trønnes @ 2019-05-04 12:34 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx
Den 25.04.2019 10.31, skrev Noralf Trønnes:
> 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.
>
> v2:
> - Remove drm_fb_helper_is_bound() (Daniel Vetter)
> - No need to check fb_helper->dev->master in
> drm_fb_helper_single_fb_probe(), restore_fbdev_mode() has the check.
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_auth.c | 20 ++++++++
> drivers/gpu/drm/drm_fb_helper.c | 90 ++++++++++++++++-----------------
> drivers/gpu/drm/drm_internal.h | 2 +
> 3 files changed, 67 insertions(+), 45 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 2339f0f8f5a8..578428461391 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -44,6 +44,7 @@
>
> #include "drm_crtc_internal.h"
> #include "drm_crtc_helper_internal.h"
> +#include "drm_internal.h"
>
> static bool drm_fbdev_emulation = true;
> module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
> @@ -509,7 +510,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
> return ret;
> }
>
> -static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> +static int restore_fbdev_mode_force(struct drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
>
> @@ -519,6 +520,21 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> return restore_fbdev_mode_legacy(fb_helper);
> }
>
> +static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> +{
> + struct drm_device *dev = fb_helper->dev;
> + int ret;
> +
> + if (!drm_master_internal_acquire(dev))
> + return -EBUSY;
> +
> + ret = restore_fbdev_mode_force(fb_helper);
> +
> + drm_master_internal_release(dev);
> +
> + return ret;
> +}
> +
> /**
> * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
> * @fb_helper: driver-allocated fbdev helper, can be NULL
The Intel CI doesn't like this patch. AFAICT the reason is that the
igt@kms_fbcon_fbt@psr-suspend test expects fbcon to work while it has an
open fd that is master. This doesn't match the new rule of bailing out
if there's a master.
Adding this debug output:
@@ -558,6 +558,17 @@ int
drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
return 0;
mutex_lock(&fb_helper->lock);
+if (READ_ONCE(fb_helper->dev->master)) {
+ int level = default_message_loglevel;
+
+ default_message_loglevel = LOGLEVEL_DEBUG;
+ printk("\n");
+ printk("\n");
+ printk("%s\n", __func__);
+ printk(" THERE IS A MASTER\n");
+ dump_stack();
+ default_message_loglevel = level;
+}
ret = restore_fbdev_mode_force(fb_helper);
do_delayed = fb_helper->delayed_hotplug;
Gives these log entries:
<7> [1857.940072] drm_fb_helper_restore_fbdev_mode_unlocked
<7> [1857.940074] THERE IS A MASTER
<7> [1857.940079] CPU: 4 PID: 8209 Comm: kms_fbcon_fbt Tainted: G U
5.1.0-rc7-CI-Trybot_4252+ #1
<7> [1857.940081] Hardware name: Intel Corporation Ice Lake Client
Platform/IceLake U DDR4 SODIMM PD RVP, BIOS
ICLSFWR1.R00.3121.A00.1903190527 03/19/2019
<7> [1857.940083] Call Trace:
<7> [1857.940091] dump_stack+0x67/0x9b
<7> [1857.940099] drm_fb_helper_restore_fbdev_mode_unlocked+0xda/0xf0
<7> [1857.940104] drm_fb_helper_set_par+0x24/0x50
<7> [1857.940188] intel_fbdev_set_par+0x11/0x40 [i915]
<7> [1857.940197] fb_set_var+0x17a/0x3f0
<7> [1857.940212] ? __lock_acquire+0x49f/0x1590
<7> [1857.940230] fbcon_blank+0x192/0x2e0
<7> [1857.940235] ? __lock_acquire+0x49f/0x1590
<7> [1857.940254] do_unblank_screen+0xa1/0x170
<7> [1857.940260] vt_ioctl+0x505/0x11d0
<7> [1857.940270] tty_ioctl+0xee/0x940
<7> [1857.940274] ? lockdep_hardirqs_off+0x94/0xd0
<7> [1857.940278] ? __slab_free+0x24e/0x4f0
<7> [1857.940287] ? _raw_spin_unlock_irqrestore+0x39/0x60
<7> [1857.940291] ? kmem_cache_free+0x279/0x2e0
<7> [1857.940299] do_vfs_ioctl+0xa0/0x6e0
<7> [1857.940305] ? do_sys_open+0x13b/0x250
<7> [1857.940311] ? rcu_read_lock_sched_held+0x6f/0x80
<7> [1857.940313] ? kmem_cache_free+0x283/0x2e0
<7> [1857.940321] ksys_ioctl+0x35/0x60
<7> [1857.940328] __x64_sys_ioctl+0x11/0x20
<7> [1857.940332] do_syscall_64+0x55/0x190
<7> [1857.940337] entry_SYSCALL_64_after_hwframe+0x49/0xbe
<7> [1857.940341] RIP: 0033:0x7faf8f0825d7
<7> [1857.940345] Code: b3 66 90 48 8b 05 b1 48 2d 00 64 c7 00 26 00 00
00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00
0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 81 48 2d 00 f7 d8 64 89 01 48
<7> [1857.940347] RSP: 002b:00007ffddf6a94b8 EFLAGS: 00000246 ORIG_RAX:
0000000000000010
<7> [1857.940351] RAX: ffffffffffffffda RBX: 0000000000000000 RCX:
00007faf8f0825d7
<7> [1857.940353] RDX: 0000000000000000 RSI: 0000000000004b3a RDI:
000000000000000a
<7> [1857.940355] RBP: 0000000000000000 R08: 0000000000000000 R09:
00000000000000ab
<7> [1857.940357] R10: 0000000000000000 R11: 0000000000000246 R12:
0000000000004b3a
<7> [1857.940359] R13: 000000000000000a R14: 0000000000000000 R15:
0000000000000000
<6> [1861.481078] [IGT] kms_fbcon_fbt: exiting, ret=99
Patches:
https://patchwork.freedesktop.org/series/59951/#rev8
Failing test:
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_4252/shard-iclb7/igt@kms_fbcon_fbt@psr-suspend.html
If I make this change and don't check for master, then it's fine:
@@ -542,7 +558,7 @@ int
drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
return 0;
mutex_lock(&fb_helper->lock);
- ret = restore_fbdev_mode(fb_helper);
+ ret = restore_fbdev_mode_force(fb_helper);
do_delayed = fb_helper->delayed_hotplug;
if (do_delayed)
https://patchwork.freedesktop.org/series/59951/#rev9
Should I make this change, or should the test change?
Noralf.
> @@ -556,34 +572,6 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> }
> EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
>
> -static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
> -{
> - struct drm_device *dev = fb_helper->dev;
> - struct drm_crtc *crtc;
> - int bound = 0, crtcs_bound = 0;
> -
> - /*
> - * Sometimes user space wants everything disabled, so don't steal the
> - * display if there's a master.
> - */
> - if (READ_ONCE(dev->master))
> - return false;
> -
> - drm_for_each_crtc(crtc, dev) {
> - drm_modeset_lock(&crtc->mutex, NULL);
> - if (crtc->primary->fb)
> - crtcs_bound++;
> - if (crtc->primary->fb == fb_helper->fb)
> - bound++;
> - drm_modeset_unlock(&crtc->mutex);
> - }
> -
> - if (bound < crtcs_bound)
> - return false;
> -
> - return true;
> -}
> -
> #ifdef CONFIG_MAGIC_SYSRQ
> /*
> * restore fbcon display for all kms driver's using this helper, used for sysrq
> @@ -604,7 +592,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
> continue;
>
> mutex_lock(&helper->lock);
> - ret = restore_fbdev_mode(helper);
> + ret = restore_fbdev_mode_force(helper);
> if (ret)
> error = true;
> mutex_unlock(&helper->lock);
> @@ -663,20 +651,22 @@ static void dpms_legacy(struct drm_fb_helper *fb_helper, int dpms_mode)
> 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)) {
> - mutex_unlock(&fb_helper->lock);
> - return;
> - }
> + if (!drm_master_internal_acquire(dev))
> + goto unlock;
>
> - if (drm_drv_uses_atomic_modeset(fb_helper->dev))
> + if (drm_drv_uses_atomic_modeset(dev))
> restore_fbdev_mode_atomic(fb_helper, dpms_mode == DRM_MODE_DPMS_ON);
> else
> dpms_legacy(fb_helper, dpms_mode);
> +
> + drm_master_internal_release(dev);
> +unlock:
> mutex_unlock(&fb_helper->lock);
> }
>
> @@ -1509,6 +1499,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)
> @@ -1516,9 +1507,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)
> @@ -1528,7 +1519,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;
> @@ -1548,12 +1540,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_mode_set *mode_set;
> 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;
> }
> @@ -1591,11 +1584,12 @@ 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;
> @@ -1847,15 +1841,18 @@ 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;
> @@ -2014,7 +2011,7 @@ 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))
> + if (!fb_helper->deferred_setup)
> restore_fbdev_mode(fb_helper);
> return -EAGAIN;
> }
> @@ -3028,6 +3025,7 @@ 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 || !fb_helper)
> @@ -3040,12 +3038,14 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> return err;
> }
>
> - if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
> + if (!fb_helper->fb || !drm_master_internal_acquire(dev)) {
> fb_helper->delayed_hotplug = true;
> mutex_unlock(&fb_helper->lock);
> return err;
> }
>
> + drm_master_internal_release(dev);
> +
> DRM_DEBUG_KMS("\n");
>
> drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index d9a483a5fce0..3ee97c9998a2 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;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/1] drm/fb-helper: Avoid race with DRM userspace
2019-05-04 12:34 ` Noralf Trønnes
@ 2019-05-05 9:16 ` Noralf Trønnes
2019-05-06 7:54 ` Daniel Vetter
0 siblings, 1 reply; 9+ messages in thread
From: Noralf Trønnes @ 2019-05-05 9:16 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx
Den 04.05.2019 14.34, skrev Noralf Trønnes:
>
>
> Den 25.04.2019 10.31, skrev Noralf Trønnes:
>> 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.
>>
>> v2:
>> - Remove drm_fb_helper_is_bound() (Daniel Vetter)
>> - No need to check fb_helper->dev->master in
>> drm_fb_helper_single_fb_probe(), restore_fbdev_mode() has the check.
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>> drivers/gpu/drm/drm_auth.c | 20 ++++++++
>> drivers/gpu/drm/drm_fb_helper.c | 90 ++++++++++++++++-----------------
>> drivers/gpu/drm/drm_internal.h | 2 +
>> 3 files changed, 67 insertions(+), 45 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 2339f0f8f5a8..578428461391 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -44,6 +44,7 @@
>>
>> #include "drm_crtc_internal.h"
>> #include "drm_crtc_helper_internal.h"
>> +#include "drm_internal.h"
>>
>> static bool drm_fbdev_emulation = true;
>> module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
>> @@ -509,7 +510,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
>> return ret;
>> }
>>
>> -static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>> +static int restore_fbdev_mode_force(struct drm_fb_helper *fb_helper)
>> {
>> struct drm_device *dev = fb_helper->dev;
>>
>> @@ -519,6 +520,21 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>> return restore_fbdev_mode_legacy(fb_helper);
>> }
>>
>> +static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>> +{
>> + struct drm_device *dev = fb_helper->dev;
>> + int ret;
>> +
>> + if (!drm_master_internal_acquire(dev))
>> + return -EBUSY;
>> +
>> + ret = restore_fbdev_mode_force(fb_helper);
>> +
>> + drm_master_internal_release(dev);
>> +
>> + return ret;
>> +}
>> +
>> /**
>> * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
>> * @fb_helper: driver-allocated fbdev helper, can be NULL
>
> The Intel CI doesn't like this patch. AFAICT the reason is that the
> igt@kms_fbcon_fbt@psr-suspend test expects fbcon to work while it has an
> open fd that is master. This doesn't match the new rule of bailing out
> if there's a master.
>
> Adding this debug output:
>
> @@ -558,6 +558,17 @@ int
> drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> return 0;
>
> mutex_lock(&fb_helper->lock);
> +if (READ_ONCE(fb_helper->dev->master)) {
> + int level = default_message_loglevel;
> +
> + default_message_loglevel = LOGLEVEL_DEBUG;
> + printk("\n");
> + printk("\n");
> + printk("%s\n", __func__);
> + printk(" THERE IS A MASTER\n");
> + dump_stack();
> + default_message_loglevel = level;
> +}
> ret = restore_fbdev_mode_force(fb_helper);
>
> do_delayed = fb_helper->delayed_hotplug;
>
> Gives these log entries:
>
> <7> [1857.940072] drm_fb_helper_restore_fbdev_mode_unlocked
> <7> [1857.940074] THERE IS A MASTER
> <7> [1857.940079] CPU: 4 PID: 8209 Comm: kms_fbcon_fbt Tainted: G U
> 5.1.0-rc7-CI-Trybot_4252+ #1
> <7> [1857.940081] Hardware name: Intel Corporation Ice Lake Client
> Platform/IceLake U DDR4 SODIMM PD RVP, BIOS
> ICLSFWR1.R00.3121.A00.1903190527 03/19/2019
> <7> [1857.940083] Call Trace:
> <7> [1857.940091] dump_stack+0x67/0x9b
> <7> [1857.940099] drm_fb_helper_restore_fbdev_mode_unlocked+0xda/0xf0
> <7> [1857.940104] drm_fb_helper_set_par+0x24/0x50
> <7> [1857.940188] intel_fbdev_set_par+0x11/0x40 [i915]
> <7> [1857.940197] fb_set_var+0x17a/0x3f0
> <7> [1857.940212] ? __lock_acquire+0x49f/0x1590
> <7> [1857.940230] fbcon_blank+0x192/0x2e0
> <7> [1857.940235] ? __lock_acquire+0x49f/0x1590
> <7> [1857.940254] do_unblank_screen+0xa1/0x170
> <7> [1857.940260] vt_ioctl+0x505/0x11d0
> <7> [1857.940270] tty_ioctl+0xee/0x940
> <7> [1857.940274] ? lockdep_hardirqs_off+0x94/0xd0
> <7> [1857.940278] ? __slab_free+0x24e/0x4f0
> <7> [1857.940287] ? _raw_spin_unlock_irqrestore+0x39/0x60
> <7> [1857.940291] ? kmem_cache_free+0x279/0x2e0
> <7> [1857.940299] do_vfs_ioctl+0xa0/0x6e0
> <7> [1857.940305] ? do_sys_open+0x13b/0x250
> <7> [1857.940311] ? rcu_read_lock_sched_held+0x6f/0x80
> <7> [1857.940313] ? kmem_cache_free+0x283/0x2e0
> <7> [1857.940321] ksys_ioctl+0x35/0x60
> <7> [1857.940328] __x64_sys_ioctl+0x11/0x20
> <7> [1857.940332] do_syscall_64+0x55/0x190
> <7> [1857.940337] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> <7> [1857.940341] RIP: 0033:0x7faf8f0825d7
> <7> [1857.940345] Code: b3 66 90 48 8b 05 b1 48 2d 00 64 c7 00 26 00 00
> 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00
> 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 81 48 2d 00 f7 d8 64 89 01 48
> <7> [1857.940347] RSP: 002b:00007ffddf6a94b8 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000010
> <7> [1857.940351] RAX: ffffffffffffffda RBX: 0000000000000000 RCX:
> 00007faf8f0825d7
> <7> [1857.940353] RDX: 0000000000000000 RSI: 0000000000004b3a RDI:
> 000000000000000a
> <7> [1857.940355] RBP: 0000000000000000 R08: 0000000000000000 R09:
> 00000000000000ab
> <7> [1857.940357] R10: 0000000000000000 R11: 0000000000000246 R12:
> 0000000000004b3a
> <7> [1857.940359] R13: 000000000000000a R14: 0000000000000000 R15:
> 0000000000000000
> <6> [1861.481078] [IGT] kms_fbcon_fbt: exiting, ret=99
>
> Patches:
> https://patchwork.freedesktop.org/series/59951/#rev8
> Failing test:
> https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_4252/shard-iclb7/igt@kms_fbcon_fbt@psr-suspend.html
>
Actually there's two failing tests, this one as well:
* igt at kms_fbcon_fbt@psr:
- shard-skl: NOTRUN -> [FAIL][1]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_4252/shard-skl7/igt@kms_fbcon_fbt@psr.html
>
> If I make this change and don't check for master, then it's fine:
>
> @@ -542,7 +558,7 @@ int
> drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> return 0;
>
> mutex_lock(&fb_helper->lock);
> - ret = restore_fbdev_mode(fb_helper);
> + ret = restore_fbdev_mode_force(fb_helper);
>
> do_delayed = fb_helper->delayed_hotplug;
> if (do_delayed)
>
> https://patchwork.freedesktop.org/series/59951/#rev9
>
> Should I make this change, or should the test change?
>
> Noralf.
>
>> @@ -556,34 +572,6 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>> }
>> EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
>>
>> -static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
>> -{
>> - struct drm_device *dev = fb_helper->dev;
>> - struct drm_crtc *crtc;
>> - int bound = 0, crtcs_bound = 0;
>> -
>> - /*
>> - * Sometimes user space wants everything disabled, so don't steal the
>> - * display if there's a master.
>> - */
>> - if (READ_ONCE(dev->master))
>> - return false;
>> -
>> - drm_for_each_crtc(crtc, dev) {
>> - drm_modeset_lock(&crtc->mutex, NULL);
>> - if (crtc->primary->fb)
>> - crtcs_bound++;
>> - if (crtc->primary->fb == fb_helper->fb)
>> - bound++;
>> - drm_modeset_unlock(&crtc->mutex);
>> - }
>> -
>> - if (bound < crtcs_bound)
>> - return false;
>> -
>> - return true;
>> -}
>> -
>> #ifdef CONFIG_MAGIC_SYSRQ
>> /*
>> * restore fbcon display for all kms driver's using this helper, used for sysrq
>> @@ -604,7 +592,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
>> continue;
>>
>> mutex_lock(&helper->lock);
>> - ret = restore_fbdev_mode(helper);
>> + ret = restore_fbdev_mode_force(helper);
>> if (ret)
>> error = true;
>> mutex_unlock(&helper->lock);
>> @@ -663,20 +651,22 @@ static void dpms_legacy(struct drm_fb_helper *fb_helper, int dpms_mode)
>> 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)) {
>> - mutex_unlock(&fb_helper->lock);
>> - return;
>> - }
>> + if (!drm_master_internal_acquire(dev))
>> + goto unlock;
>>
>> - if (drm_drv_uses_atomic_modeset(fb_helper->dev))
>> + if (drm_drv_uses_atomic_modeset(dev))
>> restore_fbdev_mode_atomic(fb_helper, dpms_mode == DRM_MODE_DPMS_ON);
>> else
>> dpms_legacy(fb_helper, dpms_mode);
>> +
>> + drm_master_internal_release(dev);
>> +unlock:
>> mutex_unlock(&fb_helper->lock);
>> }
>>
>> @@ -1509,6 +1499,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)
>> @@ -1516,9 +1507,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)
>> @@ -1528,7 +1519,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;
>> @@ -1548,12 +1540,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_mode_set *mode_set;
>> 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;
>> }
>> @@ -1591,11 +1584,12 @@ 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;
>> @@ -1847,15 +1841,18 @@ 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;
>> @@ -2014,7 +2011,7 @@ 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))
>> + if (!fb_helper->deferred_setup)
>> restore_fbdev_mode(fb_helper);
>> return -EAGAIN;
>> }
>> @@ -3028,6 +3025,7 @@ 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 || !fb_helper)
>> @@ -3040,12 +3038,14 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>> return err;
>> }
>>
>> - if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
>> + if (!fb_helper->fb || !drm_master_internal_acquire(dev)) {
>> fb_helper->delayed_hotplug = true;
>> mutex_unlock(&fb_helper->lock);
>> return err;
>> }
>>
>> + drm_master_internal_release(dev);
>> +
>> DRM_DEBUG_KMS("\n");
>>
>> drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
>> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
>> index d9a483a5fce0..3ee97c9998a2 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;
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/1] drm/fb-helper: Avoid race with DRM userspace
2019-05-05 9:16 ` Noralf Trønnes
@ 2019-05-06 7:54 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2019-05-06 7:54 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Sun, May 05, 2019 at 11:16:54AM +0200, Noralf Trønnes wrote:
>
>
> Den 04.05.2019 14.34, skrev Noralf Trønnes:
> >
> >
> > Den 25.04.2019 10.31, skrev Noralf Trønnes:
> >> 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.
> >>
> >> v2:
> >> - Remove drm_fb_helper_is_bound() (Daniel Vetter)
> >> - No need to check fb_helper->dev->master in
> >> drm_fb_helper_single_fb_probe(), restore_fbdev_mode() has the check.
> >>
> >> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> ---
> >> drivers/gpu/drm/drm_auth.c | 20 ++++++++
> >> drivers/gpu/drm/drm_fb_helper.c | 90 ++++++++++++++++-----------------
> >> drivers/gpu/drm/drm_internal.h | 2 +
> >> 3 files changed, 67 insertions(+), 45 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 2339f0f8f5a8..578428461391 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -44,6 +44,7 @@
> >>
> >> #include "drm_crtc_internal.h"
> >> #include "drm_crtc_helper_internal.h"
> >> +#include "drm_internal.h"
> >>
> >> static bool drm_fbdev_emulation = true;
> >> module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
> >> @@ -509,7 +510,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
> >> return ret;
> >> }
> >>
> >> -static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> >> +static int restore_fbdev_mode_force(struct drm_fb_helper *fb_helper)
> >> {
> >> struct drm_device *dev = fb_helper->dev;
> >>
> >> @@ -519,6 +520,21 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> >> return restore_fbdev_mode_legacy(fb_helper);
> >> }
> >>
> >> +static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> >> +{
> >> + struct drm_device *dev = fb_helper->dev;
> >> + int ret;
> >> +
> >> + if (!drm_master_internal_acquire(dev))
> >> + return -EBUSY;
> >> +
> >> + ret = restore_fbdev_mode_force(fb_helper);
> >> +
> >> + drm_master_internal_release(dev);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> /**
> >> * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
> >> * @fb_helper: driver-allocated fbdev helper, can be NULL
> >
> > The Intel CI doesn't like this patch. AFAICT the reason is that the
> > igt@kms_fbcon_fbt@psr-suspend test expects fbcon to work while it has an
> > open fd that is master. This doesn't match the new rule of bailing out
> > if there's a master.
> >
> > Adding this debug output:
> >
> > @@ -558,6 +558,17 @@ int
> > drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> > return 0;
> >
> > mutex_lock(&fb_helper->lock);
> > +if (READ_ONCE(fb_helper->dev->master)) {
> > + int level = default_message_loglevel;
> > +
> > + default_message_loglevel = LOGLEVEL_DEBUG;
> > + printk("\n");
> > + printk("\n");
> > + printk("%s\n", __func__);
> > + printk(" THERE IS A MASTER\n");
> > + dump_stack();
> > + default_message_loglevel = level;
> > +}
> > ret = restore_fbdev_mode_force(fb_helper);
> >
> > do_delayed = fb_helper->delayed_hotplug;
> >
> > Gives these log entries:
> >
> > <7> [1857.940072] drm_fb_helper_restore_fbdev_mode_unlocked
> > <7> [1857.940074] THERE IS A MASTER
> > <7> [1857.940079] CPU: 4 PID: 8209 Comm: kms_fbcon_fbt Tainted: G U
> > 5.1.0-rc7-CI-Trybot_4252+ #1
> > <7> [1857.940081] Hardware name: Intel Corporation Ice Lake Client
> > Platform/IceLake U DDR4 SODIMM PD RVP, BIOS
> > ICLSFWR1.R00.3121.A00.1903190527 03/19/2019
> > <7> [1857.940083] Call Trace:
> > <7> [1857.940091] dump_stack+0x67/0x9b
> > <7> [1857.940099] drm_fb_helper_restore_fbdev_mode_unlocked+0xda/0xf0
> > <7> [1857.940104] drm_fb_helper_set_par+0x24/0x50
> > <7> [1857.940188] intel_fbdev_set_par+0x11/0x40 [i915]
> > <7> [1857.940197] fb_set_var+0x17a/0x3f0
> > <7> [1857.940212] ? __lock_acquire+0x49f/0x1590
> > <7> [1857.940230] fbcon_blank+0x192/0x2e0
> > <7> [1857.940235] ? __lock_acquire+0x49f/0x1590
> > <7> [1857.940254] do_unblank_screen+0xa1/0x170
> > <7> [1857.940260] vt_ioctl+0x505/0x11d0
> > <7> [1857.940270] tty_ioctl+0xee/0x940
> > <7> [1857.940274] ? lockdep_hardirqs_off+0x94/0xd0
> > <7> [1857.940278] ? __slab_free+0x24e/0x4f0
> > <7> [1857.940287] ? _raw_spin_unlock_irqrestore+0x39/0x60
> > <7> [1857.940291] ? kmem_cache_free+0x279/0x2e0
> > <7> [1857.940299] do_vfs_ioctl+0xa0/0x6e0
> > <7> [1857.940305] ? do_sys_open+0x13b/0x250
> > <7> [1857.940311] ? rcu_read_lock_sched_held+0x6f/0x80
> > <7> [1857.940313] ? kmem_cache_free+0x283/0x2e0
> > <7> [1857.940321] ksys_ioctl+0x35/0x60
> > <7> [1857.940328] __x64_sys_ioctl+0x11/0x20
> > <7> [1857.940332] do_syscall_64+0x55/0x190
> > <7> [1857.940337] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > <7> [1857.940341] RIP: 0033:0x7faf8f0825d7
> > <7> [1857.940345] Code: b3 66 90 48 8b 05 b1 48 2d 00 64 c7 00 26 00 00
> > 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00
> > 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 81 48 2d 00 f7 d8 64 89 01 48
> > <7> [1857.940347] RSP: 002b:00007ffddf6a94b8 EFLAGS: 00000246 ORIG_RAX:
> > 0000000000000010
> > <7> [1857.940351] RAX: ffffffffffffffda RBX: 0000000000000000 RCX:
> > 00007faf8f0825d7
> > <7> [1857.940353] RDX: 0000000000000000 RSI: 0000000000004b3a RDI:
> > 000000000000000a
> > <7> [1857.940355] RBP: 0000000000000000 R08: 0000000000000000 R09:
> > 00000000000000ab
> > <7> [1857.940357] R10: 0000000000000000 R11: 0000000000000246 R12:
> > 0000000000004b3a
> > <7> [1857.940359] R13: 000000000000000a R14: 0000000000000000 R15:
> > 0000000000000000
> > <6> [1861.481078] [IGT] kms_fbcon_fbt: exiting, ret=99
> >
> > Patches:
> > https://patchwork.freedesktop.org/series/59951/#rev8
> > Failing test:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_4252/shard-iclb7/igt@kms_fbcon_fbt@psr-suspend.html
> >
>
> Actually there's two failing tests, this one as well:
>
> * igt at kms_fbcon_fbt@psr:
> - shard-skl: NOTRUN -> [FAIL][1]
> [1]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_4252/shard-skl7/igt@kms_fbcon_fbt@psr.html
I think the test should change. Our igt fbcon/fbdev tests started as some
quick hacks and evolved from there ...
-Daniel
>
>
> >
> > If I make this change and don't check for master, then it's fine:
> >
> > @@ -542,7 +558,7 @@ int
> > drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> > return 0;
> >
> > mutex_lock(&fb_helper->lock);
> > - ret = restore_fbdev_mode(fb_helper);
> > + ret = restore_fbdev_mode_force(fb_helper);
> >
> > do_delayed = fb_helper->delayed_hotplug;
> > if (do_delayed)
> >
> > https://patchwork.freedesktop.org/series/59951/#rev9
> >
> > Should I make this change, or should the test change?
> >
> > Noralf.
> >
> >> @@ -556,34 +572,6 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> >> }
> >> EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
> >>
> >> -static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
> >> -{
> >> - struct drm_device *dev = fb_helper->dev;
> >> - struct drm_crtc *crtc;
> >> - int bound = 0, crtcs_bound = 0;
> >> -
> >> - /*
> >> - * Sometimes user space wants everything disabled, so don't steal the
> >> - * display if there's a master.
> >> - */
> >> - if (READ_ONCE(dev->master))
> >> - return false;
> >> -
> >> - drm_for_each_crtc(crtc, dev) {
> >> - drm_modeset_lock(&crtc->mutex, NULL);
> >> - if (crtc->primary->fb)
> >> - crtcs_bound++;
> >> - if (crtc->primary->fb == fb_helper->fb)
> >> - bound++;
> >> - drm_modeset_unlock(&crtc->mutex);
> >> - }
> >> -
> >> - if (bound < crtcs_bound)
> >> - return false;
> >> -
> >> - return true;
> >> -}
> >> -
> >> #ifdef CONFIG_MAGIC_SYSRQ
> >> /*
> >> * restore fbcon display for all kms driver's using this helper, used for sysrq
> >> @@ -604,7 +592,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
> >> continue;
> >>
> >> mutex_lock(&helper->lock);
> >> - ret = restore_fbdev_mode(helper);
> >> + ret = restore_fbdev_mode_force(helper);
> >> if (ret)
> >> error = true;
> >> mutex_unlock(&helper->lock);
> >> @@ -663,20 +651,22 @@ static void dpms_legacy(struct drm_fb_helper *fb_helper, int dpms_mode)
> >> 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)) {
> >> - mutex_unlock(&fb_helper->lock);
> >> - return;
> >> - }
> >> + if (!drm_master_internal_acquire(dev))
> >> + goto unlock;
> >>
> >> - if (drm_drv_uses_atomic_modeset(fb_helper->dev))
> >> + if (drm_drv_uses_atomic_modeset(dev))
> >> restore_fbdev_mode_atomic(fb_helper, dpms_mode == DRM_MODE_DPMS_ON);
> >> else
> >> dpms_legacy(fb_helper, dpms_mode);
> >> +
> >> + drm_master_internal_release(dev);
> >> +unlock:
> >> mutex_unlock(&fb_helper->lock);
> >> }
> >>
> >> @@ -1509,6 +1499,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)
> >> @@ -1516,9 +1507,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)
> >> @@ -1528,7 +1519,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;
> >> @@ -1548,12 +1540,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_mode_set *mode_set;
> >> 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;
> >> }
> >> @@ -1591,11 +1584,12 @@ 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;
> >> @@ -1847,15 +1841,18 @@ 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;
> >> @@ -2014,7 +2011,7 @@ 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))
> >> + if (!fb_helper->deferred_setup)
> >> restore_fbdev_mode(fb_helper);
> >> return -EAGAIN;
> >> }
> >> @@ -3028,6 +3025,7 @@ 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 || !fb_helper)
> >> @@ -3040,12 +3038,14 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> >> return err;
> >> }
> >>
> >> - if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
> >> + if (!fb_helper->fb || !drm_master_internal_acquire(dev)) {
> >> fb_helper->delayed_hotplug = true;
> >> mutex_unlock(&fb_helper->lock);
> >> return err;
> >> }
> >>
> >> + drm_master_internal_release(dev);
> >> +
> >> DRM_DEBUG_KMS("\n");
> >>
> >> drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
> >> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> >> index d9a483a5fce0..3ee97c9998a2 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;
> >>
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-05-06 7:54 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-25 8:31 [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client Noralf Trønnes
2019-04-25 8:31 ` [PATCH v4 1/1] drm/fb-helper: Avoid race with DRM userspace Noralf Trønnes
2019-05-04 12:34 ` Noralf Trønnes
2019-05-05 9:16 ` Noralf Trønnes
2019-05-06 7:54 ` Daniel Vetter
2019-04-25 9:10 ` ✓ Fi.CI.BAT: success for drm/fb-helper: Move modesetting code to drm_client (rev5) Patchwork
2019-04-25 9:25 ` [Intel-gfx] [PATCH v4 0/1] drm/fb-helper: Move modesetting code to drm_client Daniel Vetter
2019-04-25 11:56 ` Noralf Trønnes
2019-04-25 15:40 ` ✗ Fi.CI.IGT: failure for drm/fb-helper: Move modesetting code to drm_client (rev5) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.