Currently tests modify i915.enable_psr and then do a modeset cycle
to change PSR. We can write a value to i915_edp_psr_debug to force
a certain PSR mode without a modeset.

To retain compatibility with older userspace, we also still allow
the override through the module parameter, and add some tracking
to check whether a debugfs mode is specified.

Changes since v1:
- Rename dev_priv->psr.enabled to .dp, and .hw_configured to .enabled.
- Fix i915_psr_debugfs_mode to match the writes to debugfs.
- Rename __i915_edp_psr_write to intel_psr_set_debugfs_mode, simplify
  it and move it to intel_psr.c. This keeps all internals in intel_psr.c
- Perform an interruptible wait for hw completion outside of the psr
  lock, instead of being forced to trywait and return -EBUSY.
Changes since v2:
- Rebase on top of intel_psr changes.
Changes since v3:
- Assign psr.dp during init. (dhnkrn)
- Add prepared bool, which should be used instead of relying on psr.dp. (dhnkrn)
- Fix -EDEADLK handling in debugfs. (dhnkrn)
- Clean up waiting for idle in intel_psr_set_debugfs_mode.
- Print PSR mode when trying to enable PSR. (dhnkrn)
- Move changing psr debug setting to i915_edp_psr_debug_set. (dhnkrn)

Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  21 ++++-
 drivers/gpu/drm/i915/i915_drv.h     |  12 ++-
 drivers/gpu/drm/i915/i915_irq.c     |   2 +-
 drivers/gpu/drm/i915/intel_drv.h    |   5 +-
 drivers/gpu/drm/i915/intel_psr.c    | 136 +++++++++++++++++++++++-----
 5 files changed, 144 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 59dc0610ea44..a7e927413e8e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2705,7 +2705,7 @@ static int i915_edp_psr_status(struct seq_file *m, void 
*data)
        intel_runtime_pm_get(dev_priv);
 
        mutex_lock(&dev_priv->psr.lock);
-       seq_printf(m, "Enabled: %s\n", yesno((bool)dev_priv->psr.enabled));
+       seq_printf(m, "Enabled: %s\n", yesno(dev_priv->psr.enabled));
        seq_printf(m, "Busy frontbuffer bits: 0x%03x\n",
                   dev_priv->psr.busy_frontbuffer_bits);
 
@@ -2747,14 +2747,29 @@ static int
 i915_edp_psr_debug_set(void *data, u64 val)
 {
        struct drm_i915_private *dev_priv = data;
+       struct drm_modeset_acquire_ctx ctx;
+       int ret;
 
        if (!CAN_PSR(dev_priv))
                return -ENODEV;
 
-       DRM_DEBUG_KMS("PSR debug %s\n", enableddisabled(val));
+       DRM_DEBUG_KMS("Setting PSR debug to %llx\n", val);
 
        intel_runtime_pm_get(dev_priv);
-       intel_psr_irq_control(dev_priv, !!val);
+
+       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);
+
        intel_runtime_pm_put(dev_priv);
 
        return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0f49f9988dfa..ef04b6cd7863 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -611,8 +611,17 @@ struct i915_drrs {
 
 struct i915_psr {
        struct mutex lock;
+
+#define I915_PSR_DEBUG_IRQ     BIT(0)
+#define I915_PSR_DEBUG_MODE_MASK (7 << 1)
+#define I915_PSR_DEBUG_DEFAULT (0 << 1)
+#define I915_PSR_DEBUG_DISABLE (1 << 1)
+#define I915_PSR_DEBUG_ENABLE  (2 << 1)
+
+       u32 debug;
        bool sink_support;
-       struct intel_dp *enabled;
+       bool prepared, enabled;
+       struct intel_dp *dp;
        bool active;
        struct work_struct work;
        unsigned busy_frontbuffer_bits;
@@ -622,7 +631,6 @@ struct i915_psr {
        bool alpm;
        bool psr2_enabled;
        u8 sink_sync_latency;
-       bool debug;
        ktime_t last_entry_attempt;
        ktime_t last_exit;
 };
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5dadefca2ad2..defd696e71fc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4051,7 +4051,7 @@ static int ironlake_irq_postinstall(struct drm_device 
*dev)
 
        if (IS_HASWELL(dev_priv)) {
                gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
-               intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
+               intel_psr_irq_control(dev_priv, dev_priv->psr.debug & 
I915_PSR_DEBUG_IRQ);
                display_mask |= DE_EDP_PSR_INT_HSW;
        }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 99a5f5be5b82..f4cfb7ed2262 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1930,6 +1930,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_invalidate(struct drm_i915_private *dev_priv,
                          unsigned frontbuffer_bits,
                          enum fb_op_origin origin);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 4bd5768731ee..7848829094ca 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,6 +56,18 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
+static bool psr_global_enabled(u32 debug)
+{
+       switch (debug & I915_PSR_DEBUG_MODE_MASK) {
+       case I915_PSR_DEBUG_DEFAULT:
+               return i915_modparams.enable_psr;
+       case I915_PSR_DEBUG_DISABLE:
+               return false;
+       default:
+               return true;
+       }
+}
+
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug)
 {
        u32 debug_mask, mask;
@@ -80,7 +92,6 @@ void intel_psr_irq_control(struct drm_i915_private *dev_priv, 
bool debug)
        if (debug)
                mask |= debug_mask;
 
-       WRITE_ONCE(dev_priv->psr.debug, debug);
        I915_WRITE(EDP_PSR_IMR, ~mask);
 }
 
@@ -212,6 +223,7 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
        dev_priv->psr.sink_support = true;
        dev_priv->psr.sink_sync_latency =
                intel_dp_get_sink_sync_latency(intel_dp);
+       dev_priv->psr.dp = intel_dp;
 
        if (INTEL_GEN(dev_priv) >= 9 &&
            (intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_IS_SUPPORTED)) {
@@ -471,11 +483,6 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
        if (!CAN_PSR(dev_priv))
                return;
 
-       if (!i915_modparams.enable_psr) {
-               DRM_DEBUG_KMS("PSR disable by flag\n");
-               return;
-       }
-
        /*
         * HSW spec explicitly says PSR is tied to port A.
         * BDW+ platforms with DDI implementation of PSR have different
@@ -517,7 +524,6 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 
        crtc_state->has_psr = true;
        crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
-       DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
 }
 
 static void intel_psr_activate(struct intel_dp *intel_dp)
@@ -589,6 +595,23 @@ static void intel_psr_enable_source(struct intel_dp 
*intel_dp,
        }
 }
 
+static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
+                                   const struct intel_crtc_state *crtc_state)
+{
+       struct intel_dp *intel_dp = dev_priv->psr.dp;
+
+       if (dev_priv->psr.enabled)
+               return;
+
+       DRM_DEBUG_KMS("Enabling PSR%s\n", dev_priv->psr.psr2_enabled ? "2" : 
"1");
+       intel_psr_setup_vsc(intel_dp, crtc_state);
+       intel_psr_enable_sink(intel_dp);
+       intel_psr_enable_source(intel_dp, crtc_state);
+       dev_priv->psr.enabled = true;
+
+       intel_psr_activate(intel_dp);
+}
+
 /**
  * intel_psr_enable - Enable PSR
  * @intel_dp: Intel DP
@@ -611,20 +634,20 @@ 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.enabled) {
+       if (dev_priv->psr.prepared) {
                DRM_DEBUG_KMS("PSR already in use\n");
                goto unlock;
        }
 
        dev_priv->psr.psr2_enabled = crtc_state->has_psr2;
        dev_priv->psr.busy_frontbuffer_bits = 0;
+       dev_priv->psr.prepared = true;
+       dev_priv->psr.dp = intel_dp;
 
-       intel_psr_setup_vsc(intel_dp, crtc_state);
-       intel_psr_enable_sink(intel_dp);
-       intel_psr_enable_source(intel_dp, crtc_state);
-       dev_priv->psr.enabled = intel_dp;
-
-       intel_psr_activate(intel_dp);
+       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");
 
 unlock:
        mutex_unlock(&dev_priv->psr.lock);
@@ -683,12 +706,13 @@ static void intel_psr_disable_locked(struct intel_dp 
*intel_dp)
        if (!dev_priv->psr.enabled)
                return;
 
+       DRM_DEBUG_KMS("Disabling PSR\n");
        intel_psr_disable_source(intel_dp);
 
        /* Disable PSR on Sink */
        drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
 
-       dev_priv->psr.enabled = NULL;
+       dev_priv->psr.enabled = false;
 }
 
 /**
@@ -712,7 +736,14 @@ void intel_psr_disable(struct intel_dp *intel_dp,
                return;
 
        mutex_lock(&dev_priv->psr.lock);
+       if (intel_dp != dev_priv->psr.dp || !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);
 }
@@ -724,9 +755,6 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state 
*new_crtc_state)
        i915_reg_t reg;
        u32 mask;
 
-       if (!new_crtc_state->has_psr)
-               return 0;
-
        /*
         * The sole user right now is intel_pipe_update_start(),
         * which won't race with psr_enable/disable, which is
@@ -737,6 +765,9 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state 
*new_crtc_state)
         * not needed and will induce latencies in the atomic
         * update path.
         */
+       if (!dev_priv->psr.enabled)
+               return 0;
+
        if (dev_priv->psr.psr2_enabled) {
                reg = EDP_PSR2_STATUS;
                mask = EDP_PSR2_STATUS_STATE_MASK;
@@ -756,13 +787,11 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state 
*new_crtc_state)
 
 static bool __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv)
 {
-       struct intel_dp *intel_dp;
        i915_reg_t reg;
        u32 mask;
        int err;
 
-       intel_dp = dev_priv->psr.enabled;
-       if (!intel_dp)
+       if (!dev_priv->psr.enabled)
                return false;
 
        if (dev_priv->psr.psr2_enabled) {
@@ -784,6 +813,62 @@ static bool __psr_wait_for_idle_locked(struct 
drm_i915_private *dev_priv)
        return err == 0 && dev_priv->psr.enabled;
 }
 
+int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
+                              struct drm_modeset_acquire_ctx *ctx,
+                              u64 val)
+{
+       struct drm_device *dev = &dev_priv->drm;
+       struct drm_connector_state *conn_state;
+       struct drm_crtc *crtc;
+       struct intel_dp *dp;
+       int ret;
+       bool enable;
+
+       if (val & ~(I915_PSR_DEBUG_IRQ | I915_PSR_DEBUG_MODE_MASK) ||
+           (val & I915_PSR_DEBUG_MODE_MASK) > I915_PSR_DEBUG_ENABLE) {
+               DRM_DEBUG_KMS("Invalid debug mask %llx\n", val);
+               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;
+
+               ret = 
wait_for_completion_interruptible(&crtc->state->commit->hw_done);
+       } else
+               ret = 
wait_for_completion_interruptible(&conn_state->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)
+               intel_psr_disable_locked(dev_priv->psr.dp);
+
+       dev_priv->psr.debug = val;
+       intel_psr_irq_control(dev_priv, dev_priv->psr.debug & 
I915_PSR_DEBUG_IRQ);
+
+       if (dev_priv->psr.prepared && enable)
+               intel_psr_enable_locked(dev_priv, 
to_intel_crtc_state(crtc->state));
+
+       mutex_unlock(&dev_priv->psr.lock);
+       return ret;
+}
+
 static void intel_psr_work(struct work_struct *work)
 {
        struct drm_i915_private *dev_priv =
@@ -811,7 +896,8 @@ static void intel_psr_work(struct work_struct *work)
        if (dev_priv->psr.busy_frontbuffer_bits || dev_priv->psr.active)
                goto unlock;
 
-       intel_psr_activate(dev_priv->psr.enabled);
+       intel_psr_activate(dev_priv->psr.dp
+);
 unlock:
        mutex_unlock(&dev_priv->psr.lock);
 }
@@ -866,7 +952,7 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
                return;
        }
 
-       crtc = dp_to_dig_port(dev_priv->psr.enabled)->base.base.crtc;
+       crtc = dp_to_dig_port(dev_priv->psr.dp)->base.base.crtc;
        pipe = to_intel_crtc(crtc)->pipe;
 
        frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
@@ -909,7 +995,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
                return;
        }
 
-       crtc = dp_to_dig_port(dev_priv->psr.enabled)->base.base.crtc;
+       crtc = dp_to_dig_port(dev_priv->psr.dp)->base.base.crtc;
        pipe = to_intel_crtc(crtc)->pipe;
 
        frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
@@ -991,7 +1077,7 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
 
        mutex_lock(&psr->lock);
 
-       if (psr->enabled != intel_dp)
+       if (!psr->enabled || psr->dp != intel_dp)
                goto exit;
 
        if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_STATUS, &val) != 1) {
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to