Re: [PATCH v3 08/23] drm/i915/tgl: Access the right register when handling PSR interruptions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 26, 2019 at 03:23:55PM +0530, Anshuman Gupta wrote:
On 2019-08-23 at 01:20:40 -0700, Lucas De Marchi wrote:
From: José Roberto de Souza <jose.souza@xxxxxxxxx>

For older gens PSR IIR and IMR had a fixed address that was not
relative to anything, but from TGL those registers moved to each
transcoder offset.

So here adding a new macro and a new PSR irq handler with the
transcoder parameter.

There are few minor comments below, apart from below comments
patch is looks ok to me.

Those are fixed and queued for next revision.

Reviewed-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>

Thanks
Lucas De Marchi

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 67 ++++++++++++++++++------
 drivers/gpu/drm/i915/display/intel_psr.h |  1 +
 drivers/gpu/drm/i915/i915_irq.c          | 52 +++++++++++++++---
 drivers/gpu/drm/i915/i915_reg.h          | 10 +++-
 4 files changed, 107 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 2429328f963e..c33aa16ed038 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -91,20 +91,33 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
 static void psr_irq_control(struct drm_i915_private *dev_priv)
 {
 	enum transcoder trans = dev_priv->psr.transcoder;
-	u32 val, mask;
+	u32 psr_error, psr_entry, psr_exit, mask, val;
+	i915_reg_t mask_reg;
+
+	if (INTEL_GEN(dev_priv) >= 12) {
+		psr_error = TRANS_PSR_ERROR;
+		psr_entry = TRANS_PSR_PRE_ENTRY;
+		psr_exit = TRANS_PSR_POST_EXIT;
+		mask_reg = TRANS_PSR_IMR(trans);
+	} else {
+		psr_error = EDP_PSR_ERROR(trans);
+		psr_entry = EDP_PSR_PRE_ENTRY(trans);
+		psr_exit = EDP_PSR_POST_EXIT(trans);
+		mask_reg = EDP_PSR_IMR;
+	}

-	mask = EDP_PSR_ERROR(trans);
+	mask = psr_error;
 	if (dev_priv->psr.debug & I915_PSR_DEBUG_IRQ)
-		mask |= EDP_PSR_POST_EXIT(trans) | EDP_PSR_PRE_ENTRY(trans);
+		mask |= psr_exit | psr_entry;

 	/*
 	 * TODO: when handling multiple PSR instances a global spinlock will be
 	 * needed to synchronize the value of shared register
 	 */
-	val = I915_READ(EDP_PSR_IMR);
-	val &= ~EDP_PSR_TRANS_MASK(trans);
+	val = I915_READ(mask_reg);
+	val &= ~(psr_error | psr_entry | psr_exit);
 	val |= ~mask;
-	I915_WRITE(EDP_PSR_IMR, val);
+	I915_WRITE(mask_reg, val);
 }

 static void psr_event_print(u32 val, bool psr2_enabled)
@@ -147,9 +160,21 @@ static void psr_event_print(u32 val, bool psr2_enabled)
 void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
 {
 	enum transcoder cpu_transcoder = dev_priv->psr.transcoder;
+	u32 psr_error, psr_entry, psr_exit;
 	ktime_t time_ns =  ktime_get();

-	if (psr_iir & EDP_PSR_ERROR(cpu_transcoder)) {
+	if (INTEL_GEN(dev_priv) >= 12) {
+		psr_error = TRANS_PSR_ERROR;
+		psr_entry = TRANS_PSR_PRE_ENTRY;
+		psr_exit = TRANS_PSR_POST_EXIT;
+	} else {
+		psr_error = EDP_PSR_ERROR(cpu_transcoder);
+		psr_entry = EDP_PSR_PRE_ENTRY(cpu_transcoder);
+		psr_exit = EDP_PSR_POST_EXIT(cpu_transcoder);
+	}
+
+	if (psr_iir & psr_error) {
+		i915_reg_t mask_reg;
 		u32 val;

 		DRM_WARN("[transcoder %s] PSR aux error\n",
@@ -168,20 +193,25 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
 		 * TODO: when handling multiple PSR instances a global spinlock
 		 * will be needed to synchronize the value of shared register
 		 */
-		val = I915_READ(EDP_PSR_IMR);
-		val |= EDP_PSR_ERROR(cpu_transcoder);
-		I915_WRITE(EDP_PSR_IMR, val);
+		if (INTEL_GEN(dev_priv) >= 12)
+			mask_reg = TRANS_PSR_IMR(cpu_transcoder);
+		else
+			mask_reg = EDP_PSR_IMR;
+
+		val = I915_READ(mask_reg);
+		val |= psr_error;
+		I915_WRITE(mask_reg, val);

 		schedule_work(&dev_priv->psr.work);
 	}

-	if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
+	if (psr_iir & psr_entry) {
 		dev_priv->psr.last_entry_attempt = time_ns;
 		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)) {
+	if (psr_iir & psr_exit) {
 		dev_priv->psr.last_exit = time_ns;
 		DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
 			      transcoder_name(cpu_transcoder));
@@ -632,7 +662,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);

 	if (INTEL_GEN(dev_priv) >= 9 &&
-	    psr2_supported(dev_priv, dev_priv->psr.transcoder))
+	    transcoder_has_psr2(dev_priv, dev_priv->psr.transcoder))
 		WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)) & EDP_PSR2_ENABLE);
 	WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & EDP_PSR_ENABLE);
 	WARN_ON(dev_priv->psr.active);
@@ -730,8 +760,13 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
 	 * first time that PSR HW tries to activate so lets keep PSR disabled
 	 * to avoid any rendering problems.
 	 */
-	val = I915_READ(EDP_PSR_IIR);
-	val &= EDP_PSR_ERROR(dev_priv->psr.transcoder);
+	if (INTEL_GEN(dev_priv) >= 12) {
+		val = I915_READ(TRANS_PSR_IIR(dev_priv->psr.transcoder));
+		val &= TRANS_PSR_ERROR;
+	} else {
+		val = I915_READ(EDP_PSR_IIR);
+		val &= EDP_PSR_ERROR(dev_priv->psr.transcoder);
+	}
 	if (val) {
 		dev_priv->psr.sink_not_reliable = true;
 		DRM_DEBUG_KMS("PSR interruption error set, not enabling PSR\n");
@@ -787,7 +822,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)

 	if (!dev_priv->psr.active) {
 		if (INTEL_GEN(dev_priv) >= 9 &&
-		    psr2_supported(dev_priv, dev_priv->psr.transcoder)) {
+		    transcoder_has_psr2(dev_priv, dev_priv->psr.transcoder)) {
 			val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
 			WARN_ON(val & EDP_PSR2_ENABLE);
 		}
diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
index 46e4de8b8cd5..6570a23a68b2 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.h
+++ b/drivers/gpu/drm/i915/display/intel_psr.h
@@ -6,6 +6,7 @@
 #ifndef __INTEL_PSR_H__
 #define __INTEL_PSR_H__

+#include "intel_display.h"
IMO we don't need to include this header.
 #include "intel_frontbuffer.h"

 struct drm_i915_private;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 77391d8325bf..6024a6ef1c76 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2655,11 +2655,22 @@ gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32 iir)
 	}

 	if (iir & GEN8_DE_EDP_PSR) {
-		u32 psr_iir = I915_READ(EDP_PSR_IIR);
+		u32 psr_iir;
+
+		if (INTEL_GEN(dev_priv) >= 12) {
+			enum transcoder trans = dev_priv->psr.transcoder;
+
+			psr_iir = I915_READ(TRANS_PSR_IIR(trans));
+			I915_WRITE(TRANS_PSR_IIR(trans), psr_iir);
+		} else {
+			psr_iir = I915_READ(EDP_PSR_IIR);
+			I915_WRITE(EDP_PSR_IIR, psr_iir);
+		}
+
+		if (psr_iir)
+			found = true;

 		intel_psr_irq_handler(dev_priv, psr_iir);
-		I915_WRITE(EDP_PSR_IIR, psr_iir);
-		found = true;
 	}

 	if (!found)
@@ -3279,8 +3290,23 @@ static void gen11_irq_reset(struct drm_i915_private *dev_priv)

 	intel_uncore_write(uncore, GEN11_DISPLAY_INT_CTL, 0);

-	intel_uncore_write(uncore, EDP_PSR_IMR, 0xffffffff);
-	intel_uncore_write(uncore, EDP_PSR_IIR, 0xffffffff);
+	if (INTEL_GEN(dev_priv) >= 12) {
+		enum transcoder trans;
+
+		for (trans = TRANSCODER_A; trans <= TRANSCODER_D; trans++) {
+			enum intel_display_power_domain domain;
+
+			domain = POWER_DOMAIN_TRANSCODER(trans);
+			if (!intel_display_power_is_enabled(dev_priv, domain))
+				continue;
+
+			intel_uncore_write(uncore, TRANS_PSR_IMR(trans), 0xffffffff);
+			intel_uncore_write(uncore, TRANS_PSR_IIR(trans), 0xffffffff);
Lines over 80 char.
+		}
+	} else {
+		intel_uncore_write(uncore, EDP_PSR_IMR, 0xffffffff);
+		intel_uncore_write(uncore, EDP_PSR_IIR, 0xffffffff);
+	}

 	for_each_pipe(dev_priv, pipe)
 		if (intel_display_power_is_enabled(dev_priv,
@@ -3793,7 +3819,21 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	else if (IS_BROADWELL(dev_priv))
 		de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;

-	gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
+	if (INTEL_GEN(dev_priv) >= 12) {
+		enum transcoder trans;
+
+		for (trans = TRANSCODER_A; trans <= TRANSCODER_D; trans++) {
+			enum intel_display_power_domain domain;
+
+			domain = POWER_DOMAIN_TRANSCODER(trans);
+			if (!intel_display_power_is_enabled(dev_priv, domain))
+				continue;
+
+			gen3_assert_iir_is_zero(uncore, TRANS_PSR_IIR(trans));
+		}
+	} else {
+		gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
+	}

 	for_each_pipe(dev_priv, pipe) {
 		dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1c6d99944630..3de02683d856 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4222,7 +4222,7 @@ enum {
 #define   EDP_PSR_TP1_TIME_0us			(3 << 4)
 #define   EDP_PSR_IDLE_FRAME_SHIFT		0

-/* Bspec claims those aren't shifted but stay at 0x64800 */
+/* Bspec claims those aren't shifted but stay at 0x64800 until TGL */
 #define EDP_PSR_IMR				_MMIO(0x64834)
 #define EDP_PSR_IIR				_MMIO(0x64838)
 #define   _EDP_PSR_TRANS_SHIFT(trans)		((trans) == TRANSCODER_EDP ? \
@@ -4232,6 +4232,14 @@ enum {
 #define   EDP_PSR_POST_EXIT(trans)		(0x2 << _EDP_PSR_TRANS_SHIFT(trans))
 #define   EDP_PSR_PRE_ENTRY(trans)		(0x1 << _EDP_PSR_TRANS_SHIFT(trans))

+#define _PSR_IMR_A				0x60814
+#define _PSR_IIR_A				0x60818
+#define TRANS_PSR_IMR(tran)			_MMIO_TRANS2(tran, _PSR_IMR_A) /* TGL+ */
+#define TRANS_PSR_IIR(tran)			_MMIO_TRANS2(tran, _PSR_IIR_A) /* TGL+ */
IMO if /* TGL+ */ comment shifted above, it will satisfy the 80 char limit.
+#define   TRANS_PSR_ERROR			(1 << 2)
+#define   TRANS_PSR_POST_EXIT			(1 << 1)
+#define   TRANS_PSR_PRE_ENTRY			(1 << 0)
+
 #define _SRD_AUX_CTL_A				0x60810
 #define _SRD_AUX_CTL_EDP			0x6f810
 #define EDP_PSR_AUX_CTL(tran)			_MMIO(_PSR_ADJ(tran, _SRD_AUX_CTL_A))
--
2.23.0

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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux