drm/i915: introduce intel_fbc_{enable,disable}

The goal is to call FBC enable/disable only once per modeset, while
activate/deactivate/update will be called multiple times.

The enable() function will be responsible for deciding if a CRTC will
have FBC on it and then it will "lock" FBC on this CRTC: it won't be
possible to change FBC's CRTC until disable(). With this, all checks
and resource acquisition that only need to be done once per modeset
can be moved from update() to enable(). And then the update(),
activate() and deactivate() code will also get simpler since they
won't need to worry about the CRTC being changed.

The disable() function will do the reverse operation of enable(). One
of its features is that it should only be called while the pipe is
already off. This guarantees that FBC is stopped and nothing is
using the CFB.

With this, the activate() and deactivate() functions just start and
temporarily stop FBC. They are the ones touching the hardware enable
bit, so HW state reflects dev_priv->crtc.active.

The last function remaining is update(). A lot of times I thought
about renaming update() to activate() or try_to_activate() since it's
called when we want to activate FBC. The thing is that update() may
not only decide to activate FBC, but also deactivate or keep it on the
same state, so I'll leave this name for now.

Moving code to enable() and disable() will also help in case we decide
to move FBC to pipe_config or something else later.

The current patch only puts the very basic code on enable() and
disable(). The next commits will take care of moving more stuff from
update() to the new functions.

v2:
  - Rebase.
  - Improve commit message (Chris).
v3: Rebase after changing the patch order.
v4: Rebase again after upstream changes.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 03d6823077bdd20241e72f8bed47a575ed70e9fd..2493ad2f5c12d198159d304719f23c1669713538 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -915,6 +915,7 @@ struct i915_fbc {
 
        bool false_color;
 
+       bool enabled;
        bool active;
 
        struct intel_fbc_work {

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 246a9f70bdc09b25a88bfbd12c50a9d6c90bac9c..602e2be1c3d8425eaa5474f3890138e4ff76c1b7 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4820,7 +4820,7 @@ static void intel_pre_plane_update(struct intel_crtc *crtc)
        struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
 
        if (atomic->disable_fbc)
-               intel_fbc_disable_crtc(crtc);
+               intel_fbc_deactivate(crtc);
 
        if (crtc->atomic.disable_ips)
                hsw_disable_ips(crtc);
@@ -4928,6 +4928,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
        if (intel_crtc->config->has_pch_encoder)
                intel_wait_for_vblank(dev, pipe);
        intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
+
+       intel_fbc_enable(intel_crtc);
 }
 
 /* IPS only exists on ULT machines and is tied to pipe A. */
@@ -5040,6 +5042,8 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
                intel_wait_for_vblank(dev, hsw_workaround_pipe);
                intel_wait_for_vblank(dev, hsw_workaround_pipe);
        }
+
+       intel_fbc_enable(intel_crtc);
 }
 
 static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
@@ -5120,6 +5124,8 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
        }
 
        intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
+
+       intel_fbc_disable_crtc(intel_crtc);
 }
 
 static void haswell_crtc_disable(struct drm_crtc *crtc)
@@ -5170,6 +5176,8 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
        if (intel_crtc->config->has_pch_encoder)
                intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
                                                      true);
+
+       intel_fbc_disable_crtc(intel_crtc);
 }
 
 static void i9xx_pfit_enable(struct intel_crtc *crtc)
@@ -6262,6 +6270,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
        for_each_encoder_on_crtc(dev, crtc, encoder)
                encoder->enable(encoder);
+
+       intel_fbc_enable(intel_crtc);
 }
 
 static void i9xx_pfit_disable(struct intel_crtc *crtc)
@@ -6324,6 +6334,8 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 
        if (!IS_GEN2(dev))
                intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+
+       intel_fbc_disable_crtc(intel_crtc);
 }
 
 static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
@@ -11585,7 +11597,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
                          to_intel_plane(primary)->frontbuffer_bit);
        mutex_unlock(&dev->struct_mutex);
 
-       intel_fbc_disable_crtc(intel_crtc);
+       intel_fbc_deactivate(intel_crtc);
        intel_frontbuffer_flip_prepare(dev,
                                       to_intel_plane(primary)->frontbuffer_bit);
 

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9d77e2aa327ce7fa35a3b26ea4e8533341a98185..3517cd1ce266b50ed2f57aff6a1fa39ce345665b 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1323,8 +1323,10 @@ static inline void intel_fbdev_restore_mode(struct drm_device *dev)
 
 /* intel_fbc.c */
 bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
+void intel_fbc_deactivate(struct intel_crtc *crtc);
 void intel_fbc_update(struct intel_crtc *crtc);
 void intel_fbc_init(struct drm_i915_private *dev_priv);
+void intel_fbc_enable(struct intel_crtc *crtc);
 void intel_fbc_disable(struct drm_i915_private *dev_priv);
 void intel_fbc_disable_crtc(struct intel_crtc *crtc);
 void intel_fbc_invalidate(struct drm_i915_private *dev_priv,

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index d8486824291f4051e4fe6a286e1fb7104e6b0222..6125c7b15966c80773699dfa8aba10e4be4e3502 100644 (file)
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -399,7 +399,6 @@ static void intel_fbc_schedule_activation(struct intel_crtc *crtc)
        WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
 
        intel_fbc_cancel_work(dev_priv);
-       dev_priv->fbc.crtc = crtc;
 
        work = kzalloc(sizeof(*work), GFP_KERNEL);
        if (work == NULL) {
@@ -429,7 +428,7 @@ static void intel_fbc_schedule_activation(struct intel_crtc *crtc)
        schedule_delayed_work(&work->work, msecs_to_jiffies(50));
 }
 
-static void intel_fbc_deactivate(struct drm_i915_private *dev_priv)
+static void __intel_fbc_deactivate(struct drm_i915_private *dev_priv)
 {
        WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
 
@@ -439,35 +438,13 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv)
                dev_priv->fbc.deactivate(dev_priv);
 }
 
-static void __intel_fbc_disable(struct drm_i915_private *dev_priv)
-{
-       intel_fbc_deactivate(dev_priv);
-       dev_priv->fbc.crtc = NULL;
-}
-
-/**
- * intel_fbc_disable - disable FBC
- * @dev_priv: i915 device instance
- *
- * This function disables FBC.
- */
-void intel_fbc_disable(struct drm_i915_private *dev_priv)
-{
-       if (!fbc_supported(dev_priv))
-               return;
-
-       mutex_lock(&dev_priv->fbc.lock);
-       __intel_fbc_disable(dev_priv);
-       mutex_unlock(&dev_priv->fbc.lock);
-}
-
 /*
- * intel_fbc_disable_crtc - disable FBC if it's associated with crtc
+ * intel_fbc_deactivate - deactivate FBC if it's associated with crtc
  * @crtc: the CRTC
  *
- * This function disables FBC if it's associated with the provided CRTC.
+ * This function deactivates FBC if it's associated with the provided CRTC.
  */
-void intel_fbc_disable_crtc(struct intel_crtc *crtc)
+void intel_fbc_deactivate(struct intel_crtc *crtc)
 {
        struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
 
@@ -476,7 +453,7 @@ void intel_fbc_disable_crtc(struct intel_crtc *crtc)
 
        mutex_lock(&dev_priv->fbc.lock);
        if (dev_priv->fbc.crtc == crtc)
-               __intel_fbc_disable(dev_priv);
+               __intel_fbc_deactivate(dev_priv);
        mutex_unlock(&dev_priv->fbc.lock);
 }
 
@@ -490,13 +467,18 @@ static void set_no_fbc_reason(struct drm_i915_private *dev_priv,
        DRM_DEBUG_KMS("Disabling FBC: %s\n", reason);
 }
 
-static bool crtc_is_valid(struct intel_crtc *crtc)
+static bool crtc_can_fbc(struct intel_crtc *crtc)
 {
        struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
 
        if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A)
                return false;
 
+       return true;
+}
+
+static bool crtc_is_valid(struct intel_crtc *crtc)
+{
        if (!intel_crtc_active(&crtc->base))
                return false;
 
@@ -790,11 +772,11 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
 }
 
 /**
- * __intel_fbc_update - enable/disable FBC as needed, unlocked
+ * __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 enables,
- * disables or maintains it on the same state.
+ * 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)
 {
@@ -810,22 +792,9 @@ static void __intel_fbc_update(struct intel_crtc *crtc)
                goto out_disable;
        }
 
-       if (dev_priv->fbc.crtc != NULL && dev_priv->fbc.crtc != crtc)
+       if (!dev_priv->fbc.enabled || dev_priv->fbc.crtc != crtc)
                return;
 
-       if (intel_vgpu_active(dev_priv->dev))
-               i915.enable_fbc = 0;
-
-       if (i915.enable_fbc < 0) {
-               set_no_fbc_reason(dev_priv, "disabled per chip default");
-               goto out_disable;
-       }
-
-       if (!i915.enable_fbc) {
-               set_no_fbc_reason(dev_priv, "disabled per module param");
-               goto out_disable;
-       }
-
        if (!crtc_is_valid(crtc)) {
                set_no_fbc_reason(dev_priv, "no output");
                goto out_disable;
@@ -924,8 +893,8 @@ static void __intel_fbc_update(struct intel_crtc *crtc)
                 * 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_fbc_disable(dev_priv);
+               DRM_DEBUG_KMS("deactivating FBC for update\n");
+               __intel_fbc_deactivate(dev_priv);
        }
 
        intel_fbc_schedule_activation(crtc);
@@ -935,17 +904,17 @@ static void __intel_fbc_update(struct intel_crtc *crtc)
 out_disable:
        /* Multiple disables should be harmless */
        if (intel_fbc_is_active(dev_priv)) {
-               DRM_DEBUG_KMS("unsupported config, disabling FBC\n");
-               __intel_fbc_disable(dev_priv);
+               DRM_DEBUG_KMS("unsupported config, deactivating FBC\n");
+               __intel_fbc_deactivate(dev_priv);
        }
        __intel_fbc_cleanup_cfb(dev_priv);
 }
 
 /*
- * intel_fbc_update - enable/disable FBC as needed
+ * intel_fbc_update - activate/deactivate FBC as needed
  * @crtc: the CRTC that triggered the update
  *
- * This function reevaluates the overall state and enables or disables FBC.
+ * This function reevaluates the overall state and activates or deactivates FBC.
  */
 void intel_fbc_update(struct intel_crtc *crtc)
 {
@@ -973,7 +942,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
 
        mutex_lock(&dev_priv->fbc.lock);
 
-       if (dev_priv->fbc.active || dev_priv->fbc.fbc_work)
+       if (dev_priv->fbc.enabled)
                fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
        else
                fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
@@ -981,7 +950,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
        dev_priv->fbc.busy_bits |= (fbc_bits & frontbuffer_bits);
 
        if (dev_priv->fbc.busy_bits)
-               intel_fbc_deactivate(dev_priv);
+               __intel_fbc_deactivate(dev_priv);
 
        mutex_unlock(&dev_priv->fbc.lock);
 }
@@ -999,14 +968,128 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
 
        dev_priv->fbc.busy_bits &= ~frontbuffer_bits;
 
-       if (!dev_priv->fbc.busy_bits && dev_priv->fbc.crtc) {
-               intel_fbc_deactivate(dev_priv);
+       if (!dev_priv->fbc.busy_bits && dev_priv->fbc.enabled) {
+               __intel_fbc_deactivate(dev_priv);
                __intel_fbc_update(dev_priv->fbc.crtc);
        }
 
        mutex_unlock(&dev_priv->fbc.lock);
 }
 
+/**
+ * intel_fbc_enable: tries to enable FBC on the CRTC
+ * @crtc: the CRTC
+ *
+ * This function checks if it's possible to enable FBC on the following CRTC,
+ * then enables it. Notice that it doesn't activate FBC.
+ */
+void intel_fbc_enable(struct intel_crtc *crtc)
+{
+       struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+
+       if (!fbc_supported(dev_priv))
+               return;
+
+       mutex_lock(&dev_priv->fbc.lock);
+
+       if (dev_priv->fbc.enabled) {
+               WARN_ON(dev_priv->fbc.crtc == crtc);
+               goto out;
+       }
+
+       WARN_ON(dev_priv->fbc.active);
+       WARN_ON(dev_priv->fbc.crtc != NULL);
+
+       if (intel_vgpu_active(dev_priv->dev)) {
+               set_no_fbc_reason(dev_priv, "VGPU is active");
+               goto out;
+       }
+
+       if (i915.enable_fbc < 0) {
+               set_no_fbc_reason(dev_priv, "disabled per chip default");
+               goto out;
+       }
+
+       if (!i915.enable_fbc) {
+               set_no_fbc_reason(dev_priv, "disabled per module param");
+               goto out;
+       }
+
+       if (!crtc_can_fbc(crtc)) {
+               set_no_fbc_reason(dev_priv, "no enabled pipes can have FBC");
+               goto out;
+       }
+
+       DRM_DEBUG_KMS("Enabling FBC on pipe %c\n", pipe_name(crtc->pipe));
+       dev_priv->fbc.no_fbc_reason = "FBC enabled but not active yet\n";
+
+       dev_priv->fbc.enabled = true;
+       dev_priv->fbc.crtc = crtc;
+out:
+       mutex_unlock(&dev_priv->fbc.lock);
+}
+
+/**
+ * __intel_fbc_disable - disable FBC
+ * @dev_priv: i915 device instance
+ *
+ * This is the low level function that actually disables FBC. Callers should
+ * grab the FBC lock.
+ */
+static void __intel_fbc_disable(struct drm_i915_private *dev_priv)
+{
+       struct intel_crtc *crtc = dev_priv->fbc.crtc;
+
+       WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
+       WARN_ON(!dev_priv->fbc.enabled);
+       WARN_ON(dev_priv->fbc.active);
+       assert_pipe_disabled(dev_priv, crtc->pipe);
+
+       DRM_DEBUG_KMS("Disabling FBC on pipe %c\n", pipe_name(crtc->pipe));
+
+       dev_priv->fbc.enabled = false;
+       dev_priv->fbc.crtc = NULL;
+}
+
+/**
+ * intel_fbc_disable_crtc - disable FBC if it's associated with crtc
+ * @crtc: the CRTC
+ *
+ * This function disables FBC if it's associated with the provided CRTC.
+ */
+void intel_fbc_disable_crtc(struct intel_crtc *crtc)
+{
+       struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+
+       if (!fbc_supported(dev_priv))
+               return;
+
+       mutex_lock(&dev_priv->fbc.lock);
+       if (dev_priv->fbc.crtc == crtc) {
+               WARN_ON(!dev_priv->fbc.enabled);
+               WARN_ON(dev_priv->fbc.active);
+               __intel_fbc_disable(dev_priv);
+       }
+       mutex_unlock(&dev_priv->fbc.lock);
+}
+
+/**
+ * intel_fbc_disable - globally disable FBC
+ * @dev_priv: i915 device instance
+ *
+ * This function disables FBC regardless of which CRTC is associated with it.
+ */
+void intel_fbc_disable(struct drm_i915_private *dev_priv)
+{
+       if (!fbc_supported(dev_priv))
+               return;
+
+       mutex_lock(&dev_priv->fbc.lock);
+       if (dev_priv->fbc.enabled)
+               __intel_fbc_disable(dev_priv);
+       mutex_unlock(&dev_priv->fbc.lock);
+}
+
 /**
  * intel_fbc_init - Initialize FBC
  * @dev_priv: the i915 device
@@ -1018,6 +1101,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
        enum pipe pipe;
 
        mutex_init(&dev_priv->fbc.lock);
+       dev_priv->fbc.enabled = false;
        dev_priv->fbc.active = false;
 
        if (!HAS_FBC(dev_priv)) {