diff mbox

[v3] drm/i915/psr: Control PSR interrupts via debugfs

Message ID 20180330211545.4115-1-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dhinakaran Pandiyan March 30, 2018, 9:15 p.m. UTC 
Interrupts other than the one for AUX errors are required only for debug,
so unmask them via debugfs when the user requests debug.

User can make such a request with
echo 1 > <DEBUG_FS>/dri/0/i915_edp_psr_debug

There are no locks to serialize PSR debug enabling from
irq_postinstall() and debugfs for simplicity. As irq_postinstall() is
called only during module initialization/resume and IGT subtests
aren't expected to modify PSR debug at those times, we should be safe.

v2: Unroll loops (Ville)
    Avoid resetting error mask bits.

v3: Unmask interrupts in postinstall() if debug was still enabled.
    Avoid RMW (Ville)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 33 +++++++++++++++++++-
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_irq.c     | 57 +++++++++++------------------------
 drivers/gpu/drm/i915/intel_drv.h    |  2 ++
 drivers/gpu/drm/i915/intel_psr.c    | 60 +++++++++++++++++++++++++++++++++++++
 5 files changed, 113 insertions(+), 40 deletions(-)

Comments

Dhinakaran Pandiyan March 30, 2018, 9:22 p.m. UTC | #1 
On Fri, 2018-03-30 at 14:15 -0700, Dhinakaran Pandiyan wrote:
> Interrupts other than the one for AUX errors are required only for debug,

> so unmask them via debugfs when the user requests debug.

> 

> User can make such a request with

> echo 1 > <DEBUG_FS>/dri/0/i915_edp_psr_debug

> 

> There are no locks to serialize PSR debug enabling from

> irq_postinstall() and debugfs for simplicity. As irq_postinstall() is

> called only during module initialization/resume and IGT subtests

> aren't expected to modify PSR debug at those times, we should be safe.


Let me know what you think, I am not completely sure about this.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ff90577da450..5e697867a8ca 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2690,6 +2690,36 @@  static int i915_edp_psr_status(struct seq_file *m, void *data)
 	return 0;
 }
 
+static int
+i915_edp_psr_debug_set(void *data, u64 val)
+{
+	struct drm_i915_private *dev_priv = data;
+
+	if (!CAN_PSR(dev_priv))
+		return -ENODEV;
+
+	DRM_DEBUG_KMS("PSR debug %s\n", enableddisabled(val));
+	intel_psr_debug_control(dev_priv, !!val);
+
+	return 0;
+}
+
+static int
+i915_edp_psr_debug_get(void *data, u64 *val)
+{
+	struct drm_i915_private *dev_priv = data;
+
+	if (!CAN_PSR(dev_priv))
+		return -ENODEV;
+
+	*val = READ_ONCE(dev_priv->psr.debug);
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_edp_psr_debug_fops,
+			i915_edp_psr_debug_get, i915_edp_psr_debug_set,
+			"%llu\n");
+
 static int i915_sink_crc(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4812,7 +4842,8 @@  static const struct i915_debugfs_files {
 	{"i915_guc_log_relay", &i915_guc_log_relay_fops},
 	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
 	{"i915_ipc_status", &i915_ipc_status_fops},
-	{"i915_drrs_ctl", &i915_drrs_ctl_fops}
+	{"i915_drrs_ctl", &i915_drrs_ctl_fops},
+	{"i915_edp_psr_debug", &i915_edp_psr_debug_fops}
 };
 
 int i915_debugfs_register(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 800230ba1c3b..71577ebf6a60 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -609,6 +609,7 @@  struct i915_psr {
 	bool colorimetry_support;
 	bool alpm;
 	bool has_hw_tracking;
+	bool debug;
 
 	void (*enable_source)(struct intel_dp *,
 			      const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8a894adf2ca1..714570955196 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2391,40 +2391,6 @@  static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
 		ironlake_rps_change_irq_handler(dev_priv);
 }
 
-static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
-{
-	u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
-	u32 edp_psr_imr = I915_READ(EDP_PSR_IMR);
-	u32 mask = BIT(TRANSCODER_EDP);
-	enum transcoder cpu_transcoder;
-
-	if (INTEL_GEN(dev_priv) >= 8)
-		mask |= BIT(TRANSCODER_A) |
-			BIT(TRANSCODER_B) |
-			BIT(TRANSCODER_C);
-
-	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, mask) {
-		if (edp_psr_iir & EDP_PSR_ERROR(cpu_transcoder))
-			DRM_DEBUG_KMS("Transcoder %s PSR error\n",
-				      transcoder_name(cpu_transcoder));
-
-		if (edp_psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
-			DRM_DEBUG_KMS("Transcoder %s PSR prepare entry in 2 vblanks\n",
-				      transcoder_name(cpu_transcoder));
-			edp_psr_imr |= EDP_PSR_PRE_ENTRY(cpu_transcoder);
-		}
-
-		if (edp_psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
-			DRM_DEBUG_KMS("Transcoder %s PSR exit completed\n",
-				      transcoder_name(cpu_transcoder));
-			edp_psr_imr &= ~EDP_PSR_PRE_ENTRY(cpu_transcoder);
-		}
-	}
-
-	I915_WRITE(EDP_PSR_IMR, edp_psr_imr);
-	I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
-}
-
 static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
 				    u32 de_iir)
 {
@@ -2437,8 +2403,12 @@  static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
 	if (de_iir & DE_ERR_INT_IVB)
 		ivb_err_int_handler(dev_priv);
 
-	if (de_iir & DE_EDP_PSR_INT_HSW)
-		hsw_edp_psr_irq_handler(dev_priv);
+	if (de_iir & DE_EDP_PSR_INT_HSW) {
+		u32 psr_iir = I915_READ(EDP_PSR_IIR);
+
+		intel_psr_irq_handler(dev_priv, psr_iir);
+		I915_WRITE(EDP_PSR_IIR, psr_iir);
+	}
 
 	if (de_iir & DE_AUX_CHANNEL_A_IVB)
 		dp_aux_irq_handler(dev_priv);
@@ -2580,7 +2550,10 @@  gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 			}
 
 			if (iir & GEN8_DE_EDP_PSR) {
-				hsw_edp_psr_irq_handler(dev_priv);
+				u32 psr_iir = I915_READ(EDP_PSR_IIR);
+
+				intel_psr_irq_handler(dev_priv, psr_iir);
+				I915_WRITE(EDP_PSR_IIR, psr_iir);
 				found = true;
 			}
 
@@ -3729,7 +3702,8 @@  static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	if (IS_HASWELL(dev_priv)) {
 		gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
-		I915_WRITE(EDP_PSR_IMR, 0);
+		I915_WRITE(EDP_PSR_IMR, ~EDP_PSR_ERROR(TRANSCODER_EDP));
+		intel_psr_debug_control(dev_priv, dev_priv->psr.debug);
 		display_mask |= DE_EDP_PSR_INT_HSW;
 	}
 
@@ -3845,6 +3819,7 @@  static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	u32 de_port_enables;
 	u32 de_misc_masked = GEN8_DE_MISC_GSE | GEN8_DE_EDP_PSR;
 	enum pipe pipe;
+	u32 psr_masked;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
 		de_pipe_masked |= GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
@@ -3869,7 +3844,11 @@  static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 		de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
 
 	gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
-	I915_WRITE(EDP_PSR_IMR, 0);
+	psr_masked = EDP_PSR_ERROR(TRANSCODER_EDP) |
+		     EDP_PSR_ERROR(TRANSCODER_A) | EDP_PSR_ERROR(TRANSCODER_B) |
+		     EDP_PSR_ERROR(TRANSCODER_C);
+	I915_WRITE(EDP_PSR_IMR, ~psr_masked);
+	intel_psr_debug_control(dev_priv, dev_priv->psr.debug);
 
 	for_each_pipe(dev_priv, pipe) {
 		dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d1452fd2a58d..53cb76b920e6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1891,6 +1891,8 @@  void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
 				   unsigned frontbuffer_bits);
 void intel_psr_compute_config(struct intel_dp *intel_dp,
 			      struct intel_crtc_state *crtc_state);
+void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool enable);
+void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir);
 
 /* intel_runtime_pm.c */
 int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b8e083e10029..0d9305f98d33 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -93,6 +93,66 @@  static void psr_aux_io_power_put(struct intel_dp *intel_dp)
 	intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
 }
 
+void intel_psr_debug_control(struct drm_i915_private *dev_priv, bool enable)
+{
+	u32 debug_mask, error_mask;
+
+	/* No PSR interrupts on VLV/CHV */
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		return;
+
+	error_mask = EDP_PSR_ERROR(TRANSCODER_EDP);
+	debug_mask = error_mask | EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
+		     EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
+
+	if (INTEL_GEN(dev_priv) >= 8) {
+		error_mask |= EDP_PSR_ERROR(TRANSCODER_A) |
+			      EDP_PSR_ERROR(TRANSCODER_B) |
+			      EDP_PSR_ERROR(TRANSCODER_C);
+
+		debug_mask |= error_mask | EDP_PSR_POST_EXIT(TRANSCODER_A) |
+			      EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
+			      EDP_PSR_POST_EXIT(TRANSCODER_B) |
+			      EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
+			      EDP_PSR_POST_EXIT(TRANSCODER_C) |
+			      EDP_PSR_PRE_ENTRY(TRANSCODER_C);
+	}
+
+	if (enable) {
+		WRITE_ONCE(dev_priv->psr.debug, true);
+		I915_WRITE(EDP_PSR_IMR, ~debug_mask);
+	} else {
+		I915_WRITE(EDP_PSR_IMR, ~error_mask);
+		WRITE_ONCE(dev_priv->psr.debug, false);
+	}
+}
+
+void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
+{
+	u32 transcoders = BIT(TRANSCODER_EDP);
+	enum transcoder cpu_transcoder;
+
+	if (INTEL_GEN(dev_priv) >= 8)
+		transcoders |= BIT(TRANSCODER_A) |
+			       BIT(TRANSCODER_B) |
+			       BIT(TRANSCODER_C);
+
+	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
+		/* FIXME: Exit PSR and link train manually when this happens. */
+		if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
+			DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
+				      transcoder_name(cpu_transcoder));
+
+		if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder))
+			DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
+				      transcoder_name(cpu_transcoder));
+
+		if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder))
+			DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
+				      transcoder_name(cpu_transcoder));
+	}
+}
+
 static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
 {
 	uint8_t psr_caps = 0;