Messages in this thread | | | Date | Sun, 05 Aug 2012 09:11:58 +0200 | From | Takashi Iwai <> | 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@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) >
| |