i915 regression on 3.6-rc1: lid blanks screen

i915 regression on 3.6-rc1: lid blanks screen

[Hugh Dickins]

Sorry to report that with 3.6-rc1, closing and opening the lid on
this ThinkPad T420s leaves the screen blank, and I have to reboot.

I understand there's also an nVidia graphics device in here,
but I have that configured out, preferring to use the i915:

00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09) (prog-if 00 [VGA controller])
	Subsystem: Lenovo Device 21d3
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 41
	Region 0: Memory at f1400000 (64-bit, non-prefetchable) [size=4M]
	Region 2: Memory at e0000000 (64-bit, prefetchable) [size=256M]
	Region 4: I/O ports at 5000 [size=64]
	Expansion ROM at <unassigned> [disabled]
	Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit-
		Address: fee0100c  Data: 41c2
	Capabilities: [d0] Power Management version 2
		Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [a4] PCI Advanced Features
		AFCap: TP+ FLR+
		AFCtrl: FLR-
		AFStatus: TP-
	Kernel driver in use: i915

Bisection led to this commit, and reverting indeed gets my screen back:

commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Wed Jul 11 16:27:52 2012 +0200

    drm/i915/lvds: ditch ->prepare special case
    
    LVDS is the first output where dpms on/off and prepare/commit don't
    perfectly match. Now the idea behind this special case seems to be
    that for simple resolution changes on the LVDS we don't need to stop
    the pipe, because (at least on newer chips) we can adjust the panel
    fitter on the fly.
    
    There are a few problems with the current code though:
    - We still stop and restart the pipe unconditionally, because the crtc
      helper code isn't flexible enough.
    - We show some ugly flickering, especially when changing crtcs (this
      the crtc helper would actually take into account, but we don't
      implement the encoder->get_crtc callback required to make this work
      properly).
    
    So it doesn't even work as advertised. I agree that it would be nice
    to do resolution changes on LVDS (and also eDP) whithout blacking the
    screen where the panel fitter allows to do that. But imo we should
    implement this as a special case a few layers up in the mode set code,
    akin to how we already detect simple framebuffer changes (and only
    update the required registers with ->mode_set_base).
    
    Until this is all in place, make our lives easier and just rip it out.
    
    Also note that this seems to fix actual bugs with enabling the lvds
    output, see:
    
    http://lists.freedesktop.org/archives/intel-gfx/2012-July/018614.html
    
    Cc: Takashi Iwai <tiwai@suse.de>
    Cc: Giacomo Comes <comes@naic.edu>
    Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
    Tested-by: Takashi Iwai <tiwai@suse.de>
    Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 49f09a8..e05c0d3 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -409,13 +409,7 @@ static void intel_lvds_prepare(struct drm_encoder *encoder)
 {
 	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
 
-	/*
-	 * Prior to Ironlake, we must disable the pipe if we want to adjust
-	 * the panel fitter. However at all other times we can just reset
-	 * the registers regardless.
-	 */
-	if (!HAS_PCH_SPLIT(encoder->dev) && intel_lvds->pfit_dirty)
-		intel_lvds_disable(intel_lvds);
+	intel_lvds_disable(intel_lvds);
 }
 
 static void intel_lvds_commit(struct drm_encoder *encoder)

Re: i915 regression on 3.6-rc1: lid blanks screen

[Takashi Iwai]

At Sat, 4 Aug 2012 10:01:13 -0700 (PDT),
Hugh Dickins wrote:
> 
> Sorry to report that with 3.6-rc1, closing and opening the lid on
> this ThinkPad T420s leaves the screen blank, and I have to reboot.
> 
> I understand there's also an nVidia graphics device in here,
> but I have that configured out, preferring to use the i915:
> 
> 00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09) (prog-if 00 [VGA controller])
> 	Subsystem: Lenovo Device 21d3
> 	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
> 	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> 	Latency: 0
> 	Interrupt: pin A routed to IRQ 41
> 	Region 0: Memory at f1400000 (64-bit, non-prefetchable) [size=4M]
> 	Region 2: Memory at e0000000 (64-bit, prefetchable) [size=256M]
> 	Region 4: I/O ports at 5000 [size=64]
> 	Expansion ROM at <unassigned> [disabled]
> 	Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit-
> 		Address: fee0100c  Data: 41c2
> 	Capabilities: [d0] Power Management version 2
> 		Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
> 		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
> 	Capabilities: [a4] PCI Advanced Features
> 		AFCap: TP+ FLR+
> 		AFCtrl: FLR-
> 		AFStatus: TP-
> 	Kernel driver in use: i915
> 
> Bisection led to this commit, and reverting indeed gets my screen back:
> 
> commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Jul 11 16:27:52 2012 +0200
> 
>     drm/i915/lvds: ditch ->prepare special case
>     
>     LVDS is the first output where dpms on/off and prepare/commit don't
>     perfectly match. Now the idea behind this special case seems to be
>     that for simple resolution changes on the LVDS we don't need to stop
>     the pipe, because (at least on newer chips) we can adjust the panel
>     fitter on the fly.
>     
>     There are a few problems with the current code though:
>     - We still stop and restart the pipe unconditionally, because the crtc
>       helper code isn't flexible enough.
>     - We show some ugly flickering, especially when changing crtcs (this
>       the crtc helper would actually take into account, but we don't
>       implement the encoder->get_crtc callback required to make this work
>       properly).
>     
>     So it doesn't even work as advertised. I agree that it would be nice
>     to do resolution changes on LVDS (and also eDP) whithout blacking the
>     screen where the panel fitter allows to do that. But imo we should
>     implement this as a special case a few layers up in the mode set code,
>     akin to how we already detect simple framebuffer changes (and only
>     update the required registers with ->mode_set_base).
>     
>     Until this is all in place, make our lives easier and just rip it out.
>     
>     Also note that this seems to fix actual bugs with enabling the lvds
>     output, see:
>     
>     http://lists.freedesktop.org/archives/intel-gfx/2012-July/018614.html
>     
>     Cc: Takashi Iwai <tiwai@suse.de>
>     Cc: Giacomo Comes <comes@naic.edu>
>     Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
>     Tested-by: Takashi Iwai <tiwai@suse.de>
>     Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Hm, it's surprising.

Could you check whether the counter-part intel_lvds_enable() is
called?  If the prepare callback affects, it must be from the mode
setting (drm_crtc_helper_set_mode()).


Takashi

> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 49f09a8..e05c0d3 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -409,13 +409,7 @@ static void intel_lvds_prepare(struct drm_encoder *encoder)
>  {
>  	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
>  
> -	/*
> -	 * Prior to Ironlake, we must disable the pipe if we want to adjust
> -	 * the panel fitter. However at all other times we can just reset
> -	 * the registers regardless.
> -	 */
> -	if (!HAS_PCH_SPLIT(encoder->dev) && intel_lvds->pfit_dirty)
> -		intel_lvds_disable(intel_lvds);
> +	intel_lvds_disable(intel_lvds);
>  }
>  
>  static void intel_lvds_commit(struct drm_encoder *encoder)
> 

Re: i915 regression on 3.6-rc1: lid blanks screen

[Hugh Dickins]

On Sun, 5 Aug 2012, Takashi Iwai wrote:
> At Sat, 4 Aug 2012 10:01:13 -0700 (PDT),
> Hugh Dickins wrote:
> > 
> > Sorry to report that with 3.6-rc1, closing and opening the lid on
> > this ThinkPad T420s leaves the screen blank, and I have to reboot.
> > 
> > I understand there's also an nVidia graphics device in here,
> > but I have that configured out, preferring to use the i915:
> > 
> > 00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09) (prog-if 00 [VGA controller])
> > 	Subsystem: Lenovo Device 21d3
> > 	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
> > 	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> > 	Latency: 0
> > 	Interrupt: pin A routed to IRQ 41
> > 	Region 0: Memory at f1400000 (64-bit, non-prefetchable) [size=4M]
> > 	Region 2: Memory at e0000000 (64-bit, prefetchable) [size=256M]
> > 	Region 4: I/O ports at 5000 [size=64]
> > 	Expansion ROM at <unassigned> [disabled]
> > 	Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit-
> > 		Address: fee0100c  Data: 41c2
> > 	Capabilities: [d0] Power Management version 2
> > 		Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
> > 		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
> > 	Capabilities: [a4] PCI Advanced Features
> > 		AFCap: TP+ FLR+
> > 		AFCtrl: FLR-
> > 		AFStatus: TP-
> > 	Kernel driver in use: i915
> > 
> > Bisection led to this commit, and reverting indeed gets my screen back:
> > 
> > commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Date:   Wed Jul 11 16:27:52 2012 +0200
> > 
> >     drm/i915/lvds: ditch ->prepare special case
...
> 
> Hm, it's surprising.
> 
> Could you check whether the counter-part intel_lvds_enable() is
> called?  If the prepare callback affects, it must be from the mode
> setting (drm_crtc_helper_set_mode()).

Yes, I put a dump_stack() in both, and intel_lvds_enable() gets called
about 0.28 seconds after the intel_lvds_disable() when I lift the lid;
but with no video display until I revert that commit.

Hugh

> 
> 
> Takashi
> 
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index 49f09a8..e05c0d3 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -409,13 +409,7 @@ static void intel_lvds_prepare(struct drm_encoder *encoder)
> >  {
> >  	struct intel_lvds *intel_lvds = to_intel_lvds(encoder);
> >  
> > -	/*
> > -	 * Prior to Ironlake, we must disable the pipe if we want to adjust
> > -	 * the panel fitter. However at all other times we can just reset
> > -	 * the registers regardless.
> > -	 */
> > -	if (!HAS_PCH_SPLIT(encoder->dev) && intel_lvds->pfit_dirty)
> > -		intel_lvds_disable(intel_lvds);
> > +	intel_lvds_disable(intel_lvds);
> >  }
> >  
> >  static void intel_lvds_commit(struct drm_encoder *encoder)

Re: i915 regression on 3.6-rc1: lid blanks screen

[Daniel Vetter]

On Mon, Aug 6, 2012 at 6:21 AM, Hugh Dickins <hughd@google.com> wrote:
> On Sun, 5 Aug 2012, Takashi Iwai wrote:
>> At Sat, 4 Aug 2012 10:01:13 -0700 (PDT),
>> Hugh Dickins wrote:
>> >
>> > Sorry to report that with 3.6-rc1, closing and opening the lid on
>> > this ThinkPad T420s leaves the screen blank, and I have to reboot.
>> >
>> > I understand there's also an nVidia graphics device in here,
>> > but I have that configured out, preferring to use the i915:
>> >
>> > 00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller (rev 09) (prog-if 00 [VGA controller])
>> >     Subsystem: Lenovo Device 21d3
>> >     Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
>> >     Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>> >     Latency: 0
>> >     Interrupt: pin A routed to IRQ 41
>> >     Region 0: Memory at f1400000 (64-bit, non-prefetchable) [size=4M]
>> >     Region 2: Memory at e0000000 (64-bit, prefetchable) [size=256M]
>> >     Region 4: I/O ports at 5000 [size=64]
>> >     Expansion ROM at <unassigned> [disabled]
>> >     Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit-
>> >             Address: fee0100c  Data: 41c2
>> >     Capabilities: [d0] Power Management version 2
>> >             Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
>> >             Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
>> >     Capabilities: [a4] PCI Advanced Features
>> >             AFCap: TP+ FLR+
>> >             AFCtrl: FLR-
>> >             AFStatus: TP-
>> >     Kernel driver in use: i915
>> >
>> > Bisection led to this commit, and reverting indeed gets my screen back:
>> >
>> > commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
>> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> > Date:   Wed Jul 11 16:27:52 2012 +0200
>> >
>> >     drm/i915/lvds: ditch ->prepare special case
> ...
>>
>> Hm, it's surprising.
>>
>> Could you check whether the counter-part intel_lvds_enable() is
>> called?  If the prepare callback affects, it must be from the mode
>> setting (drm_crtc_helper_set_mode()).
>
> Yes, I put a dump_stack() in both, and intel_lvds_enable() gets called
> about 0.28 seconds after the intel_lvds_disable() when I lift the lid;
> but with no video display until I revert that commit.

Can you please boot with drm.debug=0xe added to your kernel cmdline,
reproduce the issue (with the two dump_stack calls added) and then
attach the full dmesg?

Also a few other things to try: What happens if you do a modeset on
the LVDS while it's still working, e.g.

xrandr --outpu LVDS1 --auto --crtc 1

then switch back to crtc 0 with

xrandr --outpu LVDS1 --auto --crtc 0

Would also be interesting to know whether this can resurrect your machine.

Also, how blank is the screen? I.e. is only the backlight off, but you
can (dimly) see some screen contents, or is it completely off?

Thanks, Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: i915 regression on 3.6-rc1: lid blanks screen

[Hugh Dickins]

On Mon, 6 Aug 2012, Daniel Vetter wrote:
> On Mon, Aug 6, 2012 at 6:21 AM, Hugh Dickins <hughd@google.com> wrote:
> > On Sun, 5 Aug 2012, Takashi Iwai wrote:
> >> At Sat, 4 Aug 2012 10:01:13 -0700 (PDT),
> >> Hugh Dickins wrote:
> >> >
> >> > Sorry to report that with 3.6-rc1, closing and opening the lid on
> >> > this ThinkPad T420s leaves the screen blank, and I have to reboot.
> >> >
> >> > Bisection led to this commit, and reverting indeed gets my screen back:
> >> >
> >> > commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
> >> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> > Date:   Wed Jul 11 16:27:52 2012 +0200
> >> >
> >> >     drm/i915/lvds: ditch ->prepare special case
> > ...
> >>
> >> Hm, it's surprising.
> >>
> >> Could you check whether the counter-part intel_lvds_enable() is
> >> called?  If the prepare callback affects, it must be from the mode
> >> setting (drm_crtc_helper_set_mode()).
> >
> > Yes, I put a dump_stack() in both, and intel_lvds_enable() gets called
> > about 0.28 seconds after the intel_lvds_disable() when I lift the lid;
> > but with no video display until I revert that commit.
> 
> Can you please boot with drm.debug=0xe added to your kernel cmdline,
> reproduce the issue (with the two dump_stack calls added) and then
> attach the full dmesg?

Collected, I'll send it to you both privately in a moment.

> 
> Also a few other things to try: What happens if you do a modeset on
> the LVDS while it's still working, e.g.

In the dmesg, I've only gone to runlevel 3, simply working on the
console without startx.  For these xrandrs to work, I did startx
and used the graphics screen.

> 
> xrandr --outpu LVDS1 --auto --crtc 1

Blanks and restores the screen.

> 
> then switch back to crtc 0 with
> 
> xrandr --outpu LVDS1 --auto --crtc 0

Blanks and restores the screen.

> 
> Would also be interesting to know whether this can resurrect your machine.

Indeed it does: the first (--crtc 1) restores the display from
its blank state after opening the lid, the second (--crtc 0) then
behaves as before, briefly blanking then restoring the display.

> 
> Also, how blank is the screen? I.e. is only the backlight off, but you
> can (dimly) see some screen contents, or is it completely off?

Completely off.

Thanks,
Hugh

Re: i915 regression on 3.6-rc1: lid blanks screen

[Takashi Iwai]

At Mon, 6 Aug 2012 11:25:30 -0700 (PDT),
Hugh Dickins wrote:
> 
> On Mon, 6 Aug 2012, Daniel Vetter wrote:
> > On Mon, Aug 6, 2012 at 6:21 AM, Hugh Dickins <hughd@google.com> wrote:
> > > On Sun, 5 Aug 2012, Takashi Iwai wrote:
> > >> At Sat, 4 Aug 2012 10:01:13 -0700 (PDT),
> > >> Hugh Dickins wrote:
> > >> >
> > >> > Sorry to report that with 3.6-rc1, closing and opening the lid on
> > >> > this ThinkPad T420s leaves the screen blank, and I have to reboot.
> > >> >
> > >> > Bisection led to this commit, and reverting indeed gets my screen back:
> > >> >
> > >> > commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
> > >> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >> > Date:   Wed Jul 11 16:27:52 2012 +0200
> > >> >
> > >> >     drm/i915/lvds: ditch ->prepare special case
> > > ...
> > >>
> > >> Hm, it's surprising.
> > >>
> > >> Could you check whether the counter-part intel_lvds_enable() is
> > >> called?  If the prepare callback affects, it must be from the mode
> > >> setting (drm_crtc_helper_set_mode()).
> > >
> > > Yes, I put a dump_stack() in both, and intel_lvds_enable() gets called
> > > about 0.28 seconds after the intel_lvds_disable() when I lift the lid;
> > > but with no video display until I revert that commit.
> > 
> > Can you please boot with drm.debug=0xe added to your kernel cmdline,
> > reproduce the issue (with the two dump_stack calls added) and then
> > attach the full dmesg?
> 
> Collected, I'll send it to you both privately in a moment.
> 
> > 
> > Also a few other things to try: What happens if you do a modeset on
> > the LVDS while it's still working, e.g.
> 
> In the dmesg, I've only gone to runlevel 3, simply working on the
> console without startx.  For these xrandrs to work, I did startx
> and used the graphics screen.

OK, now I can see the problem here, too.  The key is that it happens
only on Linux console, not on X.  That's why no one else reported.
I guess the problem can be seen on many laptops with LVDS on PCH.

Looking at intel_reg_dumper output, BLC_PWM_CPU_CTL is 0 while other
registers are set correctly.  This seems coming from the rewrite of 
backlight control code by commit
  24ded204: drm/i915: properly enable the blc controller on the right pipe
and
  a4f32fc3: drm/i915: don't forget the PCH backlight registers
    
While the latter fixes the regression by the former commit, it still
doesn't cover this regression.

I don't know the exact hardware behavior, but it looks like that
resetting BLC_PWM_PCH_CTL2 and BLC_PWM_PCH_CTL1 clears the
BLC_PWM_CPU_CTL (oh what confusing reg names).

FWIW, the commit 520c41cf you mentioned is no direct cause.  This
patch works fine on the top of 3.5 kernel.  But it's like a bad drug,
the combination of this and other two commits break things.

The patch below is my quick fix.  It worked on an HP laptop.
Hugh, could you give it a try?


thanks,

Takashi

===
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] drm/i915: Fix blank panel at reopening lid on Linux console

When you reopen the lid on Linux console on a laptop with PCH, the
panel suddenly goes blank.  It seems because BLC_PWM_CPU_CTL register
is cleared when BLC_PWM_PCH_CTL1 and BLC_PWM_PCH_CTL2 registers are
played.

This patch fixes the problem by setting BLC_PWM_CPU_CTL after enabling
BLC_PWM_PCH_CTL_1 and _2 registers.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/i915/intel_panel.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 10c7d39..d640425 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -311,9 +311,6 @@ void intel_panel_enable_backlight(struct drm_device *dev,
 	if (dev_priv->backlight_level == 0)
 		dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
 
-	dev_priv->backlight_enabled = true;
-	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
-
 	if (INTEL_INFO(dev)->gen >= 4) {
 		uint32_t reg, tmp;
 
@@ -326,7 +323,7 @@ void intel_panel_enable_backlight(struct drm_device *dev,
 		 * we don't track the backlight dpms state, hence check whether
 		 * we have to do anything first. */
 		if (tmp & BLM_PWM_ENABLE)
-			return;
+			goto set_level;
 
 		if (dev_priv->num_pipe == 3)
 			tmp &= ~BLM_PIPE_SELECT_IVB;
@@ -347,6 +344,10 @@ void intel_panel_enable_backlight(struct drm_device *dev,
 			I915_WRITE(BLC_PWM_PCH_CTL1, tmp);
 		}
 	}
+
+set_level:
+	dev_priv->backlight_enabled = true;
+	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
 }
 
 static void intel_panel_init_backlight(struct drm_device *dev)
-- 
1.7.11.4

Re: i915 regression on 3.6-rc1: lid blanks screen

[Daniel Vetter]

On Fri, Aug 10, 2012 at 1:59 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Mon, 6 Aug 2012 11:25:30 -0700 (PDT),
> Hugh Dickins wrote:
>>
>> On Mon, 6 Aug 2012, Daniel Vetter wrote:
>> > On Mon, Aug 6, 2012 at 6:21 AM, Hugh Dickins <hughd@google.com> wrote:
>> > > On Sun, 5 Aug 2012, Takashi Iwai wrote:
>> > >> At Sat, 4 Aug 2012 10:01:13 -0700 (PDT),
>> > >> Hugh Dickins wrote:
>> > >> >
>> > >> > Sorry to report that with 3.6-rc1, closing and opening the lid on
>> > >> > this ThinkPad T420s leaves the screen blank, and I have to reboot.
>> > >> >
>> > >> > Bisection led to this commit, and reverting indeed gets my screen back:
>> > >> >
>> > >> > commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
>> > >> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> > >> > Date:   Wed Jul 11 16:27:52 2012 +0200
>> > >> >
>> > >> >     drm/i915/lvds: ditch ->prepare special case
>> > > ...
>> > >>
>> > >> Hm, it's surprising.
>> > >>
>> > >> Could you check whether the counter-part intel_lvds_enable() is
>> > >> called?  If the prepare callback affects, it must be from the mode
>> > >> setting (drm_crtc_helper_set_mode()).
>> > >
>> > > Yes, I put a dump_stack() in both, and intel_lvds_enable() gets called
>> > > about 0.28 seconds after the intel_lvds_disable() when I lift the lid;
>> > > but with no video display until I revert that commit.
>> >
>> > Can you please boot with drm.debug=0xe added to your kernel cmdline,
>> > reproduce the issue (with the two dump_stack calls added) and then
>> > attach the full dmesg?
>>
>> Collected, I'll send it to you both privately in a moment.
>>
>> >
>> > Also a few other things to try: What happens if you do a modeset on
>> > the LVDS while it's still working, e.g.
>>
>> In the dmesg, I've only gone to runlevel 3, simply working on the
>> console without startx.  For these xrandrs to work, I did startx
>> and used the graphics screen.
>
> OK, now I can see the problem here, too.  The key is that it happens
> only on Linux console, not on X.  That's why no one else reported.
> I guess the problem can be seen on many laptops with LVDS on PCH.
>
> Looking at intel_reg_dumper output, BLC_PWM_CPU_CTL is 0 while other
> registers are set correctly.  This seems coming from the rewrite of
> backlight control code by commit
>   24ded204: drm/i915: properly enable the blc controller on the right pipe
> and
>   a4f32fc3: drm/i915: don't forget the PCH backlight registers
>
> While the latter fixes the regression by the former commit, it still
> doesn't cover this regression.
>
> I don't know the exact hardware behavior, but it looks like that
> resetting BLC_PWM_PCH_CTL2 and BLC_PWM_PCH_CTL1 clears the
> BLC_PWM_CPU_CTL (oh what confusing reg names).
>
> FWIW, the commit 520c41cf you mentioned is no direct cause.  This
> patch works fine on the top of 3.5 kernel.  But it's like a bad drug,
> the combination of this and other two commits break things.
>
> The patch below is my quick fix.  It worked on an HP laptop.
> Hugh, could you give it a try?

Hm, this sounds eerily familiar to the backlight bug you've recently fixed in

commit 6db65cbb941f9d433659bdad02b307f6d94465df
Author: Takashi Iwai <tiwai@suse.de>
Date:   Thu Jun 21 15:30:41 2012 +0200

    drm/i915: Fix eDP blank screen after S3 resume on HP desktops

Have you checked other code-paths for such issues? The resume code
seems to follow this order already ... also, when you submit this
patch, can you please add a small comment to explain the ordering
constraint, like in the resume register restore function?

Thanks, Daniel

>
>
> thanks,
>
> Takashi
>
> ===
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] drm/i915: Fix blank panel at reopening lid on Linux console
>
> When you reopen the lid on Linux console on a laptop with PCH, the
> panel suddenly goes blank.  It seems because BLC_PWM_CPU_CTL register
> is cleared when BLC_PWM_PCH_CTL1 and BLC_PWM_PCH_CTL2 registers are
> played.
>
> This patch fixes the problem by setting BLC_PWM_CPU_CTL after enabling
> BLC_PWM_PCH_CTL_1 and _2 registers.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 10c7d39..d640425 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -311,9 +311,6 @@ void intel_panel_enable_backlight(struct drm_device *dev,
>         if (dev_priv->backlight_level == 0)
>                 dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
>
> -       dev_priv->backlight_enabled = true;
> -       intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
> -
>         if (INTEL_INFO(dev)->gen >= 4) {
>                 uint32_t reg, tmp;
>
> @@ -326,7 +323,7 @@ void intel_panel_enable_backlight(struct drm_device *dev,
>                  * we don't track the backlight dpms state, hence check whether
>                  * we have to do anything first. */
>                 if (tmp & BLM_PWM_ENABLE)
> -                       return;
> +                       goto set_level;
>
>                 if (dev_priv->num_pipe == 3)
>                         tmp &= ~BLM_PIPE_SELECT_IVB;
> @@ -347,6 +344,10 @@ void intel_panel_enable_backlight(struct drm_device *dev,
>                         I915_WRITE(BLC_PWM_PCH_CTL1, tmp);
>                 }
>         }
> +
> +set_level:
> +       dev_priv->backlight_enabled = true;
> +       intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
>  }
>
>  static void intel_panel_init_backlight(struct drm_device *dev)
> --
> 1.7.11.4
>



-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: i915 regression on 3.6-rc1: lid blanks screen

[Takashi Iwai]

At Fri, 10 Aug 2012 14:35:13 +0200,
Daniel Vetter wrote:
> 
> On Fri, Aug 10, 2012 at 1:59 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > At Mon, 6 Aug 2012 11:25:30 -0700 (PDT),
> > Hugh Dickins wrote:
> >>
> >> On Mon, 6 Aug 2012, Daniel Vetter wrote:
> >> > On Mon, Aug 6, 2012 at 6:21 AM, Hugh Dickins <hughd@google.com> wrote:
> >> > > On Sun, 5 Aug 2012, Takashi Iwai wrote:
> >> > >> At Sat, 4 Aug 2012 10:01:13 -0700 (PDT),
> >> > >> Hugh Dickins wrote:
> >> > >> >
> >> > >> > Sorry to report that with 3.6-rc1, closing and opening the lid on
> >> > >> > this ThinkPad T420s leaves the screen blank, and I have to reboot.
> >> > >> >
> >> > >> > Bisection led to this commit, and reverting indeed gets my screen back:
> >> > >> >
> >> > >> > commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
> >> > >> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> > >> > Date:   Wed Jul 11 16:27:52 2012 +0200
> >> > >> >
> >> > >> >     drm/i915/lvds: ditch ->prepare special case
> >> > > ...
> >> > >>
> >> > >> Hm, it's surprising.
> >> > >>
> >> > >> Could you check whether the counter-part intel_lvds_enable() is
> >> > >> called?  If the prepare callback affects, it must be from the mode
> >> > >> setting (drm_crtc_helper_set_mode()).
> >> > >
> >> > > Yes, I put a dump_stack() in both, and intel_lvds_enable() gets called
> >> > > about 0.28 seconds after the intel_lvds_disable() when I lift the lid;
> >> > > but with no video display until I revert that commit.
> >> >
> >> > Can you please boot with drm.debug=0xe added to your kernel cmdline,
> >> > reproduce the issue (with the two dump_stack calls added) and then
> >> > attach the full dmesg?
> >>
> >> Collected, I'll send it to you both privately in a moment.
> >>
> >> >
> >> > Also a few other things to try: What happens if you do a modeset on
> >> > the LVDS while it's still working, e.g.
> >>
> >> In the dmesg, I've only gone to runlevel 3, simply working on the
> >> console without startx.  For these xrandrs to work, I did startx
> >> and used the graphics screen.
> >
> > OK, now I can see the problem here, too.  The key is that it happens
> > only on Linux console, not on X.  That's why no one else reported.
> > I guess the problem can be seen on many laptops with LVDS on PCH.
> >
> > Looking at intel_reg_dumper output, BLC_PWM_CPU_CTL is 0 while other
> > registers are set correctly.  This seems coming from the rewrite of
> > backlight control code by commit
> >   24ded204: drm/i915: properly enable the blc controller on the right pipe
> > and
> >   a4f32fc3: drm/i915: don't forget the PCH backlight registers
> >
> > While the latter fixes the regression by the former commit, it still
> > doesn't cover this regression.
> >
> > I don't know the exact hardware behavior, but it looks like that
> > resetting BLC_PWM_PCH_CTL2 and BLC_PWM_PCH_CTL1 clears the
> > BLC_PWM_CPU_CTL (oh what confusing reg names).
> >
> > FWIW, the commit 520c41cf you mentioned is no direct cause.  This
> > patch works fine on the top of 3.5 kernel.  But it's like a bad drug,
> > the combination of this and other two commits break things.
> >
> > The patch below is my quick fix.  It worked on an HP laptop.
> > Hugh, could you give it a try?
> 
> Hm, this sounds eerily familiar to the backlight bug you've recently fixed in
> 
> commit 6db65cbb941f9d433659bdad02b307f6d94465df
> Author: Takashi Iwai <tiwai@suse.de>
> Date:   Thu Jun 21 15:30:41 2012 +0200
> 
>     drm/i915: Fix eDP blank screen after S3 resume on HP desktops
> 
> Have you checked other code-paths for such issues?

Not yet, I'd leave such a joy rather to you guys :)

> The resume code
> seems to follow this order already ... also, when you submit this
> patch, can you please add a small comment to explain the ordering
> constraint, like in the resume register restore function?

Sure, I'll add a comment and resubmit once when I hear it really fixes
on Hugh's machine, too.


thanks,

Takashi

> 
> Thanks, Daniel
> 
> >
> >
> > thanks,
> >
> > Takashi
> >
> > ===
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] drm/i915: Fix blank panel at reopening lid on Linux console
> >
> > When you reopen the lid on Linux console on a laptop with PCH, the
> > panel suddenly goes blank.  It seems because BLC_PWM_CPU_CTL register
> > is cleared when BLC_PWM_PCH_CTL1 and BLC_PWM_PCH_CTL2 registers are
> > played.
> >
> > This patch fixes the problem by setting BLC_PWM_CPU_CTL after enabling
> > BLC_PWM_PCH_CTL_1 and _2 registers.
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/gpu/drm/i915/intel_panel.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 10c7d39..d640425 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -311,9 +311,6 @@ void intel_panel_enable_backlight(struct drm_device *dev,
> >         if (dev_priv->backlight_level == 0)
> >                 dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
> >
> > -       dev_priv->backlight_enabled = true;
> > -       intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
> > -
> >         if (INTEL_INFO(dev)->gen >= 4) {
> >                 uint32_t reg, tmp;
> >
> > @@ -326,7 +323,7 @@ void intel_panel_enable_backlight(struct drm_device *dev,
> >                  * we don't track the backlight dpms state, hence check whether
> >                  * we have to do anything first. */
> >                 if (tmp & BLM_PWM_ENABLE)
> > -                       return;
> > +                       goto set_level;
> >
> >                 if (dev_priv->num_pipe == 3)
> >                         tmp &= ~BLM_PIPE_SELECT_IVB;
> > @@ -347,6 +344,10 @@ void intel_panel_enable_backlight(struct drm_device *dev,
> >                         I915_WRITE(BLC_PWM_PCH_CTL1, tmp);
> >                 }
> >         }
> > +
> > +set_level:
> > +       dev_priv->backlight_enabled = true;
> > +       intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
> >  }
> >
> >  static void intel_panel_init_backlight(struct drm_device *dev)
> > --
> > 1.7.11.4
> >
> 
> 
> 
> -- 
> Daniel Vetter
> daniel.vetter@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 

Re: i915 regression on 3.6-rc1: lid blanks screen

[Hugh Dickins]

On Fri, 10 Aug 2012, Takashi Iwai wrote:
> At Fri, 10 Aug 2012 14:35:13 +0200,
> Daniel Vetter wrote:
> > 
> > On Fri, Aug 10, 2012 at 1:59 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > > At Mon, 6 Aug 2012 11:25:30 -0700 (PDT),
> > > Hugh Dickins wrote:
> > >>
> > >> On Mon, 6 Aug 2012, Daniel Vetter wrote:
> > >> > On Mon, Aug 6, 2012 at 6:21 AM, Hugh Dickins <hughd@google.com> wrote:
> > >> > > On Sun, 5 Aug 2012, Takashi Iwai wrote:
> > >> > >> At Sat, 4 Aug 2012 10:01:13 -0700 (PDT),
> > >> > >> Hugh Dickins wrote:
> > >> > >> >
> > >> > >> > Sorry to report that with 3.6-rc1, closing and opening the lid on
> > >> > >> > this ThinkPad T420s leaves the screen blank, and I have to reboot.
> > >> > >> >
> > >> > >> > Bisection led to this commit, and reverting indeed gets my screen back:
> > >> > >> >
> > >> > >> > commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
> > >> > >> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >> > >> > Date:   Wed Jul 11 16:27:52 2012 +0200
> > >> > >> >
> > >> > >> >     drm/i915/lvds: ditch ->prepare special case
> > >> > > ...
> > >> > >>
> > >> > >> Hm, it's surprising.
> > >> > >>
> > >> > >> Could you check whether the counter-part intel_lvds_enable() is
> > >> > >> called?  If the prepare callback affects, it must be from the mode
> > >> > >> setting (drm_crtc_helper_set_mode()).
> > >> > >
> > >> > > Yes, I put a dump_stack() in both, and intel_lvds_enable() gets called
> > >> > > about 0.28 seconds after the intel_lvds_disable() when I lift the lid;
> > >> > > but with no video display until I revert that commit.
> > >> >
> > >> > Can you please boot with drm.debug=0xe added to your kernel cmdline,
> > >> > reproduce the issue (with the two dump_stack calls added) and then
> > >> > attach the full dmesg?
> > >>
> > >> Collected, I'll send it to you both privately in a moment.
> > >>
> > >> >
> > >> > Also a few other things to try: What happens if you do a modeset on
> > >> > the LVDS while it's still working, e.g.
> > >>
> > >> In the dmesg, I've only gone to runlevel 3, simply working on the
> > >> console without startx.  For these xrandrs to work, I did startx
> > >> and used the graphics screen.
> > >
> > > OK, now I can see the problem here, too.  The key is that it happens
> > > only on Linux console, not on X.  That's why no one else reported.
> > > I guess the problem can be seen on many laptops with LVDS on PCH.

A correction there: for me it was happening both on X and on console;
but once I found that it happened even on the simple console, I mostly
stuck to bisecting and testing on that.

> > >
> > > Looking at intel_reg_dumper output, BLC_PWM_CPU_CTL is 0 while other
> > > registers are set correctly.  This seems coming from the rewrite of
> > > backlight control code by commit
> > >   24ded204: drm/i915: properly enable the blc controller on the right pipe
> > > and
> > >   a4f32fc3: drm/i915: don't forget the PCH backlight registers
> > >
> > > While the latter fixes the regression by the former commit, it still
> > > doesn't cover this regression.
> > >
> > > I don't know the exact hardware behavior, but it looks like that
> > > resetting BLC_PWM_PCH_CTL2 and BLC_PWM_PCH_CTL1 clears the
> > > BLC_PWM_CPU_CTL (oh what confusing reg names).
> > >
> > > FWIW, the commit 520c41cf you mentioned is no direct cause.  This
> > > patch works fine on the top of 3.5 kernel.  But it's like a bad drug,
> > > the combination of this and other two commits break things.
> > >
> > > The patch below is my quick fix.  It worked on an HP laptop.
> > > Hugh, could you give it a try?
> > 
> > Hm, this sounds eerily familiar to the backlight bug you've recently fixed in
> > 
> > commit 6db65cbb941f9d433659bdad02b307f6d94465df
> > Author: Takashi Iwai <tiwai@suse.de>
> > Date:   Thu Jun 21 15:30:41 2012 +0200
> > 
> >     drm/i915: Fix eDP blank screen after S3 resume on HP desktops
> > 
> > Have you checked other code-paths for such issues?
> 
> Not yet, I'd leave such a joy rather to you guys :)
> 
> > The resume code
> > seems to follow this order already ... also, when you submit this
> > patch, can you please add a small comment to explain the ordering
> > constraint, like in the resume register restore function?
> 
> Sure, I'll add a comment and resubmit once when I hear it really fixes
> on Hugh's machine, too.

Indeed, your patch really fixes it on my machine: many thanks!

Hugh

> 
> 
> thanks,
> 
> Takashi
> 
> > 
> > Thanks, Daniel
> > 
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > > ===
> > > From: Takashi Iwai <tiwai@suse.de>
> > > Subject: [PATCH] drm/i915: Fix blank panel at reopening lid on Linux console
> > >
> > > When you reopen the lid on Linux console on a laptop with PCH, the
> > > panel suddenly goes blank.  It seems because BLC_PWM_CPU_CTL register
> > > is cleared when BLC_PWM_PCH_CTL1 and BLC_PWM_PCH_CTL2 registers are
> > > played.
> > >
> > > This patch fixes the problem by setting BLC_PWM_CPU_CTL after enabling
> > > BLC_PWM_PCH_CTL_1 and _2 registers.
> > >
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > ---
> > >  drivers/gpu/drm/i915/intel_panel.c | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > > index 10c7d39..d640425 100644
> > > --- a/drivers/gpu/drm/i915/intel_panel.c
> > > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > > @@ -311,9 +311,6 @@ void intel_panel_enable_backlight(struct drm_device *dev,
> > >         if (dev_priv->backlight_level == 0)
> > >                 dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
> > >
> > > -       dev_priv->backlight_enabled = true;
> > > -       intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
> > > -
> > >         if (INTEL_INFO(dev)->gen >= 4) {
> > >                 uint32_t reg, tmp;
> > >
> > > @@ -326,7 +323,7 @@ void intel_panel_enable_backlight(struct drm_device *dev,
> > >                  * we don't track the backlight dpms state, hence check whether
> > >                  * we have to do anything first. */
> > >                 if (tmp & BLM_PWM_ENABLE)
> > > -                       return;
> > > +                       goto set_level;
> > >
> > >                 if (dev_priv->num_pipe == 3)
> > >                         tmp &= ~BLM_PIPE_SELECT_IVB;
> > > @@ -347,6 +344,10 @@ void intel_panel_enable_backlight(struct drm_device *dev,
> > >                         I915_WRITE(BLC_PWM_PCH_CTL1, tmp);
> > >                 }
> > >         }
> > > +
> > > +set_level:
> > > +       dev_priv->backlight_enabled = true;
> > > +       intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
> > >  }
> > >
> > >  static void intel_panel_init_backlight(struct drm_device *dev)
> > > --
> > > 1.7.11.4
> > >
> > 
> > 
> > 
> > -- 
> > Daniel Vetter
> > daniel.vetter@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: i915 regression on 3.6-rc1: lid blanks screen

[Takashi Iwai]

At Fri, 10 Aug 2012 11:53:51 -0700 (PDT),
Hugh Dickins wrote:
> 
> On Fri, 10 Aug 2012, Takashi Iwai wrote:
> > At Fri, 10 Aug 2012 14:35:13 +0200,
> > Daniel Vetter wrote:
> > > 
> > > On Fri, Aug 10, 2012 at 1:59 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > > > At Mon, 6 Aug 2012 11:25:30 -0700 (PDT),
> > > > Hugh Dickins wrote:
> > > >>
> > > >> On Mon, 6 Aug 2012, Daniel Vetter wrote:
> > > >> > On Mon, Aug 6, 2012 at 6:21 AM, Hugh Dickins <hughd@google.com> wrote:
> > > >> > > On Sun, 5 Aug 2012, Takashi Iwai wrote:
> > > >> > >> At Sat, 4 Aug 2012 10:01:13 -0700 (PDT),
> > > >> > >> Hugh Dickins wrote:
> > > >> > >> >
> > > >> > >> > Sorry to report that with 3.6-rc1, closing and opening the lid on
> > > >> > >> > this ThinkPad T420s leaves the screen blank, and I have to reboot.
> > > >> > >> >
> > > >> > >> > Bisection led to this commit, and reverting indeed gets my screen back:
> > > >> > >> >
> > > >> > >> > commit 520c41cf2fa029d1e8b923ac2026f96664f17c4b
> > > >> > >> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > >> > >> > Date:   Wed Jul 11 16:27:52 2012 +0200
> > > >> > >> >
> > > >> > >> >     drm/i915/lvds: ditch ->prepare special case
> > > >> > > ...
> > > >> > >>
> > > >> > >> Hm, it's surprising.
> > > >> > >>
> > > >> > >> Could you check whether the counter-part intel_lvds_enable() is
> > > >> > >> called?  If the prepare callback affects, it must be from the mode
> > > >> > >> setting (drm_crtc_helper_set_mode()).
> > > >> > >
> > > >> > > Yes, I put a dump_stack() in both, and intel_lvds_enable() gets called
> > > >> > > about 0.28 seconds after the intel_lvds_disable() when I lift the lid;
> > > >> > > but with no video display until I revert that commit.
> > > >> >
> > > >> > Can you please boot with drm.debug=0xe added to your kernel cmdline,
> > > >> > reproduce the issue (with the two dump_stack calls added) and then
> > > >> > attach the full dmesg?
> > > >>
> > > >> Collected, I'll send it to you both privately in a moment.
> > > >>
> > > >> >
> > > >> > Also a few other things to try: What happens if you do a modeset on
> > > >> > the LVDS while it's still working, e.g.
> > > >>
> > > >> In the dmesg, I've only gone to runlevel 3, simply working on the
> > > >> console without startx.  For these xrandrs to work, I did startx
> > > >> and used the graphics screen.
> > > >
> > > > OK, now I can see the problem here, too.  The key is that it happens
> > > > only on Linux console, not on X.  That's why no one else reported.
> > > > I guess the problem can be seen on many laptops with LVDS on PCH.
> 
> A correction there: for me it was happening both on X and on console;
> but once I found that it happened even on the simple console, I mostly
> stuck to bisecting and testing on that.

Maybe depending on the desktop setup.  When a power-management stuff
is running, it may readjust the backlight level upon lid open, then it
fixes the issue casually.


> > > >
> > > > Looking at intel_reg_dumper output, BLC_PWM_CPU_CTL is 0 while other
> > > > registers are set correctly.  This seems coming from the rewrite of
> > > > backlight control code by commit
> > > >   24ded204: drm/i915: properly enable the blc controller on the right pipe
> > > > and
> > > >   a4f32fc3: drm/i915: don't forget the PCH backlight registers
> > > >
> > > > While the latter fixes the regression by the former commit, it still
> > > > doesn't cover this regression.
> > > >
> > > > I don't know the exact hardware behavior, but it looks like that
> > > > resetting BLC_PWM_PCH_CTL2 and BLC_PWM_PCH_CTL1 clears the
> > > > BLC_PWM_CPU_CTL (oh what confusing reg names).
> > > >
> > > > FWIW, the commit 520c41cf you mentioned is no direct cause.  This
> > > > patch works fine on the top of 3.5 kernel.  But it's like a bad drug,
> > > > the combination of this and other two commits break things.
> > > >
> > > > The patch below is my quick fix.  It worked on an HP laptop.
> > > > Hugh, could you give it a try?
> > > 
> > > Hm, this sounds eerily familiar to the backlight bug you've recently fixed in
> > > 
> > > commit 6db65cbb941f9d433659bdad02b307f6d94465df
> > > Author: Takashi Iwai <tiwai@suse.de>
> > > Date:   Thu Jun 21 15:30:41 2012 +0200
> > > 
> > >     drm/i915: Fix eDP blank screen after S3 resume on HP desktops
> > > 
> > > Have you checked other code-paths for such issues?
> > 
> > Not yet, I'd leave such a joy rather to you guys :)
> > 
> > > The resume code
> > > seems to follow this order already ... also, when you submit this
> > > patch, can you please add a small comment to explain the ordering
> > > constraint, like in the resume register restore function?
> > 
> > Sure, I'll add a comment and resubmit once when I hear it really fixes
> > on Hugh's machine, too.
> 
> Indeed, your patch really fixes it on my machine: many thanks!

OK, I'll resend the patch with more comments.


thanks,

Takashi

[PATCH v2] drm/i915: Fix blank panel at reopening lid

[Takashi Iwai]

When you reopen the lid on a laptop with PCH, the panel suddenly goes
blank sometimes.  It seems because BLC_PWM_CPU_CTL register is cleared
to zero when BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1 registers are
enabled.

This patch fixes the problem by moving the call of the function setting
BLC_PWM_CPU_CTL after enabling other two registers.

Reported-and-tested-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
v1->v2: add comments, fixed changelog

 drivers/gpu/drm/i915/intel_panel.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 10c7d39..d564c05 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -311,9 +311,6 @@ void intel_panel_enable_backlight(struct drm_device *dev,
 	if (dev_priv->backlight_level == 0)
 		dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
 
-	dev_priv->backlight_enabled = true;
-	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
-
 	if (INTEL_INFO(dev)->gen >= 4) {
 		uint32_t reg, tmp;
 
@@ -326,7 +323,7 @@ void intel_panel_enable_backlight(struct drm_device *dev,
 		 * we don't track the backlight dpms state, hence check whether
 		 * we have to do anything first. */
 		if (tmp & BLM_PWM_ENABLE)
-			return;
+			goto set_level;
 
 		if (dev_priv->num_pipe == 3)
 			tmp &= ~BLM_PIPE_SELECT_IVB;
@@ -347,6 +344,14 @@ void intel_panel_enable_backlight(struct drm_device *dev,
 			I915_WRITE(BLC_PWM_PCH_CTL1, tmp);
 		}
 	}
+
+set_level:
+	/* Call below after setting BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1.
+	 * BLC_PWM_CPU_CTL may be cleared to zero automatically when these
+	 * registers are set.
+	 */
+	dev_priv->backlight_enabled = true;
+	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
 }
 
 static void intel_panel_init_backlight(struct drm_device *dev)
-- 
1.7.11.4

Re: [PATCH v2] drm/i915: Fix blank panel at reopening lid

[Daniel Vetter]

On Sat, Aug 11, 2012 at 08:56:42AM +0200, Takashi Iwai wrote:
> When you reopen the lid on a laptop with PCH, the panel suddenly goes
> blank sometimes.  It seems because BLC_PWM_CPU_CTL register is cleared
> to zero when BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1 registers are
> enabled.
> 
> This patch fixes the problem by moving the call of the function setting
> BLC_PWM_CPU_CTL after enabling other two registers.
> 
> Reported-and-tested-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> v1->v2: add comments, fixed changelog
Picked up for -fixes, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48