linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lyude <cpaul@redhat.com>
To: intel-gfx@lists.freedesktop.org,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Lyude <cpaul@redhat.com>,
	stable@vger.kernel.org, Daniel Vetter <daniel.vetter@intel.com>,
	Radhakrishna Sripada <radhakrishna.sripada@intel.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Matt Roper <matthew.d.roper@intel.com>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	David Airlie <airlied@linux.ie>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: [PATCH v4 5/6] drm/i915/skl: Update plane watermarks atomically during plane updates
Date: Tue, 26 Jul 2016 13:34:41 -0400	[thread overview]
Message-ID: <1469554483-24999-6-git-send-email-cpaul@redhat.com> (raw)
In-Reply-To: <1469554483-24999-1-git-send-email-cpaul@redhat.com>

Thanks to Ville for suggesting this as a potential solution to pipe
underruns on Skylake.

On Skylake all of the registers for configuring planes, including the
registers for configuring their watermarks, are double buffered. New
values written to them won't take effect until said registers are
"armed", which is done by writing to the PLANE_SURF (or in the case of
cursor planes, the CURBASE register) register.

With this in mind, up until now we've been updating watermarks on skl
like this:

  non-modeset {
   - calculate (during atomic check phase)
   - finish_atomic_commit:
     - intel_pre_plane_update:
        - intel_update_watermarks()
     - {vblank happens; new watermarks + old plane values => underrun }
     - drm_atomic_helper_commit_planes_on_crtc:
        - start vblank evasion
        - write new plane registers
        - end vblank evasion
  }

  or

  modeset {
   - calculate (during atomic check phase)
   - finish_atomic_commit:
     - crtc_enable:
        - intel_update_watermarks()
     - {vblank happens; new watermarks + old plane values => underrun }
     - drm_atomic_helper_commit_planes_on_crtc:
        - start vblank evasion
        - write new plane registers
        - end vblank evasion
  }

Now we update watermarks atomically like this:

  non-modeset {
   - calculate (during atomic check phase)
   - finish_atomic_commit:
     - intel_pre_plane_update:
        - intel_update_watermarks() (wm values aren't written yet)
     - drm_atomic_helper_commit_planes_on_crtc:
        - start vblank evasion
        - write new plane registers
        - write new wm values
        - end vblank evasion
  }

  modeset {
   - calculate (during atomic check phase)
   - finish_atomic_commit:
     - crtc_enable:
        - intel_update_watermarks() (actual wm values aren't written
          yet)
     - drm_atomic_helper_commit_planes_on_crtc:
        - start vblank evasion
        - write new plane registers
	- write new wm values
        - end vblank evasion
  }

So this patch moves all of the watermark writes into the right place;
inside of the vblank evasion where we update all of the registers for
each plane. While this patch doesn't fix everything, it does allow us to
update the watermark values in the way the hardware expects us to.

Changes since original patch series:
 - Remove mutex_lock/mutex_unlock since they don't do anything and we're
   not touching global state
 - Move skl_write_cursor_wm/skl_write_plane_wm functions into
   intel_pm.c, make externally visible
 - Add skl_write_plane_wm calls to skl_update_plane
 - Fix conditional for for loop in skl_write_plane_wm (level < max_level
   should be level <= max_level)
 - Make diagram in commit more accurate to what's actually happening
 - Add Fixes:

Changes since v1:
 - Use IS_GEN9() instead of IS_SKYLAKE() since these fixes apply to more
   then just Skylake
 - Update description to make it clear this patch doesn't fix everything
 - Check if pipes were actually changed before writing watermarks

Changes since v2:
 - Write PIPE_WM_LINETIME during vblank evasion

Changes since v3:
 - Rebase against new SAGV patch changes

Fixes: 2d41c0b59afc ("drm/i915/skl: SKL Watermark Computation")
Signed-off-by: Lyude <cpaul@redhat.com>
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  5 ++++
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_pm.c      | 58 ++++++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_sprite.c  |  2 ++
 4 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b80c051..cd67945 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3031,6 +3031,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	intel_crtc->adjusted_x = x_offset;
 	intel_crtc->adjusted_y = y_offset;
 
+	skl_write_plane_wm(intel_crtc, 0);
+
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
 	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
@@ -10261,6 +10263,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 	int pipe = intel_crtc->pipe;
 	uint32_t cntl = 0;
 
+	if (IS_GEN9(dev_priv))
+		skl_write_cursor_wm(intel_crtc);
+
 	if (plane_state && plane_state->visible) {
 		cntl = MCURSOR_GAMMA_ENABLE;
 		switch (plane_state->base.crtc_w) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 113bf48..e212ed9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1711,6 +1711,8 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 			  struct skl_ddb_allocation *ddb /* out */);
 int skl_enable_sagv(struct drm_i915_private *dev_priv);
 int skl_disable_sagv(struct drm_i915_private *dev_priv);
+void skl_write_cursor_wm(struct intel_crtc *intel_crtc);
+void skl_write_plane_wm(struct intel_crtc *intel_crtc, int plane);
 uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
 bool ilk_disable_lp_wm(struct drm_device *dev);
 int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fb5d2eb..d469ad2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3783,6 +3783,47 @@ static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
 		I915_WRITE(reg, 0);
 }
 
+void skl_write_plane_wm(struct intel_crtc *intel_crtc,
+			int plane)
+{
+	struct drm_crtc *crtc = &intel_crtc->base;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+	int level, max_level = ilk_wm_max_level(dev);
+	enum pipe pipe = intel_crtc->pipe;
+
+	if (!(wm->dirty_pipes & drm_crtc_mask(crtc)))
+		return;
+
+	I915_WRITE(PIPE_WM_LINETIME(pipe), wm->wm_linetime[pipe]);
+
+	for (level = 0; level <= max_level; level++) {
+		I915_WRITE(PLANE_WM(pipe, plane, level),
+			   wm->plane[pipe][plane][level]);
+	}
+	I915_WRITE(PLANE_WM_TRANS(pipe, plane), wm->plane_trans[pipe][plane]);
+}
+
+void skl_write_cursor_wm(struct intel_crtc *intel_crtc)
+{
+	struct drm_crtc *crtc = &intel_crtc->base;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+	int level, max_level = ilk_wm_max_level(dev);
+	enum pipe pipe = intel_crtc->pipe;
+
+	if (!(wm->dirty_pipes & drm_crtc_mask(crtc)))
+		return;
+
+	for (level = 0; level <= max_level; level++) {
+		I915_WRITE(CUR_WM(pipe, level),
+			   wm->plane[pipe][PLANE_CURSOR][level]);
+	}
+	I915_WRITE(CUR_WM_TRANS(pipe), wm->plane_trans[pipe][PLANE_CURSOR]);
+}
+
 static void skl_write_wm_values(struct drm_i915_private *dev_priv,
 				const struct skl_wm_values *new)
 {
@@ -3790,7 +3831,7 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
 	struct intel_crtc *crtc;
 
 	for_each_intel_crtc(dev, crtc) {
-		int i, level, max_level = ilk_wm_max_level(dev);
+		int i;
 		enum pipe pipe = crtc->pipe;
 
 		if ((new->dirty_pipes & drm_crtc_mask(&crtc->base)) == 0)
@@ -3798,21 +3839,6 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
 		if (!crtc->active)
 			continue;
 
-		I915_WRITE(PIPE_WM_LINETIME(pipe), new->wm_linetime[pipe]);
-
-		for (level = 0; level <= max_level; level++) {
-			for (i = 0; i < intel_num_planes(crtc); i++)
-				I915_WRITE(PLANE_WM(pipe, i, level),
-					   new->plane[pipe][i][level]);
-			I915_WRITE(CUR_WM(pipe, level),
-				   new->plane[pipe][PLANE_CURSOR][level]);
-		}
-		for (i = 0; i < intel_num_planes(crtc); i++)
-			I915_WRITE(PLANE_WM_TRANS(pipe, i),
-				   new->plane_trans[pipe][i]);
-		I915_WRITE(CUR_WM_TRANS(pipe),
-			   new->plane_trans[pipe][PLANE_CURSOR]);
-
 		for (i = 0; i < intel_num_planes(crtc); i++) {
 			skl_ddb_entry_write(dev_priv,
 					    PLANE_BUF_CFG(pipe, i),
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0de935a..50026f1 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -238,6 +238,8 @@ skl_update_plane(struct drm_plane *drm_plane,
 	crtc_w--;
 	crtc_h--;
 
+	skl_write_plane_wm(to_intel_crtc(crtc_state->base.crtc), plane);
+
 	if (key->flags) {
 		I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
 		I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value);
-- 
2.7.4

  parent reply	other threads:[~2016-07-26 17:35 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-26 17:34 [PATCH v4 0/6] Finally fix watermarks Lyude
2016-07-26 17:34 ` [PATCH v4 1/6] drm/i915/skl: Add support for the SAGV, fix underrun hangs Lyude
2016-07-28 13:13   ` Matt Roper
2016-08-02 11:16   ` [v4,1/6] " Hans de Goede
2016-07-26 17:34 ` [PATCH v4 2/6] drm/i915/gen9: Only copy WM results for changed pipes to skl_hw Lyude
2016-07-26 17:34 ` [PATCH v4 3/6] drm/i915/skl: Only flush pipes when we change the ddb allocation Lyude
2016-07-28 13:14   ` Matt Roper
2016-07-26 17:34 ` [PATCH v4 4/6] drm/i915/skl: Fix extra whitespace in skl_flush_wm_values() Lyude
2016-07-26 17:34 ` Lyude [this message]
2016-07-28 13:15   ` [PATCH v4 5/6] drm/i915/skl: Update plane watermarks atomically during plane updates Matt Roper
2016-07-26 17:34 ` [PATCH v4 6/6] drm/i915/skl: Always wait for pipes to update after a flush Lyude
2016-07-29  0:03 ` [Intel-gfx] [PATCH v4 0/6] Finally fix watermarks Matt Roper
2016-07-29  9:39   ` Ville Syrjälä
2016-07-29 18:48     ` Lyude
2016-07-29 19:26       ` Ville Syrjälä
2016-07-29 19:46         ` Lyude
2016-07-29 20:41         ` Matt Roper
2016-08-01 11:50           ` Ville Syrjälä
2016-07-29 20:33     ` Matt Roper
2016-08-01  8:48       ` Maarten Lankhorst
2016-08-01 11:48         ` Ville Syrjälä
2016-08-02 15:41           ` Maarten Lankhorst
2016-08-02 15:59             ` Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1469554483-24999-6-git-send-email-cpaul@redhat.com \
    --to=cpaul@redhat.com \
    --cc=airlied@linux.ie \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=matthew.d.roper@intel.com \
    --cc=radhakrishna.sripada@intel.com \
    --cc=stable@vger.kernel.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).