On Thu, Nov 29, 2018 at 03:12:15PM +0100, Greg Kroah-Hartman wrote:
> 4.19-stable review patch. If anyone has any objections, please let me know.
This one apparently introduces some annoying dmesg errors:
[ 3.487895] [drm:intel_print_wm_latency [i915]] *ERROR* Primary WM3 latency not provided
[ 3.487926] [drm:intel_print_wm_latency [i915]] *ERROR* Sprite WM3 latency not provided
[ 3.487955] [drm:intel_print_wm_latency [i915]] *ERROR* Cursor WM3 latency not provided
To silence those please also backport
commit 274dba1ae8ff ("drm/i915: Downgrade Gen9 Plane WM latency error")
>
> ------------------
>
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> commit 21556350ade3cb5d7afecc8b3544e56431d21695 upstream.
>
> 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.
>
> v2: s/USHRT_MAX/U32_MAX/ for consistency with the types (Chris)
>
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Acked-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> 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>
> Link: https://patchwork.freedesktop.org/patch/msgid/20181114173440.6730-1-ville.syrjala@xxxxxxxxxxxxxxx
> (cherry picked from commit 03981c6ebec4fc7056b9b45f847393aeac90d060)
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2492,6 +2492,9 @@ static uint32_t ilk_compute_pri_wm(const
> uint32_t method1, method2;
> int cpp;
>
> + if (mem_value == 0)
> + return U32_MAX;
> +
> if (!intel_wm_plane_visible(cstate, pstate))
> return 0;
>
> @@ -2521,6 +2524,9 @@ static uint32_t ilk_compute_spr_wm(const
> uint32_t method1, method2;
> int cpp;
>
> + if (mem_value == 0)
> + return U32_MAX;
> +
> if (!intel_wm_plane_visible(cstate, pstate))
> return 0;
>
> @@ -2544,6 +2550,9 @@ static uint32_t ilk_compute_cur_wm(const
> {
> int cpp;
>
> + if (mem_value == 0)
> + return U32_MAX;
> +
> if (!intel_wm_plane_visible(cstate, pstate))
> return 0;
>
> @@ -2998,6 +3007,34 @@ static void snb_wm_latency_quirk(struct
> 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;
> +
> + DRM_DEBUG_KMS("LP3 watermarks disabled due to potential for lost interrupts\n");
> + intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
> + intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
> + intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
> +}
> +
> static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
> {
> intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
> @@ -3014,8 +3051,10 @@ static void ilk_setup_wm_latency(struct
> intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
> intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
>
> - if (IS_GEN6(dev_priv))
> + if (IS_GEN6(dev_priv)) {
> snb_wm_latency_quirk(dev_priv);
> + snb_wm_lp3_irq_quirk(dev_priv);
> + }
> }
>
> static void skl_setup_wm_latency(struct drm_i915_private *dev_priv)
>
--
Ville Syrjälä
Intel
