drm/i915/psr: Execute the default PSR code path when setting i915_edp_psr_debug
authorJosé Roberto de Souza <jose.souza@intel.com>
Wed, 6 Feb 2019 21:18:45 +0000 (13:18 -0800)
committerJosé Roberto de Souza <jose.souza@intel.com>
Thu, 7 Feb 2019 21:16:53 +0000 (13:16 -0800)
Changing the i915_edp_psr_debug was enabling, disabling or switching
PSR version by directly calling intel_psr_disable_locked() and
intel_psr_enable_locked(), what is not the default PSR path that will
be executed by real users.

So lets force a fastset in the PSR CRTC to trigger a pipe update and
stress the default code path.

Recently a bug was found when switching from PSR2 to PSR1 while
enable_psr kernel parameter was set to the default parameter, this
changes fix it and also fixes the bug linked bellow were DRRS was
left enabled together with PSR when enabling PSR from debugfs.

v2: Handling missing case: disabled to PSR1

v3: Not duplicating the whole atomic state(Maarten)

v4: Adding back the missing call to intel_psr_irq_control(Dhinakaran)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108341
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190206211845.5322-1-jose.souza@intel.com
drivers/gpu/drm/i915/i915_debugfs.c
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/intel_ddi.c
drivers/gpu/drm/i915/intel_drv.h
drivers/gpu/drm/i915/intel_psr.c

