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@xxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxx>
> >> > >> > 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@xxxxxxx>
> 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@xxxxxxx>
> > 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@xxxxxxx>
> > ---
> >  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@xxxxxxxx - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/