Bug 76151 - [HSW-M GT3e bisected]Blackscreen after booting with HDMI only
Summary: [HSW-M GT3e bisected]Blackscreen after booting with HDMI only
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: XOrg git
Hardware: Other All
: highest normal
Assignee: Paulo Zanoni
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-14 02:52 UTC by Guo Jinxian
Modified: 2016-10-13 08:31 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
dmesg (126.15 KB, text/plain)
2014-03-14 02:52 UTC, Guo Jinxian
no flags Details
fix runtime pm inbalance (4.80 KB, patch)
2014-03-14 07:48 UTC, Daniel Vetter
no flags Details | Splinter Review
dmesg (97.64 KB, text/plain)
2014-05-22 01:38 UTC, Guo Jinxian
no flags Details

Description Guo Jinxian 2014-03-14 02:52:02 UTC
Created attachment 95768 [details]
dmesg

system Environment:
--------------------------
platform: HSW-M GT3 
Kernel:	(drm-intel-next-queued) c2831a94b5e77a407db0708816949d4a87416a8e

Bug detailed description:
-------------------------
Boot the OS with HDMI only, blackscreen shows. The screen is able to be lighten by testdisplay or X.

It's a regression bug, the first bad commit is 6d88064edcfc5e5893371f7c06b9f3078dc1edf6 

Reproduce steps:
--------------------
boot the OS with HDMI only
Comment 1 Jani Nikula 2014-03-14 06:06:36 UTC
(In reply to comment #0)
> It's a regression bug, the first bad commit is
> 6d88064edcfc5e5893371f7c06b9f3078dc1edf6 

Please always include the first lines of the commit:

commit 6d88064edcfc5e5893371f7c06b9f3078dc1edf6
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date:   Fri Feb 21 17:58:29 2014 -0300

    drm/i915: put runtime PM only when we actually release force_wake

and for bisected regressions always assign the bug to the author of the bad commit. Done now. Thanks.
Comment 2 Daniel Vetter 2014-03-14 06:20:53 UTC
Can you please tests the below patch?

http://patchwork.freedesktop.org/patch/21687/
Comment 3 Guo Jinxian 2014-03-14 07:22:49 UTC
(In reply to comment #2)
> Can you please tests the below patch?
> 
> http://patchwork.freedesktop.org/patch/21687/

It's pass with this patch, thanks.
Comment 4 Daniel Vetter 2014-03-14 07:48:37 UTC
Created attachment 95781 [details] [review]
fix runtime pm inbalance

Ok, slightly improved patch (Paulo's is just a revert). Please test.
Comment 5 Chris Wilson 2014-03-14 08:03:12 UTC
Ugh, that interleaves the lifetime of the pm vs forcewake ref.

reg read: -> deferred forcewake.
gen6_gt_get: -> pm_get, forcewake++
gen6_gt_put: -> --forcewake, pm_put

device is now asleep

forcewake_timer: boom, invalid reg access.


diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b8b59752562a..e92bdcef5342 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5877,6 +5877,8 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
 	if (!HAS_RUNTIME_PM(dev))
 		return;
 
+	del_timer_sync(&dev_priv->uncore.force_wake_timer);
+
 	pm_runtime_mark_last_busy(device);
 	pm_runtime_put_autosuspend(device);
 }
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index e6bb421a3dbd..527527382361 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -304,8 +304,6 @@ static void gen6_force_wake_timer(unsigned long arg)
 	if (--dev_priv->uncore.forcewake_count == 0)
 		dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
-
-	intel_runtime_pm_put(dev_priv);
 }
 
 static void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
@@ -439,7 +437,6 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
 void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
 {
 	unsigned long irqflags;
-	bool delayed = false;
 
 	if (!dev_priv->uncore.funcs.force_wake_put)
 		return;
@@ -450,19 +447,17 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
 		goto out;
 	}
 
-
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 	if (--dev_priv->uncore.forcewake_count == 0) {
 		dev_priv->uncore.forcewake_count++;
-		delayed = true;
 		mod_timer_pinned(&dev_priv->uncore.force_wake_timer,
 				 jiffies + 1);
 	}
+
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 
 out:
-	if (!delayed)
-		intel_runtime_pm_put(dev_priv);
+	intel_runtime_pm_put(dev_priv);
 }
 
 void assert_force_wake_inactive(struct drm_i915_private *dev_priv)
Comment 6 Daniel Vetter 2014-03-14 08:23:38 UTC
(In reply to comment #5)
> Ugh, that interleaves the lifetime of the pm vs forcewake ref.
> 
> reg read: -> deferred forcewake.
> gen6_gt_get: -> pm_get, forcewake++
> gen6_gt_put: -> --forcewake, pm_put
> 
> device is now asleep
> 
> forcewake_timer: boom, invalid reg access.

You can only do reg I/O if you already have a runtime pm reference. But yeah I've missed that we don't actually call the full machinery on suspend ...

Both approaches have the ugly issue though that the del_timer_sync will leak the forcewake reference. So I think we need to call the timer function if we've actually deactivated it ... fun stuff.
Comment 7 Daniel Vetter 2014-05-19 09:13:33 UTC
Is this still an issue on latest -nightly? Please retest.
Comment 8 Guo Jinxian 2014-05-22 01:38:00 UTC
Created attachment 99548 [details]
dmesg

Booting with HDMI only on latest -nightly(36765340cb068dec1216342bfcdbf2678ec2986). HDMI monitor works well.
Comment 9 Paulo Zanoni 2014-06-30 20:51:51 UTC
(In reply to comment #8)
> Created attachment 99548 [details]
> dmesg
> 
> Booting with HDMI only on latest
> -nightly(36765340cb068dec1216342bfcdbf2678ec2986). HDMI monitor works well.

Closing bug then. If it still happens, please reopen.
Comment 10 Guo Jinxian 2014-07-07 08:33:50 UTC
(In reply to comment #8)
> Created attachment 99548 [details]
> dmesg
> 
> Booting with HDMI only on latest
> -nightly(36765340cb068dec1216342bfcdbf2678ec2986). HDMI monitor works well.
Comment 11 Jari Tahvanainen 2016-10-13 08:31:53 UTC
Closing verified+fixed.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.