index e6b5bd845245b5a8a7b59080730bc04999632952..53ec81a4e5f1c0c2db02e6ba5910394439bbe2a0 100644 (file)
@@ -2606,7 +2606,6 @@ static int
 i915_edp_psr_debug_set(void *data, u64 val)
 {
        struct drm_i915_private *dev_priv = data;
-       struct drm_modeset_acquire_ctx ctx;
        intel_wakeref_t wakeref;
        int ret;
 
@@ -2617,18 +2616,7 @@ i915_edp_psr_debug_set(void *data, u64 val)
 
        wakeref = intel_runtime_pm_get(dev_priv);
 
-       drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
-
-retry:
-       ret = intel_psr_set_debugfs_mode(dev_priv, &ctx, val);
-       if (ret == -EDEADLK) {
-               ret = drm_modeset_backoff(&ctx);
-               if (!ret)
-                       goto retry;
-       }
-
-       drm_modeset_drop_locks(&ctx);
-       drm_modeset_acquire_fini(&ctx);
+       ret = intel_psr_debug_set(dev_priv, val);
 
        intel_runtime_pm_put(dev_priv, wakeref);
 
index 9adc7bb9e69ccfec96e468f95435b83e084ffcce..4a423753a71c7509b86fc448f12023d688b1bf55 100644 (file)
@@ -508,7 +508,7 @@ struct i915_psr {
 
        u32 debug;
        bool sink_support;
-       bool prepared, enabled;
+       bool enabled;
        struct intel_dp *dp;
        enum pipe pipe;
        bool active;
index 2323b7cb1d38c77da0f0839463acb1384fe254fa..49fd45c9185ec3a0cf6ca72935ee3ae849e22aa4 100644 (file)
@@ -3556,7 +3556,7 @@ static void intel_ddi_update_pipe_dp(struct intel_encoder *encoder,
 {
        struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
-       intel_psr_enable(intel_dp, crtc_state);
+       intel_psr_update(intel_dp, crtc_state);
        intel_edp_drrs_enable(intel_dp, crtc_state);
 
        intel_panel_update_backlight(encoder, crtc_state, conn_state);
index aa003ab8c4aa5df43d343fc7e43b496f64d125d8..2b851e6144602e7acb6703756d0d9c5bd8b4c8b3 100644 (file)
@@ -2077,9 +2077,9 @@ void intel_psr_enable(struct intel_dp *intel_dp,
                      const struct intel_crtc_state *crtc_state);
 void intel_psr_disable(struct intel_dp *intel_dp,
                      const struct intel_crtc_state *old_crtc_state);
-int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
-                              struct drm_modeset_acquire_ctx *ctx,
-                              u64 value);
+void intel_psr_update(struct intel_dp *intel_dp,
+                     const struct intel_crtc_state *crtc_state);
+int intel_psr_debug_set(struct drm_i915_private *dev_priv, u64 value);
 void intel_psr_invalidate(struct drm_i915_private *dev_priv,
                          unsigned frontbuffer_bits,
                          enum fb_op_origin origin);
index 84a0fb98156135839b5bc966dfcd09ea81f98556..75c1a5deebf5725949630f200899fe37323a0c2c 100644 (file)
@@ -51,6 +51,8 @@
  * must be correctly synchronized/cancelled when shutting down the pipe."
  */
 
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
 
 #include "intel_drv.h"
 #include "i915_drv.h"
@@ -718,8 +720,11 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
 {
        struct intel_dp *intel_dp = dev_priv->psr.dp;
 
-       if (dev_priv->psr.enabled)
-               return;
+       WARN_ON(dev_priv->psr.enabled);
+
+       dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
+       dev_priv->psr.busy_frontbuffer_bits = 0;
+       dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
 
        DRM_DEBUG_KMS("Enabling PSR%s\n",
                      dev_priv->psr.psr2_enabled ? "2" : "1");
@@ -752,20 +757,13 @@ void intel_psr_enable(struct intel_dp *intel_dp,
        WARN_ON(dev_priv->drrs.dp);
 
        mutex_lock(&dev_priv->psr.lock);
-       if (dev_priv->psr.prepared) {
-               DRM_DEBUG_KMS("PSR already in use\n");
+
+       if (!psr_global_enabled(dev_priv->psr.debug)) {
+               DRM_DEBUG_KMS("PSR disabled by flag\n");
                goto unlock;
        }
 
-       dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
-       dev_priv->psr.busy_frontbuffer_bits = 0;
-       dev_priv->psr.prepared = true;
-       dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
-
-       if (psr_global_enabled(dev_priv->psr.debug))
-               intel_psr_enable_locked(dev_priv, crtc_state);
-       else
-               DRM_DEBUG_KMS("PSR disabled by flag\n");
+       intel_psr_enable_locked(dev_priv, crtc_state);
 
 unlock:
        mutex_unlock(&dev_priv->psr.lock);
@@ -848,18 +846,54 @@ void intel_psr_disable(struct intel_dp *intel_dp,
                return;
 
        mutex_lock(&dev_priv->psr.lock);
-       if (!dev_priv->psr.prepared) {
-               mutex_unlock(&dev_priv->psr.lock);
-               return;
-       }
 
        intel_psr_disable_locked(intel_dp);
 
-       dev_priv->psr.prepared = false;
        mutex_unlock(&dev_priv->psr.lock);
        cancel_work_sync(&dev_priv->psr.work);
 }
 
+/**
+ * intel_psr_update - Update PSR state
+ * @intel_dp: Intel DP
+ * @crtc_state: new CRTC state
+ *
+ * This functions will update PSR states, disabling, enabling or switching PSR
+ * version when executing fastsets. For full modeset, intel_psr_disable() and
+ * intel_psr_enable() should be called instead.
+ */
+void intel_psr_update(struct intel_dp *intel_dp,
+                     const struct intel_crtc_state *crtc_state)
+{
+       struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+       struct i915_psr *psr = &dev_priv->psr;
+       bool enable, psr2_enable;
+
+       if (!CAN_PSR(dev_priv) || READ_ONCE(psr->dp) != intel_dp)
+               return;
+
+       mutex_lock(&dev_priv->psr.lock);
+
+       enable = crtc_state->has_psr && psr_global_enabled(psr->debug);
+       psr2_enable = intel_psr2_enabled(dev_priv, crtc_state);
+
+       if (enable == psr->enabled && psr2_enable == psr->psr2_enabled)
+               goto unlock;
+
+       if (psr->enabled) {
+               if (!enable || psr2_enable != psr->psr2_enabled)
+                       intel_psr_disable_locked(intel_dp);
+       }
+
+       if (enable) {
+               if (!psr->enabled || psr2_enable != psr->psr2_enabled)
+                       intel_psr_enable_locked(dev_priv, crtc_state);
+       }
+
+unlock:
+       mutex_unlock(&dev_priv->psr.lock);
+}
+
 /**
  * intel_psr_wait_for_idle - wait for PSR1 to idle
  * @new_crtc_state: new CRTC state
@@ -924,36 +958,64 @@ static bool __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv)
        return err == 0 && dev_priv->psr.enabled;
 }
 
-static bool switching_psr(struct drm_i915_private *dev_priv,
-                         struct intel_crtc_state *crtc_state,
-                         u32 mode)
+static int intel_psr_fastset_force(struct drm_i915_private *dev_priv)
 {
-       /* Can't switch psr state anyway if PSR2 is not supported. */
-       if (!crtc_state || !crtc_state->has_psr2)
-               return false;
+       struct drm_device *dev = &dev_priv->drm;
+       struct drm_modeset_acquire_ctx ctx;
+       struct drm_atomic_state *state;
+       struct drm_crtc *crtc;
+       int err;
 
-       if (dev_priv->psr.psr2_enabled && mode == I915_PSR_DEBUG_FORCE_PSR1)
-               return true;
+       state = drm_atomic_state_alloc(dev);
+       if (!state)
+               return -ENOMEM;
 
-       if (!dev_priv->psr.psr2_enabled && mode != I915_PSR_DEBUG_FORCE_PSR1)
-               return true;
+       drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
+       state->acquire_ctx = &ctx;
+
+retry:
+       drm_for_each_crtc(crtc, dev) {
+               struct drm_crtc_state *crtc_state;
+               struct intel_crtc_state *intel_crtc_state;
+
+               crtc_state = drm_atomic_get_crtc_state(state, crtc);
+               if (IS_ERR(crtc_state)) {
+                       err = PTR_ERR(crtc_state);
+                       goto error;
+               }
+
+               intel_crtc_state = to_intel_crtc_state(crtc_state);
+
+               if (intel_crtc_has_type(intel_crtc_state, INTEL_OUTPUT_EDP) &&
+                   intel_crtc_state->has_psr) {
+                       /* Mark mode as changed to trigger a pipe->update() */
+                       crtc_state->mode_changed = true;
+                       break;
+               }
+       }
+
+       err = drm_atomic_commit(state);
 
-       return false;
+error:
+       if (err == -EDEADLK) {
+               drm_atomic_state_clear(state);
+               err = drm_modeset_backoff(&ctx);
+               if (!err)
+                       goto retry;
+       }
+
+       drm_modeset_drop_locks(&ctx);
+       drm_modeset_acquire_fini(&ctx);
+       drm_atomic_state_put(state);
+
+       return err;
 }
 
-int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
-                              struct drm_modeset_acquire_ctx *ctx,
-                              u64 val)
+int intel_psr_debug_set(struct drm_i915_private *dev_priv, u64 val)
 {
-       struct drm_device *dev = &dev_priv->drm;
-       struct drm_connector_state *conn_state;
-       struct intel_crtc_state *crtc_state = NULL;
-       struct drm_crtc_commit *commit;
-       struct drm_crtc *crtc;
-       struct intel_dp *dp;
+       const u32 mode = val & I915_PSR_DEBUG_MODE_MASK;
+       u32 old_mode;
        int ret;
-       bool enable;
-       u32 mode = val & I915_PSR_DEBUG_MODE_MASK;
 
        if (val & ~(I915_PSR_DEBUG_IRQ | I915_PSR_DEBUG_MODE_MASK) ||
            mode > I915_PSR_DEBUG_FORCE_PSR1) {
@@ -961,49 +1023,19 @@ int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
                return -EINVAL;
        }
 
-       ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
-       if (ret)
-               return ret;
-
-       /* dev_priv->psr.dp should be set once and then never touched again. */
-       dp = READ_ONCE(dev_priv->psr.dp);
-       conn_state = dp->attached_connector->base.state;
-       crtc = conn_state->crtc;
-       if (crtc) {
-               ret = drm_modeset_lock(&crtc->mutex, ctx);
-               if (ret)
-                       return ret;
-
-               crtc_state = to_intel_crtc_state(crtc->state);
-               commit = crtc_state->base.commit;
-       } else {
-               commit = conn_state->commit;
-       }
-       if (commit) {
-               ret = wait_for_completion_interruptible(&commit->hw_done);
-               if (ret)
-                       return ret;
-       }
-
        ret = mutex_lock_interruptible(&dev_priv->psr.lock);
        if (ret)
                return ret;
 
-       enable = psr_global_enabled(val);
-
-       if (!enable || switching_psr(dev_priv, crtc_state, mode))
-               intel_psr_disable_locked(dev_priv->psr.dp);
-
+       old_mode = dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK;
        dev_priv->psr.debug = val;
-       if (crtc)
-               dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
-
        intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
 
-       if (dev_priv->psr.prepared && enable)
-               intel_psr_enable_locked(dev_priv, crtc_state);
-
        mutex_unlock(&dev_priv->psr.lock);
+
+       if (old_mode != mode)
+               ret = intel_psr_fastset_force(dev_priv);
+
        return ret;
 }