| Message ID | s5htxryovbh.wl%tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 7, 2012 at 2:17 PM, Takashi Iwai <tiwai@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@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 ... Yours, Daniel > > drivers/gpu/drm/i915/intel_pm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 58c2f21..a544029 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3446,6 +3446,9 @@ static void cpt_init_clock_gating(struct drm_device *dev) > I915_WRITE(SOUTH_DSPCLK_GATE_D, PCH_DPLSUNIT_CLOCK_GATE_DISABLE); > I915_WRITE(SOUTH_CHICKEN2, I915_READ(SOUTH_CHICKEN2) | > DPLS_EDP_PPS_FIX_DIS); > + /* Without this, mode sets may fail silently on FDI */ > + for_each_pipe(pipe) > + I915_WRITE(TRANS_CHICKEN2(pipe), TRANS_CHICKEN2_TIMING_OVERRIDE); > /* WADP0ClockGatingDisable */ > for_each_pipe(pipe) { > I915_WRITE(TRANS_CHICKEN1(pipe), > -- > 1.8.0.1 >
At Fri, 7 Dec 2012 16:47:05 +0100, Daniel Vetter wrote: > > On Fri, Dec 7, 2012 at 2:17 PM, Takashi Iwai <tiwai@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@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. Also, looking at the display more closely, I found that the whole screen is shifted downward for a few pixels. Moving the w/a to other place: I tried to put it to modeset, but it didn't help. Will try to enable code... thanks, Takashi > > Yours, Daniel > > > > > drivers/gpu/drm/i915/intel_pm.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 58c2f21..a544029 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3446,6 +3446,9 @@ static void cpt_init_clock_gating(struct drm_device *dev) > > I915_WRITE(SOUTH_DSPCLK_GATE_D, PCH_DPLSUNIT_CLOCK_GATE_DISABLE); > > I915_WRITE(SOUTH_CHICKEN2, I915_READ(SOUTH_CHICKEN2) | > > DPLS_EDP_PPS_FIX_DIS); > > + /* Without this, mode sets may fail silently on FDI */ > > + for_each_pipe(pipe) > > + I915_WRITE(TRANS_CHICKEN2(pipe), TRANS_CHICKEN2_TIMING_OVERRIDE); > > /* WADP0ClockGatingDisable */ > > for_each_pipe(pipe) { > > I915_WRITE(TRANS_CHICKEN1(pipe), > > -- > > 1.8.0.1 > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch >
On Fri, Dec 7, 2012 at 5:28 PM, Takashi Iwai <tiwai@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@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@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. /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. Cheers, Daniel
At Fri, 7 Dec 2012 18:04:06 +0100, Daniel Vetter wrote: > > On Fri, Dec 7, 2012 at 5:28 PM, Takashi Iwai <tiwai@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@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@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. > /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? thanks, Takashi
On Fri, Dec 7, 2012 at 6:10 PM, Takashi Iwai <tiwai@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@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@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@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
On Fri, Dec 7, 2012 at 7:05 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> 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. Can you please check whether this is a dual-link lvds machine? Some other w/a would apply in this case. -Daniel
On Fri, Dec 07, 2012 at 07:07:32PM +0100, Daniel Vetter wrote: > On Fri, Dec 7, 2012 at 7:05 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > >> 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. > > Can you please check whether this is a dual-link lvds machine? Some > other w/a would apply in this case. Also please retest with latest drm-intel-nightly and the following patch: https://patchwork.kernel.org/patch/1852411/ Otherwise we might be hitting fallout from know issues. -Daniel
At Fri, 7 Dec 2012 19:07:32 +0100, Daniel Vetter wrote: > > On Fri, Dec 7, 2012 at 7:05 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > >> 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. > > Can you please check whether this is a dual-link lvds machine? Some > other w/a would apply in this case. The machine is a single link with 1366x768. Takashi
At Sat, 8 Dec 2012 13:01:29 +0100, Daniel Vetter wrote: > > On Fri, Dec 07, 2012 at 07:07:32PM +0100, Daniel Vetter wrote: > > On Fri, Dec 7, 2012 at 7:05 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > >> 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. > > > > Can you please check whether this is a dual-link lvds machine? Some > > other w/a would apply in this case. > > Also please retest with latest drm-intel-nightly and the following patch: > > https://patchwork.kernel.org/patch/1852411/ Tried that, but didn't help, unfortunately. > Otherwise we might be hitting fallout from know issues. Looks so... Takashi
On Mon, Dec 10, 2012 at 11:19 AM, Takashi Iwai <tiwai@suse.de> wrote: >> Also please retest with latest drm-intel-nightly and the following patch: >> >> https://patchwork.kernel.org/patch/1852411/ > > Tried that, but didn't help, unfortunately. Before we give up, can you try the for-ickle branch from http://cgit.freedesktop.org/~danvet/drm Specifically the reorder enhanced framing from "drm/i915: set fdi TX enhanced framing before enabling port". Yours, Daniel
At Mon, 10 Dec 2012 11:30:23 +0100, Daniel Vetter wrote: > > On Mon, Dec 10, 2012 at 11:19 AM, Takashi Iwai <tiwai@suse.de> wrote: > >> Also please retest with latest drm-intel-nightly and the following patch: > >> > >> https://patchwork.kernel.org/patch/1852411/ > > > > Tried that, but didn't help, unfortunately. > > Before we give up, can you try the for-ickle branch from > > http://cgit.freedesktop.org/~danvet/drm > > Specifically the reorder enhanced framing from "drm/i915: set fdi TX > enhanced framing before enabling port". Still didn't work... Takashi
On Mon, Dec 10, 2012 at 11:56:05AM +0100, Takashi Iwai wrote: > At Mon, 10 Dec 2012 11:30:23 +0100, > Daniel Vetter wrote: > > > > On Mon, Dec 10, 2012 at 11:19 AM, Takashi Iwai <tiwai@suse.de> wrote: > > >> Also please retest with latest drm-intel-nightly and the following patch: > > >> > > >> https://patchwork.kernel.org/patch/1852411/ > > > > > > Tried that, but didn't help, unfortunately. > > > > Before we give up, can you try the for-ickle branch from > > > > http://cgit.freedesktop.org/~danvet/drm > > > > Specifically the reorder enhanced framing from "drm/i915: set fdi TX > > enhanced framing before enabling port". > > Still didn't work... Blargh, I guess we need your hack. Can you please augment the comment in the code that the issue is only with LVDS and resubmit? Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 58c2f21..a544029 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3446,6 +3446,9 @@ static void cpt_init_clock_gating(struct drm_device *dev) I915_WRITE(SOUTH_DSPCLK_GATE_D, PCH_DPLSUNIT_CLOCK_GATE_DISABLE); I915_WRITE(SOUTH_CHICKEN2, I915_READ(SOUTH_CHICKEN2) | DPLS_EDP_PPS_FIX_DIS); + /* Without this, mode sets may fail silently on FDI */ + for_each_pipe(pipe) + I915_WRITE(TRANS_CHICKEN2(pipe), TRANS_CHICKEN2_TIMING_OVERRIDE); /* WADP0ClockGatingDisable */ for_each_pipe(pipe) { I915_WRITE(TRANS_CHICKEN1(pipe),
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@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. drivers/gpu/drm/i915/intel_pm.c | 3 +++ 1 file changed, 3 insertions(+)