Quoting Ville Syrjala (2018-11-13 18:10:23) > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > I have a Thinkpad X220 Tablet in my hands that is losing vblank > interrupts whenever LP3 watermarks are used. > > If I nudge the latency value written to the WM3 register just > by one in either direction the problem disappears. That to me > suggests that the punit will not enter the corrsponding > powersave mode (MPLL shutdown IIRC) unless the latency value > in the register matches exactly what we read from SSKPD. Ie. > it's not really a latency value but rather just a cookie > by which the punit can identify the desired power saving state. > On HSW/BDW this was changed such that we actually just write > the WM level number into those bits, which makes much more > sense given the observed behaviour. > > We could try to handle this by disallowing LP3 watermarks > only when vblank interrupts are enabled but we'd first have > to prove that only vblank interrupts are affected, which > seems unlikely. Also we can't grab the wm mutex from the > vblank enable/disable hooks because those are called with > various spinlocks held. Thus we'd have to redesigne the > watermark locking. So to play it safe and keep the code > simple we simply disable LP3 watermarks on all SNB machines. > > To do that we simply zero out the latency values for > watermark level 3, and we adjust the watermark computation > to check for that. The behaviour now matches that of the > g4x/vlv/skl wm code in the presence of a zeroed latency > value. > > Cc: stable@xxxxxxxxxxxxxxx > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101269 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103713 > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_pm.c | 41 ++++++++++++++++++++++++++++++++- > 1 file changed, 40 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 27498ded4949..ef1ae2ede379 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2493,6 +2493,9 @@ static uint32_t ilk_compute_pri_wm(const struct intel_crtc_state *cstate, > uint32_t method1, method2; > int cpp; > > + if (mem_value == 0) > + return USHRT_MAX; > + > if (!intel_wm_plane_visible(cstate, pstate)) > return 0; > > @@ -2522,6 +2525,9 @@ static uint32_t ilk_compute_spr_wm(const struct intel_crtc_state *cstate, > uint32_t method1, method2; > int cpp; > > + if (mem_value == 0) > + return USHRT_MAX; > + > if (!intel_wm_plane_visible(cstate, pstate)) > return 0; > > @@ -2545,6 +2551,9 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate, > { > int cpp; > > + if (mem_value == 0) > + return USHRT_MAX; > + > if (!intel_wm_plane_visible(cstate, pstate)) > return 0; > > @@ -3008,6 +3017,34 @@ static void snb_wm_latency_quirk(struct drm_i915_private *dev_priv) > intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency); > } > > +static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv) > +{ > + /* > + * On some SNB machines (Thinkpad X220 Tablet at least) > + * LP3 usage can cause vblank interrupts to be lost. > + * The DEIIR bit will go high but it looks like the CPU > + * never gets interrupted. > + * > + * It's not clear whether other interrupt source could > + * be affected or if this is somehow limited to vblank > + * interrupts only. To play it safe we disable LP3 > + * watermarks entirely. > + */ > + if (dev_priv->wm.pri_latency[3] == 0 && > + dev_priv->wm.spr_latency[3] == 0 && > + dev_priv->wm.cur_latency[3] == 0) > + return; > + > + dev_priv->wm.pri_latency[3] = 0; > + dev_priv->wm.spr_latency[3] = 0; > + dev_priv->wm.cur_latency[3] = 0; -> return USHRT_MAX for pri/spr/cur planes. -> result->enable = false; Only question then why USHRT_MAX for a u32 parameter? Seems to be copied from gm45 where it is a u16 parameter instead. -Chris
