[PATCH 1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment

[PATCH 1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment

[José Roberto de Souza]

Turn out it is not a DMC bug it is actually a HW one, so this
workaround will be needed for current gens, lets update the comment
and remove the FIXME.

BSpec: 7723
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index ec874d802d48..c80bb3003a7d 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -531,10 +531,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 		val |= EDP_PSR2_TP2_TIME_2500us;
 
 	/*
-	 * FIXME: There is probably a issue in DMC firmwares(icl_dmc_ver1_07.bin
-	 * and kbl_dmc_ver1_04.bin at least) that causes PSR2 SU to fail after
-	 * exiting DC6 if EDP_PSR_TP1_TP3_SEL is kept in PSR_CTL, so for now
-	 * lets workaround the issue by cleaning PSR_CTL before enable PSR2.
+	 * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec is
+	 * recommending keep this bit unset while PSR2 is enabled.
 	 */
 	I915_WRITE(EDP_PSR_CTL, 0);
 
-- 
2.21.0

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

[PATCH 2/7] drm/i915: Remove unused VLV/CHV PSR registers

[José Roberto de Souza]

PSR support for VLV and CHV was dropped in commit ce3508fd2a77
("drm/i915/psr: Nuke PSR support for VLV and CHV") so no need to keep
this registers around.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 36 ---------------------------------
 1 file changed, 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 00e03560c4e7..c59cfa83dbaf 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4209,42 +4209,6 @@ enum {
 #define PIPESRC(trans)		_MMIO_TRANS2(trans, _PIPEASRC)
 #define PIPE_MULT(trans)	_MMIO_TRANS2(trans, _PIPE_MULT_A)
 
-/* VLV eDP PSR registers */
-#define _PSRCTLA				(VLV_DISPLAY_BASE + 0x60090)
-#define _PSRCTLB				(VLV_DISPLAY_BASE + 0x61090)
-#define  VLV_EDP_PSR_ENABLE			(1 << 0)
-#define  VLV_EDP_PSR_RESET			(1 << 1)
-#define  VLV_EDP_PSR_MODE_MASK			(7 << 2)
-#define  VLV_EDP_PSR_MODE_HW_TIMER		(1 << 3)
-#define  VLV_EDP_PSR_MODE_SW_TIMER		(1 << 2)
-#define  VLV_EDP_PSR_SINGLE_FRAME_UPDATE	(1 << 7)
-#define  VLV_EDP_PSR_ACTIVE_ENTRY		(1 << 8)
-#define  VLV_EDP_PSR_SRC_TRANSMITTER_STATE	(1 << 9)
-#define  VLV_EDP_PSR_DBL_FRAME			(1 << 10)
-#define  VLV_EDP_PSR_FRAME_COUNT_MASK		(0xff << 16)
-#define  VLV_EDP_PSR_IDLE_FRAME_SHIFT		16
-#define VLV_PSRCTL(pipe)	_MMIO_PIPE(pipe, _PSRCTLA, _PSRCTLB)
-
-#define _VSCSDPA			(VLV_DISPLAY_BASE + 0x600a0)
-#define _VSCSDPB			(VLV_DISPLAY_BASE + 0x610a0)
-#define  VLV_EDP_PSR_SDP_FREQ_MASK	(3 << 30)
-#define  VLV_EDP_PSR_SDP_FREQ_ONCE	(1 << 31)
-#define  VLV_EDP_PSR_SDP_FREQ_EVFRAME	(1 << 30)
-#define VLV_VSCSDP(pipe)	_MMIO_PIPE(pipe, _VSCSDPA, _VSCSDPB)
-
-#define _PSRSTATA			(VLV_DISPLAY_BASE + 0x60094)
-#define _PSRSTATB			(VLV_DISPLAY_BASE + 0x61094)
-#define  VLV_EDP_PSR_LAST_STATE_MASK	(7 << 3)
-#define  VLV_EDP_PSR_CURR_STATE_MASK	7
-#define  VLV_EDP_PSR_DISABLED		(0 << 0)
-#define  VLV_EDP_PSR_INACTIVE		(1 << 0)
-#define  VLV_EDP_PSR_IN_TRANS_TO_ACTIVE	(2 << 0)
-#define  VLV_EDP_PSR_ACTIVE_NORFB_UP	(3 << 0)
-#define  VLV_EDP_PSR_ACTIVE_SF_UPDATE	(4 << 0)
-#define  VLV_EDP_PSR_EXIT		(5 << 0)
-#define  VLV_EDP_PSR_IN_TRANS		(1 << 7)
-#define VLV_PSRSTAT(pipe)	_MMIO_PIPE(pipe, _PSRSTATA, _PSRSTATB)
-
 /* HSW+ eDP PSR registers */
 #define HSW_EDP_PSR_BASE	0x64800
 #define BDW_EDP_PSR_BASE	0x6f800
-- 
2.21.0

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

[PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable

[José Roberto de Souza]

Even when driver is reloaded and hits this scenario the PSR mutex
should be initialized, otherwise reading PSR debugfs status will
execute mutex_lock() over a mutex that was not initialized.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c80bb3003a7d..a84da931c3be 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	if (val) {
 		DRM_DEBUG_KMS("PSR interruption error set\n");
 		dev_priv->psr.sink_not_reliable = true;
-		return;
 	}
 
 	/* Set link_standby x link_off defaults */
-- 
2.21.0

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

[PATCH 4/7] drm/i915/psr: Do not enable PSR in interlaced mode for all GENs

[José Roberto de Souza]

This interlaced restriction applies to all gens, not only to Haswell.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index a84da931c3be..bb97c1657493 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -627,8 +627,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 		return;
 	}
 
-	if (IS_HASWELL(dev_priv) &&
-	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
+	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
 		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
 		return;
 	}
-- 
2.21.0

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

[PATCH 5/7] drm/i915/bdw+: Move misc display IRQ handling to it own function

[José Roberto de Souza]

Just moving it to reduce the tabs and avoid break code lines.
No behavior changes intended here.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 63 +++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index aa107a78cb36..527d5cb21baa 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2702,41 +2702,50 @@ static u32 gen8_de_port_aux_mask(struct drm_i915_private *dev_priv)
 	return mask;
 }
 
-static irqreturn_t
-gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
+static enum irqreturn
+gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv)
 {
-	irqreturn_t ret = IRQ_NONE;
-	u32 iir;
-	enum pipe pipe;
+	u32 iir = I915_READ(GEN8_DE_MISC_IIR);
+	enum irqreturn ret = IRQ_NONE;
+	bool found = false;
 
-	if (master_ctl & GEN8_DE_MISC_IRQ) {
-		iir = I915_READ(GEN8_DE_MISC_IIR);
-		if (iir) {
-			bool found = false;
-
-			I915_WRITE(GEN8_DE_MISC_IIR, iir);
-			ret = IRQ_HANDLED;
+	if (!iir) {
+		DRM_ERROR("The master control interrupt lied (DE MISC)!\n");
+		return ret;
+	}
 
-			if (iir & GEN8_DE_MISC_GSE) {
-				intel_opregion_asle_intr(dev_priv);
-				found = true;
-			}
+	I915_WRITE(GEN8_DE_MISC_IIR, iir);
+	ret = IRQ_HANDLED;
 
-			if (iir & GEN8_DE_EDP_PSR) {
-				u32 psr_iir = I915_READ(EDP_PSR_IIR);
+	if (iir & GEN8_DE_MISC_GSE) {
+		intel_opregion_asle_intr(dev_priv);
+		found = true;
+	}
 
-				intel_psr_irq_handler(dev_priv, psr_iir);
-				I915_WRITE(EDP_PSR_IIR, psr_iir);
-				found = true;
-			}
+	if (iir & GEN8_DE_EDP_PSR) {
+		u32 psr_iir = I915_READ(EDP_PSR_IIR);
 
-			if (!found)
-				DRM_ERROR("Unexpected DE Misc interrupt\n");
-		}
-		else
-			DRM_ERROR("The master control interrupt lied (DE MISC)!\n");
+		intel_psr_irq_handler(dev_priv, psr_iir);
+		I915_WRITE(EDP_PSR_IIR, psr_iir);
+		found = true;
 	}
 
+	if (!found)
+		DRM_ERROR("Unexpected DE Misc interrupt\n");
+
+	return ret;
+}
+
+static irqreturn_t
+gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
+{
+	irqreturn_t ret = IRQ_NONE;
+	u32 iir;
+	enum pipe pipe;
+
+	if (master_ctl & GEN8_DE_MISC_IRQ)
+		ret = gen8_de_misc_irq_handler(dev_priv);
+
 	if (INTEL_GEN(dev_priv) >= 11 && (master_ctl & GEN11_DE_HPD_IRQ)) {
 		iir = I915_READ(GEN11_DE_HPD_IIR);
 		if (iir) {
-- 
2.21.0

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

[PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders

[José Roberto de Souza]

PSR is only supported in eDP transcoder and there is only one
instance of it, so lets drop all of this code.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  17 +---
 drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++-----------------------
 2 files changed, 42 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c59cfa83dbaf..18e2991b376d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4241,13 +4241,9 @@ enum {
 /* Bspec claims those aren't shifted but stay at 0x64800 */
 #define EDP_PSR_IMR				_MMIO(0x64834)
 #define EDP_PSR_IIR				_MMIO(0x64838)
-#define   EDP_PSR_ERROR(shift)			(1 << ((shift) + 2))
-#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift) + 1))
-#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
-#define   EDP_PSR_TRANSCODER_C_SHIFT		24
-#define   EDP_PSR_TRANSCODER_B_SHIFT		16
-#define   EDP_PSR_TRANSCODER_A_SHIFT		8
-#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
+#define   EDP_PSR_ERROR				(1 << 2)
+#define   EDP_PSR_POST_EXIT			(1 << 1)
+#define   EDP_PSR_PRE_ENTRY			(1 << 0)
 
 #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
 #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
@@ -4312,12 +4308,7 @@ enum {
 #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
 #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
 
-#define _PSR_EVENT_TRANS_A			0x60848
-#define _PSR_EVENT_TRANS_B			0x61848
-#define _PSR_EVENT_TRANS_C			0x62848
-#define _PSR_EVENT_TRANS_D			0x63848
-#define _PSR_EVENT_TRANS_EDP			0x6F848
-#define PSR_EVENT(trans)			_MMIO_TRANS2(trans, _PSR_EVENT_TRANS_A)
+#define PSR_EVENT				_MMIO(0x6F848)
 #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
 #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
 #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index bb97c1657493..b984e005b72e 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -84,46 +84,12 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
 	}
 }
 
-static int edp_psr_shift(enum transcoder cpu_transcoder)
-{
-	switch (cpu_transcoder) {
-	case TRANSCODER_A:
-		return EDP_PSR_TRANSCODER_A_SHIFT;
-	case TRANSCODER_B:
-		return EDP_PSR_TRANSCODER_B_SHIFT;
-	case TRANSCODER_C:
-		return EDP_PSR_TRANSCODER_C_SHIFT;
-	default:
-		MISSING_CASE(cpu_transcoder);
-		/* fallthrough */
-	case TRANSCODER_EDP:
-		return EDP_PSR_TRANSCODER_EDP_SHIFT;
-	}
-}
-
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32 debug)
 {
-	u32 debug_mask, mask;
-	enum transcoder cpu_transcoder;
-	u32 transcoders = BIT(TRANSCODER_EDP);
-
-	if (INTEL_GEN(dev_priv) >= 8)
-		transcoders |= BIT(TRANSCODER_A) |
-			       BIT(TRANSCODER_B) |
-			       BIT(TRANSCODER_C);
-
-	debug_mask = 0;
-	mask = 0;
-	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
-		int shift = edp_psr_shift(cpu_transcoder);
-
-		mask |= EDP_PSR_ERROR(shift);
-		debug_mask |= EDP_PSR_POST_EXIT(shift) |
-			      EDP_PSR_PRE_ENTRY(shift);
-	}
+	u32 mask = EDP_PSR_ERROR;
 
 	if (debug & I915_PSR_DEBUG_IRQ)
-		mask |= debug_mask;
+		mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY;
 
 	I915_WRITE(EDP_PSR_IMR, ~mask);
 }
@@ -167,62 +133,47 @@ static void psr_event_print(u32 val, bool psr2_enabled)
 
 void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
 {
-	u32 transcoders = BIT(TRANSCODER_EDP);
-	enum transcoder cpu_transcoder;
-	ktime_t time_ns =  ktime_get();
-	u32 mask = 0;
+	ktime_t time_ns = ktime_get();
 
-	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) {
-		int shift = edp_psr_shift(cpu_transcoder);
-
-		if (psr_iir & EDP_PSR_ERROR(shift)) {
-			DRM_WARN("[transcoder %s] PSR aux error\n",
-				 transcoder_name(cpu_transcoder));
-
-			dev_priv->psr.irq_aux_error = true;
-
-			/*
-			 * If this interruption is not masked it will keep
-			 * interrupting so fast that it prevents the scheduled
-			 * work to run.
-			 * Also after a PSR error, we don't want to arm PSR
-			 * again so we don't care about unmask the interruption
-			 * or unset irq_aux_error.
-			 */
-			mask |= EDP_PSR_ERROR(shift);
-		}
+	if (psr_iir & EDP_PSR_ERROR) {
+		u32 mask;
 
-		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
-			dev_priv->psr.last_entry_attempt = time_ns;
-			DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
-				      transcoder_name(cpu_transcoder));
-		}
+		DRM_WARN("[transcoder %s] PSR aux error\n",
+			 transcoder_name(TRANSCODER_EDP));
 
-		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
-			dev_priv->psr.last_exit = time_ns;
-			DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
-				      transcoder_name(cpu_transcoder));
+		dev_priv->psr.irq_aux_error = true;
 
-			if (INTEL_GEN(dev_priv) >= 9) {
-				u32 val = I915_READ(PSR_EVENT(cpu_transcoder));
-				bool psr2_enabled = dev_priv->psr.psr2_enabled;
+		/*
+		 * If this interruption is not masked it will keep
+		 * interrupting so fast that it prevents the scheduled
+		 * work to run.
+		 * Also after a PSR error, we don't want to arm PSR
+		 * again so we don't care about unmask the interruption
+		 * or unset irq_aux_error.
+		 */
+		mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR;
+		I915_WRITE(EDP_PSR_IMR, mask);
 
-				I915_WRITE(PSR_EVENT(cpu_transcoder), val);
-				psr_event_print(val, psr2_enabled);
-			}
-		}
+		schedule_work(&dev_priv->psr.work);
 	}
 
-	if (mask) {
-		mask |= I915_READ(EDP_PSR_IMR);
-		I915_WRITE(EDP_PSR_IMR, mask);
+	if (psr_iir & EDP_PSR_PRE_ENTRY) {
+		dev_priv->psr.last_entry_attempt = time_ns;
+		DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
+			      transcoder_name(TRANSCODER_EDP));
+	}
 
-		schedule_work(&dev_priv->psr.work);
+	if (psr_iir & EDP_PSR_POST_EXIT) {
+		DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
+			      transcoder_name(TRANSCODER_EDP));
+
+		if (INTEL_GEN(dev_priv) >= 9) {
+			u32 val = I915_READ(PSR_EVENT);
+			bool psr2_enabled = dev_priv->psr.psr2_enabled;
+
+			I915_WRITE(PSR_EVENT, val);
+			psr_event_print(val, psr2_enabled);
+		}
 	}
 }
 
@@ -669,30 +620,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 	dev_priv->psr.active = true;
 }
 
-static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
-					 enum transcoder cpu_transcoder)
-{
-	static const i915_reg_t regs[] = {
-		[TRANSCODER_A] = CHICKEN_TRANS_A,
-		[TRANSCODER_B] = CHICKEN_TRANS_B,
-		[TRANSCODER_C] = CHICKEN_TRANS_C,
-		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
-	};
-
-	WARN_ON(INTEL_GEN(dev_priv) < 9);
-
-	if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) ||
-		    !regs[cpu_transcoder].reg))
-		cpu_transcoder = TRANSCODER_A;
-
-	return regs[cpu_transcoder];
-}
-
 static void intel_psr_enable_source(struct intel_dp *intel_dp,
 				    const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	u32 mask;
 
 	/* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+
@@ -703,13 +634,11 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
 
 	if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9) &&
 					   !IS_GEMINILAKE(dev_priv))) {
-		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
-							cpu_transcoder);
-		u32 chicken = I915_READ(reg);
+		u32 chicken = I915_READ(CHICKEN_TRANS_EDP);
 
 		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
 			   PSR2_ADD_VERTICAL_LINE_COUNT;
-		I915_WRITE(reg, chicken);
+		I915_WRITE(CHICKEN_TRANS_EDP, chicken);
 	}
 
 	/*
@@ -1222,7 +1151,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	 * to avoid any rendering problems.
 	 */
 	val = I915_READ(EDP_PSR_IIR);
-	val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
+	val &= EDP_PSR_ERROR;
 	if (val) {
 		DRM_DEBUG_KMS("PSR interruption error set\n");
 		dev_priv->psr.sink_not_reliable = true;
-- 
2.21.0

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

[PATCH 7/7] drm/i915: Make PSR registers relative to transcoders

[José Roberto de Souza]

PSR registers are a mess, some have the full address while others just
have the additional offset from psr_mmio_base.

psr_mmio_base is nothing more than TRANSCODER_EDP_OFFSET + 0x800 and
using it makes more difficult for people with an PSR register address
from BSpec to search the register name in i915 as also the BSpec name
don't match with the name in i915.

The other option would be use the whole hard-coded address but this is
not future proof, so here going in the middle ground by making every
PSR register relative to transcoder(that is EDP transcoder), the only
exception is PSR_IMR/IIR that is not relative to nothing.
For the _TRANS2() macros to work it needs the address of the register
from the TRANSCODER_A, so adding it to every register together with
the register address from the EDP transcoder so it will make easy for
people searching with BSpec address also adding those with the BSpec
name.

For Haswell all the PSR register are relative to 0x64000, so
mmio_base_adjust was added and used to take care of that.

Also removing BDW_EDP_PSR_BASE from GVT because it is not used as
the only PSR register that GVT have is this one(SRD/PSR_CTL).

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/gvt/handlers.c |  1 -
 drivers/gpu/drm/i915/i915_drv.h     |  5 ++-
 drivers/gpu/drm/i915/i915_reg.h     | 59 ++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_psr.c    | 11 ++++--
 4 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index 86761b1def1e..d09b798e93cb 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -2739,7 +2739,6 @@ static int init_broadwell_mmio_info(struct intel_gvt *gvt)
 	MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS);
 
 	MMIO_D(WM_MISC, D_BDW);
-	MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW);
 
 	MMIO_D(_MMIO(0x6671c), D_BDW_PLUS);
 	MMIO_D(_MMIO(0x66c00), D_BDW_PLUS);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8f38d03b1c4e..9ce46a7dabfd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -501,6 +501,8 @@ struct i915_drrs {
 };
 
 struct i915_psr {
+	/* different than zero only on HSW see _TRANS2_PSR() for more info */
+	u32 mmio_base_adjust;
 	struct mutex lock;
 
 #define I915_PSR_DEBUG_MODE_MASK	0x0f
@@ -515,6 +517,7 @@ struct i915_psr {
 	bool enabled;
 	struct intel_dp *dp;
 	enum pipe pipe;
+	enum transcoder transcoder;
 	bool active;
 	struct work_struct work;
 	unsigned busy_frontbuffer_bits;
@@ -1541,8 +1544,6 @@ struct drm_i915_private {
 	/* MMIO base address for MIPI regs */
 	u32 mipi_mmio_base;
 
-	u32 psr_mmio_base;
-
 	u32 pps_mmio_base;
 
 	wait_queue_head_t gmbus_wait_queue;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 18e2991b376d..4df56c118cd2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -250,9 +250,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define _MMIO_PIPE2(pipe, reg)		_MMIO(INTEL_INFO(dev_priv)->pipe_offsets[pipe] - \
 					      INTEL_INFO(dev_priv)->pipe_offsets[PIPE_A] + (reg) + \
 					      DISPLAY_MMIO_BASE(dev_priv))
-#define _MMIO_TRANS2(pipe, reg)		_MMIO(INTEL_INFO(dev_priv)->trans_offsets[(pipe)] - \
-					      INTEL_INFO(dev_priv)->trans_offsets[TRANSCODER_A] + (reg) + \
-					      DISPLAY_MMIO_BASE(dev_priv))
+#define _TRANS2(trans, reg)		(INTEL_INFO(dev_priv)->trans_offsets[(trans)] - \
+					 INTEL_INFO(dev_priv)->trans_offsets[TRANSCODER_A] + (reg) + \
+					 DISPLAY_MMIO_BASE(dev_priv))
+#define _MMIO_TRANS2(trans, reg)	_MMIO(_TRANS2(trans, reg))
 #define _CURSOR2(pipe, reg)		_MMIO(INTEL_INFO(dev_priv)->cursor_offsets[(pipe)] - \
 					      INTEL_INFO(dev_priv)->cursor_offsets[PIPE_A] + (reg) + \
 					      DISPLAY_MMIO_BASE(dev_priv))
@@ -4210,9 +4211,15 @@ enum {
 #define PIPE_MULT(trans)	_MMIO_TRANS2(trans, _PIPE_MULT_A)
 
 /* HSW+ eDP PSR registers */
-#define HSW_EDP_PSR_BASE	0x64800
-#define BDW_EDP_PSR_BASE	0x6f800
-#define EDP_PSR_CTL				_MMIO(dev_priv->psr_mmio_base + 0)
+#define HSW_EDP_PSR_BASE	0x64000
+
+/* PSR registers on HSW is not relative to eDP transcoder */
+#define _TRANS2_PSR(reg)	(_TRANS2(dev_priv->psr.transcoder, (reg)) - dev_priv->psr.mmio_base_adjust)
+#define _MMIO_TRANS2_PSR(reg)	_MMIO(_TRANS2_PSR(reg))
+
+#define _SRD_CTL_A				0x60800
+#define _SRD_CTL_EDP				0x6F800
+#define EDP_PSR_CTL				_MMIO_TRANS2_PSR(_SRD_CTL_A)
 #define   EDP_PSR_ENABLE			(1 << 31)
 #define   BDW_PSR_SINGLE_FRAME			(1 << 30)
 #define   EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK	(1 << 29) /* SW can't modify */
@@ -4245,16 +4252,22 @@ enum {
 #define   EDP_PSR_POST_EXIT			(1 << 1)
 #define   EDP_PSR_PRE_ENTRY			(1 << 0)
 
-#define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
+#define _SRD_AUX_CTL_A				0x60810
+#define _SRD_AUX_CTL_EDP			0x6F810
+#define EDP_PSR_AUX_CTL				_MMIO_TRANS2_PSR(_SRD_AUX_CTL_A)
 #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
 #define   EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK	(0x1f << 20)
 #define   EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK	(0xf << 16)
 #define   EDP_PSR_AUX_CTL_ERROR_INTERRUPT	(1 << 11)
 #define   EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK	(0x7ff)
 
-#define EDP_PSR_AUX_DATA(i)			_MMIO(dev_priv->psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */
+#define _SRD_AUX_DATA_A				0x60814
+#define _SRD_AUX_DATA_EDP			0x6F814
+#define EDP_PSR_AUX_DATA(i)			_MMIO(_TRANS2_PSR(_SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */
 
-#define EDP_PSR_STATUS				_MMIO(dev_priv->psr_mmio_base + 0x40)
+#define _SRD_STATUS_A				0x60840
+#define	_SRD_STATUS_EDP				0x6F840
+#define EDP_PSR_STATUS				_MMIO_TRANS2_PSR(_SRD_STATUS_A)
 #define   EDP_PSR_STATUS_STATE_MASK		(7 << 29)
 #define   EDP_PSR_STATUS_STATE_SHIFT		29
 #define   EDP_PSR_STATUS_STATE_IDLE		(0 << 29)
@@ -4279,10 +4292,15 @@ enum {
 #define   EDP_PSR_STATUS_SENDING_TP1		(1 << 4)
 #define   EDP_PSR_STATUS_IDLE_MASK		0xf
 
-#define EDP_PSR_PERF_CNT		_MMIO(dev_priv->psr_mmio_base + 0x44)
+#define _SRD_PERF_CNT_A			0x60844
+#define _SRD_PERF_CNT_EDP		0x6F844
+#define EDP_PSR_PERF_CNT		_MMIO_TRANS2_PSR(_SRD_PERF_CNT_A)
 #define   EDP_PSR_PERF_CNT_MASK		0xffffff
 
-#define EDP_PSR_DEBUG				_MMIO(dev_priv->psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */
+/* PSR_MASK on SKL+ */
+#define _SRD_DEBUG_A				0x60860
+#define _SRD_DEBUG_EDP				0x6F860
+#define EDP_PSR_DEBUG				_MMIO_TRANS2_PSR(_SRD_DEBUG_A)
 #define   EDP_PSR_DEBUG_MASK_MAX_SLEEP         (1 << 28)
 #define   EDP_PSR_DEBUG_MASK_LPSP              (1 << 27)
 #define   EDP_PSR_DEBUG_MASK_MEMUP             (1 << 26)
@@ -4290,7 +4308,9 @@ enum {
 #define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /* Reserved in ICL+ */
 #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
 
-#define EDP_PSR2_CTL			_MMIO(0x6f900)
+#define _PSR2_CTL_A			0x60900
+#define _PSR2_CTL_EDP			0x6F900
+#define EDP_PSR2_CTL			_MMIO_TRANS2_PSR(_PSR2_CTL_A)
 #define   EDP_PSR2_ENABLE		(1 << 31)
 #define   EDP_SU_TRACK_ENABLE		(1 << 30)
 #define   EDP_Y_COORDINATE_VALID	(1 << 26) /* GLK and CNL+ */
@@ -4308,7 +4328,9 @@ enum {
 #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
 #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
 
-#define PSR_EVENT				_MMIO(0x6F848)
+#define _PSR_EVENT_A				0x60848
+#define _PSR_EVENT_EDP				0x6F848
+#define PSR_EVENT				_MMIO_TRANS2_PSR(_PSR_EVENT_A)
 #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
 #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
 #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
@@ -4326,14 +4348,15 @@ enum {
 #define  PSR_EVENT_LPSP_MODE_EXIT		(1 << 1)
 #define  PSR_EVENT_PSR_DISABLE			(1 << 0)
 
-#define EDP_PSR2_STATUS			_MMIO(0x6f940)
+#define _PSR2_STATUS_A			0x60940
+#define _PSR2_STATUS_EDP		0x6F940
+#define EDP_PSR2_STATUS			_MMIO_TRANS2_PSR(_PSR2_STATUS_A)
 #define EDP_PSR2_STATUS_STATE_MASK     (0xf << 28)
 #define EDP_PSR2_STATUS_STATE_SHIFT    28
 
-#define _PSR2_SU_STATUS_0		0x6F914
-#define _PSR2_SU_STATUS_1		0x6F918
-#define _PSR2_SU_STATUS_2		0x6F91C
-#define _PSR2_SU_STATUS(index)		_MMIO(_PICK_EVEN((index), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1))
+#define _PSR2_SU_STATUS_A		0x60914
+#define _PSR2_SU_STATUS_EDP		0x6F914
+#define _PSR2_SU_STATUS(index)		_MMIO(_TRANS2_PSR(_PSR2_SU_STATUS_A) + (index) * 4)
 #define PSR2_SU_STATUS(frame)		(_PSR2_SU_STATUS((frame) / 3))
 #define PSR2_SU_STATUS_SHIFT(frame)	(((frame) % 3) * 10)
 #define PSR2_SU_STATUS_MASK(frame)	(0x3ff << PSR2_SU_STATUS_SHIFT(frame))
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b984e005b72e..ba88464cbbe8 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -668,6 +668,14 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
 	dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 	dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+	dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
+
+	if (IS_HASWELL(dev_priv)) {
+		u32 trans_offset = INTEL_INFO(dev_priv)->trans_offsets[dev_priv->psr.transcoder];
+
+		WARN_ON(trans_offset < HSW_EDP_PSR_BASE);
+		dev_priv->psr.mmio_base_adjust = trans_offset - HSW_EDP_PSR_BASE;
+	}
 
 	DRM_DEBUG_KMS("Enabling PSR%s\n",
 		      dev_priv->psr.psr2_enabled ? "2" : "1");
@@ -1132,9 +1140,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	if (!HAS_PSR(dev_priv))
 		return;
 
-	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
-		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
-
 	if (!dev_priv->psr.sink_support)
 		return;
 
-- 
2.21.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/7] drm/i915/psr: Update PSR2 SU corruption workaround comment

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment
URL   : https://patchwork.freedesktop.org/series/58974/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a823afc06f70 drm/i915/psr: Update PSR2 SU corruption workaround comment
e483edd370ea drm/i915: Remove unused VLV/CHV PSR registers
997ec2ab8f6d drm/i915/psr: Initialize PSR mutex even when sink is not reliable
841cd7fa823e drm/i915/psr: Do not enable PSR in interlaced mode for all GENs
1a978559512b drm/i915/bdw+: Move misc display IRQ handling to it own function
39c5d9840b98 drm/i915/psr: Remove partial PSR support on multiple transcoders
2ec4ae3c2d94 drm/i915: Make PSR registers relative to transcoders
-:93: WARNING:LONG_LINE: line over 100 characters
#93: FILE: drivers/gpu/drm/i915/i915_reg.h:254:
+					 INTEL_INFO(dev_priv)->trans_offsets[TRANSCODER_A] + (reg) + \

-:109: WARNING:LONG_LINE: line over 100 characters
#109: FILE: drivers/gpu/drm/i915/i915_reg.h:4217:
+#define _TRANS2_PSR(reg)	(_TRANS2(dev_priv->psr.transcoder, (reg)) - dev_priv->psr.mmio_base_adjust)

-:135: WARNING:LONG_LINE_COMMENT: line over 100 characters
#135: FILE: drivers/gpu/drm/i915/i915_reg.h:4266:
+#define EDP_PSR_AUX_DATA(i)			_MMIO(_TRANS2_PSR(_SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */

total: 0 errors, 3 warnings, 0 checks, 166 lines checked

_______________________________________________
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/7] drm/i915/psr: Update PSR2 SU corruption workaround comment

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment
URL   : https://patchwork.freedesktop.org/series/58974/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/psr: Update PSR2 SU corruption workaround comment
Okay!

Commit: drm/i915: Remove unused VLV/CHV PSR registers
Okay!

Commit: drm/i915/psr: Initialize PSR mutex even when sink is not reliable
Okay!

Commit: drm/i915/psr: Do not enable PSR in interlaced mode for all GENs
Okay!

Commit: drm/i915/bdw+: Move misc display IRQ handling to it own function
Okay!

Commit: drm/i915/psr: Remove partial PSR support on multiple transcoders
Okay!

Commit: drm/i915: Make PSR registers relative to transcoders
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3623:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3624: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: failure for series starting with [1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment
URL   : https://patchwork.freedesktop.org/series/58974/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5867 -> Patchwork_12677
====================================================

Summary
-------

  **FAILURE**

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

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

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_close_race@basic-threads:
    - fi-icl-u2:          PASS -> INCOMPLETE

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       PASS -> DMESG-WARN [fdo#108965]

  * igt@gem_exec_basic@readonly-bsd2:
    - fi-skl-6770hq:      PASS -> DMESG-WARN [fdo#105541]

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       PASS -> DMESG-WARN [fdo#107709]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> FAIL [fdo#107709]

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u3}:        FAIL [fdo#103167] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-y:           INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569]

  
  {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#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965


Participating hosts (50 -> 43)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5867 -> Patchwork_12677

  CI_DRM_5867: 4a41303673f5b18b2b176182dd220d455f33d204 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4926: c9a9cf357b6b2a304623790bf8dae797e12888a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12677: 2ec4ae3c2d94eb1a433c5d6a2739fe6a8e480fb0 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2ec4ae3c2d94 drm/i915: Make PSR registers relative to transcoders
39c5d9840b98 drm/i915/psr: Remove partial PSR support on multiple transcoders
1a978559512b drm/i915/bdw+: Move misc display IRQ handling to it own function
841cd7fa823e drm/i915/psr: Do not enable PSR in interlaced mode for all GENs
997ec2ab8f6d drm/i915/psr: Initialize PSR mutex even when sink is not reliable
e483edd370ea drm/i915: Remove unused VLV/CHV PSR registers
a823afc06f70 drm/i915/psr: Update PSR2 SU corruption workaround comment

== Logs ==

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

Re: [PATCH 1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment

[Rodrigo Vivi]

On Wed, Apr 03, 2019 at 04:35:33PM -0700, José Roberto de Souza wrote:
> Turn out it is not a DMC bug it is actually a HW one, so this
> workaround will be needed for current gens, lets update the comment
> and remove the FIXME.

Do we have a Wa #number for this? p[art of workaround page
or just part of programming sequence?

> 
> BSpec: 7723
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index ec874d802d48..c80bb3003a7d 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -531,10 +531,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  		val |= EDP_PSR2_TP2_TIME_2500us;
>  
>  	/*
> -	 * FIXME: There is probably a issue in DMC firmwares(icl_dmc_ver1_07.bin
> -	 * and kbl_dmc_ver1_04.bin at least) that causes PSR2 SU to fail after
> -	 * exiting DC6 if EDP_PSR_TP1_TP3_SEL is kept in PSR_CTL, so for now
> -	 * lets workaround the issue by cleaning PSR_CTL before enable PSR2.
> +	 * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec is
> +	 * recommending keep this bit unset while PSR2 is enabled.
>  	 */
>  	I915_WRITE(EDP_PSR_CTL, 0);
>  
> -- 
> 2.21.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915: Remove unused VLV/CHV PSR registers

[Rodrigo Vivi]

On Wed, Apr 03, 2019 at 04:35:34PM -0700, José Roberto de Souza wrote:
> PSR support for VLV and CHV was dropped in commit ce3508fd2a77
> ("drm/i915/psr: Nuke PSR support for VLV and CHV") so no need to keep
> this registers around.

o.O


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 36 ---------------------------------
>  1 file changed, 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00e03560c4e7..c59cfa83dbaf 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4209,42 +4209,6 @@ enum {
>  #define PIPESRC(trans)		_MMIO_TRANS2(trans, _PIPEASRC)
>  #define PIPE_MULT(trans)	_MMIO_TRANS2(trans, _PIPE_MULT_A)
>  
> -/* VLV eDP PSR registers */
> -#define _PSRCTLA				(VLV_DISPLAY_BASE + 0x60090)
> -#define _PSRCTLB				(VLV_DISPLAY_BASE + 0x61090)
> -#define  VLV_EDP_PSR_ENABLE			(1 << 0)
> -#define  VLV_EDP_PSR_RESET			(1 << 1)
> -#define  VLV_EDP_PSR_MODE_MASK			(7 << 2)
> -#define  VLV_EDP_PSR_MODE_HW_TIMER		(1 << 3)
> -#define  VLV_EDP_PSR_MODE_SW_TIMER		(1 << 2)
> -#define  VLV_EDP_PSR_SINGLE_FRAME_UPDATE	(1 << 7)
> -#define  VLV_EDP_PSR_ACTIVE_ENTRY		(1 << 8)
> -#define  VLV_EDP_PSR_SRC_TRANSMITTER_STATE	(1 << 9)
> -#define  VLV_EDP_PSR_DBL_FRAME			(1 << 10)
> -#define  VLV_EDP_PSR_FRAME_COUNT_MASK		(0xff << 16)
> -#define  VLV_EDP_PSR_IDLE_FRAME_SHIFT		16
> -#define VLV_PSRCTL(pipe)	_MMIO_PIPE(pipe, _PSRCTLA, _PSRCTLB)
> -
> -#define _VSCSDPA			(VLV_DISPLAY_BASE + 0x600a0)
> -#define _VSCSDPB			(VLV_DISPLAY_BASE + 0x610a0)
> -#define  VLV_EDP_PSR_SDP_FREQ_MASK	(3 << 30)
> -#define  VLV_EDP_PSR_SDP_FREQ_ONCE	(1 << 31)
> -#define  VLV_EDP_PSR_SDP_FREQ_EVFRAME	(1 << 30)
> -#define VLV_VSCSDP(pipe)	_MMIO_PIPE(pipe, _VSCSDPA, _VSCSDPB)
> -
> -#define _PSRSTATA			(VLV_DISPLAY_BASE + 0x60094)
> -#define _PSRSTATB			(VLV_DISPLAY_BASE + 0x61094)
> -#define  VLV_EDP_PSR_LAST_STATE_MASK	(7 << 3)
> -#define  VLV_EDP_PSR_CURR_STATE_MASK	7
> -#define  VLV_EDP_PSR_DISABLED		(0 << 0)
> -#define  VLV_EDP_PSR_INACTIVE		(1 << 0)
> -#define  VLV_EDP_PSR_IN_TRANS_TO_ACTIVE	(2 << 0)
> -#define  VLV_EDP_PSR_ACTIVE_NORFB_UP	(3 << 0)
> -#define  VLV_EDP_PSR_ACTIVE_SF_UPDATE	(4 << 0)
> -#define  VLV_EDP_PSR_EXIT		(5 << 0)
> -#define  VLV_EDP_PSR_IN_TRANS		(1 << 7)
> -#define VLV_PSRSTAT(pipe)	_MMIO_PIPE(pipe, _PSRSTATA, _PSRSTATB)
> -
>  /* HSW+ eDP PSR registers */
>  #define HSW_EDP_PSR_BASE	0x64800
>  #define BDW_EDP_PSR_BASE	0x6f800
> -- 
> 2.21.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable

[Rodrigo Vivi]

On Wed, Apr 03, 2019 at 04:35:35PM -0700, José Roberto de Souza wrote:
> Even when driver is reloaded and hits this scenario the PSR mutex
> should be initialized, otherwise reading PSR debugfs status will
> execute mutex_lock() over a mutex that was not initialized.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c80bb3003a7d..a84da931c3be 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  	if (val) {
>  		DRM_DEBUG_KMS("PSR interruption error set\n");
>  		dev_priv->psr.sink_not_reliable = true;
> -		return;

There are other returns above and if debugfs hits this case maybe it
is worth to move the mutex initialization up instead?

>  	}
>  
>  	/* Set link_standby x link_off defaults */
> -- 
> 2.21.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915/psr: Do not enable PSR in interlaced mode for all GENs

[Rodrigo Vivi]

On Wed, Apr 03, 2019 at 04:35:36PM -0700, José Roberto de Souza wrote:
> This interlaced restriction applies to all gens, not only to Haswell.

I believe this came from VLV times and I doubt we would be
impacted by it ever, but better to protect just in case:


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index a84da931c3be..bb97c1657493 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -627,8 +627,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>  		return;
>  	}
>  
> -	if (IS_HASWELL(dev_priv) &&
> -	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
>  		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
>  		return;
>  	}
> -- 
> 2.21.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders

[Rodrigo Vivi]

On Wed, Apr 03, 2019 at 04:35:38PM -0700, José Roberto de Souza wrote:
> PSR is only supported in eDP transcoder and there is only one
> instance of it, so lets drop all of this code.

Is this sentence true? I mean, in the way it is written it
seems like HW doesn't actually support it...
Or should we re-phrase for we are not really enabling support
for other transcoders than eDP and we do not have plans to do
it so soon so let's clean the code...
or something like that?

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  17 +---
>  drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++-----------------------
>  2 files changed, 42 insertions(+), 122 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c59cfa83dbaf..18e2991b376d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4241,13 +4241,9 @@ enum {
>  /* Bspec claims those aren't shifted but stay at 0x64800 */
>  #define EDP_PSR_IMR				_MMIO(0x64834)
>  #define EDP_PSR_IIR				_MMIO(0x64838)
> -#define   EDP_PSR_ERROR(shift)			(1 << ((shift) + 2))
> -#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift) + 1))
> -#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
> -#define   EDP_PSR_TRANSCODER_C_SHIFT		24
> -#define   EDP_PSR_TRANSCODER_B_SHIFT		16
> -#define   EDP_PSR_TRANSCODER_A_SHIFT		8
> -#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
> +#define   EDP_PSR_ERROR				(1 << 2)
> +#define   EDP_PSR_POST_EXIT			(1 << 1)
> +#define   EDP_PSR_PRE_ENTRY			(1 << 0)
>  
>  #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
>  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
> @@ -4312,12 +4308,7 @@ enum {
>  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
>  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
>  
> -#define _PSR_EVENT_TRANS_A			0x60848
> -#define _PSR_EVENT_TRANS_B			0x61848
> -#define _PSR_EVENT_TRANS_C			0x62848
> -#define _PSR_EVENT_TRANS_D			0x63848
> -#define _PSR_EVENT_TRANS_EDP			0x6F848
> -#define PSR_EVENT(trans)			_MMIO_TRANS2(trans, _PSR_EVENT_TRANS_A)
> +#define PSR_EVENT				_MMIO(0x6F848)
>  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
>  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
>  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index bb97c1657493..b984e005b72e 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -84,46 +84,12 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> -static int edp_psr_shift(enum transcoder cpu_transcoder)
> -{
> -	switch (cpu_transcoder) {
> -	case TRANSCODER_A:
> -		return EDP_PSR_TRANSCODER_A_SHIFT;
> -	case TRANSCODER_B:
> -		return EDP_PSR_TRANSCODER_B_SHIFT;
> -	case TRANSCODER_C:
> -		return EDP_PSR_TRANSCODER_C_SHIFT;
> -	default:
> -		MISSING_CASE(cpu_transcoder);
> -		/* fallthrough */
> -	case TRANSCODER_EDP:
> -		return EDP_PSR_TRANSCODER_EDP_SHIFT;
> -	}
> -}
> -
>  void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32 debug)
>  {
> -	u32 debug_mask, mask;
> -	enum transcoder cpu_transcoder;
> -	u32 transcoders = BIT(TRANSCODER_EDP);
> -
> -	if (INTEL_GEN(dev_priv) >= 8)
> -		transcoders |= BIT(TRANSCODER_A) |
> -			       BIT(TRANSCODER_B) |
> -			       BIT(TRANSCODER_C);
> -
> -	debug_mask = 0;
> -	mask = 0;
> -	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
> -		int shift = edp_psr_shift(cpu_transcoder);
> -
> -		mask |= EDP_PSR_ERROR(shift);
> -		debug_mask |= EDP_PSR_POST_EXIT(shift) |
> -			      EDP_PSR_PRE_ENTRY(shift);
> -	}
> +	u32 mask = EDP_PSR_ERROR;
>  
>  	if (debug & I915_PSR_DEBUG_IRQ)
> -		mask |= debug_mask;
> +		mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY;
>  
>  	I915_WRITE(EDP_PSR_IMR, ~mask);
>  }
> @@ -167,62 +133,47 @@ static void psr_event_print(u32 val, bool psr2_enabled)
>  
>  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
>  {
> -	u32 transcoders = BIT(TRANSCODER_EDP);
> -	enum transcoder cpu_transcoder;
> -	ktime_t time_ns =  ktime_get();
> -	u32 mask = 0;
> +	ktime_t time_ns = ktime_get();
>  
> -	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) {
> -		int shift = edp_psr_shift(cpu_transcoder);
> -
> -		if (psr_iir & EDP_PSR_ERROR(shift)) {
> -			DRM_WARN("[transcoder %s] PSR aux error\n",
> -				 transcoder_name(cpu_transcoder));
> -
> -			dev_priv->psr.irq_aux_error = true;
> -
> -			/*
> -			 * If this interruption is not masked it will keep
> -			 * interrupting so fast that it prevents the scheduled
> -			 * work to run.
> -			 * Also after a PSR error, we don't want to arm PSR
> -			 * again so we don't care about unmask the interruption
> -			 * or unset irq_aux_error.
> -			 */
> -			mask |= EDP_PSR_ERROR(shift);
> -		}
> +	if (psr_iir & EDP_PSR_ERROR) {
> +		u32 mask;
>  
> -		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
> -			dev_priv->psr.last_entry_attempt = time_ns;
> -			DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
> -				      transcoder_name(cpu_transcoder));
> -		}
> +		DRM_WARN("[transcoder %s] PSR aux error\n",
> +			 transcoder_name(TRANSCODER_EDP));
>  
> -		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
> -			dev_priv->psr.last_exit = time_ns;
> -			DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> -				      transcoder_name(cpu_transcoder));
> +		dev_priv->psr.irq_aux_error = true;
>  
> -			if (INTEL_GEN(dev_priv) >= 9) {
> -				u32 val = I915_READ(PSR_EVENT(cpu_transcoder));
> -				bool psr2_enabled = dev_priv->psr.psr2_enabled;
> +		/*
> +		 * If this interruption is not masked it will keep
> +		 * interrupting so fast that it prevents the scheduled
> +		 * work to run.
> +		 * Also after a PSR error, we don't want to arm PSR
> +		 * again so we don't care about unmask the interruption
> +		 * or unset irq_aux_error.
> +		 */
> +		mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR;
> +		I915_WRITE(EDP_PSR_IMR, mask);
>  
> -				I915_WRITE(PSR_EVENT(cpu_transcoder), val);
> -				psr_event_print(val, psr2_enabled);
> -			}
> -		}
> +		schedule_work(&dev_priv->psr.work);
>  	}
>  
> -	if (mask) {
> -		mask |= I915_READ(EDP_PSR_IMR);
> -		I915_WRITE(EDP_PSR_IMR, mask);
> +	if (psr_iir & EDP_PSR_PRE_ENTRY) {
> +		dev_priv->psr.last_entry_attempt = time_ns;
> +		DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
> +			      transcoder_name(TRANSCODER_EDP));
> +	}
>  
> -		schedule_work(&dev_priv->psr.work);
> +	if (psr_iir & EDP_PSR_POST_EXIT) {
> +		DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> +			      transcoder_name(TRANSCODER_EDP));
> +
> +		if (INTEL_GEN(dev_priv) >= 9) {
> +			u32 val = I915_READ(PSR_EVENT);
> +			bool psr2_enabled = dev_priv->psr.psr2_enabled;
> +
> +			I915_WRITE(PSR_EVENT, val);
> +			psr_event_print(val, psr2_enabled);
> +		}
>  	}
>  }
>  
> @@ -669,30 +620,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
>  	dev_priv->psr.active = true;
>  }
>  
> -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
> -					 enum transcoder cpu_transcoder)
> -{
> -	static const i915_reg_t regs[] = {
> -		[TRANSCODER_A] = CHICKEN_TRANS_A,
> -		[TRANSCODER_B] = CHICKEN_TRANS_B,
> -		[TRANSCODER_C] = CHICKEN_TRANS_C,
> -		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
> -	};
> -
> -	WARN_ON(INTEL_GEN(dev_priv) < 9);
> -
> -	if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) ||
> -		    !regs[cpu_transcoder].reg))
> -		cpu_transcoder = TRANSCODER_A;
> -
> -	return regs[cpu_transcoder];
> -}
> -
>  static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  				    const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	u32 mask;
>  
>  	/* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+
> @@ -703,13 +634,11 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  
>  	if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9) &&
>  					   !IS_GEMINILAKE(dev_priv))) {
> -		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> -							cpu_transcoder);
> -		u32 chicken = I915_READ(reg);
> +		u32 chicken = I915_READ(CHICKEN_TRANS_EDP);
>  
>  		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
>  			   PSR2_ADD_VERTICAL_LINE_COUNT;
> -		I915_WRITE(reg, chicken);
> +		I915_WRITE(CHICKEN_TRANS_EDP, chicken);
>  	}
>  
>  	/*
> @@ -1222,7 +1151,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  	 * to avoid any rendering problems.
>  	 */
>  	val = I915_READ(EDP_PSR_IIR);
> -	val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
> +	val &= EDP_PSR_ERROR;
>  	if (val) {
>  		DRM_DEBUG_KMS("PSR interruption error set\n");
>  		dev_priv->psr.sink_not_reliable = true;
> -- 
> 2.21.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment

[Runyan, Arthur J]

I update the Bspec as general programming.  SRD_CTL TP2 TP3 Select - "This bit impacts PSR2. Clear it before enabling PSR2 and do not set it while PSR2 is enabled."
I haven't seen the hardware bug report come through yet to establish the wa number.

> -----Original Message-----
> From: Vivi, Rodrigo
> Sent: Wednesday, 3 April, 2019 5:22 PM
> To: Souza, Jose <jose.souza@intel.com>; Runyan, Arthur J
> <arthur.j.runyan@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>
> Subject: Re: [PATCH 1/7] drm/i915/psr: Update PSR2 SU corruption workaround
> comment
> 
> On Wed, Apr 03, 2019 at 04:35:33PM -0700, José Roberto de Souza wrote:
> > Turn out it is not a DMC bug it is actually a HW one, so this
> > workaround will be needed for current gens, lets update the comment
> > and remove the FIXME.
> 
> Do we have a Wa #number for this? p[art of workaround page
> or just part of programming sequence?
> 
> >
> > BSpec: 7723
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> > index ec874d802d48..c80bb3003a7d 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -531,10 +531,8 @@ static void hsw_activate_psr2(struct intel_dp
> *intel_dp)
> >  		val |= EDP_PSR2_TP2_TIME_2500us;
> >
> >  	/*
> > -	 * FIXME: There is probably a issue in DMC
> firmwares(icl_dmc_ver1_07.bin
> > -	 * and kbl_dmc_ver1_04.bin at least) that causes PSR2 SU to fail after
> > -	 * exiting DC6 if EDP_PSR_TP1_TP3_SEL is kept in PSR_CTL, so for now
> > -	 * lets workaround the issue by cleaning PSR_CTL before enable PSR2.
> > +	 * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec is
> > +	 * recommending keep this bit unset while PSR2 is enabled.
> >  	 */
> >  	I915_WRITE(EDP_PSR_CTL, 0);
> >
> > --
> > 2.21.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment

[Rodrigo Vivi]

On Wed, Apr 03, 2019 at 05:39:40PM -0700, Runyan, Arthur J wrote:
> I update the Bspec as general programming.  SRD_CTL TP2 TP3 Select - "This bit impacts PSR2. Clear it before enabling PSR2 and do not set it while PSR2 is enabled."
> I haven't seen the hardware bug report come through yet to establish the wa number.

Thanks


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> 
> > -----Original Message-----
> > From: Vivi, Rodrigo
> > Sent: Wednesday, 3 April, 2019 5:22 PM
> > To: Souza, Jose <jose.souza@intel.com>; Runyan, Arthur J
> > <arthur.j.runyan@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Pandiyan, Dhinakaran
> > <dhinakaran.pandiyan@intel.com>
> > Subject: Re: [PATCH 1/7] drm/i915/psr: Update PSR2 SU corruption workaround
> > comment
> > 
> > On Wed, Apr 03, 2019 at 04:35:33PM -0700, José Roberto de Souza wrote:
> > > Turn out it is not a DMC bug it is actually a HW one, so this
> > > workaround will be needed for current gens, lets update the comment
> > > and remove the FIXME.
> > 
> > Do we have a Wa #number for this? p[art of workaround page
> > or just part of programming sequence?
> > 
> > >
> > > BSpec: 7723
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > > index ec874d802d48..c80bb3003a7d 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -531,10 +531,8 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> > >  		val |= EDP_PSR2_TP2_TIME_2500us;
> > >
> > >  	/*
> > > -	 * FIXME: There is probably a issue in DMC
> > firmwares(icl_dmc_ver1_07.bin
> > > -	 * and kbl_dmc_ver1_04.bin at least) that causes PSR2 SU to fail after
> > > -	 * exiting DC6 if EDP_PSR_TP1_TP3_SEL is kept in PSR_CTL, so for now
> > > -	 * lets workaround the issue by cleaning PSR_CTL before enable PSR2.
> > > +	 * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec is
> > > +	 * recommending keep this bit unset while PSR2 is enabled.
> > >  	 */
> > >  	I915_WRITE(EDP_PSR_CTL, 0);
> > >
> > > --
> > > 2.21.0
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 1492 bytes --]

On Wed, 2019-04-03 at 17:27 -0700, Rodrigo Vivi wrote:
> On Wed, Apr 03, 2019 at 04:35:35PM -0700, José Roberto de Souza
> wrote:
> > Even when driver is reloaded and hits this scenario the PSR mutex
> > should be initialized, otherwise reading PSR debugfs status will
> > execute mutex_lock() over a mutex that was not initialized.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index c80bb3003a7d..a84da931c3be 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> >  	if (val) {
> >  		DRM_DEBUG_KMS("PSR interruption error set\n");
> >  		dev_priv->psr.sink_not_reliable = true;
> > -		return;
> 
> There are other returns above and if debugfs hits this case maybe it
> is worth to move the mutex initialization up instead?


We have those two returns in PSR debugfs, !HAS_PSR(dev_priv) and !psr-
>sink_support and in this cases we don't have any PSR functionality so
not worthy to initialize anything PSR related.

> 
> >  	}
> >  
> >  	/* Set link_standby x link_off defaults */
> > -- 
> > 2.21.0
> > 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 10349 bytes --]

On Wed, 2019-04-03 at 17:31 -0700, Rodrigo Vivi wrote:
> On Wed, Apr 03, 2019 at 04:35:38PM -0700, José Roberto de Souza
> wrote:
> > PSR is only supported in eDP transcoder and there is only one
> > instance of it, so lets drop all of this code.
> 
> Is this sentence true? I mean, in the way it is written it
> seems like HW doesn't actually support it...
> Or should we re-phrase for we are not really enabling support
> for other transcoders than eDP and we do not have plans to do
> it so soon so let's clean the code...
> or something like that?

Okay, what about replace it for:

Since BDW all transcoders have PSR registers but only EDP transcoder
can drive a EDP panel and as PSR is only part of the EDP specification,
for real usage PSR is only supported in EDP panel, so lets drop all of
this useless code.



> 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |  17 +---
> >  drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++-------------------
> > ----
> >  2 files changed, 42 insertions(+), 122 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index c59cfa83dbaf..18e2991b376d 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4241,13 +4241,9 @@ enum {
> >  /* Bspec claims those aren't shifted but stay at 0x64800 */
> >  #define EDP_PSR_IMR				_MMIO(0x64834)
> >  #define EDP_PSR_IIR				_MMIO(0x64838)
> > -#define   EDP_PSR_ERROR(shift)			(1 << ((shift)
> > + 2))
> > -#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift) + 1))
> > -#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
> > -#define   EDP_PSR_TRANSCODER_C_SHIFT		24
> > -#define   EDP_PSR_TRANSCODER_B_SHIFT		16
> > -#define   EDP_PSR_TRANSCODER_A_SHIFT		8
> > -#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
> > +#define   EDP_PSR_ERROR				(1 << 2)
> > +#define   EDP_PSR_POST_EXIT			(1 << 1)
> > +#define   EDP_PSR_PRE_ENTRY			(1 << 0)
> >  
> >  #define EDP_PSR_AUX_CTL				_MMIO(dev_priv-
> > >psr_mmio_base + 0x10)
> >  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
> > @@ -4312,12 +4308,7 @@ enum {
> >  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> >  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> >  
> > -#define _PSR_EVENT_TRANS_A			0x60848
> > -#define _PSR_EVENT_TRANS_B			0x61848
> > -#define _PSR_EVENT_TRANS_C			0x62848
> > -#define _PSR_EVENT_TRANS_D			0x63848
> > -#define _PSR_EVENT_TRANS_EDP			0x6F848
> > -#define PSR_EVENT(trans)			_MMIO_TRANS2(trans,
> > _PSR_EVENT_TRANS_A)
> > +#define PSR_EVENT				_MMIO(0x6F848)
> >  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
> >  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
> >  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index bb97c1657493..b984e005b72e 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -84,46 +84,12 @@ static bool intel_psr2_enabled(struct
> > drm_i915_private *dev_priv,
> >  	}
> >  }
> >  
> > -static int edp_psr_shift(enum transcoder cpu_transcoder)
> > -{
> > -	switch (cpu_transcoder) {
> > -	case TRANSCODER_A:
> > -		return EDP_PSR_TRANSCODER_A_SHIFT;
> > -	case TRANSCODER_B:
> > -		return EDP_PSR_TRANSCODER_B_SHIFT;
> > -	case TRANSCODER_C:
> > -		return EDP_PSR_TRANSCODER_C_SHIFT;
> > -	default:
> > -		MISSING_CASE(cpu_transcoder);
> > -		/* fallthrough */
> > -	case TRANSCODER_EDP:
> > -		return EDP_PSR_TRANSCODER_EDP_SHIFT;
> > -	}
> > -}
> > -
> >  void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32
> > debug)
> >  {
> > -	u32 debug_mask, mask;
> > -	enum transcoder cpu_transcoder;
> > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > -
> > -	if (INTEL_GEN(dev_priv) >= 8)
> > -		transcoders |= BIT(TRANSCODER_A) |
> > -			       BIT(TRANSCODER_B) |
> > -			       BIT(TRANSCODER_C);
> > -
> > -	debug_mask = 0;
> > -	mask = 0;
> > -	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > transcoders) {
> > -		int shift = edp_psr_shift(cpu_transcoder);
> > -
> > -		mask |= EDP_PSR_ERROR(shift);
> > -		debug_mask |= EDP_PSR_POST_EXIT(shift) |
> > -			      EDP_PSR_PRE_ENTRY(shift);
> > -	}
> > +	u32 mask = EDP_PSR_ERROR;
> >  
> >  	if (debug & I915_PSR_DEBUG_IRQ)
> > -		mask |= debug_mask;
> > +		mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY;
> >  
> >  	I915_WRITE(EDP_PSR_IMR, ~mask);
> >  }
> > @@ -167,62 +133,47 @@ static void psr_event_print(u32 val, bool
> > psr2_enabled)
> >  
> >  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> > psr_iir)
> >  {
> > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > -	enum transcoder cpu_transcoder;
> > -	ktime_t time_ns =  ktime_get();
> > -	u32 mask = 0;
> > +	ktime_t time_ns = ktime_get();
> >  
> > -	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) {
> > -		int shift = edp_psr_shift(cpu_transcoder);
> > -
> > -		if (psr_iir & EDP_PSR_ERROR(shift)) {
> > -			DRM_WARN("[transcoder %s] PSR aux error\n",
> > -				 transcoder_name(cpu_transcoder));
> > -
> > -			dev_priv->psr.irq_aux_error = true;
> > -
> > -			/*
> > -			 * If this interruption is not masked it will
> > keep
> > -			 * interrupting so fast that it prevents the
> > scheduled
> > -			 * work to run.
> > -			 * Also after a PSR error, we don't want to arm
> > PSR
> > -			 * again so we don't care about unmask the
> > interruption
> > -			 * or unset irq_aux_error.
> > -			 */
> > -			mask |= EDP_PSR_ERROR(shift);
> > -		}
> > +	if (psr_iir & EDP_PSR_ERROR) {
> > +		u32 mask;
> >  
> > -		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
> > -			dev_priv->psr.last_entry_attempt = time_ns;
> > -			DRM_DEBUG_KMS("[transcoder %s] PSR entry
> > attempt in 2 vblanks\n",
> > -				      transcoder_name(cpu_transcoder));
> > -		}
> > +		DRM_WARN("[transcoder %s] PSR aux error\n",
> > +			 transcoder_name(TRANSCODER_EDP));
> >  
> > -		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
> > -			dev_priv->psr.last_exit = time_ns;
> > -			DRM_DEBUG_KMS("[transcoder %s] PSR exit
> > completed\n",
> > -				      transcoder_name(cpu_transcoder));
> > +		dev_priv->psr.irq_aux_error = true;
> >  
> > -			if (INTEL_GEN(dev_priv) >= 9) {
> > -				u32 val =
> > I915_READ(PSR_EVENT(cpu_transcoder));
> > -				bool psr2_enabled = dev_priv-
> > >psr.psr2_enabled;
> > +		/*
> > +		 * If this interruption is not masked it will keep
> > +		 * interrupting so fast that it prevents the scheduled
> > +		 * work to run.
> > +		 * Also after a PSR error, we don't want to arm PSR
> > +		 * again so we don't care about unmask the interruption
> > +		 * or unset irq_aux_error.
> > +		 */
> > +		mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR;
> > +		I915_WRITE(EDP_PSR_IMR, mask);
> >  
> > -				I915_WRITE(PSR_EVENT(cpu_transcoder),
> > val);
> > -				psr_event_print(val, psr2_enabled);
> > -			}
> > -		}
> > +		schedule_work(&dev_priv->psr.work);
> >  	}
> >  
> > -	if (mask) {
> > -		mask |= I915_READ(EDP_PSR_IMR);
> > -		I915_WRITE(EDP_PSR_IMR, mask);
> > +	if (psr_iir & EDP_PSR_PRE_ENTRY) {
> > +		dev_priv->psr.last_entry_attempt = time_ns;
> > +		DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2
> > vblanks\n",
> > +			      transcoder_name(TRANSCODER_EDP));
> > +	}
> >  
> > -		schedule_work(&dev_priv->psr.work);
> > +	if (psr_iir & EDP_PSR_POST_EXIT) {
> > +		DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> > +			      transcoder_name(TRANSCODER_EDP));
> > +
> > +		if (INTEL_GEN(dev_priv) >= 9) {
> > +			u32 val = I915_READ(PSR_EVENT);
> > +			bool psr2_enabled = dev_priv->psr.psr2_enabled;
> > +
> > +			I915_WRITE(PSR_EVENT, val);
> > +			psr_event_print(val, psr2_enabled);
> > +		}
> >  	}
> >  }
> >  
> > @@ -669,30 +620,10 @@ static void intel_psr_activate(struct
> > intel_dp *intel_dp)
> >  	dev_priv->psr.active = true;
> >  }
> >  
> > -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private
> > *dev_priv,
> > -					 enum transcoder
> > cpu_transcoder)
> > -{
> > -	static const i915_reg_t regs[] = {
> > -		[TRANSCODER_A] = CHICKEN_TRANS_A,
> > -		[TRANSCODER_B] = CHICKEN_TRANS_B,
> > -		[TRANSCODER_C] = CHICKEN_TRANS_C,
> > -		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
> > -	};
> > -
> > -	WARN_ON(INTEL_GEN(dev_priv) < 9);
> > -
> > -	if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) ||
> > -		    !regs[cpu_transcoder].reg))
> > -		cpu_transcoder = TRANSCODER_A;
> > -
> > -	return regs[cpu_transcoder];
> > -}
> > -
> >  static void intel_psr_enable_source(struct intel_dp *intel_dp,
> >  				    const struct intel_crtc_state
> > *crtc_state)
> >  {
> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > -	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  	u32 mask;
> >  
> >  	/* Only HSW and BDW have PSR AUX registers that need to be
> > setup. SKL+
> > @@ -703,13 +634,11 @@ static void intel_psr_enable_source(struct
> > intel_dp *intel_dp,
> >  
> >  	if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9) &&
> >  					   !IS_GEMINILAKE(dev_priv))) {
> > -		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> > -							cpu_transcoder)
> > ;
> > -		u32 chicken = I915_READ(reg);
> > +		u32 chicken = I915_READ(CHICKEN_TRANS_EDP);
> >  
> >  		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
> >  			   PSR2_ADD_VERTICAL_LINE_COUNT;
> > -		I915_WRITE(reg, chicken);
> > +		I915_WRITE(CHICKEN_TRANS_EDP, chicken);
> >  	}
> >  
> >  	/*
> > @@ -1222,7 +1151,7 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> >  	 * to avoid any rendering problems.
> >  	 */
> >  	val = I915_READ(EDP_PSR_IIR);
> > -	val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
> > +	val &= EDP_PSR_ERROR;
> >  	if (val) {
> >  		DRM_DEBUG_KMS("PSR interruption error set\n");
> >  		dev_priv->psr.sink_not_reliable = true;
> > -- 
> > 2.21.0
> > 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
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/7] drm/i915/psr: Update PSR2 SU corruption workaround comment (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment (rev2)
URL   : https://patchwork.freedesktop.org/series/58974/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
73fa7ed439e6 drm/i915/psr: Update PSR2 SU corruption workaround comment
0ca6002ada07 drm/i915: Remove unused VLV/CHV PSR registers
5ce75180483f drm/i915/psr: Initialize PSR mutex even when sink is not reliable
e47e5b39db34 drm/i915/psr: Do not enable PSR in interlaced mode for all GENs
9f93f7096863 drm/i915/bdw+: Move misc display IRQ handling to it own function
43add19f1fa5 drm/i915/psr: Remove partial PSR support on multiple transcoders
a46b5a8681d1 drm/i915: Make PSR registers relative to transcoders
-:93: WARNING:LONG_LINE: line over 100 characters
#93: FILE: drivers/gpu/drm/i915/i915_reg.h:254:
+					 INTEL_INFO(dev_priv)->trans_offsets[TRANSCODER_A] + (reg) + \

-:109: WARNING:LONG_LINE: line over 100 characters
#109: FILE: drivers/gpu/drm/i915/i915_reg.h:4217:
+#define _TRANS2_PSR(reg)	(_TRANS2(dev_priv->psr.transcoder, (reg)) - dev_priv->psr.mmio_base_adjust)

-:135: WARNING:LONG_LINE_COMMENT: line over 100 characters
#135: FILE: drivers/gpu/drm/i915/i915_reg.h:4266:
+#define EDP_PSR_AUX_DATA(i)			_MMIO(_TRANS2_PSR(_SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */

total: 0 errors, 3 warnings, 0 checks, 166 lines checked

_______________________________________________
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/7] drm/i915/psr: Update PSR2 SU corruption workaround comment (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment (rev2)
URL   : https://patchwork.freedesktop.org/series/58974/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/psr: Update PSR2 SU corruption workaround comment
Okay!

Commit: drm/i915: Remove unused VLV/CHV PSR registers
Okay!

Commit: drm/i915/psr: Initialize PSR mutex even when sink is not reliable
Okay!

Commit: drm/i915/psr: Do not enable PSR in interlaced mode for all GENs
Okay!

Commit: drm/i915/bdw+: Move misc display IRQ handling to it own function
Okay!

Commit: drm/i915/psr: Remove partial PSR support on multiple transcoders
Okay!

Commit: drm/i915: Make PSR registers relative to transcoders
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3623:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3624: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/7] drm/i915/psr: Update PSR2 SU corruption workaround comment (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment (rev2)
URL   : https://patchwork.freedesktop.org/series/58974/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5875 -> Patchwork_12686
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@basic-bsd2:
    - fi-kbl-7500u:       NOTRUN -> SKIP [fdo#109271] +9

  * igt@gem_exec_basic@gtt-bsd2:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +57

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      PASS -> DMESG-FAIL [fdo#110235 ]

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        PASS -> DMESG-FAIL [fdo#110210]

  * igt@kms_busy@basic-flip-c:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       NOTRUN -> DMESG-WARN [fdo#103841]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] +20

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       NOTRUN -> INCOMPLETE [fdo#107718]

  * igt@runner@aborted:
    - fi-kbl-7500u:       NOTRUN -> FAIL [fdo#103841]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       INCOMPLETE [fdo#108602] / [fdo#108744] -> PASS

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - fi-bsw-n3050:       FAIL [fdo#100368] -> PASS

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

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (48 -> 42)
------------------------------

  Additional (2): fi-byt-clapper fi-kbl-7500u 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-whl-u fi-icl-y 


Build changes
-------------

    * Linux: CI_DRM_5875 -> Patchwork_12686

  CI_DRM_5875: 5cc7c47c44aaef5bfe07e7307d06caa98e401fad @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4928: 014a6fa238322b497116b359cb92df1ce7fa8847 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12686: a46b5a8681d15f9c7888adf75ea6f574446690f0 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a46b5a8681d1 drm/i915: Make PSR registers relative to transcoders
43add19f1fa5 drm/i915/psr: Remove partial PSR support on multiple transcoders
9f93f7096863 drm/i915/bdw+: Move misc display IRQ handling to it own function
e47e5b39db34 drm/i915/psr: Do not enable PSR in interlaced mode for all GENs
5ce75180483f drm/i915/psr: Initialize PSR mutex even when sink is not reliable
0ca6002ada07 drm/i915: Remove unused VLV/CHV PSR registers
73fa7ed439e6 drm/i915/psr: Update PSR2 SU corruption workaround comment

== Logs ==

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

Re: [PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders

[Rodrigo Vivi]

On Thu, Apr 04, 2019 at 12:40:34PM -0700, Souza, Jose wrote:
> On Wed, 2019-04-03 at 17:31 -0700, Rodrigo Vivi wrote:
> > On Wed, Apr 03, 2019 at 04:35:38PM -0700, José Roberto de Souza
> > wrote:
> > > PSR is only supported in eDP transcoder and there is only one
> > > instance of it, so lets drop all of this code.
> > 
> > Is this sentence true? I mean, in the way it is written it
> > seems like HW doesn't actually support it...
> > Or should we re-phrase for we are not really enabling support
> > for other transcoders than eDP and we do not have plans to do
> > it so soon so let's clean the code...
> > or something like that?
> 
> Okay, what about replace it for:
> 
> Since BDW all transcoders have PSR registers but only EDP transcoder
> can drive a EDP panel and as PSR is only part of the EDP specification,
> for real usage PSR is only supported in EDP panel, so lets drop all of
> this useless code.

well, this still looks like HW limitation. If PSR is supported by HW on
different transcoders is because there's some possibility of adding
eDP on other transcoders. They wouldn't waste so many register space
if that wasn't the case.

Even though we have a dedicated transcoder for eDP I don't
believe we can assume that eDP is not supported on the other ones.

> 
> 
> 
> > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h  |  17 +---
> > >  drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++-------------------
> > > ----
> > >  2 files changed, 42 insertions(+), 122 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index c59cfa83dbaf..18e2991b376d 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4241,13 +4241,9 @@ enum {
> > >  /* Bspec claims those aren't shifted but stay at 0x64800 */
> > >  #define EDP_PSR_IMR				_MMIO(0x64834)
> > >  #define EDP_PSR_IIR				_MMIO(0x64838)
> > > -#define   EDP_PSR_ERROR(shift)			(1 << ((shift)
> > > + 2))
> > > -#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift) + 1))
> > > -#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
> > > -#define   EDP_PSR_TRANSCODER_C_SHIFT		24
> > > -#define   EDP_PSR_TRANSCODER_B_SHIFT		16
> > > -#define   EDP_PSR_TRANSCODER_A_SHIFT		8
> > > -#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
> > > +#define   EDP_PSR_ERROR				(1 << 2)
> > > +#define   EDP_PSR_POST_EXIT			(1 << 1)
> > > +#define   EDP_PSR_PRE_ENTRY			(1 << 0)
> > >  
> > >  #define EDP_PSR_AUX_CTL				_MMIO(dev_priv-
> > > >psr_mmio_base + 0x10)
> > >  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
> > > @@ -4312,12 +4308,7 @@ enum {
> > >  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> > >  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> > >  
> > > -#define _PSR_EVENT_TRANS_A			0x60848
> > > -#define _PSR_EVENT_TRANS_B			0x61848
> > > -#define _PSR_EVENT_TRANS_C			0x62848
> > > -#define _PSR_EVENT_TRANS_D			0x63848
> > > -#define _PSR_EVENT_TRANS_EDP			0x6F848
> > > -#define PSR_EVENT(trans)			_MMIO_TRANS2(trans,
> > > _PSR_EVENT_TRANS_A)
> > > +#define PSR_EVENT				_MMIO(0x6F848)
> > >  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
> > >  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
> > >  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index bb97c1657493..b984e005b72e 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -84,46 +84,12 @@ static bool intel_psr2_enabled(struct
> > > drm_i915_private *dev_priv,
> > >  	}
> > >  }
> > >  
> > > -static int edp_psr_shift(enum transcoder cpu_transcoder)
> > > -{
> > > -	switch (cpu_transcoder) {
> > > -	case TRANSCODER_A:
> > > -		return EDP_PSR_TRANSCODER_A_SHIFT;
> > > -	case TRANSCODER_B:
> > > -		return EDP_PSR_TRANSCODER_B_SHIFT;
> > > -	case TRANSCODER_C:
> > > -		return EDP_PSR_TRANSCODER_C_SHIFT;
> > > -	default:
> > > -		MISSING_CASE(cpu_transcoder);
> > > -		/* fallthrough */
> > > -	case TRANSCODER_EDP:
> > > -		return EDP_PSR_TRANSCODER_EDP_SHIFT;
> > > -	}
> > > -}
> > > -
> > >  void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32
> > > debug)
> > >  {
> > > -	u32 debug_mask, mask;
> > > -	enum transcoder cpu_transcoder;
> > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > -
> > > -	if (INTEL_GEN(dev_priv) >= 8)
> > > -		transcoders |= BIT(TRANSCODER_A) |
> > > -			       BIT(TRANSCODER_B) |
> > > -			       BIT(TRANSCODER_C);
> > > -
> > > -	debug_mask = 0;
> > > -	mask = 0;
> > > -	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > > transcoders) {
> > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > -
> > > -		mask |= EDP_PSR_ERROR(shift);
> > > -		debug_mask |= EDP_PSR_POST_EXIT(shift) |
> > > -			      EDP_PSR_PRE_ENTRY(shift);
> > > -	}
> > > +	u32 mask = EDP_PSR_ERROR;
> > >  
> > >  	if (debug & I915_PSR_DEBUG_IRQ)
> > > -		mask |= debug_mask;
> > > +		mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY;
> > >  
> > >  	I915_WRITE(EDP_PSR_IMR, ~mask);
> > >  }
> > > @@ -167,62 +133,47 @@ static void psr_event_print(u32 val, bool
> > > psr2_enabled)
> > >  
> > >  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> > > psr_iir)
> > >  {
> > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > -	enum transcoder cpu_transcoder;
> > > -	ktime_t time_ns =  ktime_get();
> > > -	u32 mask = 0;
> > > +	ktime_t time_ns = ktime_get();
> > >  
> > > -	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) {
> > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > -
> > > -		if (psr_iir & EDP_PSR_ERROR(shift)) {
> > > -			DRM_WARN("[transcoder %s] PSR aux error\n",
> > > -				 transcoder_name(cpu_transcoder));
> > > -
> > > -			dev_priv->psr.irq_aux_error = true;
> > > -
> > > -			/*
> > > -			 * If this interruption is not masked it will
> > > keep
> > > -			 * interrupting so fast that it prevents the
> > > scheduled
> > > -			 * work to run.
> > > -			 * Also after a PSR error, we don't want to arm
> > > PSR
> > > -			 * again so we don't care about unmask the
> > > interruption
> > > -			 * or unset irq_aux_error.
> > > -			 */
> > > -			mask |= EDP_PSR_ERROR(shift);
> > > -		}
> > > +	if (psr_iir & EDP_PSR_ERROR) {
> > > +		u32 mask;
> > >  
> > > -		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
> > > -			dev_priv->psr.last_entry_attempt = time_ns;
> > > -			DRM_DEBUG_KMS("[transcoder %s] PSR entry
> > > attempt in 2 vblanks\n",
> > > -				      transcoder_name(cpu_transcoder));
> > > -		}
> > > +		DRM_WARN("[transcoder %s] PSR aux error\n",
> > > +			 transcoder_name(TRANSCODER_EDP));
> > >  
> > > -		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
> > > -			dev_priv->psr.last_exit = time_ns;
> > > -			DRM_DEBUG_KMS("[transcoder %s] PSR exit
> > > completed\n",
> > > -				      transcoder_name(cpu_transcoder));
> > > +		dev_priv->psr.irq_aux_error = true;
> > >  
> > > -			if (INTEL_GEN(dev_priv) >= 9) {
> > > -				u32 val =
> > > I915_READ(PSR_EVENT(cpu_transcoder));
> > > -				bool psr2_enabled = dev_priv-
> > > >psr.psr2_enabled;
> > > +		/*
> > > +		 * If this interruption is not masked it will keep
> > > +		 * interrupting so fast that it prevents the scheduled
> > > +		 * work to run.
> > > +		 * Also after a PSR error, we don't want to arm PSR
> > > +		 * again so we don't care about unmask the interruption
> > > +		 * or unset irq_aux_error.
> > > +		 */
> > > +		mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR;
> > > +		I915_WRITE(EDP_PSR_IMR, mask);
> > >  
> > > -				I915_WRITE(PSR_EVENT(cpu_transcoder),
> > > val);
> > > -				psr_event_print(val, psr2_enabled);
> > > -			}
> > > -		}
> > > +		schedule_work(&dev_priv->psr.work);
> > >  	}
> > >  
> > > -	if (mask) {
> > > -		mask |= I915_READ(EDP_PSR_IMR);
> > > -		I915_WRITE(EDP_PSR_IMR, mask);
> > > +	if (psr_iir & EDP_PSR_PRE_ENTRY) {
> > > +		dev_priv->psr.last_entry_attempt = time_ns;
> > > +		DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2
> > > vblanks\n",
> > > +			      transcoder_name(TRANSCODER_EDP));
> > > +	}
> > >  
> > > -		schedule_work(&dev_priv->psr.work);
> > > +	if (psr_iir & EDP_PSR_POST_EXIT) {
> > > +		DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> > > +			      transcoder_name(TRANSCODER_EDP));
> > > +
> > > +		if (INTEL_GEN(dev_priv) >= 9) {
> > > +			u32 val = I915_READ(PSR_EVENT);
> > > +			bool psr2_enabled = dev_priv->psr.psr2_enabled;
> > > +
> > > +			I915_WRITE(PSR_EVENT, val);
> > > +			psr_event_print(val, psr2_enabled);
> > > +		}
> > >  	}
> > >  }
> > >  
> > > @@ -669,30 +620,10 @@ static void intel_psr_activate(struct
> > > intel_dp *intel_dp)
> > >  	dev_priv->psr.active = true;
> > >  }
> > >  
> > > -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private
> > > *dev_priv,
> > > -					 enum transcoder
> > > cpu_transcoder)
> > > -{
> > > -	static const i915_reg_t regs[] = {
> > > -		[TRANSCODER_A] = CHICKEN_TRANS_A,
> > > -		[TRANSCODER_B] = CHICKEN_TRANS_B,
> > > -		[TRANSCODER_C] = CHICKEN_TRANS_C,
> > > -		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
> > > -	};
> > > -
> > > -	WARN_ON(INTEL_GEN(dev_priv) < 9);
> > > -
> > > -	if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) ||
> > > -		    !regs[cpu_transcoder].reg))
> > > -		cpu_transcoder = TRANSCODER_A;
> > > -
> > > -	return regs[cpu_transcoder];
> > > -}
> > > -
> > >  static void intel_psr_enable_source(struct intel_dp *intel_dp,
> > >  				    const struct intel_crtc_state
> > > *crtc_state)
> > >  {
> > >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > -	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > >  	u32 mask;
> > >  
> > >  	/* Only HSW and BDW have PSR AUX registers that need to be
> > > setup. SKL+
> > > @@ -703,13 +634,11 @@ static void intel_psr_enable_source(struct
> > > intel_dp *intel_dp,
> > >  
> > >  	if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9) &&
> > >  					   !IS_GEMINILAKE(dev_priv))) {
> > > -		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> > > -							cpu_transcoder)
> > > ;
> > > -		u32 chicken = I915_READ(reg);
> > > +		u32 chicken = I915_READ(CHICKEN_TRANS_EDP);
> > >  
> > >  		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
> > >  			   PSR2_ADD_VERTICAL_LINE_COUNT;
> > > -		I915_WRITE(reg, chicken);
> > > +		I915_WRITE(CHICKEN_TRANS_EDP, chicken);
> > >  	}
> > >  
> > >  	/*
> > > @@ -1222,7 +1151,7 @@ void intel_psr_init(struct drm_i915_private
> > > *dev_priv)
> > >  	 * to avoid any rendering problems.
> > >  	 */
> > >  	val = I915_READ(EDP_PSR_IIR);
> > > -	val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
> > > +	val &= EDP_PSR_ERROR;
> > >  	if (val) {
> > >  		DRM_DEBUG_KMS("PSR interruption error set\n");
> > >  		dev_priv->psr.sink_not_reliable = true;
> > > -- 
> > > 2.21.0
> > > 


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

Re: [PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders

[Pandiyan, Dhinakaran]

On Thu, 2019-04-04 at 14:20 -0700, Rodrigo Vivi wrote:
> On Thu, Apr 04, 2019 at 12:40:34PM -0700, Souza, Jose wrote:
> > On Wed, 2019-04-03 at 17:31 -0700, Rodrigo Vivi wrote:
> > > On Wed, Apr 03, 2019 at 04:35:38PM -0700, José Roberto de Souza
> > > wrote:
> > > > PSR is only supported in eDP transcoder and there is only one
> > > > instance of it, so lets drop all of this code.
> > > 
> > > Is this sentence true? I mean, in the way it is written it
> > > seems like HW doesn't actually support it...
> > > Or should we re-phrase for we are not really enabling support
> > > for other transcoders than eDP and we do not have plans to do
> > > it so soon so let's clean the code...
> > > or something like that?
> > 
> > Okay, what about replace it for:
> > 
> > Since BDW all transcoders have PSR registers but only EDP transcoder
> > can drive a EDP panel and as PSR is only part of the EDP specification,
> > for real usage PSR is only supported in EDP panel, so lets drop all of
> > this useless code.
> 
> well, this still looks like HW limitation. If PSR is supported by HW on
> different transcoders is because there's some possibility of adding
> eDP on other transcoders. They wouldn't waste so many register space
> if that wasn't the case.
> 
> Even though we have a dedicated transcoder for eDP I don't
> believe we can assume that eDP is not supported on the other ones.
> 

Why not write something like (or exactly) this 

"i915 does not support enabling PSR on any transcoder other than eDP. Clean up
the misleading non-eDP code that currently exists to allow for the rework of PSR
register definitions in the next patch"


-DK
> > 
> > 
> > 
> > > 
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_reg.h  |  17 +---
> > > >  drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++-------------------
> > > > ----
> > > >  2 files changed, 42 insertions(+), 122 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > index c59cfa83dbaf..18e2991b376d 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -4241,13 +4241,9 @@ enum {
> > > >  /* Bspec claims those aren't shifted but stay at 0x64800 */
> > > >  #define EDP_PSR_IMR				_MMIO(0x64834)
> > > >  #define EDP_PSR_IIR				_MMIO(0x64838)
> > > > -#define   EDP_PSR_ERROR(shift)			(1 << ((shift)
> > > > + 2))
> > > > -#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift) + 1))
> > > > -#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
> > > > -#define   EDP_PSR_TRANSCODER_C_SHIFT		24
> > > > -#define   EDP_PSR_TRANSCODER_B_SHIFT		16
> > > > -#define   EDP_PSR_TRANSCODER_A_SHIFT		8
> > > > -#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
> > > > +#define   EDP_PSR_ERROR				(1 << 2)
> > > > +#define   EDP_PSR_POST_EXIT			(1 << 1)
> > > > +#define   EDP_PSR_PRE_ENTRY			(1 << 0)
> > > >  
> > > >  #define EDP_PSR_AUX_CTL				_MMIO(dev_priv-
> > > > > psr_mmio_base + 0x10)
> > > > 
> > > >  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
> > > > @@ -4312,12 +4308,7 @@ enum {
> > > >  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> > > >  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> > > >  
> > > > -#define _PSR_EVENT_TRANS_A			0x60848
> > > > -#define _PSR_EVENT_TRANS_B			0x61848
> > > > -#define _PSR_EVENT_TRANS_C			0x62848
> > > > -#define _PSR_EVENT_TRANS_D			0x63848
> > > > -#define _PSR_EVENT_TRANS_EDP			0x6F848
> > > > -#define PSR_EVENT(trans)			_MMIO_TRANS2(trans,
> > > > _PSR_EVENT_TRANS_A)
> > > > +#define PSR_EVENT				_MMIO(0x6F848)
> > > >  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
> > > >  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
> > > >  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index bb97c1657493..b984e005b72e 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -84,46 +84,12 @@ static bool intel_psr2_enabled(struct
> > > > drm_i915_private *dev_priv,
> > > >  	}
> > > >  }
> > > >  
> > > > -static int edp_psr_shift(enum transcoder cpu_transcoder)
> > > > -{
> > > > -	switch (cpu_transcoder) {
> > > > -	case TRANSCODER_A:
> > > > -		return EDP_PSR_TRANSCODER_A_SHIFT;
> > > > -	case TRANSCODER_B:
> > > > -		return EDP_PSR_TRANSCODER_B_SHIFT;
> > > > -	case TRANSCODER_C:
> > > > -		return EDP_PSR_TRANSCODER_C_SHIFT;
> > > > -	default:
> > > > -		MISSING_CASE(cpu_transcoder);
> > > > -		/* fallthrough */
> > > > -	case TRANSCODER_EDP:
> > > > -		return EDP_PSR_TRANSCODER_EDP_SHIFT;
> > > > -	}
> > > > -}
> > > > -
> > > >  void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32
> > > > debug)
> > > >  {
> > > > -	u32 debug_mask, mask;
> > > > -	enum transcoder cpu_transcoder;
> > > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > > -
> > > > -	if (INTEL_GEN(dev_priv) >= 8)
> > > > -		transcoders |= BIT(TRANSCODER_A) |
> > > > -			       BIT(TRANSCODER_B) |
> > > > -			       BIT(TRANSCODER_C);
> > > > -
> > > > -	debug_mask = 0;
> > > > -	mask = 0;
> > > > -	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > > > transcoders) {
> > > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > > -
> > > > -		mask |= EDP_PSR_ERROR(shift);
> > > > -		debug_mask |= EDP_PSR_POST_EXIT(shift) |
> > > > -			      EDP_PSR_PRE_ENTRY(shift);
> > > > -	}
> > > > +	u32 mask = EDP_PSR_ERROR;
> > > >  
> > > >  	if (debug & I915_PSR_DEBUG_IRQ)
> > > > -		mask |= debug_mask;
> > > > +		mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY;
> > > >  
> > > >  	I915_WRITE(EDP_PSR_IMR, ~mask);
> > > >  }
> > > > @@ -167,62 +133,47 @@ static void psr_event_print(u32 val, bool
> > > > psr2_enabled)
> > > >  
> > > >  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> > > > psr_iir)
> > > >  {
> > > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > > -	enum transcoder cpu_transcoder;
> > > > -	ktime_t time_ns =  ktime_get();
> > > > -	u32 mask = 0;
> > > > +	ktime_t time_ns = ktime_get();
> > > >  
> > > > -	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) {
> > > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > > -
> > > > -		if (psr_iir & EDP_PSR_ERROR(shift)) {
> > > > -			DRM_WARN("[transcoder %s] PSR aux error\n",
> > > > -				 transcoder_name(cpu_transcoder));
> > > > -
> > > > -			dev_priv->psr.irq_aux_error = true;
> > > > -
> > > > -			/*
> > > > -			 * If this interruption is not masked it will
> > > > keep
> > > > -			 * interrupting so fast that it prevents the
> > > > scheduled
> > > > -			 * work to run.
> > > > -			 * Also after a PSR error, we don't want to arm
> > > > PSR
> > > > -			 * again so we don't care about unmask the
> > > > interruption
> > > > -			 * or unset irq_aux_error.
> > > > -			 */
> > > > -			mask |= EDP_PSR_ERROR(shift);
> > > > -		}
> > > > +	if (psr_iir & EDP_PSR_ERROR) {
> > > > +		u32 mask;
> > > >  
> > > > -		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
> > > > -			dev_priv->psr.last_entry_attempt = time_ns;
> > > > -			DRM_DEBUG_KMS("[transcoder %s] PSR entry
> > > > attempt in 2 vblanks\n",
> > > > -				      transcoder_name(cpu_transcoder));
> > > > -		}
> > > > +		DRM_WARN("[transcoder %s] PSR aux error\n",
> > > > +			 transcoder_name(TRANSCODER_EDP));
> > > >  
> > > > -		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
> > > > -			dev_priv->psr.last_exit = time_ns;
> > > > -			DRM_DEBUG_KMS("[transcoder %s] PSR exit
> > > > completed\n",
> > > > -				      transcoder_name(cpu_transcoder));
> > > > +		dev_priv->psr.irq_aux_error = true;
> > > >  
> > > > -			if (INTEL_GEN(dev_priv) >= 9) {
> > > > -				u32 val =
> > > > I915_READ(PSR_EVENT(cpu_transcoder));
> > > > -				bool psr2_enabled = dev_priv-
> > > > > psr.psr2_enabled;
> > > > 
> > > > +		/*
> > > > +		 * If this interruption is not masked it will keep
> > > > +		 * interrupting so fast that it prevents the scheduled
> > > > +		 * work to run.
> > > > +		 * Also after a PSR error, we don't want to arm PSR
> > > > +		 * again so we don't care about unmask the interruption
> > > > +		 * or unset irq_aux_error.
> > > > +		 */
> > > > +		mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR;
> > > > +		I915_WRITE(EDP_PSR_IMR, mask);
> > > >  
> > > > -				I915_WRITE(PSR_EVENT(cpu_transcoder),
> > > > val);
> > > > -				psr_event_print(val, psr2_enabled);
> > > > -			}
> > > > -		}
> > > > +		schedule_work(&dev_priv->psr.work);
> > > >  	}
> > > >  
> > > > -	if (mask) {
> > > > -		mask |= I915_READ(EDP_PSR_IMR);
> > > > -		I915_WRITE(EDP_PSR_IMR, mask);
> > > > +	if (psr_iir & EDP_PSR_PRE_ENTRY) {
> > > > +		dev_priv->psr.last_entry_attempt = time_ns;
> > > > +		DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2
> > > > vblanks\n",
> > > > +			      transcoder_name(TRANSCODER_EDP));
> > > > +	}
> > > >  
> > > > -		schedule_work(&dev_priv->psr.work);
> > > > +	if (psr_iir & EDP_PSR_POST_EXIT) {
> > > > +		DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> > > > +			      transcoder_name(TRANSCODER_EDP));
> > > > +
> > > > +		if (INTEL_GEN(dev_priv) >= 9) {
> > > > +			u32 val = I915_READ(PSR_EVENT);
> > > > +			bool psr2_enabled = dev_priv->psr.psr2_enabled;
> > > > +
> > > > +			I915_WRITE(PSR_EVENT, val);
> > > > +			psr_event_print(val, psr2_enabled);
> > > > +		}
> > > >  	}
> > > >  }
> > > >  
> > > > @@ -669,30 +620,10 @@ static void intel_psr_activate(struct
> > > > intel_dp *intel_dp)
> > > >  	dev_priv->psr.active = true;
> > > >  }
> > > >  
> > > > -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private
> > > > *dev_priv,
> > > > -					 enum transcoder
> > > > cpu_transcoder)
> > > > -{
> > > > -	static const i915_reg_t regs[] = {
> > > > -		[TRANSCODER_A] = CHICKEN_TRANS_A,
> > > > -		[TRANSCODER_B] = CHICKEN_TRANS_B,
> > > > -		[TRANSCODER_C] = CHICKEN_TRANS_C,
> > > > -		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
> > > > -	};
> > > > -
> > > > -	WARN_ON(INTEL_GEN(dev_priv) < 9);
> > > > -
> > > > -	if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) ||
> > > > -		    !regs[cpu_transcoder].reg))
> > > > -		cpu_transcoder = TRANSCODER_A;
> > > > -
> > > > -	return regs[cpu_transcoder];
> > > > -}
> > > > -
> > > >  static void intel_psr_enable_source(struct intel_dp *intel_dp,
> > > >  				    const struct intel_crtc_state
> > > > *crtc_state)
> > > >  {
> > > >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > > -	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > >  	u32 mask;
> > > >  
> > > >  	/* Only HSW and BDW have PSR AUX registers that need to be
> > > > setup. SKL+
> > > > @@ -703,13 +634,11 @@ static void intel_psr_enable_source(struct
> > > > intel_dp *intel_dp,
> > > >  
> > > >  	if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9) &&
> > > >  					   !IS_GEMINILAKE(dev_priv))) {
> > > > -		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> > > > -							cpu_transcoder)
> > > > ;
> > > > -		u32 chicken = I915_READ(reg);
> > > > +		u32 chicken = I915_READ(CHICKEN_TRANS_EDP);
> > > >  
> > > >  		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
> > > >  			   PSR2_ADD_VERTICAL_LINE_COUNT;
> > > > -		I915_WRITE(reg, chicken);
> > > > +		I915_WRITE(CHICKEN_TRANS_EDP, chicken);
> > > >  	}
> > > >  
> > > >  	/*
> > > > @@ -1222,7 +1151,7 @@ void intel_psr_init(struct drm_i915_private
> > > > *dev_priv)
> > > >  	 * to avoid any rendering problems.
> > > >  	 */
> > > >  	val = I915_READ(EDP_PSR_IIR);
> > > > -	val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
> > > > +	val &= EDP_PSR_ERROR;
> > > >  	if (val) {
> > > >  		DRM_DEBUG_KMS("PSR interruption error set\n");
> > > >  		dev_priv->psr.sink_not_reliable = true;
> > > > -- 
> > > > 2.21.0
> > > > 
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders

[Rodrigo Vivi]

On Thu, Apr 04, 2019 at 02:41:26PM -0700, Pandiyan, Dhinakaran wrote:
> On Thu, 2019-04-04 at 14:20 -0700, Rodrigo Vivi wrote:
> > On Thu, Apr 04, 2019 at 12:40:34PM -0700, Souza, Jose wrote:
> > > On Wed, 2019-04-03 at 17:31 -0700, Rodrigo Vivi wrote:
> > > > On Wed, Apr 03, 2019 at 04:35:38PM -0700, José Roberto de Souza
> > > > wrote:
> > > > > PSR is only supported in eDP transcoder and there is only one
> > > > > instance of it, so lets drop all of this code.
> > > > 
> > > > Is this sentence true? I mean, in the way it is written it
> > > > seems like HW doesn't actually support it...
> > > > Or should we re-phrase for we are not really enabling support
> > > > for other transcoders than eDP and we do not have plans to do
> > > > it so soon so let's clean the code...
> > > > or something like that?
> > > 
> > > Okay, what about replace it for:
> > > 
> > > Since BDW all transcoders have PSR registers but only EDP transcoder
> > > can drive a EDP panel and as PSR is only part of the EDP specification,
> > > for real usage PSR is only supported in EDP panel, so lets drop all of
> > > this useless code.
> > 
> > well, this still looks like HW limitation. If PSR is supported by HW on
> > different transcoders is because there's some possibility of adding
> > eDP on other transcoders. They wouldn't waste so many register space
> > if that wasn't the case.
> > 
> > Even though we have a dedicated transcoder for eDP I don't
> > believe we can assume that eDP is not supported on the other ones.
> > 
> 
> Why not write something like (or exactly) this 
> 
> "i915 does not support enabling PSR on any transcoder other than eDP. Clean up
> the misleading non-eDP code that currently exists to allow for the rework of PSR
> register definitions in the next patch"

+1

> 
> 
> -DK
> > > 
> > > 
> > > 
> > > > 
> > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_reg.h  |  17 +---
> > > > >  drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++-------------------
> > > > > ----
> > > > >  2 files changed, 42 insertions(+), 122 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > index c59cfa83dbaf..18e2991b376d 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > @@ -4241,13 +4241,9 @@ enum {
> > > > >  /* Bspec claims those aren't shifted but stay at 0x64800 */
> > > > >  #define EDP_PSR_IMR				_MMIO(0x64834)
> > > > >  #define EDP_PSR_IIR				_MMIO(0x64838)
> > > > > -#define   EDP_PSR_ERROR(shift)			(1 << ((shift)
> > > > > + 2))
> > > > > -#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift) + 1))
> > > > > -#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
> > > > > -#define   EDP_PSR_TRANSCODER_C_SHIFT		24
> > > > > -#define   EDP_PSR_TRANSCODER_B_SHIFT		16
> > > > > -#define   EDP_PSR_TRANSCODER_A_SHIFT		8
> > > > > -#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
> > > > > +#define   EDP_PSR_ERROR				(1 << 2)
> > > > > +#define   EDP_PSR_POST_EXIT			(1 << 1)
> > > > > +#define   EDP_PSR_PRE_ENTRY			(1 << 0)
> > > > >  
> > > > >  #define EDP_PSR_AUX_CTL				_MMIO(dev_priv-
> > > > > > psr_mmio_base + 0x10)
> > > > > 
> > > > >  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
> > > > > @@ -4312,12 +4308,7 @@ enum {
> > > > >  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> > > > >  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> > > > >  
> > > > > -#define _PSR_EVENT_TRANS_A			0x60848
> > > > > -#define _PSR_EVENT_TRANS_B			0x61848
> > > > > -#define _PSR_EVENT_TRANS_C			0x62848
> > > > > -#define _PSR_EVENT_TRANS_D			0x63848
> > > > > -#define _PSR_EVENT_TRANS_EDP			0x6F848
> > > > > -#define PSR_EVENT(trans)			_MMIO_TRANS2(trans,
> > > > > _PSR_EVENT_TRANS_A)
> > > > > +#define PSR_EVENT				_MMIO(0x6F848)
> > > > >  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
> > > > >  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
> > > > >  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > > index bb97c1657493..b984e005b72e 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -84,46 +84,12 @@ static bool intel_psr2_enabled(struct
> > > > > drm_i915_private *dev_priv,
> > > > >  	}
> > > > >  }
> > > > >  
> > > > > -static int edp_psr_shift(enum transcoder cpu_transcoder)
> > > > > -{
> > > > > -	switch (cpu_transcoder) {
> > > > > -	case TRANSCODER_A:
> > > > > -		return EDP_PSR_TRANSCODER_A_SHIFT;
> > > > > -	case TRANSCODER_B:
> > > > > -		return EDP_PSR_TRANSCODER_B_SHIFT;
> > > > > -	case TRANSCODER_C:
> > > > > -		return EDP_PSR_TRANSCODER_C_SHIFT;
> > > > > -	default:
> > > > > -		MISSING_CASE(cpu_transcoder);
> > > > > -		/* fallthrough */
> > > > > -	case TRANSCODER_EDP:
> > > > > -		return EDP_PSR_TRANSCODER_EDP_SHIFT;
> > > > > -	}
> > > > > -}
> > > > > -
> > > > >  void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32
> > > > > debug)
> > > > >  {
> > > > > -	u32 debug_mask, mask;
> > > > > -	enum transcoder cpu_transcoder;
> > > > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > > > -
> > > > > -	if (INTEL_GEN(dev_priv) >= 8)
> > > > > -		transcoders |= BIT(TRANSCODER_A) |
> > > > > -			       BIT(TRANSCODER_B) |
> > > > > -			       BIT(TRANSCODER_C);
> > > > > -
> > > > > -	debug_mask = 0;
> > > > > -	mask = 0;
> > > > > -	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > > > > transcoders) {
> > > > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > > > -
> > > > > -		mask |= EDP_PSR_ERROR(shift);
> > > > > -		debug_mask |= EDP_PSR_POST_EXIT(shift) |
> > > > > -			      EDP_PSR_PRE_ENTRY(shift);
> > > > > -	}
> > > > > +	u32 mask = EDP_PSR_ERROR;
> > > > >  
> > > > >  	if (debug & I915_PSR_DEBUG_IRQ)
> > > > > -		mask |= debug_mask;
> > > > > +		mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY;
> > > > >  
> > > > >  	I915_WRITE(EDP_PSR_IMR, ~mask);
> > > > >  }
> > > > > @@ -167,62 +133,47 @@ static void psr_event_print(u32 val, bool
> > > > > psr2_enabled)
> > > > >  
> > > > >  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> > > > > psr_iir)
> > > > >  {
> > > > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > > > -	enum transcoder cpu_transcoder;
> > > > > -	ktime_t time_ns =  ktime_get();
> > > > > -	u32 mask = 0;
> > > > > +	ktime_t time_ns = ktime_get();
> > > > >  
> > > > > -	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) {
> > > > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > > > -
> > > > > -		if (psr_iir & EDP_PSR_ERROR(shift)) {
> > > > > -			DRM_WARN("[transcoder %s] PSR aux error\n",
> > > > > -				 transcoder_name(cpu_transcoder));
> > > > > -
> > > > > -			dev_priv->psr.irq_aux_error = true;
> > > > > -
> > > > > -			/*
> > > > > -			 * If this interruption is not masked it will
> > > > > keep
> > > > > -			 * interrupting so fast that it prevents the
> > > > > scheduled
> > > > > -			 * work to run.
> > > > > -			 * Also after a PSR error, we don't want to arm
> > > > > PSR
> > > > > -			 * again so we don't care about unmask the
> > > > > interruption
> > > > > -			 * or unset irq_aux_error.
> > > > > -			 */
> > > > > -			mask |= EDP_PSR_ERROR(shift);
> > > > > -		}
> > > > > +	if (psr_iir & EDP_PSR_ERROR) {
> > > > > +		u32 mask;
> > > > >  
> > > > > -		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
> > > > > -			dev_priv->psr.last_entry_attempt = time_ns;
> > > > > -			DRM_DEBUG_KMS("[transcoder %s] PSR entry
> > > > > attempt in 2 vblanks\n",
> > > > > -				      transcoder_name(cpu_transcoder));
> > > > > -		}
> > > > > +		DRM_WARN("[transcoder %s] PSR aux error\n",
> > > > > +			 transcoder_name(TRANSCODER_EDP));
> > > > >  
> > > > > -		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
> > > > > -			dev_priv->psr.last_exit = time_ns;
> > > > > -			DRM_DEBUG_KMS("[transcoder %s] PSR exit
> > > > > completed\n",
> > > > > -				      transcoder_name(cpu_transcoder));
> > > > > +		dev_priv->psr.irq_aux_error = true;
> > > > >  
> > > > > -			if (INTEL_GEN(dev_priv) >= 9) {
> > > > > -				u32 val =
> > > > > I915_READ(PSR_EVENT(cpu_transcoder));
> > > > > -				bool psr2_enabled = dev_priv-
> > > > > > psr.psr2_enabled;
> > > > > 
> > > > > +		/*
> > > > > +		 * If this interruption is not masked it will keep
> > > > > +		 * interrupting so fast that it prevents the scheduled
> > > > > +		 * work to run.
> > > > > +		 * Also after a PSR error, we don't want to arm PSR
> > > > > +		 * again so we don't care about unmask the interruption
> > > > > +		 * or unset irq_aux_error.
> > > > > +		 */
> > > > > +		mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR;
> > > > > +		I915_WRITE(EDP_PSR_IMR, mask);
> > > > >  
> > > > > -				I915_WRITE(PSR_EVENT(cpu_transcoder),
> > > > > val);
> > > > > -				psr_event_print(val, psr2_enabled);
> > > > > -			}
> > > > > -		}
> > > > > +		schedule_work(&dev_priv->psr.work);
> > > > >  	}
> > > > >  
> > > > > -	if (mask) {
> > > > > -		mask |= I915_READ(EDP_PSR_IMR);
> > > > > -		I915_WRITE(EDP_PSR_IMR, mask);
> > > > > +	if (psr_iir & EDP_PSR_PRE_ENTRY) {
> > > > > +		dev_priv->psr.last_entry_attempt = time_ns;
> > > > > +		DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2
> > > > > vblanks\n",
> > > > > +			      transcoder_name(TRANSCODER_EDP));
> > > > > +	}
> > > > >  
> > > > > -		schedule_work(&dev_priv->psr.work);
> > > > > +	if (psr_iir & EDP_PSR_POST_EXIT) {
> > > > > +		DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> > > > > +			      transcoder_name(TRANSCODER_EDP));
> > > > > +
> > > > > +		if (INTEL_GEN(dev_priv) >= 9) {
> > > > > +			u32 val = I915_READ(PSR_EVENT);
> > > > > +			bool psr2_enabled = dev_priv->psr.psr2_enabled;
> > > > > +
> > > > > +			I915_WRITE(PSR_EVENT, val);
> > > > > +			psr_event_print(val, psr2_enabled);
> > > > > +		}
> > > > >  	}
> > > > >  }
> > > > >  
> > > > > @@ -669,30 +620,10 @@ static void intel_psr_activate(struct
> > > > > intel_dp *intel_dp)
> > > > >  	dev_priv->psr.active = true;
> > > > >  }
> > > > >  
> > > > > -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private
> > > > > *dev_priv,
> > > > > -					 enum transcoder
> > > > > cpu_transcoder)
> > > > > -{
> > > > > -	static const i915_reg_t regs[] = {
> > > > > -		[TRANSCODER_A] = CHICKEN_TRANS_A,
> > > > > -		[TRANSCODER_B] = CHICKEN_TRANS_B,
> > > > > -		[TRANSCODER_C] = CHICKEN_TRANS_C,
> > > > > -		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
> > > > > -	};
> > > > > -
> > > > > -	WARN_ON(INTEL_GEN(dev_priv) < 9);
> > > > > -
> > > > > -	if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) ||
> > > > > -		    !regs[cpu_transcoder].reg))
> > > > > -		cpu_transcoder = TRANSCODER_A;
> > > > > -
> > > > > -	return regs[cpu_transcoder];
> > > > > -}
> > > > > -
> > > > >  static void intel_psr_enable_source(struct intel_dp *intel_dp,
> > > > >  				    const struct intel_crtc_state
> > > > > *crtc_state)
> > > > >  {
> > > > >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > > > -	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > > > >  	u32 mask;
> > > > >  
> > > > >  	/* Only HSW and BDW have PSR AUX registers that need to be
> > > > > setup. SKL+
> > > > > @@ -703,13 +634,11 @@ static void intel_psr_enable_source(struct
> > > > > intel_dp *intel_dp,
> > > > >  
> > > > >  	if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9) &&
> > > > >  					   !IS_GEMINILAKE(dev_priv))) {
> > > > > -		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> > > > > -							cpu_transcoder)
> > > > > ;
> > > > > -		u32 chicken = I915_READ(reg);
> > > > > +		u32 chicken = I915_READ(CHICKEN_TRANS_EDP);
> > > > >  
> > > > >  		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
> > > > >  			   PSR2_ADD_VERTICAL_LINE_COUNT;
> > > > > -		I915_WRITE(reg, chicken);
> > > > > +		I915_WRITE(CHICKEN_TRANS_EDP, chicken);
> > > > >  	}
> > > > >  
> > > > >  	/*
> > > > > @@ -1222,7 +1151,7 @@ void intel_psr_init(struct drm_i915_private
> > > > > *dev_priv)
> > > > >  	 * to avoid any rendering problems.
> > > > >  	 */
> > > > >  	val = I915_READ(EDP_PSR_IIR);
> > > > > -	val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
> > > > > +	val &= EDP_PSR_ERROR;
> > > > >  	if (val) {
> > > > >  		DRM_DEBUG_KMS("PSR interruption error set\n");
> > > > >  		dev_priv->psr.sink_not_reliable = true;
> > > > > -- 
> > > > > 2.21.0
> > > > > 
> > 
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915/psr: Do not enable PSR in interlaced mode for all GENs

[Dhinakaran Pandiyan]

On Wed, 2019-04-03 at 17:29 -0700, Rodrigo Vivi wrote:
> On Wed, Apr 03, 2019 at 04:35:36PM -0700, José Roberto de Souza wrote:
> > This interlaced restriction applies to all gens, not only to Haswell.
> 
> I believe this came from VLV times and I doubt we would be
> impacted by it ever, but better to protect just in case:
> 
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index a84da931c3be..bb97c1657493 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -627,8 +627,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> >  		return;
> >  	}
> >  
> > -	if (IS_HASWELL(dev_priv) &&
> > -	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> > +	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> >  		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
It'd be nice to change this to "interlaced mode"

Noticed that the spec says PSR does not work with Stereo 3d mode as well. But,
that should be okay since we don't set stereo_allowed for DP.

Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> >  		return;
> >  	}
> > -- 
> > 2.21.0
> > 

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

Re: [PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders

[Dhinakaran Pandiyan]

On Thu, 2019-04-04 at 14:41 -0700, Dhinakaran Pandiyan wrote:
> On Thu, 2019-04-04 at 14:20 -0700, Rodrigo Vivi wrote:
> > On Thu, Apr 04, 2019 at 12:40:34PM -0700, Souza, Jose wrote:
> > > On Wed, 2019-04-03 at 17:31 -0700, Rodrigo Vivi wrote:
> > > > On Wed, Apr 03, 2019 at 04:35:38PM -0700, José Roberto de Souza
> > > > wrote:
> > > > > PSR is only supported in eDP transcoder and there is only one
> > > > > instance of it, so lets drop all of this code.
> > > > 
> > > > Is this sentence true? I mean, in the way it is written it
> > > > seems like HW doesn't actually support it...
> > > > Or should we re-phrase for we are not really enabling support
> > > > for other transcoders than eDP and we do not have plans to do
> > > > it so soon so let's clean the code...
> > > > or something like that?
> > > 
> > > Okay, what about replace it for:
> > > 
> > > Since BDW all transcoders have PSR registers but only EDP transcoder
> > > can drive a EDP panel and as PSR is only part of the EDP specification,
> > > for real usage PSR is only supported in EDP panel, so lets drop all of
> > > this useless code.
> > 
> > well, this still looks like HW limitation. If PSR is supported by HW on
> > different transcoders is because there's some possibility of adding
> > eDP on other transcoders. They wouldn't waste so many register space
> > if that wasn't the case.
> > 
> > Even though we have a dedicated transcoder for eDP I don't
> > believe we can assume that eDP is not supported on the other ones.
> > 
> 
> Why not write something like (or exactly) this 
> 
> "i915 does not support enabling PSR on any transcoder other than eDP. Clean up
> the misleading non-eDP code that currently exists to allow for the rework of
> PSR
> register definitions in the next patch"
> 
> 
> -DK
> > > 
> > > 
> > > 
> > > > 
> > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_reg.h  |  17 +---
> > > > >  drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++-------------------
> > > > > ----
> > > > >  2 files changed, 42 insertions(+), 122 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > index c59cfa83dbaf..18e2991b376d 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > @@ -4241,13 +4241,9 @@ enum {
> > > > >  /* Bspec claims those aren't shifted but stay at 0x64800 */
> > > > >  #define EDP_PSR_IMR				_MMIO(0x64834)
> > > > >  #define EDP_PSR_IIR				_MMIO(0x64838)
> > > > > -#define   EDP_PSR_ERROR(shift)			(1 << ((shift)
> > > > > + 2))
> > > > > -#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift) + 1))
> > > > > -#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
> > > > > -#define   EDP_PSR_TRANSCODER_C_SHIFT		24
> > > > > -#define   EDP_PSR_TRANSCODER_B_SHIFT		16
> > > > > -#define   EDP_PSR_TRANSCODER_A_SHIFT		8
> > > > > -#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
> > > > > +#define   EDP_PSR_ERROR				(1 << 2)
> > > > > +#define   EDP_PSR_POST_EXIT			(1 << 1)
> > > > > +#define   EDP_PSR_PRE_ENTRY			(1 << 0)
> > > > >  
> > > > >  #define EDP_PSR_AUX_CTL				_MMIO(dev_priv-
> > > > > > psr_mmio_base + 0x10)
> > > > > 
> > > > >  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
> > > > > @@ -4312,12 +4308,7 @@ enum {
> > > > >  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> > > > >  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> > > > >  
> > > > > -#define _PSR_EVENT_TRANS_A			0x60848
> > > > > -#define _PSR_EVENT_TRANS_B			0x61848
> > > > > -#define _PSR_EVENT_TRANS_C			0x62848
> > > > > -#define _PSR_EVENT_TRANS_D			0x63848
> > > > > -#define _PSR_EVENT_TRANS_EDP			0x6F848
> > > > > -#define PSR_EVENT(trans)			_MMIO_TRANS2(trans,
> > > > > _PSR_EVENT_TRANS_A)
> > > > > +#define PSR_EVENT				_MMIO(0x6F848)
> > > > >  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
> > > > >  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
> > > > >  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > > index bb97c1657493..b984e005b72e 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -84,46 +84,12 @@ static bool intel_psr2_enabled(struct
> > > > > drm_i915_private *dev_priv,
> > > > >  	}
> > > > >  }
> > > > >  
> > > > > -static int edp_psr_shift(enum transcoder cpu_transcoder)
> > > > > -{
> > > > > -	switch (cpu_transcoder) {
> > > > > -	case TRANSCODER_A:
> > > > > -		return EDP_PSR_TRANSCODER_A_SHIFT;
> > > > > -	case TRANSCODER_B:
> > > > > -		return EDP_PSR_TRANSCODER_B_SHIFT;
> > > > > -	case TRANSCODER_C:
> > > > > -		return EDP_PSR_TRANSCODER_C_SHIFT;
> > > > > -	default:
> > > > > -		MISSING_CASE(cpu_transcoder);
> > > > > -		/* fallthrough */
> > > > > -	case TRANSCODER_EDP:
> > > > > -		return EDP_PSR_TRANSCODER_EDP_SHIFT;
> > > > > -	}
> > > > > -}
> > > > > -
> > > > >  void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32
> > > > > debug)
> > > > >  {
> > > > > -	u32 debug_mask, mask;
> > > > > -	enum transcoder cpu_transcoder;
> > > > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > > > -
> > > > > -	if (INTEL_GEN(dev_priv) >= 8)
> > > > > -		transcoders |= BIT(TRANSCODER_A) |
> > > > > -			       BIT(TRANSCODER_B) |
> > > > > -			       BIT(TRANSCODER_C);
> > > > > -
> > > > > -	debug_mask = 0;
> > > > > -	mask = 0;
> > > > > -	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > > > > transcoders) {
> > > > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > > > -
> > > > > -		mask |= EDP_PSR_ERROR(shift);
> > > > > -		debug_mask |= EDP_PSR_POST_EXIT(shift) |
> > > > > -			      EDP_PSR_PRE_ENTRY(shift);
> > > > > -	}
> > > > > +	u32 mask = EDP_PSR_ERROR;
> > > > >  
> > > > >  	if (debug & I915_PSR_DEBUG_IRQ)
> > > > > -		mask |= debug_mask;
> > > > > +		mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY;
> > > > >  
> > > > >  	I915_WRITE(EDP_PSR_IMR, ~mask);
> > > > >  }
> > > > > @@ -167,62 +133,47 @@ static void psr_event_print(u32 val, bool
> > > > > psr2_enabled)
> > > > >  
> > > > >  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> > > > > psr_iir)
> > > > >  {
> > > > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > > > -	enum transcoder cpu_transcoder;
> > > > > -	ktime_t time_ns =  ktime_get();
> > > > > -	u32 mask = 0;
> > > > > +	ktime_t time_ns = ktime_get();
> > > > >  
> > > > > -	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) {
> > > > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > > > -
> > > > > -		if (psr_iir & EDP_PSR_ERROR(shift)) {
> > > > > -			DRM_WARN("[transcoder %s] PSR aux error\n",
> > > > > -				 transcoder_name(cpu_transcoder));
> > > > > -
> > > > > -			dev_priv->psr.irq_aux_error = true;
> > > > > -
> > > > > -			/*
> > > > > -			 * If this interruption is not masked it will
> > > > > keep
> > > > > -			 * interrupting so fast that it prevents the
> > > > > scheduled
> > > > > -			 * work to run.
> > > > > -			 * Also after a PSR error, we don't want to arm
> > > > > PSR
> > > > > -			 * again so we don't care about unmask the
> > > > > interruption
> > > > > -			 * or unset irq_aux_error.
> > > > > -			 */
> > > > > -			mask |= EDP_PSR_ERROR(shift);
> > > > > -		}

What's really the strategy though? I hope we aren't planning to add this code
back again.

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

Re: [PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 14576 bytes --]

On Thu, 2019-04-04 at 14:51 -0700, Rodrigo Vivi wrote:
> On Thu, Apr 04, 2019 at 02:41:26PM -0700, Pandiyan, Dhinakaran wrote:
> > On Thu, 2019-04-04 at 14:20 -0700, Rodrigo Vivi wrote:
> > > On Thu, Apr 04, 2019 at 12:40:34PM -0700, Souza, Jose wrote:
> > > > On Wed, 2019-04-03 at 17:31 -0700, Rodrigo Vivi wrote:
> > > > > On Wed, Apr 03, 2019 at 04:35:38PM -0700, José Roberto de
> > > > > Souza
> > > > > wrote:
> > > > > > PSR is only supported in eDP transcoder and there is only
> > > > > > one
> > > > > > instance of it, so lets drop all of this code.
> > > > > 
> > > > > Is this sentence true? I mean, in the way it is written it
> > > > > seems like HW doesn't actually support it...
> > > > > Or should we re-phrase for we are not really enabling support
> > > > > for other transcoders than eDP and we do not have plans to do
> > > > > it so soon so let's clean the code...
> > > > > or something like that?
> > > > 
> > > > Okay, what about replace it for:
> > > > 
> > > > Since BDW all transcoders have PSR registers but only EDP
> > > > transcoder
> > > > can drive a EDP panel and as PSR is only part of the EDP
> > > > specification,
> > > > for real usage PSR is only supported in EDP panel, so lets drop
> > > > all of
> > > > this useless code.
> > > 
> > > well, this still looks like HW limitation. If PSR is supported by
> > > HW on
> > > different transcoders is because there's some possibility of
> > > adding
> > > eDP on other transcoders. They wouldn't waste so many register
> > > space
> > > if that wasn't the case.
> > > 
> > > Even though we have a dedicated transcoder for eDP I don't
> > > believe we can assume that eDP is not supported on the other
> > > ones.
> > > 
> > 
> > Why not write something like (or exactly) this 
> > 
> > "i915 does not support enabling PSR on any transcoder other than
> > eDP. Clean up
> > the misleading non-eDP code that currently exists to allow for the
> > rework of PSR
> > register definitions in the next patch"
> 
> +1

Sure, going to change to that in the next version.

> 
> > 
> > -DK
> > > > 
> > > > 
> > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/i915_reg.h  |  17 +---
> > > > > >  drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++-----------
> > > > > > --------
> > > > > > ----
> > > > > >  2 files changed, 42 insertions(+), 122 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > > index c59cfa83dbaf..18e2991b376d 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > > @@ -4241,13 +4241,9 @@ enum {
> > > > > >  /* Bspec claims those aren't shifted but stay at 0x64800
> > > > > > */
> > > > > >  #define EDP_PSR_IMR				_MMIO(0
> > > > > > x64834)
> > > > > >  #define EDP_PSR_IIR				_MMIO(0
> > > > > > x64838)
> > > > > > -#define   EDP_PSR_ERROR(shift)			(1 <<
> > > > > > ((shift)
> > > > > > + 2))
> > > > > > -#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift)
> > > > > > + 1))
> > > > > > -#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
> > > > > > -#define   EDP_PSR_TRANSCODER_C_SHIFT		24
> > > > > > -#define   EDP_PSR_TRANSCODER_B_SHIFT		16
> > > > > > -#define   EDP_PSR_TRANSCODER_A_SHIFT		8
> > > > > > -#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
> > > > > > +#define   EDP_PSR_ERROR				(1 <<
> > > > > > 2)
> > > > > > +#define   EDP_PSR_POST_EXIT			(1 <<
> > > > > > 1)
> > > > > > +#define   EDP_PSR_PRE_ENTRY			(1 <<
> > > > > > 0)
> > > > > >  
> > > > > >  #define EDP_PSR_AUX_CTL				_MMIO(d
> > > > > > ev_priv-
> > > > > > > psr_mmio_base + 0x10)
> > > > > > 
> > > > > >  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 <<
> > > > > > 26)
> > > > > > @@ -4312,12 +4308,7 @@ enum {
> > > > > >  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> > > > > >  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> > > > > >  
> > > > > > -#define _PSR_EVENT_TRANS_A			0x60848
> > > > > > -#define _PSR_EVENT_TRANS_B			0x61848
> > > > > > -#define _PSR_EVENT_TRANS_C			0x62848
> > > > > > -#define _PSR_EVENT_TRANS_D			0x63848
> > > > > > -#define _PSR_EVENT_TRANS_EDP			0x6F848
> > > > > > -#define PSR_EVENT(trans)			_MMIO_TRANS2(tr
> > > > > > ans,
> > > > > > _PSR_EVENT_TRANS_A)
> > > > > > +#define PSR_EVENT				_MMIO(0x6F848)
> > > > > >  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 <<
> > > > > > 17)
> > > > > >  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
> > > > > >  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > index bb97c1657493..b984e005b72e 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > > @@ -84,46 +84,12 @@ static bool intel_psr2_enabled(struct
> > > > > > drm_i915_private *dev_priv,
> > > > > >  	}
> > > > > >  }
> > > > > >  
> > > > > > -static int edp_psr_shift(enum transcoder cpu_transcoder)
> > > > > > -{
> > > > > > -	switch (cpu_transcoder) {
> > > > > > -	case TRANSCODER_A:
> > > > > > -		return EDP_PSR_TRANSCODER_A_SHIFT;
> > > > > > -	case TRANSCODER_B:
> > > > > > -		return EDP_PSR_TRANSCODER_B_SHIFT;
> > > > > > -	case TRANSCODER_C:
> > > > > > -		return EDP_PSR_TRANSCODER_C_SHIFT;
> > > > > > -	default:
> > > > > > -		MISSING_CASE(cpu_transcoder);
> > > > > > -		/* fallthrough */
> > > > > > -	case TRANSCODER_EDP:
> > > > > > -		return EDP_PSR_TRANSCODER_EDP_SHIFT;
> > > > > > -	}
> > > > > > -}
> > > > > > -
> > > > > >  void intel_psr_irq_control(struct drm_i915_private
> > > > > > *dev_priv, u32
> > > > > > debug)
> > > > > >  {
> > > > > > -	u32 debug_mask, mask;
> > > > > > -	enum transcoder cpu_transcoder;
> > > > > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > > > > -
> > > > > > -	if (INTEL_GEN(dev_priv) >= 8)
> > > > > > -		transcoders |= BIT(TRANSCODER_A) |
> > > > > > -			       BIT(TRANSCODER_B) |
> > > > > > -			       BIT(TRANSCODER_C);
> > > > > > -
> > > > > > -	debug_mask = 0;
> > > > > > -	mask = 0;
> > > > > > -	for_each_cpu_transcoder_masked(dev_priv,
> > > > > > cpu_transcoder,
> > > > > > transcoders) {
> > > > > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > > > > -
> > > > > > -		mask |= EDP_PSR_ERROR(shift);
> > > > > > -		debug_mask |= EDP_PSR_POST_EXIT(shift) |
> > > > > > -			      EDP_PSR_PRE_ENTRY(shift);
> > > > > > -	}
> > > > > > +	u32 mask = EDP_PSR_ERROR;
> > > > > >  
> > > > > >  	if (debug & I915_PSR_DEBUG_IRQ)
> > > > > > -		mask |= debug_mask;
> > > > > > +		mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY;
> > > > > >  
> > > > > >  	I915_WRITE(EDP_PSR_IMR, ~mask);
> > > > > >  }
> > > > > > @@ -167,62 +133,47 @@ static void psr_event_print(u32 val,
> > > > > > bool
> > > > > > psr2_enabled)
> > > > > >  
> > > > > >  void intel_psr_irq_handler(struct drm_i915_private
> > > > > > *dev_priv, u32
> > > > > > psr_iir)
> > > > > >  {
> > > > > > -	u32 transcoders = BIT(TRANSCODER_EDP);
> > > > > > -	enum transcoder cpu_transcoder;
> > > > > > -	ktime_t time_ns =  ktime_get();
> > > > > > -	u32 mask = 0;
> > > > > > +	ktime_t time_ns = ktime_get();
> > > > > >  
> > > > > > -	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) {
> > > > > > -		int shift = edp_psr_shift(cpu_transcoder);
> > > > > > -
> > > > > > -		if (psr_iir & EDP_PSR_ERROR(shift)) {
> > > > > > -			DRM_WARN("[transcoder %s] PSR aux
> > > > > > error\n",
> > > > > > -				 transcoder_name(cpu_transcoder
> > > > > > ));
> > > > > > -
> > > > > > -			dev_priv->psr.irq_aux_error = true;
> > > > > > -
> > > > > > -			/*
> > > > > > -			 * If this interruption is not masked
> > > > > > it will
> > > > > > keep
> > > > > > -			 * interrupting so fast that it
> > > > > > prevents the
> > > > > > scheduled
> > > > > > -			 * work to run.
> > > > > > -			 * Also after a PSR error, we don't
> > > > > > want to arm
> > > > > > PSR
> > > > > > -			 * again so we don't care about unmask
> > > > > > the
> > > > > > interruption
> > > > > > -			 * or unset irq_aux_error.
> > > > > > -			 */
> > > > > > -			mask |= EDP_PSR_ERROR(shift);
> > > > > > -		}
> > > > > > +	if (psr_iir & EDP_PSR_ERROR) {
> > > > > > +		u32 mask;
> > > > > >  
> > > > > > -		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
> > > > > > -			dev_priv->psr.last_entry_attempt =
> > > > > > time_ns;
> > > > > > -			DRM_DEBUG_KMS("[transcoder %s] PSR
> > > > > > entry
> > > > > > attempt in 2 vblanks\n",
> > > > > > -				      transcoder_name(cpu_trans
> > > > > > coder));
> > > > > > -		}
> > > > > > +		DRM_WARN("[transcoder %s] PSR aux error\n",
> > > > > > +			 transcoder_name(TRANSCODER_EDP));
> > > > > >  
> > > > > > -		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
> > > > > > -			dev_priv->psr.last_exit = time_ns;
> > > > > > -			DRM_DEBUG_KMS("[transcoder %s] PSR exit
> > > > > > completed\n",
> > > > > > -				      transcoder_name(cpu_trans
> > > > > > coder));
> > > > > > +		dev_priv->psr.irq_aux_error = true;
> > > > > >  
> > > > > > -			if (INTEL_GEN(dev_priv) >= 9) {
> > > > > > -				u32 val =
> > > > > > I915_READ(PSR_EVENT(cpu_transcoder));
> > > > > > -				bool psr2_enabled = dev_priv-
> > > > > > > psr.psr2_enabled;
> > > > > > 
> > > > > > +		/*
> > > > > > +		 * If this interruption is not masked it will
> > > > > > keep
> > > > > > +		 * interrupting so fast that it prevents the
> > > > > > scheduled
> > > > > > +		 * work to run.
> > > > > > +		 * Also after a PSR error, we don't want to arm
> > > > > > PSR
> > > > > > +		 * again so we don't care about unmask the
> > > > > > interruption
> > > > > > +		 * or unset irq_aux_error.
> > > > > > +		 */
> > > > > > +		mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR;
> > > > > > +		I915_WRITE(EDP_PSR_IMR, mask);
> > > > > >  
> > > > > > -				I915_WRITE(PSR_EVENT(cpu_transc
> > > > > > oder),
> > > > > > val);
> > > > > > -				psr_event_print(val,
> > > > > > psr2_enabled);
> > > > > > -			}
> > > > > > -		}
> > > > > > +		schedule_work(&dev_priv->psr.work);
> > > > > >  	}
> > > > > >  
> > > > > > -	if (mask) {
> > > > > > -		mask |= I915_READ(EDP_PSR_IMR);
> > > > > > -		I915_WRITE(EDP_PSR_IMR, mask);
> > > > > > +	if (psr_iir & EDP_PSR_PRE_ENTRY) {
> > > > > > +		dev_priv->psr.last_entry_attempt = time_ns;
> > > > > > +		DRM_DEBUG_KMS("[transcoder %s] PSR entry
> > > > > > attempt in 2
> > > > > > vblanks\n",
> > > > > > +			      transcoder_name(TRANSCODER_EDP));
> > > > > > +	}
> > > > > >  
> > > > > > -		schedule_work(&dev_priv->psr.work);
> > > > > > +	if (psr_iir & EDP_PSR_POST_EXIT) {
> > > > > > +		DRM_DEBUG_KMS("[transcoder %s] PSR exit
> > > > > > completed\n",
> > > > > > +			      transcoder_name(TRANSCODER_EDP));
> > > > > > +
> > > > > > +		if (INTEL_GEN(dev_priv) >= 9) {
> > > > > > +			u32 val = I915_READ(PSR_EVENT);
> > > > > > +			bool psr2_enabled = dev_priv-
> > > > > > >psr.psr2_enabled;
> > > > > > +
> > > > > > +			I915_WRITE(PSR_EVENT, val);
> > > > > > +			psr_event_print(val, psr2_enabled);
> > > > > > +		}
> > > > > >  	}
> > > > > >  }
> > > > > >  
> > > > > > @@ -669,30 +620,10 @@ static void intel_psr_activate(struct
> > > > > > intel_dp *intel_dp)
> > > > > >  	dev_priv->psr.active = true;
> > > > > >  }
> > > > > >  
> > > > > > -static i915_reg_t gen9_chicken_trans_reg(struct
> > > > > > drm_i915_private
> > > > > > *dev_priv,
> > > > > > -					 enum transcoder
> > > > > > cpu_transcoder)
> > > > > > -{
> > > > > > -	static const i915_reg_t regs[] = {
> > > > > > -		[TRANSCODER_A] = CHICKEN_TRANS_A,
> > > > > > -		[TRANSCODER_B] = CHICKEN_TRANS_B,
> > > > > > -		[TRANSCODER_C] = CHICKEN_TRANS_C,
> > > > > > -		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
> > > > > > -	};
> > > > > > -
> > > > > > -	WARN_ON(INTEL_GEN(dev_priv) < 9);
> > > > > > -
> > > > > > -	if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) ||
> > > > > > -		    !regs[cpu_transcoder].reg))
> > > > > > -		cpu_transcoder = TRANSCODER_A;
> > > > > > -
> > > > > > -	return regs[cpu_transcoder];
> > > > > > -}
> > > > > > -
> > > > > >  static void intel_psr_enable_source(struct intel_dp
> > > > > > *intel_dp,
> > > > > >  				    const struct
> > > > > > intel_crtc_state
> > > > > > *crtc_state)
> > > > > >  {
> > > > > >  	struct drm_i915_private *dev_priv =
> > > > > > dp_to_i915(intel_dp);
> > > > > > -	enum transcoder cpu_transcoder = crtc_state-
> > > > > > >cpu_transcoder;
> > > > > >  	u32 mask;
> > > > > >  
> > > > > >  	/* Only HSW and BDW have PSR AUX registers that need to
> > > > > > be
> > > > > > setup. SKL+
> > > > > > @@ -703,13 +634,11 @@ static void
> > > > > > intel_psr_enable_source(struct
> > > > > > intel_dp *intel_dp,
> > > > > >  
> > > > > >  	if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9)
> > > > > > &&
> > > > > >  					   !IS_GEMINILAKE(dev_p
> > > > > > riv))) {
> > > > > > -		i915_reg_t reg =
> > > > > > gen9_chicken_trans_reg(dev_priv,
> > > > > > -							cpu_tra
> > > > > > nscoder)
> > > > > > ;
> > > > > > -		u32 chicken = I915_READ(reg);
> > > > > > +		u32 chicken = I915_READ(CHICKEN_TRANS_EDP);
> > > > > >  
> > > > > >  		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
> > > > > >  			   PSR2_ADD_VERTICAL_LINE_COUNT;
> > > > > > -		I915_WRITE(reg, chicken);
> > > > > > +		I915_WRITE(CHICKEN_TRANS_EDP, chicken);
> > > > > >  	}
> > > > > >  
> > > > > >  	/*
> > > > > > @@ -1222,7 +1151,7 @@ void intel_psr_init(struct
> > > > > > drm_i915_private
> > > > > > *dev_priv)
> > > > > >  	 * to avoid any rendering problems.
> > > > > >  	 */
> > > > > >  	val = I915_READ(EDP_PSR_IIR);
> > > > > > -	val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
> > > > > > +	val &= EDP_PSR_ERROR;
> > > > > >  	if (val) {
> > > > > >  		DRM_DEBUG_KMS("PSR interruption error set\n");
> > > > > >  		dev_priv->psr.sink_not_reliable = true;
> > > > > > -- 
> > > > > > 2.21.0
> > > > > > 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable

[Rodrigo Vivi]

On Thu, Apr 04, 2019 at 12:25:56PM -0700, Souza, Jose wrote:
> On Wed, 2019-04-03 at 17:27 -0700, Rodrigo Vivi wrote:
> > On Wed, Apr 03, 2019 at 04:35:35PM -0700, José Roberto de Souza
> > wrote:
> > > Even when driver is reloaded and hits this scenario the PSR mutex
> > > should be initialized, otherwise reading PSR debugfs status will
> > > execute mutex_lock() over a mutex that was not initialized.
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index c80bb3003a7d..a84da931c3be 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private
> > > *dev_priv)
> > >  	if (val) {
> > >  		DRM_DEBUG_KMS("PSR interruption error set\n");
> > >  		dev_priv->psr.sink_not_reliable = true;
> > > -		return;
> > 
> > There are other returns above and if debugfs hits this case maybe it
> > is worth to move the mutex initialization up instead?
> 
> 
> We have those two returns in PSR debugfs, !HAS_PSR(dev_priv) and !psr-
> >sink_support and in this cases we don't have any PSR functionality so
> not worthy to initialize anything PSR related.

oh, indeed.


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> 
> > 
> > >  	}
> > >  
> > >  	/* Set link_standby x link_off defaults */
> > > -- 
> > > 2.21.0
> > > 


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

Re: [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable

[Dhinakaran Pandiyan]

On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote:
> Even when driver is reloaded and hits this scenario the PSR mutex
> should be initialized, otherwise reading PSR debugfs status will
> execute mutex_lock() over a mutex that was not initialized.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index c80bb3003a7d..a84da931c3be 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  	if (val) {
>  		DRM_DEBUG_KMS("PSR interruption error set\n");
>  		dev_priv->psr.sink_not_reliable = true;
Should we just sink_support = false and keep the return? IOW is there any use
for sink_not_reliable?

> -		return;
>  	}


>  
>  	/* Set link_standby x link_off defaults */

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

Re: [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 1400 bytes --]

On Thu, 2019-04-04 at 17:22 -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote:
> > Even when driver is reloaded and hits this scenario the PSR mutex
> > should be initialized, otherwise reading PSR debugfs status will
> > execute mutex_lock() over a mutex that was not initialized.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index c80bb3003a7d..a84da931c3be 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> >  	if (val) {
> >  		DRM_DEBUG_KMS("PSR interruption error set\n");
> >  		dev_priv->psr.sink_not_reliable = true;
> Should we just sink_support = false and keep the return? IOW is there
> any use
> for sink_not_reliable?

I guess it could cause confusion as user had PSR support before the
module reload and after the load PSR debugfs will say that sink do not
support PSR.

> 
> > -		return;
> >  	}
> >  
> >  	/* Set link_standby x link_off defaults */

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 5/7] drm/i915/bdw+: Move misc display IRQ handling to it own function

[Dhinakaran Pandiyan]

On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote:
> Just moving it to reduce the tabs and avoid break code lines.
> No behavior changes intended here.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 63 +++++++++++++++++++--------------
>  1 file changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index aa107a78cb36..527d5cb21baa 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2702,41 +2702,50 @@ static u32 gen8_de_port_aux_mask(struct
> drm_i915_private *dev_priv)
>  	return mask;
>  }
>  
> -static irqreturn_t
> -gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> +static enum irqreturn
> +gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv)
>  {
> -	irqreturn_t ret = IRQ_NONE;
> -	u32 iir;
> -	enum pipe pipe;
> +	u32 iir = I915_READ(GEN8_DE_MISC_IIR);
> +	enum irqreturn ret = IRQ_NONE;
> +	bool found = false;
>  
> -	if (master_ctl & GEN8_DE_MISC_IRQ) {
> -		iir = I915_READ(GEN8_DE_MISC_IIR);
> -		if (iir) {
> -			bool found = false;
> -
> -			I915_WRITE(GEN8_DE_MISC_IIR, iir);
> -			ret = IRQ_HANDLED;
> +	if (!iir) {
> +		DRM_ERROR("The master control interrupt lied (DE MISC)!\n");
> +		return ret;
> +	}
Move the above to the caller and pass the IIR value as a parameter? That would
make it consistent with other handlers in this file. The benefit I see is that I
don't have to look beyond gen8_de_irq_handler() to know what the return value is
going to be.


> -				found = true;
> -			}
> +	I915_WRITE(GEN8_DE_MISC_IIR, iir);
> +	ret = IRQ_HANDLED;
>  
> -			if (iir & GEN8_DE_EDP_PSR) {
> -				u32 psr_iir = I915_READ(EDP_PSR_IIR);
> +	if (iir & GEN8_DE_MISC_GSE) {
> +		intel_opregion_asle_intr(dev_priv);
> +		found = true;
> +	}
>  
> -				intel_psr_irq_handler(dev_priv, psr_iir);
> -				I915_WRITE(EDP_PSR_IIR, psr_iir);
> -				found = true;
> -			}
> +	if (iir & GEN8_DE_EDP_PSR) {
> +		u32 psr_iir = I915_READ(EDP_PSR_IIR);
>  
> -			if (!found)
> -				DRM_ERROR("Unexpected DE Misc interrupt\n");
> -		}
> -		else
> -			DRM_ERROR("The master control interrupt lied (DE
> MISC)!\n");
> +		intel_psr_irq_handler(dev_priv, psr_iir);
> +		I915_WRITE(EDP_PSR_IIR, psr_iir);
> +		found = true;
>  	}
>  
> +	if (!found)
> +		DRM_ERROR("Unexpected DE Misc interrupt\n");
> +
> +	return ret;
> +}
> +
> +static irqreturn_t
> +gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> +{
> +	irqreturn_t ret = IRQ_NONE;
> +	u32 iir;
> +	enum pipe pipe;
> +
> +	if (master_ctl & GEN8_DE_MISC_IRQ)
> +		ret = gen8_de_misc_irq_handler(dev_priv);
> +
>  	if (INTEL_GEN(dev_priv) >= 11 && (master_ctl & GEN11_DE_HPD_IRQ)) {
>  		iir = I915_READ(GEN11_DE_HPD_IIR);
>  		if (iir) {

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

Re: [PATCH 3/7] drm/i915/psr: Initialize PSR mutex even when sink is not reliable

[Dhinakaran Pandiyan]

On Thu, 2019-04-04 at 17:32 -0700, Souza, Jose wrote:
> On Thu, 2019-04-04 at 17:22 -0700, Dhinakaran Pandiyan wrote:
> > On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote:
> > > Even when driver is reloaded and hits this scenario the PSR mutex
> > > should be initialized, otherwise reading PSR debugfs status will
> > > execute mutex_lock() over a mutex that was not initialized.
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index c80bb3003a7d..a84da931c3be 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -1227,7 +1227,6 @@ void intel_psr_init(struct drm_i915_private
> > > *dev_priv)
> > >  	if (val) {
> > >  		DRM_DEBUG_KMS("PSR interruption error set\n");
> > >  		dev_priv->psr.sink_not_reliable = true;
> > 
> > Should we just sink_support = false and keep the return? IOW is there
> > any use
> > for sink_not_reliable?
> 
> I guess it could cause confusion as user had PSR support before the
> module reload and after the load PSR debugfs will say that sink do not
> support PSR.

I don't think it is any more confusing than saying sink supports PSR and not
enabling it. Might as well make it clear that we are blaming the sink for not
enabling PSR.

> 
> > 
> > > -		return;
> > >  	}
> > >  
> > >  	/* Set link_standby x link_off defaults */

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

✓ Fi.CI.IGT: success for series starting with [1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915/psr: Update PSR2 SU corruption workaround comment (rev2)
URL   : https://patchwork.freedesktop.org/series/58974/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5875_full -> Patchwork_12686_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_create@create-clear:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@gem_exec_parallel@bsd1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +192

  * igt@gem_exec_parse@basic-allocation:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +46

  * igt@gem_exec_store@cachelines-blt:
    - shard-iclb:         NOTRUN -> INCOMPLETE [fdo#110246]

  * igt@gem_pread@stolen-uncached:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +13

  * igt@i915_pm_rpm@gem-idle:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807] +1

  * igt@kms_atomic_transition@3x-modeset-transitions-nonblocking:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +9

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-f:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-c:
    - shard-apl:          PASS -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#110222] +2

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-d:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_ccs@pipe-a-ccs-on-another-bo:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +78

  * igt@kms_chamelium@dp-hpd-storm:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +1

  * igt@kms_chamelium@vga-edid-read:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +15

  * igt@kms_cursor_crc@cursor-256x256-sliding:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232] +4

  * igt@kms_flip@2x-flip-vs-absolute-wf_vblank:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +2

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          NOTRUN -> FAIL [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-shrfb-fliptrack:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +2

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +7

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +4

  * igt@kms_lease@atomic_implicit_crtc:
    - shard-skl:          NOTRUN -> FAIL [fdo#110279]

  * igt@kms_lease@cursor_implicit_plane:
    - shard-skl:          NOTRUN -> FAIL [fdo#110278]

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105456]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-e:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +14

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +1

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +4

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145] +2

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_psr@cursor_mmap_gtt:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215]

  * igt@kms_psr@no_drrs:
    - shard-iclb:         PASS -> FAIL [fdo#108341]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@kms_universal_plane@disable-primary-vs-flip-pipe-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +4

  * igt@prime_nv_pcopy@test1_micro:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS

  * igt@gem_flink_race@flink_close:
    - shard-iclb:         DMESG-WARN -> PASS

  * igt@gem_mmap_gtt@forked-big-copy-xy:
    - shard-iclb:         TIMEOUT [fdo#109673] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - shard-iclb:         DMESG-WARN [fdo#110222] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] -> PASS

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
    - shard-iclb:         FAIL [fdo#103355] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +6

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-blt:
    - shard-iclb:         FAIL [fdo#105682] / [fdo#109247] -> PASS

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +14

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-glk:          SKIP [fdo#109271] -> PASS

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] -> PASS

  * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS +2

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +2

  * igt@kms_psr@sprite_mmap_gtt:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS +1

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-iclb:         FAIL [fdo#104894] -> PASS

  * igt@perf@polling:
    - shard-iclb:         FAIL [fdo#108587] -> PASS

  * igt@tools_test@tools_test:
    - shard-apl:          SKIP [fdo#109271] -> PASS

  
#### Warnings ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         INCOMPLETE [fdo#108686] -> FAIL [fdo#108686]

  
  {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#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108587]: https://bugs.freedesktop.org/show_bug.cgi?id=108587
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#110246]: https://bugs.freedesktop.org/show_bug.cgi?id=110246
  [fdo#110278]: https://bugs.freedesktop.org/show_bug.cgi?id=110278
  [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5875 -> Patchwork_12686

  CI_DRM_5875: 5cc7c47c44aaef5bfe07e7307d06caa98e401fad @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4928: 014a6fa238322b497116b359cb92df1ce7fa8847 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12686: a46b5a8681d15f9c7888adf75ea6f574446690f0 @ 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_12686/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915: Make PSR registers relative to transcoders

[Dhinakaran Pandiyan]

On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote:
> PSR registers are a mess, some have the full address while others just
> have the additional offset from psr_mmio_base.
> 
> psr_mmio_base is nothing more than TRANSCODER_EDP_OFFSET + 0x800 and
> using it makes more difficult for people with an PSR register address
> from BSpec to search the register name in i915 as also the BSpec name
> don't match with the name in i915.
> 
> The other option would be use the whole hard-coded address but this is
> not future proof, so here going in the middle ground by making every
> PSR register relative to transcoder(that is EDP transcoder), the only
> exception is PSR_IMR/IIR that is not relative to nothing.
> For the _TRANS2() macros to work it needs the address of the register
> from the TRANSCODER_A, so adding it to every register together with
> the register address from the EDP transcoder so it will make easy for
> people searching with BSpec address also adding those with the BSpec
> name.
> 
> For Haswell all the PSR register are relative to 0x64000, so
> mmio_base_adjust was added and used to take care of that.
> 
> Also removing BDW_EDP_PSR_BASE from GVT because it is not used as
> the only PSR register that GVT have is this one(SRD/PSR_CTL).
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/handlers.c |  1 -
>  drivers/gpu/drm/i915/i915_drv.h     |  5 ++-
>  drivers/gpu/drm/i915/i915_reg.h     | 59 ++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_psr.c    | 11 ++++--
>  4 files changed, 52 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index 86761b1def1e..d09b798e93cb 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -2739,7 +2739,6 @@ static int init_broadwell_mmio_info(struct intel_gvt
> *gvt)
>  	MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS);
>  
>  	MMIO_D(WM_MISC, D_BDW);
> -	MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW);
>  
>  	MMIO_D(_MMIO(0x6671c), D_BDW_PLUS);
>  	MMIO_D(_MMIO(0x66c00), D_BDW_PLUS);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8f38d03b1c4e..9ce46a7dabfd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -501,6 +501,8 @@ struct i915_drrs {
>  };
>  
>  struct i915_psr {
> +	/* different than zero only on HSW see _TRANS2_PSR() for more info */
> +	u32 mmio_base_adjust;
>  	struct mutex lock;
>  
>  #define I915_PSR_DEBUG_MODE_MASK	0x0f
> @@ -515,6 +517,7 @@ struct i915_psr {
>  	bool enabled;
>  	struct intel_dp *dp;
>  	enum pipe pipe;
> +	enum transcoder transcoder;
>  	bool active;
>  	struct work_struct work;
>  	unsigned busy_frontbuffer_bits;
> @@ -1541,8 +1544,6 @@ struct drm_i915_private {
>  	/* MMIO base address for MIPI regs */
>  	u32 mipi_mmio_base;
>  
> -	u32 psr_mmio_base;
> -
>  	u32 pps_mmio_base;
>  
>  	wait_queue_head_t gmbus_wait_queue;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 18e2991b376d..4df56c118cd2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -250,9 +250,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define _MMIO_PIPE2(pipe, reg)		_MMIO(INTEL_INFO(dev_priv)-
> >pipe_offsets[pipe] - \
>  					      INTEL_INFO(dev_priv)-
> >pipe_offsets[PIPE_A] + (reg) + \
>  					      DISPLAY_MMIO_BASE(dev_priv))
> -#define _MMIO_TRANS2(pipe, reg)		_MMIO(INTEL_INFO(dev_priv)-
> >trans_offsets[(pipe)] - \
> -					      INTEL_INFO(dev_priv)-
> >trans_offsets[TRANSCODER_A] + (reg) + \
> -					      DISPLAY_MMIO_BASE(dev_priv))
> +#define _TRANS2(trans, reg)		(INTEL_INFO(dev_priv)-
> >trans_offsets[(trans)] - \
> +					 INTEL_INFO(dev_priv)-
> >trans_offsets[TRANSCODER_A] + (reg) + \
> +					 DISPLAY_MMIO_BASE(dev_priv))
> +#define _MMIO_TRANS2(trans, reg)	_MMIO(_TRANS2(trans, reg))
>  #define _CURSOR2(pipe, reg)		_MMIO(INTEL_INFO(dev_priv)-
> >cursor_offsets[(pipe)] - \
>  					      INTEL_INFO(dev_priv)-
> >cursor_offsets[PIPE_A] + (reg) + \
>  					      DISPLAY_MMIO_BASE(dev_priv))
> @@ -4210,9 +4211,15 @@ enum {
>  #define PIPE_MULT(trans)	_MMIO_TRANS2(trans, _PIPE_MULT_A)
>  
>  /* HSW+ eDP PSR registers */
> -#define HSW_EDP_PSR_BASE	0x64800
> -#define BDW_EDP_PSR_BASE	0x6f800
> -#define EDP_PSR_CTL				_MMIO(dev_priv->psr_mmio_base +
> 0)
> +#define HSW_EDP_PSR_BASE	0x64000
> +
> +/* PSR registers on HSW is not relative to eDP transcoder */
> +#define _TRANS2_PSR(reg)	(_TRANS2(dev_priv->psr.transcoder, (reg)) -
> dev_priv->psr.mmio_base_adjust)

The error interrupt registers are still hardcoded for eDP, aren't they?

> +#define _MMIO_TRANS2_PSR(reg)	_MMIO(_TRANS2_PSR(reg))
> +
> +#define _SRD_CTL_A				0x60800
> +#define _SRD_CTL_EDP				0x6F800
> +#define EDP_PSR_CTL				_MMIO_TRANS2_PSR(_SRD_CTL_A)
Why isn't this accepting transcoder/pipe as an argument? dev_priv-
>psr.transcoder is hidden two levels below than it should be.




>  #define   EDP_PSR_ENABLE			(1 << 31)
>  #define   BDW_PSR_SINGLE_FRAME			(1 << 30)
>  #define   EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK	(1 << 29) /* SW can't
> modify */
> @@ -4245,16 +4252,22 @@ enum {
>  #define   EDP_PSR_POST_EXIT			(1 << 1)
>  #define   EDP_PSR_PRE_ENTRY			(1 << 0)
>  
> -#define EDP_PSR_AUX_CTL				_MMIO(dev_priv-
> >psr_mmio_base + 0x10)
> +#define _SRD_AUX_CTL_A				0x60810
> +#define _SRD_AUX_CTL_EDP			0x6F810
> +#define EDP_PSR_AUX_CTL				_MMIO_TRANS2_PSR(_SRD_AU
> X_CTL_A)
>  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
>  #define   EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK	(0x1f << 20)
>  #define   EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK	(0xf << 16)
>  #define   EDP_PSR_AUX_CTL_ERROR_INTERRUPT	(1 << 11)
>  #define   EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK	(0x7ff)
>  
> -#define EDP_PSR_AUX_DATA(i)			_MMIO(dev_priv->psr_mmio_base +
> 0x14 + (i) * 4) /* 5 registers */
> +#define _SRD_AUX_DATA_A				0x60814
> +#define _SRD_AUX_DATA_EDP			0x6F814
> +#define EDP_PSR_AUX_DATA(i)			_MMIO(_TRANS2_PSR(_SRD_AUX_DATA_
> A) + (i) + 4) /* 5 registers */
>  
> -#define EDP_PSR_STATUS				_MMIO(dev_priv-
> >psr_mmio_base + 0x40)
> +#define _SRD_STATUS_A				0x60840
> +#define	_SRD_STATUS_EDP				0x6F840
> +#define EDP_PSR_STATUS				_MMIO_TRANS2_PSR(_SRD_ST
> ATUS_A)
>  #define   EDP_PSR_STATUS_STATE_MASK		(7 << 29)
>  #define   EDP_PSR_STATUS_STATE_SHIFT		29
>  #define   EDP_PSR_STATUS_STATE_IDLE		(0 << 29)
> @@ -4279,10 +4292,15 @@ enum {
>  #define   EDP_PSR_STATUS_SENDING_TP1		(1 << 4)
>  #define   EDP_PSR_STATUS_IDLE_MASK		0xf
>  
> -#define EDP_PSR_PERF_CNT		_MMIO(dev_priv->psr_mmio_base + 0x44)
> +#define _SRD_PERF_CNT_A			0x60844
> +#define _SRD_PERF_CNT_EDP		0x6F844
> +#define EDP_PSR_PERF_CNT		_MMIO_TRANS2_PSR(_SRD_PERF_CNT_A)
>  #define   EDP_PSR_PERF_CNT_MASK		0xffffff
>  
> -#define EDP_PSR_DEBUG				_MMIO(dev_priv-
> >psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */
> +/* PSR_MASK on SKL+ */
> +#define _SRD_DEBUG_A				0x60860
> +#define _SRD_DEBUG_EDP				0x6F860
> +#define EDP_PSR_DEBUG				_MMIO_TRANS2_PSR(_SRD_DE
> BUG_A)
>  #define   EDP_PSR_DEBUG_MASK_MAX_SLEEP         (1 << 28)
>  #define   EDP_PSR_DEBUG_MASK_LPSP              (1 << 27)
>  #define   EDP_PSR_DEBUG_MASK_MEMUP             (1 << 26)
> @@ -4290,7 +4308,9 @@ enum {
>  #define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /* Reserved in ICL+
> */
>  #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
>  
> -#define EDP_PSR2_CTL			_MMIO(0x6f900)
> +#define _PSR2_CTL_A			0x60900
> +#define _PSR2_CTL_EDP			0x6F900
> +#define EDP_PSR2_CTL			_MMIO_TRANS2_PSR(_PSR2_CTL_A)
>  #define   EDP_PSR2_ENABLE		(1 << 31)
>  #define   EDP_SU_TRACK_ENABLE		(1 << 30)
>  #define   EDP_Y_COORDINATE_VALID	(1 << 26) /* GLK and CNL+ */
> @@ -4308,7 +4328,9 @@ enum {
>  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
>  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
>  
> -#define PSR_EVENT				_MMIO(0x6F848)
> +#define _PSR_EVENT_A				0x60848
> +#define _PSR_EVENT_EDP				0x6F848
> +#define PSR_EVENT				_MMIO_TRANS2_PSR(_PSR_EVENT_A)
>  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
>  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
>  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> @@ -4326,14 +4348,15 @@ enum {
>  #define  PSR_EVENT_LPSP_MODE_EXIT		(1 << 1)
>  #define  PSR_EVENT_PSR_DISABLE			(1 << 0)
>  
> -#define EDP_PSR2_STATUS			_MMIO(0x6f940)
> +#define _PSR2_STATUS_A			0x60940
> +#define _PSR2_STATUS_EDP		0x6F940
> +#define EDP_PSR2_STATUS			_MMIO_TRANS2_PSR(_PSR2_STATUS_A)
>  #define EDP_PSR2_STATUS_STATE_MASK     (0xf << 28)
>  #define EDP_PSR2_STATUS_STATE_SHIFT    28
>  
> -#define _PSR2_SU_STATUS_0		0x6F914
> -#define _PSR2_SU_STATUS_1		0x6F918
> -#define _PSR2_SU_STATUS_2		0x6F91C
> -#define _PSR2_SU_STATUS(index)		_MMIO(_PICK_EVEN((index),
> _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1))
> +#define _PSR2_SU_STATUS_A		0x60914
> +#define _PSR2_SU_STATUS_EDP		0x6F914
> +#define _PSR2_SU_STATUS(index)		_MMIO(_TRANS2_PSR(_PSR2_SU_STATU
> S_A) + (index) * 4)
>  #define PSR2_SU_STATUS(frame)		(_PSR2_SU_STATUS((frame) / 3))
>  #define PSR2_SU_STATUS_SHIFT(frame)	(((frame) % 3) * 10)
>  #define PSR2_SU_STATUS_MASK(frame)	(0x3ff << PSR2_SU_STATUS_SHIFT(frame))
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index b984e005b72e..ba88464cbbe8 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -668,6 +668,14 @@ static void intel_psr_enable_locked(struct
> drm_i915_private *dev_priv,
>  	dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state);
>  	dev_priv->psr.busy_frontbuffer_bits = 0;
>  	dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
> +	dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
> +
> +	if (IS_HASWELL(dev_priv)) {
> +		u32 trans_offset = INTEL_INFO(dev_priv)->trans_offsets[dev_priv-
> >psr.transcoder];
> +
> +		WARN_ON(trans_offset < HSW_EDP_PSR_BASE);
> +		dev_priv->psr.mmio_base_adjust = trans_offset -
> HSW_EDP_PSR_BASE;
> +	}
>  
>  	DRM_DEBUG_KMS("Enabling PSR%s\n",
>  		      dev_priv->psr.psr2_enabled ? "2" : "1");
> @@ -1132,9 +1140,6 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  	if (!HAS_PSR(dev_priv))
>  		return;
>  
> -	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
> -		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
> -
>  	if (!dev_priv->psr.sink_support)
>  		return;
>  

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

Re: [PATCH 7/7] drm/i915: Make PSR registers relative to transcoders

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 12043 bytes --]

On Fri, 2019-04-05 at 17:55 -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2019-04-03 at 16:35 -0700, José Roberto de Souza wrote:
> > PSR registers are a mess, some have the full address while others
> > just
> > have the additional offset from psr_mmio_base.
> > 
> > psr_mmio_base is nothing more than TRANSCODER_EDP_OFFSET + 0x800
> > and
> > using it makes more difficult for people with an PSR register
> > address
> > from BSpec to search the register name in i915 as also the BSpec
> > name
> > don't match with the name in i915.
> > 
> > The other option would be use the whole hard-coded address but this
> > is
> > not future proof, so here going in the middle ground by making
> > every
> > PSR register relative to transcoder(that is EDP transcoder), the
> > only
> > exception is PSR_IMR/IIR that is not relative to nothing.
> > For the _TRANS2() macros to work it needs the address of the
> > register
> > from the TRANSCODER_A, so adding it to every register together with
> > the register address from the EDP transcoder so it will make easy
> > for
> > people searching with BSpec address also adding those with the
> > BSpec
> > name.
> > 
> > For Haswell all the PSR register are relative to 0x64000, so
> > mmio_base_adjust was added and used to take care of that.
> > 
> > Also removing BDW_EDP_PSR_BASE from GVT because it is not used as
> > the only PSR register that GVT have is this one(SRD/PSR_CTL).
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/handlers.c |  1 -
> >  drivers/gpu/drm/i915/i915_drv.h     |  5 ++-
> >  drivers/gpu/drm/i915/i915_reg.h     | 59 ++++++++++++++++++++-----
> > ----
> >  drivers/gpu/drm/i915/intel_psr.c    | 11 ++++--
> >  4 files changed, 52 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > b/drivers/gpu/drm/i915/gvt/handlers.c
> > index 86761b1def1e..d09b798e93cb 100644
> > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > @@ -2739,7 +2739,6 @@ static int init_broadwell_mmio_info(struct
> > intel_gvt
> > *gvt)
> >  	MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS);
> >  
> >  	MMIO_D(WM_MISC, D_BDW);
> > -	MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW);
> >  
> >  	MMIO_D(_MMIO(0x6671c), D_BDW_PLUS);
> >  	MMIO_D(_MMIO(0x66c00), D_BDW_PLUS);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 8f38d03b1c4e..9ce46a7dabfd 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -501,6 +501,8 @@ struct i915_drrs {
> >  };
> >  
> >  struct i915_psr {
> > +	/* different than zero only on HSW see _TRANS2_PSR() for more
> > info */
> > +	u32 mmio_base_adjust;
> >  	struct mutex lock;
> >  
> >  #define I915_PSR_DEBUG_MODE_MASK	0x0f
> > @@ -515,6 +517,7 @@ struct i915_psr {
> >  	bool enabled;
> >  	struct intel_dp *dp;
> >  	enum pipe pipe;
> > +	enum transcoder transcoder;
> >  	bool active;
> >  	struct work_struct work;
> >  	unsigned busy_frontbuffer_bits;
> > @@ -1541,8 +1544,6 @@ struct drm_i915_private {
> >  	/* MMIO base address for MIPI regs */
> >  	u32 mipi_mmio_base;
> >  
> > -	u32 psr_mmio_base;
> > -
> >  	u32 pps_mmio_base;
> >  
> >  	wait_queue_head_t gmbus_wait_queue;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 18e2991b376d..4df56c118cd2 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -250,9 +250,10 @@ static inline bool
> > i915_mmio_reg_valid(i915_reg_t reg)
> >  #define _MMIO_PIPE2(pipe, reg)		_MMIO(INTEL_INFO(dev_pr
> > iv)-
> > > pipe_offsets[pipe] - \
> >  					      INTEL_INFO(dev_priv)-
> > > pipe_offsets[PIPE_A] + (reg) + \
> >  					      DISPLAY_MMIO_BASE(dev_pri
> > v))
> > -#define _MMIO_TRANS2(pipe, reg)		_MMIO(INTEL_INFO(dev_pr
> > iv)-
> > > trans_offsets[(pipe)] - \
> > -					      INTEL_INFO(dev_priv)-
> > > trans_offsets[TRANSCODER_A] + (reg) + \
> > -					      DISPLAY_MMIO_BASE(dev_pri
> > v))
> > +#define _TRANS2(trans, reg)		(INTEL_INFO(dev_priv)-
> > > trans_offsets[(trans)] - \
> > +					 INTEL_INFO(dev_priv)-
> > > trans_offsets[TRANSCODER_A] + (reg) + \
> > +					 DISPLAY_MMIO_BASE(dev_priv))
> > +#define _MMIO_TRANS2(trans, reg)	_MMIO(_TRANS2(trans, reg))
> >  #define _CURSOR2(pipe, reg)		_MMIO(INTEL_INFO(dev_pr
> > iv)-
> > > cursor_offsets[(pipe)] - \
> >  					      INTEL_INFO(dev_priv)-
> > > cursor_offsets[PIPE_A] + (reg) + \
> >  					      DISPLAY_MMIO_BASE(dev_pri
> > v))
> > @@ -4210,9 +4211,15 @@ enum {
> >  #define PIPE_MULT(trans)	_MMIO_TRANS2(trans, _PIPE_MULT_A)
> >  
> >  /* HSW+ eDP PSR registers */
> > -#define HSW_EDP_PSR_BASE	0x64800
> > -#define BDW_EDP_PSR_BASE	0x6f800
> > -#define EDP_PSR_CTL				_MMIO(dev_priv-
> > >psr_mmio_base +
> > 0)
> > +#define HSW_EDP_PSR_BASE	0x64000
> > +
> > +/* PSR registers on HSW is not relative to eDP transcoder */
> > +#define _TRANS2_PSR(reg)	(_TRANS2(dev_priv->psr.transcoder,
> > (reg)) -
> > dev_priv->psr.mmio_base_adjust)
> 
> The error interrupt registers are still hardcoded for eDP, aren't
> they?

Yes, the PSR_IMR and PSR_IIR have the same hardcoded address in HSW and
other platforms.

> 
> > +#define _MMIO_TRANS2_PSR(reg)	_MMIO(_TRANS2_PSR(reg))
> > +
> > +#define _SRD_CTL_A				0x60800
> > +#define _SRD_CTL_EDP				0x6F800
> > +#define EDP_PSR_CTL				_MMIO_TRANS2_PS
> > R(_SRD_CTL_A)
> Why isn't this accepting transcoder/pipe as an argument? dev_priv-
> > psr.transcoder is hidden two levels below than it should be.
> 

Mostly to avoid more changes, as we only have one psr struct now.
But if in the future we have a platform that can have more than one eDP
panel we will need for sure add a transcoder parameter.

> 
> 
> >  #define   EDP_PSR_ENABLE			(1 << 31)
> >  #define   BDW_PSR_SINGLE_FRAME			(1 << 30)
> >  #define   EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK	(1 << 29) /* SW
> > can't
> > modify */
> > @@ -4245,16 +4252,22 @@ enum {
> >  #define   EDP_PSR_POST_EXIT			(1 << 1)
> >  #define   EDP_PSR_PRE_ENTRY			(1 << 0)
> >  
> > -#define EDP_PSR_AUX_CTL				_MMIO(dev_priv-
> > > psr_mmio_base + 0x10)
> > +#define _SRD_AUX_CTL_A				0x60810
> > +#define _SRD_AUX_CTL_EDP			0x6F810
> > +#define EDP_PSR_AUX_CTL				_MMIO_TRANS2_PS
> > R(_SRD_AU
> > X_CTL_A)
> >  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
> >  #define   EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK	(0x1f << 20)
> >  #define   EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK	(0xf << 16)
> >  #define   EDP_PSR_AUX_CTL_ERROR_INTERRUPT	(1 << 11)
> >  #define   EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK	(0x7ff)
> >  
> > -#define EDP_PSR_AUX_DATA(i)			_MMIO(dev_priv-
> > >psr_mmio_base +
> > 0x14 + (i) * 4) /* 5 registers */
> > +#define _SRD_AUX_DATA_A				0x60814
> > +#define _SRD_AUX_DATA_EDP			0x6F814
> > +#define EDP_PSR_AUX_DATA(i)			_MMIO(_TRANS2_P
> > SR(_SRD_AUX_DATA_
> > A) + (i) + 4) /* 5 registers */
> >  
> > -#define EDP_PSR_STATUS				_MMIO(dev_priv-
> > > psr_mmio_base + 0x40)
> > +#define _SRD_STATUS_A				0x60840
> > +#define	_SRD_STATUS_EDP				0x6F840
> > +#define EDP_PSR_STATUS				_MMIO_TRANS2_PS
> > R(_SRD_ST
> > ATUS_A)
> >  #define   EDP_PSR_STATUS_STATE_MASK		(7 << 29)
> >  #define   EDP_PSR_STATUS_STATE_SHIFT		29
> >  #define   EDP_PSR_STATUS_STATE_IDLE		(0 << 29)
> > @@ -4279,10 +4292,15 @@ enum {
> >  #define   EDP_PSR_STATUS_SENDING_TP1		(1 << 4)
> >  #define   EDP_PSR_STATUS_IDLE_MASK		0xf
> >  
> > -#define EDP_PSR_PERF_CNT		_MMIO(dev_priv->psr_mmio_base +
> > 0x44)
> > +#define _SRD_PERF_CNT_A			0x60844
> > +#define _SRD_PERF_CNT_EDP		0x6F844
> > +#define EDP_PSR_PERF_CNT		_MMIO_TRANS2_PSR(_SRD_PERF_CNT_
> > A)
> >  #define   EDP_PSR_PERF_CNT_MASK		0xffffff
> >  
> > -#define EDP_PSR_DEBUG				_MMIO(dev_priv-
> > > psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */
> > +/* PSR_MASK on SKL+ */
> > +#define _SRD_DEBUG_A				0x60860
> > +#define _SRD_DEBUG_EDP				0x6F860
> > +#define EDP_PSR_DEBUG				_MMIO_TRANS2_PS
> > R(_SRD_DE
> > BUG_A)
> >  #define   EDP_PSR_DEBUG_MASK_MAX_SLEEP         (1 << 28)
> >  #define   EDP_PSR_DEBUG_MASK_LPSP              (1 << 27)
> >  #define   EDP_PSR_DEBUG_MASK_MEMUP             (1 << 26)
> > @@ -4290,7 +4308,9 @@ enum {
> >  #define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /*
> > Reserved in ICL+
> > */
> >  #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+
> > */
> >  
> > -#define EDP_PSR2_CTL			_MMIO(0x6f900)
> > +#define _PSR2_CTL_A			0x60900
> > +#define _PSR2_CTL_EDP			0x6F900
> > +#define EDP_PSR2_CTL			_MMIO_TRANS2_PSR(_PSR2_
> > CTL_A)
> >  #define   EDP_PSR2_ENABLE		(1 << 31)
> >  #define   EDP_SU_TRACK_ENABLE		(1 << 30)
> >  #define   EDP_Y_COORDINATE_VALID	(1 << 26) /* GLK and CNL+ */
> > @@ -4308,7 +4328,9 @@ enum {
> >  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
> >  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
> >  
> > -#define PSR_EVENT				_MMIO(0x6F848)
> > +#define _PSR_EVENT_A				0x60848
> > +#define _PSR_EVENT_EDP				0x6F848
> > +#define PSR_EVENT				_MMIO_TRANS2_PSR(_PSR_E
> > VENT_A)
> >  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
> >  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
> >  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> > @@ -4326,14 +4348,15 @@ enum {
> >  #define  PSR_EVENT_LPSP_MODE_EXIT		(1 << 1)
> >  #define  PSR_EVENT_PSR_DISABLE			(1 << 0)
> >  
> > -#define EDP_PSR2_STATUS			_MMIO(0x6f940)
> > +#define _PSR2_STATUS_A			0x60940
> > +#define _PSR2_STATUS_EDP		0x6F940
> > +#define EDP_PSR2_STATUS			_MMIO_TRANS2_PSR(_PSR2_
> > STATUS_A)
> >  #define EDP_PSR2_STATUS_STATE_MASK     (0xf << 28)
> >  #define EDP_PSR2_STATUS_STATE_SHIFT    28
> >  
> > -#define _PSR2_SU_STATUS_0		0x6F914
> > -#define _PSR2_SU_STATUS_1		0x6F918
> > -#define _PSR2_SU_STATUS_2		0x6F91C
> > -#define _PSR2_SU_STATUS(index)		_MMIO(_PICK_EVEN((index
> > ),
> > _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1))
> > +#define _PSR2_SU_STATUS_A		0x60914
> > +#define _PSR2_SU_STATUS_EDP		0x6F914
> > +#define _PSR2_SU_STATUS(index)		_MMIO(_TRANS2_PSR(_PSR2
> > _SU_STATU
> > S_A) + (index) * 4)
> >  #define PSR2_SU_STATUS(frame)		(_PSR2_SU_STATUS((frame
> > ) / 3))
> >  #define PSR2_SU_STATUS_SHIFT(frame)	(((frame) % 3) * 10)
> >  #define PSR2_SU_STATUS_MASK(frame)	(0x3ff <<
> > PSR2_SU_STATUS_SHIFT(frame))
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index b984e005b72e..ba88464cbbe8 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -668,6 +668,14 @@ static void intel_psr_enable_locked(struct
> > drm_i915_private *dev_priv,
> >  	dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv,
> > crtc_state);
> >  	dev_priv->psr.busy_frontbuffer_bits = 0;
> >  	dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)-
> > >pipe;
> > +	dev_priv->psr.transcoder = crtc_state->cpu_transcoder;
> > +
> > +	if (IS_HASWELL(dev_priv)) {
> > +		u32 trans_offset = INTEL_INFO(dev_priv)-
> > >trans_offsets[dev_priv-
> > > psr.transcoder];
> > +
> > +		WARN_ON(trans_offset < HSW_EDP_PSR_BASE);
> > +		dev_priv->psr.mmio_base_adjust = trans_offset -
> > HSW_EDP_PSR_BASE;
> > +	}
> >  
> >  	DRM_DEBUG_KMS("Enabling PSR%s\n",
> >  		      dev_priv->psr.psr2_enabled ? "2" : "1");
> > @@ -1132,9 +1140,6 @@ void intel_psr_init(struct drm_i915_private
> > *dev_priv)
> >  	if (!HAS_PSR(dev_priv))
> >  		return;
> >  
> > -	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
> > -		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
> > -
> >  	if (!dev_priv->psr.sink_support)
> >  		return;
> >  

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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