[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <s5h3941ke2p.wl%tiwai@suse.de>
Date: Sun, 05 Aug 2012 09:11:58 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Hugh Dickins <hughd@...gle.com>
Cc: Daniel Vetter <daniel.vetter@...ll.ch>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Chris Wilson <chris@...is-wilson.co.uk>,
Giacomo Comes <comes@...c.edu>, linux-kernel@...r.kernel.org
Subject: Re: i915 regression on 3.6-rc1: lid blanks screen
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@...ll.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@...e.de>
> Cc: Giacomo Comes <comes@...c.edu>
> Acked-by: Chris Wilson <chris@...is-wilson.co.uk>
> Tested-by: Takashi Iwai <tiwai@...e.de>
> Signed-Off-by: Daniel Vetter <daniel.vetter@...ll.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)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists