From 9eb0463cfe65d826c97fa26b904a64f52c94300d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
Date: Wed, 29 Apr 2020 13:10:25 +0300
Subject: drm/i915/fbc: Fix fence_y_offset handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Git-commit: 9eb0463cfe65d826c97fa26b904a64f52c94300d
Patch-mainline: v5.8-rc5
References: bsc#1152489
The current fence_y_offset calculation is broken. I think it more or
less used to do the right thing, but then I changed the plane code
to put the final x/y source offsets back into the src rectangle so
now it's just subtraacting the same value from itself. The code would
never have worked if we allowed the framebuffer to have a non-zero
offset.
Let's do this in a better way by just calculating the fence_y_offset
from the final plane surface offset. Note that we don't align the
plane surface address to fence rows so with horizontal panning there's
often a horizontal offset from the fence start to the surface address
as well. We have no way to tell the hardware about that so we just
ignore it. Based on some quick tests the invlidation still happens
correctly. I presume due to the invalidation nuking at least the full
line (or a segment of multiple lines).
Fixes: 54d4d719fa11 ("drm/i915: Overcome display engine stride limits via GTT remapping")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200429101034.8208-4-ville.syrjala@linux.intel.com
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
(cherry picked from commit 5331889b5ffb11d6257953e418291a9f04c02bed)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/i915/display/intel_display.c | 11 ++++++++
drivers/gpu/drm/i915/display/intel_display.h | 1
drivers/gpu/drm/i915/display/intel_fbc.c | 34 +++++++++------------------
drivers/gpu/drm/i915/i915_drv.h | 6 ++--
4 files changed, 27 insertions(+), 25 deletions(-)
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3385,6 +3385,17 @@ static bool skl_check_main_ccs_coordinat
return true;
}
+unsigned int
+intel_plane_fence_y_offset(const struct intel_plane_state *plane_state)
+{
+ int x = 0, y = 0;
+
+ intel_plane_adjust_aligned_offset(&x, &y, plane_state, 0,
+ plane_state->color_plane[0].offset, 0);
+
+ return y;
+}
+
static int skl_check_main_surface(struct intel_plane_state *plane_state)
{
struct drm_i915_private *dev_priv = to_i915(plane_state->base.plane->dev);
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -537,6 +537,7 @@ unsigned int i9xx_plane_max_stride(struc
u32 pixel_format, u64 modifier,
unsigned int rotation);
int bdw_get_pipemisc_bpp(struct intel_crtc *crtc);
+unsigned int intel_plane_fence_y_offset(const struct intel_plane_state *plane_state);
struct intel_display_error_state *
intel_display_capture_error_state(struct drm_i915_private *dev_priv);
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -56,19 +56,6 @@ static inline bool no_fbc_on_multiple_pi
}
/*
- * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
- * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
- * origin so the x and y offsets can actually fit the registers. As a
- * consequence, the fence doesn't really start exactly at the display plane
- * address we program because it starts at the real start of the buffer, so we
- * have to take this into consideration here.
- */
-static unsigned int get_crtc_fence_y_offset(struct intel_fbc *fbc)
-{
- return fbc->state_cache.plane.y - fbc->state_cache.plane.adjusted_y;
-}
-
-/*
* For SKL+, the plane source size used by the hardware is based on the value we
* write to the PLANE_SIZE register. For BDW-, the hardware looks at the value
* we wrote to PIPESRC.
@@ -146,7 +133,7 @@ static void i8xx_fbc_activate(struct drm
fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.i9xx_plane);
I915_WRITE(FBC_CONTROL2, fbc_ctl2);
- I915_WRITE(FBC_FENCE_OFF, params->crtc.fence_y_offset);
+ I915_WRITE(FBC_FENCE_OFF, params->fence_y_offset);
}
/* enable it... */
@@ -178,7 +165,7 @@ static void g4x_fbc_activate(struct drm_
if (params->flags & PLANE_HAS_FENCE) {
dpfc_ctl |= DPFC_CTL_FENCE_EN | params->vma->fence->id;
- I915_WRITE(DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
+ I915_WRITE(DPFC_FENCE_YOFF, params->fence_y_offset);
} else {
I915_WRITE(DPFC_FENCE_YOFF, 0);
}
@@ -243,7 +230,7 @@ static void ilk_fbc_activate(struct drm_
SNB_CPU_FENCE_ENABLE |
params->vma->fence->id);
I915_WRITE(DPFC_CPU_FENCE_OFFSET,
- params->crtc.fence_y_offset);
+ params->fence_y_offset);
}
} else {
if (IS_GEN(dev_priv, 6)) {
@@ -252,7 +239,7 @@ static void ilk_fbc_activate(struct drm_
}
}
- I915_WRITE(ILK_DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
+ I915_WRITE(ILK_DPFC_FENCE_YOFF, params->fence_y_offset);
I915_WRITE(ILK_FBC_RT_BASE,
i915_ggtt_offset(params->vma) | ILK_FBC_RT_VALID);
/* enable it... */
@@ -322,7 +309,8 @@ static void gen7_fbc_activate(struct drm
I915_WRITE(SNB_DPFC_CTL_SA,
SNB_CPU_FENCE_ENABLE |
params->vma->fence->id);
- I915_WRITE(DPFC_CPU_FENCE_OFFSET, params->crtc.fence_y_offset);
+ I915_WRITE(DPFC_CPU_FENCE_OFFSET,
+ params->fence_y_offset);
} else {
I915_WRITE(SNB_DPFC_CTL_SA,0);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, 0);
@@ -623,8 +611,8 @@ static bool pixel_format_is_valid(struct
/*
* For some reason, the hardware tracking starts looking at whatever we
* programmed as the display plane base address register. It does not look at
- * the X and Y offset registers. That's why we look at the crtc->adjusted{x,y}
- * variables instead of just looking at the pipe/plane size.
+ * the X and Y offset registers. That's why we include the src x/y offsets
+ * instead of just looking at the plane size.
*/
static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
{
@@ -681,7 +669,6 @@ static void intel_fbc_update_state_cache
cache->plane.visible = plane_state->base.visible;
cache->plane.adjusted_x = plane_state->color_plane[0].x;
cache->plane.adjusted_y = plane_state->color_plane[0].y;
- cache->plane.y = plane_state->base.src.y1 >> 16;
cache->plane.pixel_blend_mode = plane_state->base.pixel_blend_mode;
@@ -691,6 +678,8 @@ static void intel_fbc_update_state_cache
cache->fb.format = fb->format;
cache->fb.stride = fb->pitches[0];
+ cache->fence_y_offset = intel_plane_fence_y_offset(plane_state);
+
cache->vma = plane_state->vma;
cache->flags = plane_state->flags;
if (WARN_ON(cache->flags & PLANE_HAS_FENCE && !cache->vma->fence))
@@ -839,9 +828,10 @@ static void intel_fbc_get_reg_params(str
params->vma = cache->vma;
params->flags = cache->flags;
+ params->fence_y_offset = cache->fence_y_offset;
+
params->crtc.pipe = crtc->pipe;
params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)->i9xx_plane;
- params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
params->fb.format = cache->fb.format;
params->fb.stride = cache->fb.stride;
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -397,8 +397,6 @@ struct intel_fbc {
int adjusted_x;
int adjusted_y;
- int y;
-
u16 pixel_blend_mode;
} plane;
@@ -406,6 +404,8 @@ struct intel_fbc {
const struct drm_format_info *format;
unsigned int stride;
} fb;
+
+ unsigned int fence_y_offset;
} state_cache;
/*
@@ -422,7 +422,6 @@ struct intel_fbc {
struct {
enum pipe pipe;
enum i9xx_plane_id i9xx_plane;
- unsigned int fence_y_offset;
} crtc;
struct {
@@ -431,6 +430,7 @@ struct intel_fbc {
} fb;
int cfb_size;
+ unsigned int fence_y_offset;
unsigned int gen9_wa_cfb_stride;
} params;