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>
(cherry picked from commit 5331889b5ffb11d6257953e418291a9f04c02bed)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 9ea1a397d1b54e813822a42eb5cf05a85b7fe876..26996e1839e2232637c331111666c6422e3f810e 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 efb4da205ea292e7a7cecb819249bc1694e66223..3a06f72c985965818cc2068f3e77538de33c0621 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -608,6 +608,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 1c26673acb2dd8bd603c427a1eddae38b86761e6..a65d9d8b79a7674e34e84088572af5ec0fd745ea 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 adb9bf34cf97a3bb21690c8ec73bbeb382a0f7b3..f79f118bf19278ea994eaa88364166f9d8c71f65 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -410,8 +410,6 @@ struct intel_fbc {
                        int adjusted_x;
                        int adjusted_y;
 
-                       int y;
-
                        u16 pixel_blend_mode;
                } plane;
 
@@ -420,6 +418,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;
@@ -435,7 +435,6 @@ struct intel_fbc {
                struct {
                        enum pipe pipe;
                        enum i9xx_plane_id i9xx_plane;
-                       unsigned int fence_y_offset;
                } crtc;
 
                struct {
@@ -444,6 +443,7 @@ struct intel_fbc {
                } fb;
 
                int cfb_size;
+               unsigned int fence_y_offset;
                u16 gen9_wa_cfb_stride;
                s8 fence_id;
                bool plane_visible;