All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
@ 2019-04-12 22:29 José Roberto de Souza
  2019-04-12 22:29 ` [PATCH v2 2/3] drm/i915/psr: Remove partial PSR support on multiple transcoders José Roberto de Souza
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: José Roberto de Souza @ 2019-04-12 22:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

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

v2:
- Reading misc display IRQ outside of gen8_de_misc_irq_handler() as
other irq handlers (Dhinakaran)

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 45 ++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d934545445e1..820d89300454 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2740,6 +2740,28 @@ static u32 gen8_de_port_aux_mask(struct drm_i915_private *dev_priv)
 	return mask;
 }
 
+static void
+gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32 iir)
+{
+	bool found = false;
+
+	if (iir & GEN8_DE_MISC_GSE) {
+		intel_opregion_asle_intr(dev_priv);
+		found = true;
+	}
+
+	if (iir & GEN8_DE_EDP_PSR) {
+		u32 psr_iir = I915_READ(EDP_PSR_IIR);
+
+		intel_psr_irq_handler(dev_priv, psr_iir);
+		I915_WRITE(EDP_PSR_IIR, psr_iir);
+		found = true;
+	}
+
+	if (!found)
+		DRM_ERROR("Unexpected DE Misc interrupt\n");
+}
+
 static irqreturn_t
 gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 {
@@ -2750,29 +2772,12 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 	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 & GEN8_DE_MISC_GSE) {
-				intel_opregion_asle_intr(dev_priv);
-				found = true;
-			}
-
-			if (iir & GEN8_DE_EDP_PSR) {
-				u32 psr_iir = I915_READ(EDP_PSR_IIR);
-
-				intel_psr_irq_handler(dev_priv, psr_iir);
-				I915_WRITE(EDP_PSR_IIR, psr_iir);
-				found = true;
-			}
-
-			if (!found)
-				DRM_ERROR("Unexpected DE Misc interrupt\n");
-		}
-		else
+			gen8_de_misc_irq_handler(dev_priv, iir);
+		} else {
 			DRM_ERROR("The master control interrupt lied (DE MISC)!\n");
+		}
 	}
 
 	if (INTEL_GEN(dev_priv) >= 11 && (master_ctl & GEN11_DE_HPD_IRQ)) {
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] drm/i915/psr: Remove partial PSR support on multiple transcoders
  2019-04-12 22:29 [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
@ 2019-04-12 22:29 ` José Roberto de Souza
  2019-04-12 22:42   ` Rodrigo Vivi
  2019-04-12 22:29 ` [PATCH v2 3/3] drm/i915: Make PSR registers relative to transcoders José Roberto de Souza
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: José Roberto de Souza @ 2019-04-12 22:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

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.

v2:
- Commit message updated (Rodrigo and Dhinakaran)

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 c1c0f7ab03e9..e2803b120b6d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4243,13 +4243,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)
@@ -4314,12 +4310,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 963663ba0edf..581774748c4c 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -87,46 +87,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);
 }
@@ -170,62 +136,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);
+		}
 	}
 }
 
@@ -672,30 +623,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+
@@ -706,13 +637,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);
 	}
 
 	/*
@@ -1225,7 +1154,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] drm/i915: Make PSR registers relative to transcoders
  2019-04-12 22:29 [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
  2019-04-12 22:29 ` [PATCH v2 2/3] drm/i915/psr: Remove partial PSR support on multiple transcoders José Roberto de Souza
@ 2019-04-12 22:29 ` José Roberto de Souza
  2019-04-12 22:45   ` Rodrigo Vivi
  2019-04-12 22:40 ` [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function Rodrigo Vivi
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: José Roberto de Souza @ 2019-04-12 22:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

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 35d0782c077e..a9666290f0b8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -494,6 +494,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
@@ -508,6 +510,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;
@@ -1534,8 +1537,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 e2803b120b6d..094bd19abb35 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))
@@ -4212,9 +4213,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 */
@@ -4247,16 +4254,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)
@@ -4281,10 +4294,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)
@@ -4292,7 +4310,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+ */
@@ -4310,7 +4330,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)
@@ -4328,14 +4350,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 581774748c4c..4e3d74d1b227 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -671,6 +671,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");
@@ -1135,9 +1143,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
  2019-04-12 22:29 [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
  2019-04-12 22:29 ` [PATCH v2 2/3] drm/i915/psr: Remove partial PSR support on multiple transcoders José Roberto de Souza
  2019-04-12 22:29 ` [PATCH v2 3/3] drm/i915: Make PSR registers relative to transcoders José Roberto de Souza
@ 2019-04-12 22:40 ` Rodrigo Vivi
  2019-04-12 22:45   ` Souza, Jose
  2019-04-12 23:24 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] " Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Rodrigo Vivi @ 2019-04-12 22:40 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan

On Fri, Apr 12, 2019 at 03:29:07PM -0700, José Roberto de Souza wrote:
> Just moving it to reduce the tabs and avoid break code lines.
> No behavior changes intended here.

this function is indeed big and deserves a split.
I wonder why haven't you moved the entire de_misc block to a separated
function instead only the middle of the handling...

i mean, including reg read above and error message below.

> 
> v2:
> - Reading misc display IRQ outside of gen8_de_misc_irq_handler() as
> other irq handlers (Dhinakaran)
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 45 ++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d934545445e1..820d89300454 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2740,6 +2740,28 @@ static u32 gen8_de_port_aux_mask(struct drm_i915_private *dev_priv)
>  	return mask;
>  }
>  
> +static void
> +gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32 iir)
> +{
> +	bool found = false;
> +
> +	if (iir & GEN8_DE_MISC_GSE) {
> +		intel_opregion_asle_intr(dev_priv);
> +		found = true;
> +	}
> +
> +	if (iir & GEN8_DE_EDP_PSR) {
> +		u32 psr_iir = I915_READ(EDP_PSR_IIR);
> +
> +		intel_psr_irq_handler(dev_priv, psr_iir);
> +		I915_WRITE(EDP_PSR_IIR, psr_iir);
> +		found = true;
> +	}
> +
> +	if (!found)
> +		DRM_ERROR("Unexpected DE Misc interrupt\n");
> +}
> +
>  static irqreturn_t
>  gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  {
> @@ -2750,29 +2772,12 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  	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 & GEN8_DE_MISC_GSE) {
> -				intel_opregion_asle_intr(dev_priv);
> -				found = true;
> -			}
> -
> -			if (iir & GEN8_DE_EDP_PSR) {
> -				u32 psr_iir = I915_READ(EDP_PSR_IIR);
> -
> -				intel_psr_irq_handler(dev_priv, psr_iir);
> -				I915_WRITE(EDP_PSR_IIR, psr_iir);
> -				found = true;
> -			}
> -
> -			if (!found)
> -				DRM_ERROR("Unexpected DE Misc interrupt\n");
> -		}
> -		else
> +			gen8_de_misc_irq_handler(dev_priv, iir);
> +		} else {
>  			DRM_ERROR("The master control interrupt lied (DE MISC)!\n");
> +		}
>  	}
>  
>  	if (INTEL_GEN(dev_priv) >= 11 && (master_ctl & GEN11_DE_HPD_IRQ)) {
> -- 
> 2.21.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] drm/i915/psr: Remove partial PSR support on multiple transcoders
  2019-04-12 22:29 ` [PATCH v2 2/3] drm/i915/psr: Remove partial PSR support on multiple transcoders José Roberto de Souza
@ 2019-04-12 22:42   ` Rodrigo Vivi
  0 siblings, 0 replies; 11+ messages in thread
From: Rodrigo Vivi @ 2019-04-12 22:42 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan

On Fri, Apr 12, 2019 at 03:29:08PM -0700, José Roberto de Souza wrote:
> 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.
> 
> v2:
> - Commit message updated (Rodrigo and Dhinakaran)

nice code clean-up and thanks for updating the message.

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  |  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 c1c0f7ab03e9..e2803b120b6d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4243,13 +4243,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)
> @@ -4314,12 +4310,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 963663ba0edf..581774748c4c 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -87,46 +87,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);
>  }
> @@ -170,62 +136,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);
> +		}
>  	}
>  }
>  
> @@ -672,30 +623,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+
> @@ -706,13 +637,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);
>  	}
>  
>  	/*
> @@ -1225,7 +1154,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
  2019-04-12 22:40 ` [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function Rodrigo Vivi
@ 2019-04-12 22:45   ` Souza, Jose
  2019-04-12 22:53     ` Rodrigo Vivi
  0 siblings, 1 reply; 11+ messages in thread
From: Souza, Jose @ 2019-04-12 22:45 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx@lists.freedesktop.org, Pandiyan, Dhinakaran


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

On Fri, 2019-04-12 at 15:40 -0700, Rodrigo Vivi wrote:
> On Fri, Apr 12, 2019 at 03:29:07PM -0700, José Roberto de Souza
> wrote:
> > Just moving it to reduce the tabs and avoid break code lines.
> > No behavior changes intended here.
> 
> this function is indeed big and deserves a split.
> I wonder why haven't you moved the entire de_misc block to a
> separated
> function instead only the middle of the handling...
> 
> i mean, including reg read above and error message below.

It was reading and writing GEN8_DE_MISC_IIR inside of
gen8_de_misc_irq_handler() but to keep consistent with the other
handlers Dhinakaran suggested this change.

> 
> > v2:
> > - Reading misc display IRQ outside of gen8_de_misc_irq_handler() as
> > other irq handlers (Dhinakaran)
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 45 ++++++++++++++++++-----------
> > ----
> >  1 file changed, 25 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index d934545445e1..820d89300454 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2740,6 +2740,28 @@ static u32 gen8_de_port_aux_mask(struct
> > drm_i915_private *dev_priv)
> >  	return mask;
> >  }
> >  
> > +static void
> > +gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32
> > iir)
> > +{
> > +	bool found = false;
> > +
> > +	if (iir & GEN8_DE_MISC_GSE) {
> > +		intel_opregion_asle_intr(dev_priv);
> > +		found = true;
> > +	}
> > +
> > +	if (iir & GEN8_DE_EDP_PSR) {
> > +		u32 psr_iir = I915_READ(EDP_PSR_IIR);
> > +
> > +		intel_psr_irq_handler(dev_priv, psr_iir);
> > +		I915_WRITE(EDP_PSR_IIR, psr_iir);
> > +		found = true;
> > +	}
> > +
> > +	if (!found)
> > +		DRM_ERROR("Unexpected DE Misc interrupt\n");
> > +}
> > +
> >  static irqreturn_t
> >  gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32
> > master_ctl)
> >  {
> > @@ -2750,29 +2772,12 @@ gen8_de_irq_handler(struct drm_i915_private
> > *dev_priv, u32 master_ctl)
> >  	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 & GEN8_DE_MISC_GSE) {
> > -				intel_opregion_asle_intr(dev_priv);
> > -				found = true;
> > -			}
> > -
> > -			if (iir & GEN8_DE_EDP_PSR) {
> > -				u32 psr_iir = I915_READ(EDP_PSR_IIR);
> > -
> > -				intel_psr_irq_handler(dev_priv,
> > psr_iir);
> > -				I915_WRITE(EDP_PSR_IIR, psr_iir);
> > -				found = true;
> > -			}
> > -
> > -			if (!found)
> > -				DRM_ERROR("Unexpected DE Misc
> > interrupt\n");
> > -		}
> > -		else
> > +			gen8_de_misc_irq_handler(dev_priv, iir);
> > +		} else {
> >  			DRM_ERROR("The master control interrupt lied
> > (DE MISC)!\n");
> > +		}
> >  	}
> >  
> >  	if (INTEL_GEN(dev_priv) >= 11 && (master_ctl &
> > GEN11_DE_HPD_IRQ)) {
> > -- 
> > 2.21.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] drm/i915: Make PSR registers relative to transcoders
  2019-04-12 22:29 ` [PATCH v2 3/3] drm/i915: Make PSR registers relative to transcoders José Roberto de Souza
@ 2019-04-12 22:45   ` Rodrigo Vivi
  0 siblings, 0 replies; 11+ messages in thread
From: Rodrigo Vivi @ 2019-04-12 22:45 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan

On Fri, Apr 12, 2019 at 03:29:09PM -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 35d0782c077e..a9666290f0b8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -494,6 +494,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
> @@ -508,6 +510,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;
> @@ -1534,8 +1537,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 e2803b120b6d..094bd19abb35 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))

this chunk deserves a separated patch...

>  #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))
> @@ -4212,9 +4213,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))

and this is the part that I'm not entirely sure that it is a good
approach.

> +
> +#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 */
> @@ -4247,16 +4254,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)
> @@ -4281,10 +4294,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)
> @@ -4292,7 +4310,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+ */
> @@ -4310,7 +4330,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)
> @@ -4328,14 +4350,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 581774748c4c..4e3d74d1b227 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -671,6 +671,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");
> @@ -1135,9 +1143,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
  2019-04-12 22:45   ` Souza, Jose
@ 2019-04-12 22:53     ` Rodrigo Vivi
  0 siblings, 0 replies; 11+ messages in thread
From: Rodrigo Vivi @ 2019-04-12 22:53 UTC (permalink / raw)
  To: Souza, Jose, Paulo Zanoni
  Cc: intel-gfx@lists.freedesktop.org, Pandiyan, Dhinakaran

On Fri, Apr 12, 2019 at 10:45:16PM +0000, Souza, Jose wrote:
> On Fri, 2019-04-12 at 15:40 -0700, Rodrigo Vivi wrote:
> > On Fri, Apr 12, 2019 at 03:29:07PM -0700, José Roberto de Souza
> > wrote:
> > > Just moving it to reduce the tabs and avoid break code lines.
> > > No behavior changes intended here.
> > 
> > this function is indeed big and deserves a split.
> > I wonder why haven't you moved the entire de_misc block to a
> > separated
> > function instead only the middle of the handling...
> > 
> > i mean, including reg read above and error message below.
> 
> It was reading and writing GEN8_DE_MISC_IIR inside of
> gen8_de_misc_irq_handler() but to keep consistent with the other
> handlers Dhinakaran suggested this change.

oh, I think I prefer your v1 instead... and for consistency we
change the rest later hehe

but I won't block... we need to start to clean-up this function
from somewhere...

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
(preferred for v1, but feel free to use on this)

I hope this doesn't conflict with the reworks around irc Paulo is doing.
his patches is on my todo list for review.

Cc: Paulo

> 
> > 
> > > v2:
> > > - Reading misc display IRQ outside of gen8_de_misc_irq_handler() as
> > > other irq handlers (Dhinakaran)
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_irq.c | 45 ++++++++++++++++++-----------
> > > ----
> > >  1 file changed, 25 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > b/drivers/gpu/drm/i915/i915_irq.c
> > > index d934545445e1..820d89300454 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -2740,6 +2740,28 @@ static u32 gen8_de_port_aux_mask(struct
> > > drm_i915_private *dev_priv)
> > >  	return mask;
> > >  }
> > >  
> > > +static void
> > > +gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32
> > > iir)
> > > +{
> > > +	bool found = false;
> > > +
> > > +	if (iir & GEN8_DE_MISC_GSE) {
> > > +		intel_opregion_asle_intr(dev_priv);
> > > +		found = true;
> > > +	}
> > > +
> > > +	if (iir & GEN8_DE_EDP_PSR) {
> > > +		u32 psr_iir = I915_READ(EDP_PSR_IIR);
> > > +
> > > +		intel_psr_irq_handler(dev_priv, psr_iir);
> > > +		I915_WRITE(EDP_PSR_IIR, psr_iir);
> > > +		found = true;
> > > +	}
> > > +
> > > +	if (!found)
> > > +		DRM_ERROR("Unexpected DE Misc interrupt\n");
> > > +}
> > > +
> > >  static irqreturn_t
> > >  gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32
> > > master_ctl)
> > >  {
> > > @@ -2750,29 +2772,12 @@ gen8_de_irq_handler(struct drm_i915_private
> > > *dev_priv, u32 master_ctl)
> > >  	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 & GEN8_DE_MISC_GSE) {
> > > -				intel_opregion_asle_intr(dev_priv);
> > > -				found = true;
> > > -			}
> > > -
> > > -			if (iir & GEN8_DE_EDP_PSR) {
> > > -				u32 psr_iir = I915_READ(EDP_PSR_IIR);
> > > -
> > > -				intel_psr_irq_handler(dev_priv,
> > > psr_iir);
> > > -				I915_WRITE(EDP_PSR_IIR, psr_iir);
> > > -				found = true;
> > > -			}
> > > -
> > > -			if (!found)
> > > -				DRM_ERROR("Unexpected DE Misc
> > > interrupt\n");
> > > -		}
> > > -		else
> > > +			gen8_de_misc_irq_handler(dev_priv, iir);
> > > +		} else {
> > >  			DRM_ERROR("The master control interrupt lied
> > > (DE MISC)!\n");
> > > +		}
> > >  	}
> > >  
> > >  	if (INTEL_GEN(dev_priv) >= 11 && (master_ctl &
> > > GEN11_DE_HPD_IRQ)) {
> > > -- 
> > > 2.21.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
  2019-04-12 22:29 [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
                   ` (2 preceding siblings ...)
  2019-04-12 22:40 ` [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function Rodrigo Vivi
@ 2019-04-12 23:24 ` Patchwork
  2019-04-12 23:26 ` ✗ Fi.CI.SPARSE: " Patchwork
  2019-04-12 23:53 ` ✗ Fi.CI.BAT: failure " Patchwork
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-04-12 23:24 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
URL   : https://patchwork.freedesktop.org/series/59424/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5f0a6bbb4f8c drm/i915/bdw+: Move misc display IRQ handling to it own function
970cbdfd1e8d drm/i915/psr: Remove partial PSR support on multiple transcoders
0b8f4172319e 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:4219:
+#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:4268:
+#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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
  2019-04-12 22:29 [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
                   ` (3 preceding siblings ...)
  2019-04-12 23:24 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] " Patchwork
@ 2019-04-12 23:26 ` Patchwork
  2019-04-12 23:53 ` ✗ Fi.CI.BAT: failure " Patchwork
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-04-12 23:26 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
URL   : https://patchwork.freedesktop.org/series/59424/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
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:3616:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3617:16: warning: expression using sizeof(void)

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [v2,1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
  2019-04-12 22:29 [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
                   ` (4 preceding siblings ...)
  2019-04-12 23:26 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-04-12 23:53 ` Patchwork
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-04-12 23:53 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function
URL   : https://patchwork.freedesktop.org/series/59424/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5926 -> Patchwork_12788
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12788 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12788, 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/59424/revisions/1/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      PASS -> DMESG-FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-kbl-7500u:       PASS -> DMESG-WARN [fdo#105128] / [fdo#107139]

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

  * igt@kms_force_connector_basic@force-edid:
    - fi-glk-dsi:         NOTRUN -> SKIP [fdo#109271] +26

  * igt@kms_frontbuffer_tracking@basic:
    - fi-glk-dsi:         NOTRUN -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-glk-dsi:         NOTRUN -> FAIL [fdo#103191]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191]
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

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

  
#### Possible fixes ####

  * igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size:
    - fi-glk-dsi:         INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

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

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

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#105128]: https://bugs.freedesktop.org/show_bug.cgi?id=105128
  [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  Missing    (6): fi-kbl-soraka fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-icl-y fi-bdw-samus 


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

    * Linux: CI_DRM_5926 -> Patchwork_12788

  CI_DRM_5926: 2ab8e3b23618f04e84a03ecb53685e14cd2a5346 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4945: a52cc643cfe6733465cfc9ccb3d21cbdc4fd7506 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12788: 0b8f4172319e670fba37f215d2576b6b65b8643c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0b8f4172319e drm/i915: Make PSR registers relative to transcoders
970cbdfd1e8d drm/i915/psr: Remove partial PSR support on multiple transcoders
5f0a6bbb4f8c drm/i915/bdw+: Move misc display IRQ handling to it own function

== Logs ==

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-04-12 23:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-12 22:29 [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function José Roberto de Souza
2019-04-12 22:29 ` [PATCH v2 2/3] drm/i915/psr: Remove partial PSR support on multiple transcoders José Roberto de Souza
2019-04-12 22:42   ` Rodrigo Vivi
2019-04-12 22:29 ` [PATCH v2 3/3] drm/i915: Make PSR registers relative to transcoders José Roberto de Souza
2019-04-12 22:45   ` Rodrigo Vivi
2019-04-12 22:40 ` [PATCH v2 1/3] drm/i915/bdw+: Move misc display IRQ handling to it own function Rodrigo Vivi
2019-04-12 22:45   ` Souza, Jose
2019-04-12 22:53     ` Rodrigo Vivi
2019-04-12 23:24 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] " Patchwork
2019-04-12 23:26 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-04-12 23:53 ` ✗ Fi.CI.BAT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.