Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matt Roper <matthew.d.roper@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH 2/2] drm/i915: Update sprite watermarks outside vblank evasion
Date: Mon, 22 Jun 2015 18:30:33 -0700	[thread overview]
Message-ID: <1435023033-18043-3-git-send-email-matthew.d.roper@intel.com> (raw)
In-Reply-To: <1435023033-18043-1-git-send-email-matthew.d.roper@intel.com>

We never removed the sprite watermark updates from our low-level
foo_update_plane() functions; since our hardware updates happen under
vblank evasion, we're not supposed to be calling potentially sleeping
functions there (since interrupts are disabled).  We do already have a
mechanism that's supposed to be used to update sprite watermarks in the
post-evasion function intel_post_plane_update(), but at the moment it's
only being used for updates caused by plane disables.

To fix this oversight we need to make a few changes:

 * When intel_plane_atomic_calc_changes() calls intel_wm_need_update()
   to determine whether watermarks need an update, we need to set the
   'atomic.update_sprite_watermarks' flag rather than the
   'atomic.update_wm' flag when the plane in question is a sprite.  Some
   platforms don't care about this change since the two types of update
   do the same thing, but some platforms (e.g., ILK-style watermarks)
   need to specifically use the sprite update function to update cached
   watermark parameters.

 * intel_wm_need_update() needs to also look for plane size changes
   (previously it only checked tiling & rotation).

With the above changes, the need for sprite watermark updates should be
properly flagged at atomic 'check' time and handled at 'commit' time in
post-evasion, so we can drop the direct calls to
intel_update_sprite_watermarks() from all of the
intel_plane->update_plane() handlers.  We'll also toss a
WARN_ON(irqs_disabled()) into the watermark update functions to avoid
such mistakes in the future.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 23 ++++++++++++++++-------
 drivers/gpu/drm/i915/intel_pm.c      |  2 ++
 drivers/gpu/drm/i915/intel_sprite.c  | 12 ------------
 3 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e8e01c..181aedd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11575,13 +11575,17 @@ retry:
 static bool intel_wm_need_update(struct drm_plane *plane,
 				 struct drm_plane_state *state)
 {
-	/* Update watermarks on tiling changes. */
+	struct intel_plane_state *new = to_intel_plane_state(state);
+	struct intel_plane_state *cur = to_intel_plane_state(plane->state);
+
+	/* Update watermarks on tiling changes or size changes. */
 	if (!plane->state->fb || !state->fb ||
 	    plane->state->fb->modifier[0] != state->fb->modifier[0] ||
-	    plane->state->rotation != state->rotation)
-		return true;
-
-	if (plane->state->crtc_w != state->crtc_w)
+	    plane->state->rotation != state->rotation ||
+	    drm_rect_width(&new->src) != drm_rect_width(&cur->src) ||
+	    drm_rect_height(&new->src) != drm_rect_height(&cur->src) ||
+	    drm_rect_width(&new->dst) != drm_rect_width(&cur->dst) ||
+	    drm_rect_height(&new->dst) != drm_rect_height(&cur->dst))
 		return true;
 
 	return false;
@@ -11645,8 +11649,13 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 			 plane->base.id, was_visible, visible,
 			 turn_off, turn_on, mode_changed);
 
-	if (intel_wm_need_update(plane, plane_state))
-		intel_crtc->atomic.update_wm = true;
+	if (intel_wm_need_update(plane, plane_state)) {
+		if (plane->type == DRM_PLANE_TYPE_OVERLAY)
+			intel_crtc->atomic.update_sprite_watermarks |=
+				1 << i;
+		else
+			intel_crtc->atomic.update_wm = true;
+	}
 
 	switch (plane->type) {
 	case DRM_PLANE_TYPE_PRIMARY:
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4d3cb70..2470ede 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3750,6 +3750,7 @@ void intel_update_watermarks(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 
+	WARN_ON(irqs_disabled());
 	if (dev_priv->display.update_wm)
 		dev_priv->display.update_wm(crtc);
 }
@@ -3770,6 +3771,7 @@ void intel_update_sprite_watermarks(struct drm_plane *plane,
 	unsigned int dst_h = drm_rect_height(&state->dst);
 	bool scaled = (src_w != dst_w || src_h != dst_h);
 
+	WARN_ON(irqs_disabled());
 	if (dev_priv->display.update_sprite_wm)
 		dev_priv->display.update_sprite_wm(plane, crtc,
 						   sprite_width, sprite_height,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index b627067..ce2fa92 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -199,8 +199,6 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	rotation = drm_plane->state->rotation;
 	plane_ctl |= skl_plane_ctl_rotation(rotation);
 
-	intel_update_sprite_watermarks(drm_plane, crtc);
-
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
 					       fb->pixel_format);
 
@@ -282,8 +280,6 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 
 	I915_WRITE(PLANE_SURF(pipe, plane), 0);
 	POSTING_READ(PLANE_SURF(pipe, plane));
-
-	intel_update_sprite_watermarks(dplane, crtc);
 }
 
 static void
@@ -399,8 +395,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 	if (obj->tiling_mode != I915_TILING_NONE)
 		sprctl |= SP_TILED;
 
-	intel_update_sprite_watermarks(dplane, crtc);
-
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -465,8 +459,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 
 	I915_WRITE(SPSURF(pipe, plane), 0);
 	POSTING_READ(SPSURF(pipe, plane));
-
-	intel_update_sprite_watermarks(dplane, crtc);
 }
 
 static void
@@ -530,8 +522,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		sprctl |= SPRITE_PIPE_CSC_ENABLE;
 
-	intel_update_sprite_watermarks(plane, crtc);
-
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -665,8 +655,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	if (IS_GEN6(dev))
 		dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
 
-	intel_update_sprite_watermarks(plane, crtc);
-
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
-- 
2.1.4

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

  parent reply	other threads:[~2015-06-23  1:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-23  1:30 [PATCH 0/2] Prevent (sleeping) wm updates under vblank evasion Matt Roper
2015-06-23  1:30 ` [PATCH 1/2] drm/i915: Drop parameters to intel_update_sprite_watermarks() Matt Roper
2015-06-23  1:30 ` Matt Roper [this message]
2015-06-23  7:34   ` [PATCH 2/2] drm/i915: Update sprite watermarks outside vblank evasion Daniel Vetter
2015-06-23 14:29     ` Matt Roper
2015-06-23 15:24       ` Daniel Vetter

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=1435023033-18043-3-git-send-email-matthew.d.roper@intel.com \
    --to=matthew.d.roper@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /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