[PATCH] drm: Add locking to resume_force_mode to prevent multiple

[PATCH] drm: Add locking to resume_force_mode to prevent multiple

[Sean Paul]

Add a mutex to protect resume_force_mode from being called multiple
times. This fixes a bug observed on SNB where two wake sources call
resume_force_mode and the FDI training fails as a result. The user
facing result of this is complete screen corruption.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/drm_crtc_helper.c |    8 ++++++++
 drivers/gpu/drm/drm_stub.c        |    1 +
 include/drm/drmP.h                |    1 +
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 8111889..de7e492 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -882,6 +882,13 @@ int drm_helper_resume_force_mode(struct drm_device *dev)
 	struct drm_crtc_helper_funcs *crtc_funcs;
 	int ret;
 
+	/* Make sure we're not already resuming, this can really screw
+	 * things up! */
+	if (mutex_trylock(&dev->resume_mutex) == 0) {
+		DRM_DEBUG("Already in resume_force_mode\n");
+		return 0;
+	}
+
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 
 		if (!crtc->enabled)
@@ -914,6 +921,7 @@ int drm_helper_resume_force_mode(struct drm_device *dev)
 	}
 	/* disable the unused connectors while restoring the modesetting */
 	drm_helper_disable_unused_functions(dev);
+	mutex_unlock(&dev->resume_mutex);
 	return 0;
 }
 EXPORT_SYMBOL(drm_helper_resume_force_mode);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index aa454f8..f35bed5 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -273,6 +273,7 @@ int drm_fill_in_dev(struct drm_device *dev,
 	spin_lock_init(&dev->count_lock);
 	spin_lock_init(&dev->event_lock);
 	mutex_init(&dev->struct_mutex);
+	mutex_init(&dev->resume_mutex);
 	mutex_init(&dev->ctxlist_mutex);
 
 	if (drm_ht_create(&dev->map_hash, 12)) {
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 574bd1c..75395a8 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1034,6 +1034,7 @@ struct drm_device {
 	/*@{ */
 	spinlock_t count_lock;		/**< For inuse, drm_device::open_count, drm_device::buf_use */
 	struct mutex struct_mutex;	/**< For others */
+	struct mutex resume_mutex;	/**< For drm_helper_resume_force_mode */
 	/*@} */
 
 	/** \name Usage Counters */
-- 
1.7.7.3

Re: [PATCH] drm: Add locking to resume_force_mode to prevent multiple

[Chris Wilson]

On Thu, 22 Mar 2012 18:25:55 -0400, Sean Paul <seanpaul@chromium.org> wrote:
> Add a mutex to protect resume_force_mode from being called multiple
> times. This fixes a bug observed on SNB where two wake sources call
> resume_force_mode and the FDI training fails as a result. The user
> facing result of this is complete screen corruption.

Looks like a bug in i915_drv.c for calling that function without holding
the mode_config.lock
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm: Add locking to resume_force_mode to prevent multiple

[Sean Paul]

On Thu, Mar 22, 2012 at 6:57 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, 22 Mar 2012 18:25:55 -0400, Sean Paul <seanpaul@chromium.org> wrote:
>> Add a mutex to protect resume_force_mode from being called multiple
>> times. This fixes a bug observed on SNB where two wake sources call
>> resume_force_mode and the FDI training fails as a result. The user
>> facing result of this is complete screen corruption.
>
> Looks like a bug in i915_drv.c for calling that function without holding
> the mode_config.lock

Good catch, thanks for the feedback Chris.

I'll submit a new patch to intel-gfx, please drop this one.

Sean


> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre