Commit ccebc23b authored by Lyude's avatar Lyude Committed by Maarten Lankhorst

drm/i915/skl: Don't try to update plane watermarks if they haven't changed

i915 sometimes needs to disable planes in the middle of an atomic
commit, and then reenable them later in the same commit. Because of
this, we can't make the assumption that the state of the plane actually
changed. Since the state of the plane hasn't actually changed, neither
have it's watermarks. And if the watermarks hasn't changed then we
haven't populated skl_results with anything, which means we'll end up
zeroing out a plane's watermarks in the middle of the atomic commit
without restoring them later.

Simple reproduction recipe:
 - Get a SKL laptop, launch any kind of X session
 - Get two extra monitors
 - Keep hotplugging both displays (so that the display configuration
   jumps from 1 active pipe to 3 active pipes and back)
 - Eventually underrun

Changes since v1:
 - Fix incorrect use of "it's"
Changes since v2:
 - Add reproduction recipe
Signed-off-by: default avatarLyude <cpaul@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: 62e0fb88 ("drm/i915/skl: Update plane watermarks atomically during plane updates")
Signed-off-by: default avatarLyude <cpaul@redhat.com>
Testcase: kms_plane
Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1472488288-27280-1-git-send-email-cpaul@redhat.com
Cc: drm-intel-fixes@lists.freedesktop.org
parent b18c1bb4
...@@ -3448,7 +3448,12 @@ static void skylake_disable_primary_plane(struct drm_plane *primary, ...@@ -3448,7 +3448,12 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_crtc->pipe; int pipe = intel_crtc->pipe;
skl_write_plane_wm(intel_crtc, &dev_priv->wm.skl_results, 0); /*
* We only populate skl_results on watermark updates, and if the
* plane's visiblity isn't actually changing neither is its watermarks.
*/
if (!crtc->primary->state->visible)
skl_write_plane_wm(intel_crtc, &dev_priv->wm.skl_results, 0);
I915_WRITE(PLANE_CTL(pipe, 0), 0); I915_WRITE(PLANE_CTL(pipe, 0), 0);
I915_WRITE(PLANE_SURF(pipe, 0), 0); I915_WRITE(PLANE_SURF(pipe, 0), 0);
......
...@@ -292,8 +292,13 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc) ...@@ -292,8 +292,13 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
const int pipe = intel_plane->pipe; const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1; const int plane = intel_plane->plane + 1;
skl_write_plane_wm(to_intel_crtc(crtc), &dev_priv->wm.skl_results, /*
plane); * We only populate skl_results on watermark updates, and if the
* plane's visiblity isn't actually changing neither is its watermarks.
*/
if (!dplane->state->visible)
skl_write_plane_wm(to_intel_crtc(crtc),
&dev_priv->wm.skl_results, plane);
I915_WRITE(PLANE_CTL(pipe, plane), 0); I915_WRITE(PLANE_CTL(pipe, plane), 0);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment