[PATCH 1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5

[PATCH 1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5

[Maarten Lankhorst]

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)
Changes since v4:
- Return error in _set() function.
- Change flag values to make them easier to remember. (dhnkrn)
- Only assign psr.dp once. (dhnkrn)
- Only set crtc_state->has_psr on the crtc with psr.dp.
- Fix typo. (dhnkrn)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  23 ++++-
 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    | 138 +++++++++++++++++++++++-----
 5 files changed, 149 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f9ce35da4123..3e81301a94ba 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2708,7 +2708,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);
 
@@ -2750,17 +2750,32 @@ 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;
+	return ret;
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 657f46e0cae9..a3ea48ce1811 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_MODE_MASK	0x0f
+#define I915_PSR_DEBUG_DEFAULT		0x00
+#define I915_PSR_DEBUG_DISABLE		0x01
+#define I915_PSR_DEBUG_ENABLE		0x02
+#define I915_PSR_DEBUG_IRQ		0x10
+
+	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 8084e35b25c5..b2c9838442bc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4048,7 +4048,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 1ad7c1124bef..098ea2859a9b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1924,7 +1924,7 @@ int intel_hdcp_enable(struct intel_connector *connector);
 int intel_hdcp_disable(struct intel_connector *connector);
 int intel_hdcp_check_link(struct intel_connector *connector);
 bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
-
+// 
 /* intel_psr.c */
 #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
 void intel_psr_init_dpcd(struct intel_dp *intel_dp);
@@ -1932,6 +1932,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..12fbc59af5a4 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);
 }
 
@@ -213,6 +224,9 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 	dev_priv->psr.sink_sync_latency =
 		intel_dp_get_sink_sync_latency(intel_dp);
 
+	WARN_ON(dev_priv->psr.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)) {
 		bool y_req = intel_dp->psr_dpcd[1] &
@@ -471,8 +485,8 @@ 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");
+	if (intel_dp != dev_priv->psr.dp) {
+		DRM_DEBUG_KMS("Not enabling PSR: wrong connector\n");
 		return;
 	}
 
@@ -517,7 +531,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 +602,24 @@ 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
@@ -609,22 +640,20 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 	if (WARN_ON(!CAN_PSR(dev_priv)))
 		return;
 
-	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;
 
-	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 +712,14 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp)
 	if (!dev_priv->psr.enabled)
 		return;
 
+	DRM_DEBUG_KMS("Disabling PSR%s\n",
+		      dev_priv->psr.psr2_enabled ? "2" : "1");
 	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 +743,14 @@ 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);
 }
@@ -724,9 +762,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 +772,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 +794,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 +820,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 +903,7 @@ 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 +958,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 +1001,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 +1083,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

[PATCH 2/2] drm/i915/psr: Add debugfs support to force a downgrade to PSR1 mode.

[Maarten Lankhorst]

This will make it easier to test PSR1 on PSR2 capable eDP machines.

Changes since v1:
- Remove I915_PSR_DEBUG_FORCE_PSR2, it did nothing, not sure forcing
  PSR2 would even work.
- Handle NULL crtc in intel_psr_set_debugfs_mode. (dhnkrn)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 44 ++++++++++++++++++++++++++++----
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a3ea48ce1811..14883614d9e6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -616,6 +616,7 @@ struct i915_psr {
 #define I915_PSR_DEBUG_DEFAULT		0x00
 #define I915_PSR_DEBUG_DISABLE		0x01
 #define I915_PSR_DEBUG_ENABLE		0x02
+#define I915_PSR_DEBUG_FORCE_PSR1	0x03
 #define I915_PSR_DEBUG_IRQ		0x10
 
 	u32 debug;
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 12fbc59af5a4..a408faa16f90 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -68,6 +68,17 @@ static bool psr_global_enabled(u32 debug)
 	}
 }
 
+static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
+			       const struct intel_crtc_state *crtc_state)
+{
+	switch (dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
+	case I915_PSR_DEBUG_FORCE_PSR1:
+		return false;
+	default:
+		return crtc_state->has_psr2;
+	}
+}
+
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug)
 {
 	u32 debug_mask, mask;
@@ -646,7 +657,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 		goto unlock;
 	}
 
-	dev_priv->psr.psr2_enabled = crtc_state->has_psr2;
+	dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 	dev_priv->psr.prepared = true;
 
@@ -820,19 +831,38 @@ 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)
+{
+	/* Can't switch psr state anyway if PSR2 is not supported. */
+	if (!crtc_state || !crtc_state->has_psr2)
+		return false;
+
+	if (dev_priv->psr.psr2_enabled && mode == I915_PSR_DEBUG_FORCE_PSR1)
+		return true;
+
+	if (!dev_priv->psr.psr2_enabled && mode != I915_PSR_DEBUG_FORCE_PSR1)
+		return true;
+
+	return false;
+}
+
 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 intel_crtc_state *crtc_state = NULL;
 	struct drm_crtc *crtc;
 	struct intel_dp *dp;
 	int ret;
 	bool enable;
+	u32 mode = val & I915_PSR_DEBUG_MODE_MASK;
 
 	if (val & ~(I915_PSR_DEBUG_IRQ | I915_PSR_DEBUG_MODE_MASK) ||
-	    (val & I915_PSR_DEBUG_MODE_MASK) > I915_PSR_DEBUG_ENABLE) {
+	    mode > I915_PSR_DEBUG_FORCE_PSR1) {
 		DRM_DEBUG_KMS("Invalid debug mask %llx\n", val);
 		return -EINVAL;
 	}
@@ -850,7 +880,8 @@ int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
 		if (ret)
 			return ret;
 
-		ret = wait_for_completion_interruptible(&crtc->state->commit->hw_done);
+		crtc_state = to_intel_crtc_state(crtc->state);
+		ret = wait_for_completion_interruptible(&crtc_state->base.commit->hw_done);
 	} else
 		ret = wait_for_completion_interruptible(&conn_state->commit->hw_done);
 
@@ -863,14 +894,17 @@ int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
 
 	enable = psr_global_enabled(val);
 
-	if (!enable)
+	if (!enable || switching_psr(dev_priv, crtc_state, mode))
 		intel_psr_disable_locked(dev_priv->psr.dp);
 
 	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 & I915_PSR_DEBUG_IRQ);
 
 	if (dev_priv->psr.prepared && enable)
-		intel_psr_enable_locked(dev_priv, to_intel_crtc_state(crtc->state));
+		intel_psr_enable_locked(dev_priv, crtc_state);
 
 	mutex_unlock(&dev_priv->psr.lock);
 	return ret;
-- 
2.18.0

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5
URL   : https://patchwork.freedesktop.org/series/47888/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
156e45d20493 drm/i915: Allow control of PSR at runtime through debugfs, v5
-:25: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#25: 
- Add prepared bool, which should be used instead of relying on psr.dp. (dhnkrn)

-:143: ERROR:TRAILING_WHITESPACE: trailing whitespace
#143: FILE: drivers/gpu/drm/i915/intel_drv.h:1927:
+// $

-:366: CHECK:BRACES: braces {} should be used on all arms of this statement
#366: FILE: drivers/gpu/drm/i915/intel_psr.c:848:
+	if (crtc) {
[...]
+	} else
[...]

-:372: CHECK:BRACES: Unbalanced braces around else statement
#372: FILE: drivers/gpu/drm/i915/intel_psr.c:854:
+	} else

total: 1 errors, 1 warnings, 2 checks, 351 lines checked
a61846961762 drm/i915/psr: Add debugfs support to force a downgrade to PSR1 mode.

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

✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5
URL   : https://patchwork.freedesktop.org/series/47888/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Allow control of PSR at runtime through debugfs, v5
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3675:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3683:16: warning: expression using sizeof(void)

Commit: drm/i915/psr: Add debugfs support to force a downgrade to PSR1 mode.
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3683:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3684:16: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5
URL   : https://patchwork.freedesktop.org/series/47888/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4633 -> Patchwork_9886 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47888/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9886 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    {igt@amdgpu/amd_basic@userptr}:
      {fi-kbl-8809g}:     PASS -> INCOMPLETE (fdo#107402)

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         PASS -> DMESG-FAIL (fdo#107164)

    igt@drv_selftest@live_hangcheck:
      fi-cfl-s3:          PASS -> DMESG-FAIL (fdo#106560)
      fi-bxt-j4205:       PASS -> DMESG-FAIL (fdo#106560)

    igt@drv_selftest@live_workarounds:
      {fi-cfl-8109u}:     PASS -> DMESG-FAIL (fdo#107292)
      fi-cnl-psr:         PASS -> DMESG-FAIL (fdo#107292)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         DMESG-FAIL (fdo#107174) -> PASS

    igt@drv_selftest@live_workarounds:
      fi-whl-u:           DMESG-FAIL (fdo#107292) -> PASS
      fi-kbl-x1275:       DMESG-FAIL (fdo#107292) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   FAIL (fdo#103167) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
  fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402


== Participating hosts (51 -> 47) ==

  Additional (1): fi-bxt-dsi 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4633 -> Patchwork_9886

  CI_DRM_4633: ea6e3f703e4d234c9c8eaec6c533355c7454ecb6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4588: 7e5abbe4d9b2129bbbf02be77a70cad3da2ab941 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9886: a61846961762ac2ce6ac6b2c23411dd45b08b5cb @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a61846961762 drm/i915/psr: Add debugfs support to force a downgrade to PSR1 mode.
156e45d20493 drm/i915: Allow control of PSR at runtime through debugfs, v5

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9886/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5
URL   : https://patchwork.freedesktop.org/series/47888/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4633_full -> Patchwork_9886_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9886_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9886_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9886_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@pm_rpm@gem-pread:
      shard-kbl:          PASS -> WARN

    
== Known issues ==

  Here are the changes found in Patchwork_9886_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-snb:          PASS -> INCOMPLETE (fdo#106886, fdo#105411)

    igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_vblank@pipe-c-wait-forked-busy-hang:
      shard-kbl:          PASS -> DMESG-WARN (fdo#105602, fdo#103558)

    
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4633 -> Patchwork_9886

  CI_DRM_4633: ea6e3f703e4d234c9c8eaec6c533355c7454ecb6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4588: 7e5abbe4d9b2129bbbf02be77a70cad3da2ab941 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9886: a61846961762ac2ce6ac6b2c23411dd45b08b5cb @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9886/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5

[Dhinakaran Pandiyan]

On Wed, 2018-08-08 at 16:19 +0200, Maarten Lankhorst wrote:
> 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)
> Changes since v4:
> - Return error in _set() function.
> - Change flag values to make them easier to remember. (dhnkrn)
> - Only assign psr.dp once. (dhnkrn)
> - Only set crtc_state->has_psr on the crtc with psr.dp.
> - Fix typo. (dhnkrn)
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  23 ++++-
>  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    | 138 +++++++++++++++++++++++---
> --
>  5 files changed, 149 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index f9ce35da4123..3e81301a94ba 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2708,7 +2708,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);
>  
> @@ -2750,17 +2750,32 @@ 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;
> +	return ret;
>  }
>  
>  static int
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 657f46e0cae9..a3ea48ce1811 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_MODE_MASK	0x0f
> +#define I915_PSR_DEBUG_DEFAULT		0x00
> +#define I915_PSR_DEBUG_DISABLE		0x01
> +#define I915_PSR_DEBUG_ENABLE		0x02
> +#define I915_PSR_DEBUG_IRQ		0x10
> +
> +	u32 debug;
	u16?

>  	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 8084e35b25c5..b2c9838442bc 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4048,7 +4048,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 1ad7c1124bef..098ea2859a9b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1924,7 +1924,7 @@ int intel_hdcp_enable(struct intel_connector
> *connector);
>  int intel_hdcp_disable(struct intel_connector *connector);
>  int intel_hdcp_check_link(struct intel_connector *connector);
>  bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port
> port);
> -
> +//
^ seems like a ghost comment sneaked in.
>  
>  /* intel_psr.c */
>  #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv-
> >psr.sink_support)
>  void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> @@ -1932,6 +1932,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..12fbc59af5a4 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);
>  }
>  
> @@ -213,6 +224,9 @@ void intel_psr_init_dpcd(struct intel_dp
> *intel_dp)
>  	dev_priv->psr.sink_sync_latency =
>  		intel_dp_get_sink_sync_latency(intel_dp);
>  
> +	WARN_ON(dev_priv->psr.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)) {
>  		bool y_req = intel_dp->psr_dpcd[1] &
> @@ -471,8 +485,8 @@ 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");
> +	if (intel_dp != dev_priv->psr.dp) {
> +		DRM_DEBUG_KMS("Not enabling PSR: wrong
> connector\n");
This debug message will get printed for non eDP ports, return silently?

>  		return;
>  	}
>  
> @@ -517,7 +531,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 +602,24 @@ 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
> @@ -609,22 +640,20 @@ void intel_psr_enable(struct intel_dp
> *intel_dp,
>  	if (WARN_ON(!CAN_PSR(dev_priv)))
>  		return;
>  
> -	WARN_ON(dev_priv->drrs.dp);
This warning was to catch if DRRS was enabled along with PSR. the
hardware supports only one at a time.

>  	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;
>  
> -	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 +712,14 @@ static void intel_psr_disable_locked(struct
> intel_dp *intel_dp)
>  	if (!dev_priv->psr.enabled)
>  		return;
>  
> +	DRM_DEBUG_KMS("Disabling PSR%s\n",
> +		      dev_priv->psr.psr2_enabled ? "2" : "1");
>  	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 +743,14 @@ 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);
>  }
> @@ -724,9 +762,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;
> -
We'll end up waiting for PSR to idle when updating CRTCs that are not
driving a PSR panel. I should have checked this in the previous
version, my bad.

The patch looks good to me other than for the issues pointed out.

-DK

>  	/*
>  	 * The sole user right now is intel_pipe_update_start(),
>  	 * which won't race with psr_enable/disable, which is
> @@ -737,6 +772,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;



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

Re: [PATCH 2/2] drm/i915/psr: Add debugfs support to force a downgrade to PSR1 mode.

[Dhinakaran Pandiyan]

On Wed, 2018-08-08 at 16:19 +0200, Maarten Lankhorst wrote:
> This will make it easier to test PSR1 on PSR2 capable eDP machines.
> 
> Changes since v1:
> - Remove I915_PSR_DEBUG_FORCE_PSR2, it did nothing, not sure forcing
>   PSR2 would even work.
> - Handle NULL crtc in intel_psr_set_debugfs_mode. (dhnkrn)
> 
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>


> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_psr.c | 44 ++++++++++++++++++++++++++++
> ----
>  2 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index a3ea48ce1811..14883614d9e6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -616,6 +616,7 @@ struct i915_psr {
>  #define I915_PSR_DEBUG_DEFAULT		0x00
>  #define I915_PSR_DEBUG_DISABLE		0x01
>  #define I915_PSR_DEBUG_ENABLE		0x02
> +#define I915_PSR_DEBUG_FORCE_PSR1	0x03
>  #define I915_PSR_DEBUG_IRQ		0x10
>  
>  	u32 debug;
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 12fbc59af5a4..a408faa16f90 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -68,6 +68,17 @@ static bool psr_global_enabled(u32 debug)
>  	}
>  }
>  
> +static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
> +			       const struct intel_crtc_state
> *crtc_state)
> +{
> +	switch (dev_priv->psr.debug & I915_PSR_DEBUG_MODE_MASK) {
> +	case I915_PSR_DEBUG_FORCE_PSR1:
> +		return false;
> +	default:
> +		return crtc_state->has_psr2;
> +	}
> +}
> +
>  void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool
> debug)
>  {
>  	u32 debug_mask, mask;
> @@ -646,7 +657,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
>  		goto unlock;
>  	}
>  
> -	dev_priv->psr.psr2_enabled = crtc_state->has_psr2;
> +	dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv,
> crtc_state);
>  	dev_priv->psr.busy_frontbuffer_bits = 0;
>  	dev_priv->psr.prepared = true;
>  
> @@ -820,19 +831,38 @@ 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)
> +{
> +	/* Can't switch psr state anyway if PSR2 is not supported.
> */
> +	if (!crtc_state || !crtc_state->has_psr2)
> +		return false;
> +
> +	if (dev_priv->psr.psr2_enabled && mode ==
> I915_PSR_DEBUG_FORCE_PSR1)
> +		return true;
> +
> +	if (!dev_priv->psr.psr2_enabled && mode !=
> I915_PSR_DEBUG_FORCE_PSR1)
> +		return true;
> +
> +	return false;
> +}
> +
>  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 intel_crtc_state *crtc_state = NULL;
>  	struct drm_crtc *crtc;
>  	struct intel_dp *dp;
>  	int ret;
>  	bool enable;
> +	u32 mode = val & I915_PSR_DEBUG_MODE_MASK;
>  
>  	if (val & ~(I915_PSR_DEBUG_IRQ | I915_PSR_DEBUG_MODE_MASK)
> ||
> -	    (val & I915_PSR_DEBUG_MODE_MASK) >
> I915_PSR_DEBUG_ENABLE) {
> +	    mode > I915_PSR_DEBUG_FORCE_PSR1) {
>  		DRM_DEBUG_KMS("Invalid debug mask %llx\n", val);
>  		return -EINVAL;
>  	}
> @@ -850,7 +880,8 @@ int intel_psr_set_debugfs_mode(struct
> drm_i915_private *dev_priv,
>  		if (ret)
>  			return ret;
>  
> -		ret = wait_for_completion_interruptible(&crtc-
> >state->commit->hw_done);
> +		crtc_state = to_intel_crtc_state(crtc->state);
> +		ret = wait_for_completion_interruptible(&crtc_state-
> >base.commit->hw_done);
>  	} else
>  		ret = wait_for_completion_interruptible(&conn_state-
> >commit->hw_done);
>  
> @@ -863,14 +894,17 @@ int intel_psr_set_debugfs_mode(struct
> drm_i915_private *dev_priv,
>  
>  	enable = psr_global_enabled(val);
>  
> -	if (!enable)
> +	if (!enable || switching_psr(dev_priv, crtc_state, mode))
>  		intel_psr_disable_locked(dev_priv->psr.dp);
>  
>  	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 &
> I915_PSR_DEBUG_IRQ);
>  
>  	if (dev_priv->psr.prepared && enable)
> -		intel_psr_enable_locked(dev_priv,
> to_intel_crtc_state(crtc->state));
> +		intel_psr_enable_locked(dev_priv, crtc_state);
>  
>  	mutex_unlock(&dev_priv->psr.lock);
>  	return ret;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Allow control of PSR at runtime through debugfs, v5

[Maarten Lankhorst]

Op 09-08-18 om 01:44 schreef Dhinakaran Pandiyan:
> On Wed, 2018-08-08 at 16:19 +0200, Maarten Lankhorst wrote:
>> 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)
>> Changes since v4:
>> - Return error in _set() function.
>> - Change flag values to make them easier to remember. (dhnkrn)
>> - Only assign psr.dp once. (dhnkrn)
>> - Only set crtc_state->has_psr on the crtc with psr.dp.
>> - Fix typo. (dhnkrn)
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c |  23 ++++-
>>  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    | 138 +++++++++++++++++++++++---
>> --
>>  5 files changed, 149 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index f9ce35da4123..3e81301a94ba 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2708,7 +2708,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);
>>  
>> @@ -2750,17 +2750,32 @@ 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;
>> +	return ret;
>>  }
>>  
>>  static int
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 657f46e0cae9..a3ea48ce1811 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_MODE_MASK	0x0f
>> +#define I915_PSR_DEBUG_DEFAULT		0x00
>> +#define I915_PSR_DEBUG_DISABLE		0x01
>> +#define I915_PSR_DEBUG_ENABLE		0x02
>> +#define I915_PSR_DEBUG_IRQ		0x10
>> +
>> +	u32 debug;
> 	u16?
>
>>  	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 8084e35b25c5..b2c9838442bc 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -4048,7 +4048,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 1ad7c1124bef..098ea2859a9b 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1924,7 +1924,7 @@ int intel_hdcp_enable(struct intel_connector
>> *connector);
>>  int intel_hdcp_disable(struct intel_connector *connector);
>>  int intel_hdcp_check_link(struct intel_connector *connector);
>>  bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port
>> port);
>> -
>> +//
> ^ seems like a ghost comment sneaked in.
>>  
>>  /* intel_psr.c */
>>  #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv-
>>> psr.sink_support)
>>  void intel_psr_init_dpcd(struct intel_dp *intel_dp);
>> @@ -1932,6 +1932,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..12fbc59af5a4 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);
>>  }
>>  
>> @@ -213,6 +224,9 @@ void intel_psr_init_dpcd(struct intel_dp
>> *intel_dp)
>>  	dev_priv->psr.sink_sync_latency =
>>  		intel_dp_get_sink_sync_latency(intel_dp);
>>  
>> +	WARN_ON(dev_priv->psr.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)) {
>>  		bool y_req = intel_dp->psr_dpcd[1] &
>> @@ -471,8 +485,8 @@ 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");
>> +	if (intel_dp != dev_priv->psr.dp) {
>> +		DRM_DEBUG_KMS("Not enabling PSR: wrong
>> connector\n");
> This debug message will get printed for non eDP ports, return silently?
I guess move it until after the PORT_A check?
>>  		return;
>>  	}
>>  
>> @@ -517,7 +531,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 +602,24 @@ 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
>> @@ -609,22 +640,20 @@ void intel_psr_enable(struct intel_dp
>> *intel_dp,
>>  	if (WARN_ON(!CAN_PSR(dev_priv)))
>>  		return;
>>  
>> -	WARN_ON(dev_priv->drrs.dp);
> This warning was to catch if DRRS was enabled along with PSR. the
> hardware supports only one at a time.
Oh right. I'll leave it there. Maybe add a DRRS disable call around PSR debugfs?
>>  	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;
>>  
>> -	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 +712,14 @@ static void intel_psr_disable_locked(struct
>> intel_dp *intel_dp)
>>  	if (!dev_priv->psr.enabled)
>>  		return;
>>  
>> +	DRM_DEBUG_KMS("Disabling PSR%s\n",
>> +		      dev_priv->psr.psr2_enabled ? "2" : "1");
>>  	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 +743,14 @@ 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);
>>  }
>> @@ -724,9 +762,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;
>> -
> We'll end up waiting for PSR to idle when updating CRTCs that are not
> driving a PSR panel. I should have checked this in the previous
> version, my bad.
>
> The patch looks good to me other than for the issues pointed out.
Ok, will reinstate this.

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

[PATCH v6 1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6

[Maarten Lankhorst]

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)
Changes since v4:
- Return error in _set() function.
- Change flag values to make them easier to remember. (dhnkrn)
- Only assign psr.dp once. (dhnkrn)
- Only set crtc_state->has_psr on the crtc with psr.dp.
- Fix typo. (dhnkrn)
Changes since v5:
- Only wait for PSR idle on the PSR connector correctly. (dhnkrn)
- Reinstate WARN_ON(drrs.dp) in intel_psr_enable. (dhnkrn)
- Remove stray comment. (dhnkrn)
- Be silent in intel_psr_compute_config on wrong connector. (dhnkrn)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  23 ++++-
 drivers/gpu/drm/i915/i915_drv.h     |  12 ++-
 drivers/gpu/drm/i915/i915_irq.c     |   2 +-
 drivers/gpu/drm/i915/intel_drv.h    |   3 +
 drivers/gpu/drm/i915/intel_psr.c    | 134 +++++++++++++++++++++++-----
 5 files changed, 146 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f9ce35da4123..3e81301a94ba 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2708,7 +2708,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);
 
@@ -2750,17 +2750,32 @@ 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;
+	return ret;
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 657f46e0cae9..a3ea48ce1811 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_MODE_MASK	0x0f
+#define I915_PSR_DEBUG_DEFAULT		0x00
+#define I915_PSR_DEBUG_DISABLE		0x01
+#define I915_PSR_DEBUG_ENABLE		0x02
+#define I915_PSR_DEBUG_IRQ		0x10
+
+	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 8084e35b25c5..b2c9838442bc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4048,7 +4048,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 1ad7c1124bef..4ac468b7297f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1932,6 +1932,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..e9ca410e18c4 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);
 }
 
@@ -213,6 +224,9 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 	dev_priv->psr.sink_sync_latency =
 		intel_dp_get_sink_sync_latency(intel_dp);
 
+	WARN_ON(dev_priv->psr.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)) {
 		bool y_req = intel_dp->psr_dpcd[1] &
@@ -471,10 +485,8 @@ 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");
+	if (intel_dp != dev_priv->psr.dp)
 		return;
-	}
 
 	/*
 	 * HSW spec explicitly says PSR is tied to port A.
@@ -517,7 +529,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 +600,24 @@ 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
@@ -610,21 +639,21 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 		return;
 
 	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;
 
-	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 +712,14 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp)
 	if (!dev_priv->psr.enabled)
 		return;
 
+	DRM_DEBUG_KMS("Disabling PSR%s\n",
+		      dev_priv->psr.psr2_enabled ? "2" : "1");
 	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 +743,14 @@ 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);
 }
@@ -724,7 +762,7 @@ 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)
+	if (!dev_priv->psr.enabled || !new_crtc_state->has_psr)
 		return 0;
 
 	/*
@@ -756,13 +794,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 +820,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 +903,7 @@ 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 +958,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 +1001,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 +1083,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

✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6 (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v6,1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6 (rev2)
URL   : https://patchwork.freedesktop.org/series/47888/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f12e3f989693 drm/i915: Allow control of PSR at runtime through debugfs, v6
-:25: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#25: 
- Add prepared bool, which should be used instead of relying on psr.dp. (dhnkrn)

-:352: CHECK:BRACES: braces {} should be used on all arms of this statement
#352: FILE: drivers/gpu/drm/i915/intel_psr.c:848:
+	if (crtc) {
[...]
+	} else
[...]

-:358: CHECK:BRACES: Unbalanced braces around else statement
#358: FILE: drivers/gpu/drm/i915/intel_psr.c:854:
+	} else

total: 0 errors, 1 warnings, 2 checks, 334 lines checked
3a302bbbaa65 drm/i915/psr: Add debugfs support to force a downgrade to PSR1 mode.

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

✗ Fi.CI.SPARSE: warning for series starting with [v6,1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6 (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v6,1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6 (rev2)
URL   : https://patchwork.freedesktop.org/series/47888/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Allow control of PSR at runtime through debugfs, v6
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3675:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3683:16: warning: expression using sizeof(void)

Commit: drm/i915/psr: Add debugfs support to force a downgrade to PSR1 mode.
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3683:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3684:16: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for series starting with [v6,1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6 (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v6,1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6 (rev2)
URL   : https://patchwork.freedesktop.org/series/47888/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4639 -> Patchwork_9911 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47888/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9911 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362, fdo#103191)

    {igt@kms_psr@primary_mmap_gtt}:
      fi-cnl-psr:         PASS -> DMESG-WARN (fdo#107372)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    {igt@amdgpu/amd_prime@amd-to-i915}:
      fi-bxt-j4205:       INCOMPLETE (fdo#103927) -> SKIP

    {igt@amdgpu/amd_prime@i915-to-amd}:
      fi-bxt-j4205:       DMESG-FAIL -> SKIP

    igt@drv_selftest@live_workarounds:
      {fi-cfl-8109u}:     DMESG-FAIL (fdo#107292) -> PASS
      {fi-bsw-kefka}:     DMESG-FAIL (fdo#107292) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      {fi-byt-clapper}:   FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (53 -> 48) ==

  Additional (1): fi-gdg-551 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_4639 -> Patchwork_9911

  CI_DRM_4639: da34c4841d4bd5098cef0bd3ddaeed1ee3eb3103 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4591: 6cb3d7dbe5831a7b2b5b7a4638d8a8b7ac624f5f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9911: 3a302bbbaa65dc1cdc489809173533b7b0db834f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3a302bbbaa65 drm/i915/psr: Add debugfs support to force a downgrade to PSR1 mode.
f12e3f989693 drm/i915: Allow control of PSR at runtime through debugfs, v6

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9911/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [v6,1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6 (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v6,1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6 (rev2)
URL   : https://patchwork.freedesktop.org/series/47888/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4639_full -> Patchwork_9911_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9911_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9911_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9911_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_eio@reset-stress:
      shard-apl:          PASS -> FAIL

    
== Known issues ==

  Here are the changes found in Patchwork_9911_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_eio@wait-wedge-immediate:
      shard-glk:          PASS -> FAIL (fdo#105957)

    igt@gem_exec_await@wide-contexts:
      shard-apl:          PASS -> FAIL (fdo#106680, fdo#105900)

    igt@kms_atomic_transition@plane-all-modeset-transition-fencing:
      shard-hsw:          PASS -> DMESG-WARN (fdo#102614)

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-kbl:          PASS -> DMESG-WARN (fdo#103313)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-apl:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-hsw:          DMESG-WARN (fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS +1

    igt@perf@polling:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#105957 https://bugs.freedesktop.org/show_bug.cgi?id=105957
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4639 -> Patchwork_9911

  CI_DRM_4639: da34c4841d4bd5098cef0bd3ddaeed1ee3eb3103 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4591: 6cb3d7dbe5831a7b2b5b7a4638d8a8b7ac624f5f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9911: 3a302bbbaa65dc1cdc489809173533b7b0db834f @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9911/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v6 1/2] drm/i915: Allow control of PSR at runtime through debugfs, v6

[Maarten Lankhorst]

Op 09-08-18 om 16:21 schreef Maarten Lankhorst:
> 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)
> Changes since v4:
> - Return error in _set() function.
> - Change flag values to make them easier to remember. (dhnkrn)
> - Only assign psr.dp once. (dhnkrn)
> - Only set crtc_state->has_psr on the crtc with psr.dp.
> - Fix typo. (dhnkrn)
> Changes since v5:
> - Only wait for PSR idle on the PSR connector correctly. (dhnkrn)
> - Reinstate WARN_ON(drrs.dp) in intel_psr_enable. (dhnkrn)
> - Remove stray comment. (dhnkrn)
> - Be silent in intel_psr_compute_config on wrong connector. (dhnkrn)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  23 ++++-
>  drivers/gpu/drm/i915/i915_drv.h     |  12 ++-
>  drivers/gpu/drm/i915/i915_irq.c     |   2 +-
>  drivers/gpu/drm/i915/intel_drv.h    |   3 +
>  drivers/gpu/drm/i915/intel_psr.c    | 134 +++++++++++++++++++++++-----
>  5 files changed, 146 insertions(+), 28 deletions(-)
Pushed with dhnkrn's irc r-b
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx