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

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

[Philipp Gesang]

[-- Attachment #1.1.1: Type: text/plain, Size: 899 bytes --]

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



[-- Attachment #1.1.2: 0001-drm-i915-save-and-restore-BLC_HIST_CTL-during-suspen.patch --]
[-- Type: text/x-diff, Size: 2036 bytes --]

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


[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[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].

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
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Philipp Gesang]

[-- Attachment #1.1: Type: text/plain, Size: 1500 bytes --]

···<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


[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Thu, 25 Jun 2015, Philipp Gesang <phg@phi-gamma.net> wrote:
> 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.

Please try this patch:

From 018c3dab349b9ba82364ea6e7257d1bbad3df2ae Mon Sep 17 00:00:00 2001
From: Jani Nikula <jani.nikula@intel.com>
Date: Thu, 25 Jun 2015 23:55:06 +0300
Subject: [PATCH] drm/i915: fix backlight on gen2
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Cc: Jani Nikula <jani.nikula@intel.com>

Some 855gm models (at least ThinkPad X40) regressed because of

commit b0cd324faed23d10d66ba6ade66579c681feef6f
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

which tried to make our driver more robust by not blindly saving and
restoring registers, but it failed to take into account

commit 0eb96d6ed38430b72897adde58f5477a6b71757a
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Wed Oct 14 12:33:41 2009 -0700

    drm/i915: save/restore BLC histogram control reg across suspend/resume

Fix the regression by enabling hist ctl on gen2.

Reported-by: Philipp Gesang <phg@phi-gamma.net>
References: http://mid.gmane.org/20150623222648.GD12335@acheron
Fixes: b0cd324faed2 ("drm/i915: don't save/restore backlight hist ctl registers")
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    | 1 +
 drivers/gpu/drm/i915/intel_panel.c | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fa9ccb87eb66..bf7c08b94088 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3507,6 +3507,7 @@ enum skl_disp_power_wells {
 #define   BLM_POLARITY_PNV			(1 << 0) /* pnv only */
 
 #define BLC_HIST_CTL	(dev_priv->info.display_mmio_offset + 0x61260)
+#define  BLM_HISTOGRAM_ENABLE			(1 << 31)
 
 /* New registers for PCH-split platforms. Safe where new bits show up, the
  * register layout machtes with gen4 BLC_PWM_CTL[12]. */
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 7d83527f95f7..c4d884d9e223 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -907,6 +907,10 @@ static void i9xx_enable_backlight(struct intel_connector *connector)
 
 	/* XXX: combine this into above write? */
 	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+
+	/* Needed to enable backlight. */
+	if (IS_GEN2(dev))
+		I915_WRITE(BLC_HIST_CTL, BLM_HISTOGRAM_ENABLE);
 }
 
 static void i965_enable_backlight(struct intel_connector *connector)
-- 
2.1.4



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Philipp Gesang]

[-- Attachment #1.1: Type: text/plain, Size: 895 bytes --]

···<date: 2015-06-26, Friday>···<from: Jani Nikula>···

> On Thu, 25 Jun 2015, Philipp Gesang <phg@phi-gamma.net> wrote:
> > 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.
> 
> Please try this patch:
> 
> From 018c3dab349b9ba82364ea6e7257d1bbad3df2ae Mon Sep 17 00:00:00 2001
> From: Jani Nikula <jani.nikula@intel.com>
> Date: Thu, 25 Jun 2015 23:55:06 +0300
> Subject: [PATCH] drm/i915: fix backlight on gen2
> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
> Cc: Jani Nikula <jani.nikula@intel.com>

Tested based on kernel aefbef10e3…. Works great, thanks!

Best,
Philipp



[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx