From 212890cfcd25b148665585ac3b2ecb65bad0d538 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Tue, 19 Jan 2016 11:35:43 -0200 Subject: [PATCH] drm/i915/fbc: split intel_fbc_update into pre and post update So now pre_update will be responsible for unconditionally deactivating FBC and updating the state cache, while post_update will be responsible for checking if it can be enabled, then enabling it. This is one more step into proper locking. Notice that intel_fbc_flush now calls post_update directly. The FBC flush can only happen for drawing operations - since we explicitly ignore the flips -, so the FBC state is not expected to have changed at this point. With this we can just run post_update, which will make sure we won't deactivate+reactivate FBC as would be the case now if we called pre_update + post_update. Reviewed-by: Maarten Lankhorst Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-11-git-send-email-paulo.r.zanoni@intel.com --- drivers/gpu/drm/i915/intel_fbc.c | 77 +++++++++++--------------------- 1 file changed, 26 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 49d445a1854e..61523cd95ac1 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -896,24 +896,16 @@ static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params *params1, return memcmp(params1, params2, sizeof(*params1)) == 0; } -/** - * __intel_fbc_update - activate/deactivate FBC as needed, unlocked - * @crtc: the CRTC that triggered the update - * - * This function completely reevaluates the status of FBC, then activates, - * deactivates or maintains it on the same state. - */ -static void __intel_fbc_update(struct intel_crtc *crtc) +static void intel_fbc_pre_update(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; struct intel_fbc *fbc = &dev_priv->fbc; - struct intel_fbc_reg_params old_params; WARN_ON(!mutex_is_locked(&fbc->lock)); if (!multiple_pipes_ok(dev_priv)) { set_no_fbc_reason(dev_priv, "more than one pipe active"); - goto out_disable; + goto deactivate; } if (!fbc->enabled || fbc->crtc != crtc) @@ -921,8 +913,25 @@ static void __intel_fbc_update(struct intel_crtc *crtc) intel_fbc_update_state_cache(crtc); - if (!intel_fbc_can_activate(crtc)) - goto out_disable; +deactivate: + __intel_fbc_deactivate(dev_priv); +} + +static void intel_fbc_post_update(struct intel_crtc *crtc) +{ + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + struct intel_fbc *fbc = &dev_priv->fbc; + struct intel_fbc_reg_params old_params; + + WARN_ON(!mutex_is_locked(&fbc->lock)); + + if (!fbc->enabled || fbc->crtc != crtc) + return; + + if (!intel_fbc_can_activate(crtc)) { + WARN_ON(fbc->active); + return; + } old_params = fbc->params; intel_fbc_get_reg_params(crtc, &fbc->params); @@ -936,44 +945,9 @@ static void __intel_fbc_update(struct intel_crtc *crtc) intel_fbc_reg_params_equal(&old_params, &fbc->params)) return; - if (intel_fbc_is_active(dev_priv)) { - /* 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("deactivating FBC for update\n"); - __intel_fbc_deactivate(dev_priv); - } - + __intel_fbc_deactivate(dev_priv); intel_fbc_schedule_activation(crtc); - fbc->no_fbc_reason = "FBC enabled (not necessarily active)"; - return; - -out_disable: - /* Multiple disables should be harmless */ - if (intel_fbc_is_active(dev_priv)) { - DRM_DEBUG_KMS("unsupported config, deactivating FBC\n"); - __intel_fbc_deactivate(dev_priv); - } + fbc->no_fbc_reason = "FBC enabled (active or scheduled)"; } /* @@ -991,7 +965,8 @@ void intel_fbc_update(struct intel_crtc *crtc) return; mutex_lock(&fbc->lock); - __intel_fbc_update(crtc); + intel_fbc_pre_update(crtc); + intel_fbc_post_update(crtc); mutex_unlock(&fbc->lock); } @@ -1045,7 +1020,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv, if (fbc->active) intel_fbc_recompress(dev_priv); else - __intel_fbc_update(fbc->crtc); + intel_fbc_post_update(fbc->crtc); } mutex_unlock(&fbc->lock); -- 2.20.1