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@xxxxxxxxx> --- 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx