drm/i915/fbc: Fix fence_y_offset handling

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>

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index d486d675166f738d716f3b105dd0cd7c69b38860..4fdc9ccfea2aeb5538322bdd014efc0f940623f1 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3822,6 +3822,17 @@ skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state,
        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->uapi.plane->dev);

diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index b7a6d56bac5f3e27641a86a7056ed19c4c749839..f68007ff8a1303f9cb68b9d05fb1d472c45c11a9 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -615,6 +615,7 @@ unsigned int i9xx_plane_max_stride(struct intel_plane *plane,
                                   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);

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 30649e17cfb748b9a78163af2f17f89a11585d21..2312e70e2e5ed2396b5a6a3c766d55d4b09d3799 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -47,19 +47,6 @@
 #include "intel_fbc.h"
 #include "intel_frontbuffer.h"
 
-/*
- * 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
@@ -141,7 +128,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
                        fbc_ctl2 |= FBC_CTL_CPU_FENCE;
                intel_de_write(dev_priv, FBC_CONTROL2, fbc_ctl2);
                intel_de_write(dev_priv, FBC_FENCE_OFF,
-                              params->crtc.fence_y_offset);
+                              params->fence_y_offset);
        }
 
        /* enable it... */
@@ -175,7 +162,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
        if (params->fence_id >= 0) {
                dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fence_id;
                intel_de_write(dev_priv, DPFC_FENCE_YOFF,
-                              params->crtc.fence_y_offset);
+                              params->fence_y_offset);
        } else {
                intel_de_write(dev_priv, DPFC_FENCE_YOFF, 0);
        }
@@ -243,7 +230,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
                        intel_de_write(dev_priv, SNB_DPFC_CTL_SA,
                                       SNB_CPU_FENCE_ENABLE | params->fence_id);
                        intel_de_write(dev_priv, DPFC_CPU_FENCE_OFFSET,
-                                      params->crtc.fence_y_offset);
+                                      params->fence_y_offset);
                }
        } else {
                if (IS_GEN(dev_priv, 6)) {
@@ -253,7 +240,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
        }
 
        intel_de_write(dev_priv, ILK_DPFC_FENCE_YOFF,
-                      params->crtc.fence_y_offset);
+                      params->fence_y_offset);
        /* enable it... */
        intel_de_write(dev_priv, ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
@@ -320,7 +307,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
                intel_de_write(dev_priv, SNB_DPFC_CTL_SA,
                               SNB_CPU_FENCE_ENABLE | params->fence_id);
                intel_de_write(dev_priv, DPFC_CPU_FENCE_OFFSET,
-                              params->crtc.fence_y_offset);
+                              params->fence_y_offset);
        } else if (dev_priv->ggtt.num_fences) {
                intel_de_write(dev_priv, SNB_DPFC_CTL_SA, 0);
                intel_de_write(dev_priv, DPFC_CPU_FENCE_OFFSET, 0);
@@ -631,8 +618,8 @@ static bool rotation_is_valid(struct drm_i915_private *dev_priv,
 /*
  * 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)
 {
@@ -705,7 +692,6 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
        cache->plane.src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
        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->uapi.src.y1 >> 16;
 
        cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode;
 
@@ -713,6 +699,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
        cache->fb.stride = fb->pitches[0];
        cache->fb.modifier = fb->modifier;
 
+       cache->fence_y_offset = intel_plane_fence_y_offset(plane_state);
+
        drm_WARN_ON(&dev_priv->drm, plane_state->flags & PLANE_HAS_FENCE &&
                    !plane_state->vma->fence);
 
@@ -883,10 +871,10 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
        memset(params, 0, sizeof(*params));
 
        params->fence_id = cache->fence_id;
+       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;

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9aad3ec979bdb167fc9562402d0af342cf5faa63..087608d98f091a3af6fc810f14685b95e34aea7b 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -411,8 +411,6 @@ struct intel_fbc {
                        int adjusted_x;
                        int adjusted_y;
 
-                       int y;
-
                        u16 pixel_blend_mode;
                } plane;
 
@@ -421,6 +419,8 @@ struct intel_fbc {
                        unsigned int stride;
                        u64 modifier;
                } fb;
+
+               unsigned int fence_y_offset;
                u16 gen9_wa_cfb_stride;
                s8 fence_id;
        } state_cache;
@@ -436,7 +436,6 @@ struct intel_fbc {
                struct {
                        enum pipe pipe;
                        enum i9xx_plane_id i9xx_plane;
-                       unsigned int fence_y_offset;
                } crtc;
 
                struct {
@@ -445,6 +444,7 @@ struct intel_fbc {
                } fb;
 
                int cfb_size;
+               unsigned int fence_y_offset;
                u16 gen9_wa_cfb_stride;
                s8 fence_id;
                bool plane_visible;