Bug 97486

The kernel source code used is from https://code.launchpad.net/~ubuntu-kernel-test/ubuntu/+source/linux/+git/mirror-drm-intel/+ref/drm-intel-nightly, and kernel version shown is 4.8.0-rc3)

The debian binary is available @ http://people.canonical.com/~alexhung/LP1597612/

Created attachment 126045 [details]
Output of lspci

Created attachment 126046 [details]
/proc/cpuinfo

Created attachment 126047 [details]
dmesg

dmesg with the following:
1. kernel parameter with drm.debug=0x6
2. boot, suspend and resume
3. brightness by hotkeys from max to min, and from min to max

From ODM's debug result, the root cause of the issue is PWM signal is changed after suspend and resume in Linux no mater which panel is used. However BOE's panel spec does not support well when PWM signal is 1.6K.
Why PWM signal is changed to 1.6K after suspend and resume?

=================================
In Ubuntu Environment: 
1. When system boot up to the desktop, PWM signal is 220K
2. After system suspend and resume, PWM signal is 1.6K

In WIndows 10 Environment:
1. When system boot up to the desktop, PWM signal is 220K
2. After system suspend and resume, PWM signal is 220K
===================================

A register dump using 'intel_reg dump' before suspend and after resume might be interesting. intel_reg is part of intel-gpu-tools http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/

Created attachment 126114 [details]
intel_reg dump before S3

Created attachment 126115 [details]
intel_reg dump after S3

I tried to increase the value of reg 0xc8254 after S3, it seems help.
The display can become brighter after the brightness value be adjusted above 50%, and after set the reg value up to 0x05f00000, the brightness variation can be seen while changing the brightness value from 0% to 100%

Hi AceLan Kao,
Could you also help to read register 0xc2004 before and after S3?

The value of reg 0xC2004 before and after S3 is 0x01, it doesn't change.

Hi AceLan Kao,
Please also dump register 0xc2000 before and after S3. Thanks!

Before S3, 0xc2000 is 0x01
After S3, 0xc2000 is 0x00
So, after S3 I write back the value, it works.

Hi AceLan Kao,

Could you help to apply the change in below to monitor this symptom still be replicated? Thanks!

---
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_suspend.c |    6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c413587..003cbf2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1086,6 +1086,7 @@ struct i915_suspend_saved_registers {
 	uint64_t saveFENCE[I915_MAX_NUM_FENCES];
 	u32 savePCH_PORT_HOTPLUG;
 	u16 saveGCDGMBUS;
+	u32 savePWM_GRANULARITY;
 };
 
 struct vlv_s0ix_state {
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index a0af170..5bf8b88 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -40,6 +40,9 @@ static void i915_save_display(struct drm_device *dev)
 	/* save FBC interval */
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev))
 		dev_priv->regfile.saveFBC_CONTROL = I915_READ(FBC_CONTROL);
+
+	if (HAS_PCH_SPT(dev_priv))
+		dev_priv->regfile.savePWM_GRANULARITY = I915_READ(SOUTH_CHICKEN1);
 }
 
 static void i915_restore_display(struct drm_device *dev)
@@ -58,6 +61,9 @@ static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(FBC_CONTROL, dev_priv->regfile.saveFBC_CONTROL);
 
 	i915_redisable_vga(dev);
+
+	if (HAS_PCH_SPT(dev_priv))
+		I915_WRITE(SOUTH_CHICKEN1, dev_priv->regfile.savePWM_GRANULARITY);
 }
 
 int i915_save_state(struct drm_device *dev)
--

(In reply to Shawn Lee from comment #14)
> Could you help to apply the change in below to monitor this symptom still be
> replicated? Thanks!

This may be tried as a debug patch, but *no* new register saves/restores should be added to save/restore. Instead, the backlight enable hooks in intel_panel.c should make sure it contains all state that is required.

Hi Shawn Lee,
The patch works for me.

Patch is under review and available here : https://patchwork.freedesktop.org/series/12165/

Any updates of the upstreaming progress of this patch?

Pending another version from Shawn to address review comments.

Regarding a long out-standing bug on Maabook Air 6,2 (2013), there is a similar behavior on back light after S3. [Bug 67454] With the suggested test on Comment 10 to 14 here, I found that instead of SOUTH_CHICKEN1 reg., SOUTH_CHICKEN2 reg. needs to be kept and restored. The output of 'lspci -nns 2' is 

00:02.0 VGA compatible controller [0300]: Intel Corporation Haswell-ULT Integrated Graphics Controller [8086:0a26] (rev 09)

Would the proposed patch be extended and cover the issue on MBA? Thanks!

(In reply to Yuan Chao from comment #20)
> Regarding a long out-standing bug on Maabook Air 6,2 (2013), there is a
> similar behavior on back light after S3. [Bug 67454] With the suggested test
> on Comment 10 to 14 here, I found that instead of SOUTH_CHICKEN1 reg.,
> SOUTH_CHICKEN2 reg. needs to be kept and restored. The output of 'lspci -nns
> 2' is 
> 
> 00:02.0 VGA compatible controller [0300]: Intel Corporation Haswell-ULT
> Integrated Graphics Controller [8086:0a26] (rev 09)
> 
> Would the proposed patch be extended and cover the issue on MBA? Thanks!

It already covered your hardware. Did you actually try?

Anyway, here's two patches addressing the review comments:

https://patchwork.freedesktop.org/patch/111128/
https://patchwork.freedesktop.org/patch/111129/

Please test.

Fixed by commits

commit 32b421e79e6b546da1d469f1229403ac9142d695
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Mon Sep 19 13:35:25 2016 +0300

    drm/i915/backlight: setup and cache pwm alternate increment value

and

commit e29aff05f239f8dd24e9ee7816fd96726e20105a
Author: Shawn Lee <shawn.c.lee@intel.com>
Date:   Mon Sep 19 13:35:26 2016 +0300

    drm/i915/backlight: setup backlight pwm alternate increment on backlight enable

in drm-intel-next-queued.