From: Chris Wilson <chris@chris-wilson.co.uk>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH] drm/i915: Share the common work of disabling active FBC before updating
Date: Thu, 7 Jul 2011 22:14:18 +0100 [thread overview]
Message-ID: <1310073258-19976-1-git-send-email-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <yuny609lsol.fsf@aiko.keithp.com>
Upon review, all path share the same dependencies for updating the
registers and so we can benefit from sharing the code and checking
early.
This removes the unsightly intel_wait_for_vblank() from the lowlevel
functions and upon further analysis the only path that will require a
wait is if we are performing an instantaneous transition between two
valid FBC configurations. The page-flip path itself will have disabled
FBC registers and will have waited for at least one vblank before
finishing the flip and attempting to re-enable FBC.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
Respun without the blocking wait. Noting instead that since we delay
enabling FBC by 50ms we can assume that a vblank passes after disabling
FBC and before modifying the control registers.
-Chris
---
drivers/gpu/drm/i915/i915_drv.h | 6 +-
drivers/gpu/drm/i915/intel_display.c | 122 +++++++++++++++++----------------
2 files changed, 65 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2b6b3da..ad73a2b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -329,10 +329,8 @@ typedef struct drm_i915_private {
uint32_t last_instdone1;
unsigned long cfb_size;
- unsigned long cfb_pitch;
- unsigned long cfb_offset;
- int cfb_fence;
- int cfb_plane;
+ unsigned int cfb_fb;
+ enum plane cfb_plane;
int cfb_y;
struct intel_fbc_work *fbc_work;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 315191c..cbea4e6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1410,27 +1410,17 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
struct drm_i915_gem_object *obj = intel_fb->obj;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ int cfb_pitch;
int plane, i;
u32 fbc_ctl, fbc_ctl2;
- if (fb->pitch == dev_priv->cfb_pitch &&
- obj->fence_reg == dev_priv->cfb_fence &&
- intel_crtc->plane == dev_priv->cfb_plane &&
- I915_READ(FBC_CONTROL) & FBC_CTL_EN)
- return;
-
- i8xx_disable_fbc(dev);
-
- dev_priv->cfb_pitch = dev_priv->cfb_size / FBC_LL_SIZE;
-
- if (fb->pitch < dev_priv->cfb_pitch)
- dev_priv->cfb_pitch = fb->pitch;
+ cfb_pitch = dev_priv->cfb_size / FBC_LL_SIZE;
+ if (fb->pitch < cfb_pitch)
+ cfb_pitch = fb->pitch;
/* FBC_CTL wants 64B units */
- dev_priv->cfb_pitch = (dev_priv->cfb_pitch / 64) - 1;
- dev_priv->cfb_fence = obj->fence_reg;
- dev_priv->cfb_plane = intel_crtc->plane;
- plane = dev_priv->cfb_plane == 0 ? FBC_CTL_PLANEA : FBC_CTL_PLANEB;
+ cfb_pitch = (cfb_pitch / 64) - 1;
+ plane = intel_crtc->plane == 0 ? FBC_CTL_PLANEA : FBC_CTL_PLANEB;
/* Clear old tags */
for (i = 0; i < (FBC_LL_SIZE / 32) + 1; i++)
@@ -1438,8 +1428,7 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
/* Set it up... */
fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | plane;
- if (obj->tiling_mode != I915_TILING_NONE)
- fbc_ctl2 |= FBC_CTL_CPU_FENCE;
+ fbc_ctl2 |= FBC_CTL_CPU_FENCE;
I915_WRITE(FBC_CONTROL2, fbc_ctl2);
I915_WRITE(FBC_FENCE_OFF, crtc->y);
@@ -1447,14 +1436,13 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
fbc_ctl = FBC_CTL_EN | FBC_CTL_PERIODIC;
if (IS_I945GM(dev))
fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
- fbc_ctl |= (dev_priv->cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
+ fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
fbc_ctl |= (interval & 0x2fff) << FBC_CTL_INTERVAL_SHIFT;
- if (obj->tiling_mode != I915_TILING_NONE)
- fbc_ctl |= dev_priv->cfb_fence;
+ fbc_ctl |= obj->fence_reg;
I915_WRITE(FBC_CONTROL, fbc_ctl);
- DRM_DEBUG_KMS("enabled FBC, pitch %ld, yoff %d, plane %d, ",
- dev_priv->cfb_pitch, crtc->y, dev_priv->cfb_plane);
+ DRM_DEBUG_KMS("enabled FBC, pitch %d, yoff %d, plane %d, ",
+ cfb_pitch, crtc->y, intel_crtc->plane);
}
static bool i8xx_fbc_enabled(struct drm_device *dev)
@@ -1476,23 +1464,8 @@ static void g4x_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
unsigned long stall_watermark = 200;
u32 dpfc_ctl;
- dpfc_ctl = I915_READ(DPFC_CONTROL);
- if (dpfc_ctl & DPFC_CTL_EN) {
- if (dev_priv->cfb_fence == obj->fence_reg &&
- dev_priv->cfb_plane == intel_crtc->plane &&
- dev_priv->cfb_y == crtc->y)
- return;
-
- I915_WRITE(DPFC_CONTROL, dpfc_ctl & ~DPFC_CTL_EN);
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- }
-
- dev_priv->cfb_fence = obj->fence_reg;
- dev_priv->cfb_plane = intel_crtc->plane;
- dev_priv->cfb_y = crtc->y;
-
dpfc_ctl = plane | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
- dpfc_ctl |= DPFC_CTL_FENCE_EN | dev_priv->cfb_fence;
+ dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
I915_WRITE(DPFC_CHICKEN, DPFC_HT_MODIFY);
I915_WRITE(DPFC_RECOMP_CTL, DPFC_RECOMP_STALL_EN |
@@ -1561,27 +1534,11 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
u32 dpfc_ctl;
dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
- if (dpfc_ctl & DPFC_CTL_EN) {
- if (dev_priv->cfb_fence == obj->fence_reg &&
- dev_priv->cfb_plane == intel_crtc->plane &&
- dev_priv->cfb_offset == obj->gtt_offset &&
- dev_priv->cfb_y == crtc->y)
- return;
-
- I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl & ~DPFC_CTL_EN);
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- }
-
- dev_priv->cfb_fence = obj->fence_reg;
- dev_priv->cfb_plane = intel_crtc->plane;
- dev_priv->cfb_offset = obj->gtt_offset;
- dev_priv->cfb_y = crtc->y;
-
dpfc_ctl &= DPFC_RESERVED;
dpfc_ctl |= (plane | DPFC_CTL_LIMIT_1X);
/* Set persistent mode for front-buffer rendering, ala X. */
dpfc_ctl |= DPFC_CTL_PERSISTENT_MODE;
- dpfc_ctl |= (DPFC_CTL_FENCE_EN | dev_priv->cfb_fence);
+ dpfc_ctl |= (DPFC_CTL_FENCE_EN | obj->fence_reg);
I915_WRITE(ILK_DPFC_CHICKEN, DPFC_HT_MODIFY);
I915_WRITE(ILK_DPFC_RECOMP_CTL, DPFC_RECOMP_STALL_EN |
@@ -1594,7 +1551,7 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
if (IS_GEN6(dev)) {
I915_WRITE(SNB_DPFC_CTL_SA,
- SNB_CPU_FENCE_ENABLE | dev_priv->cfb_fence);
+ SNB_CPU_FENCE_ENABLE | obj->fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
sandybridge_blit_fbc_update(dev);
}
@@ -1647,10 +1604,15 @@ static void intel_fbc_work_fn(struct work_struct *__work)
/* Double check that we haven't switched fb without cancelling
* the prior work.
*/
- if (work->crtc->fb == work->fb)
+ if (work->crtc->fb == work->fb) {
dev_priv->display.enable_fbc(work->crtc,
work->interval);
+ dev_priv->cfb_plane = to_intel_crtc(work->crtc)->plane;
+ dev_priv->cfb_fb = work->crtc->fb->base.id;
+ dev_priv->cfb_y = work->crtc->y;
+ }
+
dev_priv->fbc_work = NULL;
}
mutex_unlock(&dev->struct_mutex);
@@ -1708,7 +1670,10 @@ static void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
DRM_DEBUG_KMS("scheduling delayed FBC enable\n");
/* Delay the actual enabling to let pageflipping cease and the
- * display to settle before starting the compression.
+ * display to settle before starting the compression. Note that
+ * this delay also serves a second purpose: it allows for a
+ * vblank to pass after disabling the FBC before we attempt
+ * to modify the control registers.
*
* A more complicated solution would involve tracking vblanks
* following the termination of the page-flipping sequence
@@ -1728,6 +1693,7 @@ void intel_disable_fbc(struct drm_device *dev)
return;
dev_priv->display.disable_fbc(dev);
+ dev_priv->cfb_plane = -1;
}
/**
@@ -1836,6 +1802,44 @@ static void intel_update_fbc(struct drm_device *dev)
if (in_dbg_master())
goto out_disable;
+ /* If the scanout has not changed, don't modify the FBC settings.
+ * Note that we make the fundamental assumption that the fb->obj
+ * cannot be unpinned (and have its GTT offset and fence revoked)
+ * without first being decoupled from the scanout and FBC disabled.
+ */
+ if (dev_priv->cfb_plane == intel_crtc->plane &&
+ dev_priv->cfb_fb == fb->base.id &&
+ dev_priv->cfb_y == crtc->y)
+ return;
+
+ if (intel_fbc_enabled(dev)) {
+ /* We update FBC along two paths, after changing fb/crtc
+ * configuration (modeswitching) and after page-flipping
+ * finishes. For the latter, we know that not only did
+ * we disable the FBC at the start of the page-flip
+ * sequence, but also more than one vblank has passed.
+ *
+ * For the former case of modeswitching, it is possible
+ * to switch between two FBC valid configurations
+ * instantaneously so we do need to disable the FBC
+ * before we can modify its control registers. We also
+ * have to wait for the next vblank for that to take
+ * effect. However, since we delay enabling FBC we can
+ * assume that a vblank has passed since disabling and
+ * that we can safely alter the registers in the deferred
+ * callback.
+ *
+ * In the scenario that we go from a valid to invalid
+ * and then back to valid FBC configuration we have
+ * no strict enforcement that a vblank occurred since
+ * disabling the FBC. However, along all current pipe
+ * disabling paths we do need to wait for a vblank at
+ * some point. And we wait before enabling FBC anyway.
+ */
+ DRM_DEBUG_KMS("disabling active FBC for update\n");
+ intel_disable_fbc(dev);
+ }
+
intel_enable_fbc(crtc, 500);
return;
--
1.7.5.4
next prev parent reply other threads:[~2011-07-07 21:14 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-07 11:48 FBC fixes for review and testing Chris Wilson
2011-07-07 11:48 ` [PATCH 1/6] drm/i915: Remove vestigial pitch from post-gen2 FBC control routines Chris Wilson
2011-07-07 16:12 ` Jesse Barnes
2011-07-07 11:48 ` [PATCH 2/6] drm/i915: Use of a CPU fence is mandatory to update FBC regions upon CPU writes Chris Wilson
2011-07-07 16:13 ` Jesse Barnes
2011-07-07 11:48 ` [PATCH 3/6] drm/i915: Set persistent-mode for ILK/SNB framebuffer compression Chris Wilson
2011-07-07 16:14 ` Jesse Barnes
2011-07-07 11:48 ` [PATCH 4/6] drm/i915: Disable FBC across page-flipping Chris Wilson
2011-07-07 16:15 ` Jesse Barnes
2011-07-07 11:48 ` [PATCH 5/6] drm/i915: Only export the generic intel_disable_fbc() interface Chris Wilson
2011-07-07 16:16 ` Jesse Barnes
2011-07-07 17:19 ` [PATCH] drm/i915: Replace direct calls to vfunc.disable_fbc with intel_disable_fbc() Chris Wilson
2011-07-07 11:48 ` [PATCH 6/6] drm/i915: Perform intel_enable_fbc() from a delayed task Chris Wilson
2011-07-07 16:20 ` Jesse Barnes
2011-07-07 17:12 ` [PATCH] drm/i915: Flush any scheduled tasks during unload Chris Wilson
2011-07-07 15:45 ` FBC fixes for review and testing Keith Packard
2011-07-07 20:30 ` [PATCH] drm/i915: Share the common work of disabling active FBC before updating Chris Wilson
2011-07-07 20:52 ` Keith Packard
2011-07-07 21:14 ` Chris Wilson [this message]
2011-07-07 21:26 ` Keith Packard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1310073258-19976-1-git-send-email-chris@chris-wilson.co.uk \
--to=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).