diff mbox

[bisect] regression in suspend with i915 on 82852/855GM

Message ID 20150623222648.GD12335@acheron (mailing list archive)
State New, archived
Headers show

Commit Message

Philipp Gesang June 23, 2015, 10:26 p.m. UTC 
Hi,

suspend/resume results in the backlight not coming back to life
on my X40 laptop with an “Display controller: Intel Corporation
82852/855GM Integrated Graphics Device”.

I bisected the issue. Apparently, commit
b0cd324faed23d10d66ba6ade66579c681feef6f introduced the problem.

    Author: Jani Nikula <jani.nikula@intel.com>
    Date:   Wed Nov 12 16:25:43 2014 +0200

        drm/i915: don't save/restore backlight hist ctl registers

I can confirm that suspend works as expected with a 4.1 kernel if
the lines to dump/undump the BLC_HIST_CTL register are
reintroduced; see the attached patch. (According to a web search,
the same issue seems to have arisen before in 2009, see [1].)

Let me know if you’d like me to run further tests on that
particular hardware.

Best,
Philipp

[1] http://lists.freedesktop.org/archives/intel-gfx/2009-October/004490.html

Comments

Jani Nikula June 25, 2015, 7:48 a.m. UTC | #1 
On Wed, 24 Jun 2015, Philipp Gesang <phg@phi-gamma.net> wrote:
> Hi,
>
> suspend/resume results in the backlight not coming back to life
> on my X40 laptop with an “Display controller: Intel Corporation
> 82852/855GM Integrated Graphics Device”.
>
> I bisected the issue. Apparently, commit
> b0cd324faed23d10d66ba6ade66579c681feef6f introduced the problem.
>
>     Author: Jani Nikula <jani.nikula@intel.com>
>     Date:   Wed Nov 12 16:25:43 2014 +0200
>
>         drm/i915: don't save/restore backlight hist ctl registers
>
> I can confirm that suspend works as expected with a 4.1 kernel if
> the lines to dump/undump the BLC_HIST_CTL register are
> reintroduced; see the attached patch. (According to a web search,
> the same issue seems to have arisen before in 2009, see [1].)
>
> Let me know if you’d like me to run further tests on that
> particular hardware.

Please run 'intel_reg read 0x61260' after resume both on the patched and
unpatched kernel to see what the hardware wants. The tool is in the
intel-gpu-tools package [1].

BR,
Jani.

[1] http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/

>
> Best,
> Philipp
>
> [1] http://lists.freedesktop.org/archives/intel-gfx/2009-October/004490.html
>
>
> From 2f5f519dd77cdd27f5a88413d8125fb23f6e526f Mon Sep 17 00:00:00 2001
> From: Philipp Gesang <phg@phi-gamma.net>
> Date: Tue, 23 Jun 2015 23:37:28 +0200
> Subject: [PATCH] drm/i915: save and restore BLC_HIST_CTL during suspend/resume
>
> This partially reverts b0cd324fae...
>
> Without considering the BLC_HIST_CTL register the backlight on an IBM
> X40 (855GM) remains dark after suspend/resume. Include its value again
> with the saved registers.
>
> Signed-off-by: Philipp Gesang <phg@phi-gamma.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     | 1 +
>  drivers/gpu/drm/i915/i915_suspend.c | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8ae6f7f..f68e0b5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -925,6 +925,7 @@ struct i915_suspend_saved_registers {
>  	u32 savePP_OFF;
>  	u32 savePP_CONTROL;
>  	u32 savePP_DIVISOR;
> +	u32 saveBLC_HIST_CTL;
>  	u32 saveFBC_CONTROL;
>  	u32 saveCACHE_MODE_0;
>  	u32 saveMI_ARB_STATE;
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index cf67f82..f417744 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -54,6 +54,7 @@ static void i915_save_display(struct drm_device *dev)
>  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
>  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PP_OFF_DELAYS);
>  		dev_priv->regfile.savePP_DIVISOR = I915_READ(PP_DIVISOR);
> +		dev_priv->regfile.saveBLC_HIST_CTL = I915_READ(BLC_HIST_CTL);
>  	}
>  
>  	/* save FBC interval */
> @@ -89,6 +90,7 @@ static void i915_restore_display(struct drm_device *dev)
>  		I915_WRITE(PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
>  		I915_WRITE(PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
>  		I915_WRITE(PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
> +		I915_WRITE(BLC_HIST_CTL, dev_priv->regfile.saveBLC_HIST_CTL);
>  	}
>  
>  	/* only restore FBC info on the platform that supports FBC*/
> -- 
> 2.4.2
>
Philipp Gesang June 25, 2015, 8:38 p.m. UTC | #2 
···<date: 2015-06-25, Thursday>···<from: Jani Nikula>···

> On Wed, 24 Jun 2015, Philipp Gesang <phg@phi-gamma.net> wrote:
> > Hi,
> >
> > suspend/resume results in the backlight not coming back to life
> > on my X40 laptop with an “Display controller: Intel Corporation
> > 82852/855GM Integrated Graphics Device”.
> >
> > I bisected the issue. Apparently, commit
> > b0cd324faed23d10d66ba6ade66579c681feef6f introduced the problem.
> >
> >     Author: Jani Nikula <jani.nikula@intel.com>
> >     Date:   Wed Nov 12 16:25:43 2014 +0200
> >
> >         drm/i915: don't save/restore backlight hist ctl registers
> >
> > I can confirm that suspend works as expected with a 4.1 kernel if
> > the lines to dump/undump the BLC_HIST_CTL register are
> > reintroduced; see the attached patch. (According to a web search,
> > the same issue seems to have arisen before in 2009, see [1].)
> >
> > Let me know if you’d like me to run further tests on that
> > particular hardware.
> 
> Please run 'intel_reg read 0x61260' after resume both on the patched and
> unpatched kernel to see what the hardware wants. The tool is in the
> intel-gpu-tools package [1].

Nice tool. With a vanilla kernel I get a value of 0x80000000
before suspend and 0x00000000 after resume. With the patched
kernel it’s 0x80000000 before and after.

On the vanilla kernel, issuing ``intel_reg write 0x61260 0x80000000`` 
after resume turns the backlight back on.

Best,
Philipp
diff mbox

Patch

From 2f5f519dd77cdd27f5a88413d8125fb23f6e526f Mon Sep 17 00:00:00 2001
From: Philipp Gesang <phg@phi-gamma.net>
Date: Tue, 23 Jun 2015 23:37:28 +0200
Subject: [PATCH] drm/i915: save and restore BLC_HIST_CTL during suspend/resume

This partially reverts b0cd324fae...

Without considering the BLC_HIST_CTL register the backlight on an IBM
X40 (855GM) remains dark after suspend/resume. Include its value again
with the saved registers.

Signed-off-by: Philipp Gesang <phg@phi-gamma.net>
---
 drivers/gpu/drm/i915/i915_drv.h     | 1 +
 drivers/gpu/drm/i915/i915_suspend.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8ae6f7f..f68e0b5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -925,6 +925,7 @@  struct i915_suspend_saved_registers {
 	u32 savePP_OFF;
 	u32 savePP_CONTROL;
 	u32 savePP_DIVISOR;
+	u32 saveBLC_HIST_CTL;
 	u32 saveFBC_CONTROL;
 	u32 saveCACHE_MODE_0;
 	u32 saveMI_ARB_STATE;
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index cf67f82..f417744 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -54,6 +54,7 @@  static void i915_save_display(struct drm_device *dev)
 		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
 		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PP_OFF_DELAYS);
 		dev_priv->regfile.savePP_DIVISOR = I915_READ(PP_DIVISOR);
+		dev_priv->regfile.saveBLC_HIST_CTL = I915_READ(BLC_HIST_CTL);
 	}
 
 	/* save FBC interval */
@@ -89,6 +90,7 @@  static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
 		I915_WRITE(PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
 		I915_WRITE(PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
+		I915_WRITE(BLC_HIST_CTL, dev_priv->regfile.saveBLC_HIST_CTL);
 	}
 
 	/* only restore FBC info on the platform that supports FBC*/
-- 
2.4.2