[PATCH] drm/i915/skl: Fix redundant cursor update, fix cursor underruns

[PATCH] drm/i915/skl: Fix redundant cursor update, fix cursor underruns

[Lyude]

At long last, the time has finally come for Skylake users to plug their
external displays back in.

During intel_atomic_commit() on Skylake, we've actually been arming the
registers to update the cursor information twice instead of just once.
Once in i9xx_update_cursor(), and once in skl_wm_flush_pipe(). This
isn't actually necessary, and removing the later update in
skl_wm_flush_pipe() has completely stopped the underruns on this T460p
from occurring when moving the mouse cursor from one monitor to another.

Signed-off-by: Lyude <cpaul@redhat.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_pm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7ac71ec..4771a03 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3860,7 +3860,6 @@ skl_wm_flush_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, int pass)
 		I915_WRITE(PLANE_SURF(pipe, plane),
 			   I915_READ(PLANE_SURF(pipe, plane)));
 	}
-	I915_WRITE(CURBASE(pipe), I915_READ(CURBASE(pipe)));
 }
 
 static bool
-- 
2.7.4

[PATCH] drm/i915/skl: Fix redundant cursor update, fix cursor underruns

[Lyude]

At long last, the time has finally come for Skylake users to plug their
external displays back in.

During intel_atomic_commit() on Skylake, we've actually been arming the
registers to update the cursor information twice instead of just once.
Once in i9xx_update_cursor(), and once in skl_wm_flush_pipe(). This
isn't actually necessary, and removing the later update in
skl_wm_flush_pipe() has completely stopped the underruns on this T460p
from occurring when moving the mouse cursor from one monitor to another.

Signed-off-by: Lyude <cpaul@redhat.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_pm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7ac71ec..4771a03 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3860,7 +3860,6 @@ skl_wm_flush_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, int pass)
 		I915_WRITE(PLANE_SURF(pipe, plane),
 			   I915_READ(PLANE_SURF(pipe, plane)));
 	}
-	I915_WRITE(CURBASE(pipe), I915_READ(CURBASE(pipe)));
 }
 
 static bool
-- 
2.7.4

✓ Ro.CI.BAT: success for drm/i915/skl: Fix redundant cursor update, fix cursor underruns

[Patchwork]

== Series Details ==

Series: drm/i915/skl: Fix redundant cursor update, fix cursor underruns
URL   : https://patchwork.freedesktop.org/series/9940/
State : success

== Summary ==

Series 9940v1 drm/i915/skl: Fix redundant cursor update, fix cursor underruns
http://patchwork.freedesktop.org/api/1.0/series/9940/revisions/1/mbox


fi-hsw-i7-4770k  total:242  pass:210  dwarn:0   dfail:0   fail:8   skip:24 
fi-kbl-qkkr      total:242  pass:176  dwarn:29  dfail:0   fail:7   skip:30 
fi-skl-i5-6260u  total:242  pass:219  dwarn:0   dfail:0   fail:7   skip:16 
fi-skl-i7-6700k  total:242  pass:205  dwarn:0   dfail:0   fail:7   skip:30 
fi-snb-i7-2600   total:242  pass:190  dwarn:0   dfail:0   fail:8   skip:44 
ro-bdw-i5-5250u  total:242  pass:214  dwarn:4   dfail:0   fail:7   skip:17 
ro-bdw-i7-5557U  total:242  pass:215  dwarn:0   dfail:0   fail:7   skip:20 
ro-bdw-i7-5600u  total:242  pass:199  dwarn:0   dfail:1   fail:7   skip:35 
ro-bsw-n3050     total:218  pass:173  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:242  pass:191  dwarn:0   dfail:0   fail:9   skip:42 
ro-hsw-i3-4010u  total:242  pass:206  dwarn:0   dfail:0   fail:8   skip:28 
ro-hsw-i7-4770r  total:242  pass:206  dwarn:0   dfail:0   fail:8   skip:28 
ro-ilk-i7-620lm  total:242  pass:166  dwarn:0   dfail:0   fail:9   skip:67 
ro-ilk1-i5-650   total:237  pass:166  dwarn:0   dfail:0   fail:9   skip:62 
ro-ivb-i7-3770   total:242  pass:197  dwarn:0   dfail:0   fail:8   skip:37 
ro-skl3-i5-6260u total:242  pass:218  dwarn:1   dfail:0   fail:7   skip:16 
ro-snb-i7-2620M  total:242  pass:188  dwarn:0   dfail:0   fail:9   skip:45 
fi-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1506/

2fe098f drm-intel-nightly: 2016y-07m-15d-19h-42m-01s UTC integration manifest
2074255 drm/i915/skl: Fix redundant cursor update, fix cursor underruns

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/skl: Fix redundant cursor update, fix cursor underruns

[Ville Syrjälä]

On Fri, Jul 15, 2016 at 06:13:56PM -0400, Lyude wrote:
> At long last, the time has finally come for Skylake users to plug their
> external displays back in.
> 
> During intel_atomic_commit() on Skylake, we've actually been arming the
> registers to update the cursor information twice instead of just once.
> Once in i9xx_update_cursor(), and once in skl_wm_flush_pipe(). This
> isn't actually necessary, and removing the later update in
> skl_wm_flush_pipe() has completely stopped the underruns on this T460p
> from occurring when moving the mouse cursor from one monitor to another.
> 
> Signed-off-by: Lyude <cpaul@redhat.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7ac71ec..4771a03 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3860,7 +3860,6 @@ skl_wm_flush_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, int pass)
>  		I915_WRITE(PLANE_SURF(pipe, plane),
>  			   I915_READ(PLANE_SURF(pipe, plane)));
>  	}
> -	I915_WRITE(CURBASE(pipe), I915_READ(CURBASE(pipe)));

The WM/BUF_CFG register double buffering is armed by the plane surface
register, so if you're removing this write, all you actually end up
doing is skipping the watermark update. So this is just papering over
the bug.

This might even cause more explosions when enabling pipes, as the DDB
configuration for the cursors on the already enabled pipes might not
even get updated, and hence multiple planes migth end up trying to use
the same part of the DDB for their FIFOs.

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH] drm/i915/skl: Fix redundant cursor update, fix cursor underruns

[Ville Syrjälä]

On Fri, Jul 15, 2016 at 06:13:56PM -0400, Lyude wrote:
> At long last, the time has finally come for Skylake users to plug their
> external displays back in.
> 
> During intel_atomic_commit() on Skylake, we've actually been arming the
> registers to update the cursor information twice instead of just once.
> Once in i9xx_update_cursor(), and once in skl_wm_flush_pipe(). This
> isn't actually necessary, and removing the later update in
> skl_wm_flush_pipe() has completely stopped the underruns on this T460p
> from occurring when moving the mouse cursor from one monitor to another.
> 
> Signed-off-by: Lyude <cpaul@redhat.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7ac71ec..4771a03 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3860,7 +3860,6 @@ skl_wm_flush_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, int pass)
>  		I915_WRITE(PLANE_SURF(pipe, plane),
>  			   I915_READ(PLANE_SURF(pipe, plane)));
>  	}
> -	I915_WRITE(CURBASE(pipe), I915_READ(CURBASE(pipe)));

The WM/BUF_CFG register double buffering is armed by the plane surface
register, so if you're removing this write, all you actually end up
doing is skipping the watermark update. So this is just papering over
the bug.

This might even cause more explosions when enabling pipes, as the DDB
configuration for the cursors on the already enabled pipes might not
even get updated, and hence multiple planes migth end up trying to use
the same part of the DDB for their FIFOs.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx