From: Ville Syrj?l? <ville.syrjala at linux.intel.com>
Sadly the FBC_RT_BASE register is part of the hardware context. A
context switch can therefore restore a stale value to the register.
To fix things up, always add an LRI after MI_SET_CONTEXT to update
the register value.
There's still a problem though. If we flip to another buffer between
inserting the LRI to the ring, and the CS actually executing it, a stale
value will again be restored. To fix that, insert another LRI when
updating the FBC state. This won't allow the stale value to persist
indefinitely. So any rendering happening between restoring the stale value
and the LRI fixing it up, will use the stale value. But said rendering
can't be targetting the current front buffer anyway as that would imply
that we queued up some rendering to the new front buffer, and then failed
to wait for it to finish before the flip actually happened.
Signed-off-by: Ville Syrj?l? <ville.syrjala at linux.intel.com>
---
Let's call this option a). The other options are:
b) Update FBC_RT_BASE at every execbuffer
c) Update all saved contexts manually just before we write FBC_RT_BASE
The docs dicourage manually poking the contexts though
d) some other approach I didn't consider?
I didn't actually test this (it compiles though :). Would be nice to
have a test case that targets these problems...
I also plugged it into gen7 codepaths too, in case someone would want to
test it there.
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_gem_context.c | 30 ++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++++++++++--
3 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 360b582..953b1c0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1005,6 +1005,7 @@ typedef struct drm_i915_private {
unsigned int cfb_fb;
enum plane cfb_plane;
int cfb_y;
+ uint32_t cfb_rt_base;
struct intel_fbc_work *fbc_work;
struct intel_opregion opregion;
@@ -1733,6 +1734,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
void i915_gem_context_init(struct drm_device *dev);
void i915_gem_context_fini(struct drm_device *dev);
void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
+int i915_gem_update_fbc_rt_base(struct intel_ring_buffer *ring);
int i915_switch_context(struct intel_ring_buffer *ring,
struct drm_file *file, int to_id);
void i915_gem_context_free(struct kref *ctx_ref);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 64cb190..cf3ad01 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -311,6 +311,30 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
return (struct i915_hw_context *)idr_find(&file_priv->context_idr, id);
}
+int
+i915_gem_update_fbc_rt_base(struct intel_ring_buffer *ring)
+{
+ struct drm_device *dev = ring->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ int ret;
+
+ ret = intel_ring_begin(ring, 4);
+ if (ret)
+ return ret;
+
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+ if (INTEL_INFO(dev)->gen >= 7)
+ intel_ring_emit(ring, IVB_FBC_RT_BASE);
+ else
+ intel_ring_emit(ring, ILK_FBC_RT_BASE);
+ intel_ring_emit(ring, dev_priv->cfb_rt_base);
+ intel_ring_emit(ring, MI_NOOP);
+
+ intel_ring_advance(ring);
+
+ return 0;
+}
+
static inline int
mi_set_context(struct intel_ring_buffer *ring,
struct i915_hw_context *new_context,
@@ -356,6 +380,12 @@ mi_set_context(struct intel_ring_buffer *ring,
intel_ring_advance(ring);
+ /*
+ * FBC_RT_BASE is part of the context,
+ * so reload it after each context switch.
+ */
+ i915_gem_update_fbc_rt_base(ring);
+
return ret;
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e198f38..9a8a381 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -217,7 +217,8 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
(stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
(interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y);
- I915_WRITE(ILK_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
+ dev_priv->cfb_rt_base = obj->gtt_offset | ILK_FBC_RT_VALID;
+ I915_WRITE(ILK_FBC_RT_BASE, dev_priv->cfb_rt_base);
/* enable it... */
I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
@@ -226,6 +227,13 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
SNB_CPU_FENCE_ENABLE | obj->fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
sandybridge_blit_fbc_update(dev);
+
+ /*
+ * There may be FBC_RT_BASE LRI in the ring with
+ * a stale value. Put in another one to make things
+ * right.
+ */
+ i915_gem_update_fbc_rt_base(&dev_priv->ring[RCS]);
}
DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
@@ -254,6 +262,8 @@ static void ironlake_disable_fbc(struct drm_device *dev)
I915_READ(HSW_CLKGATE_DISABLE_PART_1) &
~HSW_DPFC_GATING_DISABLE);
+ dev_priv->cfb_rt_base = 0;
+
DRM_DEBUG_KMS("disabled FBC\n");
}
}
@@ -274,7 +284,8 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
struct drm_i915_gem_object *obj = intel_fb->obj;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
+ dev_priv->cfb_rt_base = obj->gtt_offset | ILK_FBC_RT_VALID;
+ I915_WRITE(IVB_FBC_RT_BASE, dev_priv->cfb_rt_base);
I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
IVB_DPFC_CTL_FENCE_EN |
@@ -303,6 +314,13 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
sandybridge_blit_fbc_update(dev);
+ /*
+ * There may be FBC_RT_BASE LRI in the ring with
+ * a stale value. Put in another one to make things
+ * right.
+ */
+ i915_gem_update_fbc_rt_base(&dev_priv->ring[RCS]);
+
DRM_DEBUG_KMS("enabled fbc on plane %d\n", intel_crtc->plane);
}
--
1.8.1.5
