[Intel-gfx] [PATCH] drm/i915: Fix garbage pixels on top of LVDS on IVY laptop

[Daniel Vetter]

On Fri, Dec 7, 2012 at 6:10 PM, Takashi Iwai <tiwai at suse.de> wrote:
> At Fri, 7 Dec 2012 18:04:06 +0100,
> Daniel Vetter wrote:
>>
>> On Fri, Dec 7, 2012 at 5:28 PM, Takashi Iwai <tiwai at suse.de> wrote:
>> > At Fri, 7 Dec 2012 16:47:05 +0100,
>> > Daniel Vetter wrote:
>> >>
>> >> On Fri, Dec 7, 2012 at 2:17 PM, Takashi Iwai <tiwai at suse.de> wrote:
>> >> > The commit [23670b322: drm/i915: CPT+ pch transcoder workaround]
>> >> > caused a regression on some HP laptops with IvyBridge.  On the top of
>> >> > laptop screen, a few pixels height are blinking in the whole width
>> >> > constantly.  The garbage appears only on LVDS and not on other
>> >> > outputs.
>> >> >
>> >> > This patch reverts the minimum part for fixing this regression,
>> >> > namely, the setup of CHICKEN2 bit in cpt_init_clock_gating().
>> >> >
>> >> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
>> >> > ---
>> >> >
>> >> > Don't ask me why this fixes :)
>> >> > The bug is still present in drm-intel-next-queued as of today, at
>> >> > least.
>> >> >
>> >> > Let me know if a better workaround is available.
>> >>
>> >> Since you're saying it only affects LVDS - have you tried to just move
>> >> the w/a enabling earlier in the enable/modeset sequence? I'm thinking
>> >> of the LVDS pin pair enabling, which now moved into the
>> >> ->pre_pll_enable hook, but in 3.8-next it's still in the
>> >> ironlake_crtc_mode_set. That would at least make some sense, and might
>> >> also be a bit more robust since This code is only run once at
>> >> resume/boot. But if we then clear things again on a subsequent
>> >> modeset, LVDS might break when re-enabling ...
>> >
>> > You comment reminded me of testing S3/S4 I forgot, and interestingly,
>> > the phenomenon is gone after S3.
>>
>> Hm, could it be an issue due to the residual BIOS mode? Our code
>> leaves the timing override bit enabled while the pipe is running,
>> maybe the BIOS disables it once the pipe is up, leading to havoc when
>> we disable the output on driver take-over? So maybe we need to fixup
>> our idea of how the w/a should be set up in the crtc fixup code in
>> intel_sanitize_crtc.
>
> That's possible.  The BIOS version on the machine isn't the latest.
> But it shouldn't be too old, too.

Not necessarily old BIOS, just a bad interaction between our code and
the state left behind by the BIOS. After all, it seems to work when
resuming, where Linux is in control from the hw ...

>> /me is just tossing ideas around right now ...
>>
>> > Also, looking at the display more closely, I found that the whole
>> > screen is shifted downward for a few pixels.
>>
>> No idea, can't remember a similar pattern.
>>
>> > Moving the w/a to other place: I tried to put it to modeset, but it
>> > didn't help.  Will try to enable code...
>>
>> Hm, that would fit with the theory that we need this bit set while
>> disabling the BIOS LVDS mode, not require it earlier in the modeset
>> sequence.
>
> I tried to move the w/a to enable code, but also didn't help.
>
> So... your guess appears really feasible.
> Then this is a side-effect of the new modeset code by optimizing the
> enable/disable step?

Nope, the commit you've cited is way past the modeset-rework merge.
That really just moved the enable/disable points of the w/a bit around
to the supposedly correct places. Also, if this w/a fails you're
supposed to get a black screen with stuck cpu-side pipe, not a display
shifted down a few lines. So something strange seems to be going on.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch