drm/i915/psr: Fix PSR_IMR/IIR field handling

Current PSR code is supposed to use TRANSCODER_EDP to force 0 shift for
bits in PSR_IMR/IIR registers:

/*
 * gen12+ has registers relative to transcoder and one per transcoder
 * using the same bit definition: handle it as TRANSCODER_EDP to force
 * 0 shift in bit definition
 */

At the time of writing the code assumption "TRANSCODER_EDP == 0" was made.
This is not the case and all fields in PSR_IMR and PSR_IIR are shifted
incorrectly if DISPLAY_VER >= 12.

Fix this by adding separate register field defines for >=12 and add bit
getter functions to keep code readability.

v4:
 - Remove EDP from TGL definitions (José)
 - Use REG_BIT and REG_GENMASK (José)
v3:
 - Add separate register field defines (José)
 - Add bit getter functions (José)
v2:
 - Improve commit message (José)

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Fixes: 8241cfbe67f4 ("drm/i915/tgl: Access the right register when handling PSR interruptions")
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221003072011.72408-1-jouni.hogander@intel.com

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 9def8d9fade6b0b6921a12271988d59a5d123dd5..d4cce627d7a87f4b51575f1a45994f699130e019 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -116,34 +116,56 @@ static bool psr2_global_enabled(struct intel_dp *intel_dp)
        }
 }
 
+static u32 psr_irq_psr_error_bit_get(struct intel_dp *intel_dp)
+{
+       struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+       return DISPLAY_VER(dev_priv) >= 12 ? TGL_PSR_ERROR :
+               EDP_PSR_ERROR(intel_dp->psr.transcoder);
+}
+
+static u32 psr_irq_post_exit_bit_get(struct intel_dp *intel_dp)
+{
+       struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+       return DISPLAY_VER(dev_priv) >= 12 ? TGL_PSR_POST_EXIT :
+               EDP_PSR_POST_EXIT(intel_dp->psr.transcoder);
+}
+
+static u32 psr_irq_pre_entry_bit_get(struct intel_dp *intel_dp)
+{
+       struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+       return DISPLAY_VER(dev_priv) >= 12 ? TGL_PSR_PRE_ENTRY :
+               EDP_PSR_PRE_ENTRY(intel_dp->psr.transcoder);
+}
+
+static u32 psr_irq_mask_get(struct intel_dp *intel_dp)
+{
+       struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+       return DISPLAY_VER(dev_priv) >= 12 ? TGL_PSR_MASK :
+               EDP_PSR_MASK(intel_dp->psr.transcoder);
+}
+
 static void psr_irq_control(struct intel_dp *intel_dp)
 {
        struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-       enum transcoder trans_shift;
        i915_reg_t imr_reg;
        u32 mask, val;
 
-       /*
-        * gen12+ has registers relative to transcoder and one per transcoder
-        * using the same bit definition: handle it as TRANSCODER_EDP to force
-        * 0 shift in bit definition
-        */
-       if (DISPLAY_VER(dev_priv) >= 12) {
-               trans_shift = 0;
+       if (DISPLAY_VER(dev_priv) >= 12)
                imr_reg = TRANS_PSR_IMR(intel_dp->psr.transcoder);
-       } else {
-               trans_shift = intel_dp->psr.transcoder;
+       else
                imr_reg = EDP_PSR_IMR;
-       }
 
-       mask = EDP_PSR_ERROR(trans_shift);
+       mask = psr_irq_psr_error_bit_get(intel_dp);
        if (intel_dp->psr.debug & I915_PSR_DEBUG_IRQ)
-               mask |= EDP_PSR_POST_EXIT(trans_shift) |
-                       EDP_PSR_PRE_ENTRY(trans_shift);
+               mask |= psr_irq_post_exit_bit_get(intel_dp) |
+                       psr_irq_pre_entry_bit_get(intel_dp);
 
-       /* Warning: it is masking/setting reserved bits too */
        val = intel_de_read(dev_priv, imr_reg);
-       val &= ~EDP_PSR_TRANS_MASK(trans_shift);
+       val &= ~psr_irq_mask_get(intel_dp);
        val |= ~mask;
        intel_de_write(dev_priv, imr_reg, val);
 }
@@ -191,25 +213,21 @@ void intel_psr_irq_handler(struct intel_dp *intel_dp, u32 psr_iir)
        enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
        struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
        ktime_t time_ns =  ktime_get();
-       enum transcoder trans_shift;
        i915_reg_t imr_reg;
 
-       if (DISPLAY_VER(dev_priv) >= 12) {
-               trans_shift = 0;
+       if (DISPLAY_VER(dev_priv) >= 12)
                imr_reg = TRANS_PSR_IMR(intel_dp->psr.transcoder);
-       } else {
-               trans_shift = intel_dp->psr.transcoder;
+       else
                imr_reg = EDP_PSR_IMR;
-       }
 
-       if (psr_iir & EDP_PSR_PRE_ENTRY(trans_shift)) {
+       if (psr_iir & psr_irq_pre_entry_bit_get(intel_dp)) {
                intel_dp->psr.last_entry_attempt = time_ns;
                drm_dbg_kms(&dev_priv->drm,
                            "[transcoder %s] PSR entry attempt in 2 vblanks\n",
                            transcoder_name(cpu_transcoder));
        }
 
-       if (psr_iir & EDP_PSR_POST_EXIT(trans_shift)) {
+       if (psr_iir & psr_irq_post_exit_bit_get(intel_dp)) {
                intel_dp->psr.last_exit = time_ns;
                drm_dbg_kms(&dev_priv->drm,
                            "[transcoder %s] PSR exit completed\n",
@@ -226,7 +244,7 @@ void intel_psr_irq_handler(struct intel_dp *intel_dp, u32 psr_iir)
                }
        }
 
-       if (psr_iir & EDP_PSR_ERROR(trans_shift)) {
+       if (psr_iir & psr_irq_psr_error_bit_get(intel_dp)) {
                u32 val;
 
                drm_warn(&dev_priv->drm, "[transcoder %s] PSR aux error\n",
@@ -243,7 +261,7 @@ void intel_psr_irq_handler(struct intel_dp *intel_dp, u32 psr_iir)
                 * or unset irq_aux_error.
                 */
                val = intel_de_read(dev_priv, imr_reg);
-               val |= EDP_PSR_ERROR(trans_shift);
+               val |= psr_irq_psr_error_bit_get(intel_dp);
                intel_de_write(dev_priv, imr_reg, val);
 
                schedule_work(&intel_dp->psr.work);
@@ -1194,14 +1212,12 @@ static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
         * first time that PSR HW tries to activate so lets keep PSR disabled
         * to avoid any rendering problems.
         */
-       if (DISPLAY_VER(dev_priv) >= 12) {
+       if (DISPLAY_VER(dev_priv) >= 12)
                val = intel_de_read(dev_priv,
                                    TRANS_PSR_IIR(intel_dp->psr.transcoder));
-               val &= EDP_PSR_ERROR(0);
-       } else {
+       else
                val = intel_de_read(dev_priv, EDP_PSR_IIR);
-               val &= EDP_PSR_ERROR(intel_dp->psr.transcoder);
-       }
+       val &= psr_irq_psr_error_bit_get(intel_dp);
        if (val) {
                intel_dp->psr.sink_not_reliable = true;
                drm_dbg_kms(&dev_priv->drm,

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a4f0129c3f6f7e1ec50325c8fdfcd96fc7aa801e..e43bb0a911e91e789c2064cde9bc1dc681ecaa41 100644 (file)
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2157,10 +2157,18 @@
 #define TRANS_PSR_IIR(tran)                    _MMIO_TRANS2(tran, _PSR_IIR_A)
 #define   _EDP_PSR_TRANS_SHIFT(trans)          ((trans) == TRANSCODER_EDP ? \
                                                 0 : ((trans) - TRANSCODER_A + 1) * 8)
-#define   EDP_PSR_TRANS_MASK(trans)            (0x7 << _EDP_PSR_TRANS_SHIFT(trans))
-#define   EDP_PSR_ERROR(trans)                 (0x4 << _EDP_PSR_TRANS_SHIFT(trans))
-#define   EDP_PSR_POST_EXIT(trans)             (0x2 << _EDP_PSR_TRANS_SHIFT(trans))
-#define   EDP_PSR_PRE_ENTRY(trans)             (0x1 << _EDP_PSR_TRANS_SHIFT(trans))
+#define   TGL_PSR_MASK                 REG_GENMASK(2, 0)
+#define   TGL_PSR_ERROR                        REG_BIT(2)
+#define   TGL_PSR_POST_EXIT            REG_BIT(1)
+#define   TGL_PSR_PRE_ENTRY            REG_BIT(0)
+#define   EDP_PSR_MASK(trans)          (TGL_PSR_MASK <<                \
+                                        _EDP_PSR_TRANS_SHIFT(trans))
+#define   EDP_PSR_ERROR(trans)         (TGL_PSR_ERROR <<               \
+                                        _EDP_PSR_TRANS_SHIFT(trans))
+#define   EDP_PSR_POST_EXIT(trans)     (TGL_PSR_POST_EXIT <<           \
+                                        _EDP_PSR_TRANS_SHIFT(trans))
+#define   EDP_PSR_PRE_ENTRY(trans)     (TGL_PSR_PRE_ENTRY <<           \
+                                        _EDP_PSR_TRANS_SHIFT(trans))
 
 #define _SRD_AUX_DATA_A                                0x60814
 #define _SRD_AUX_DATA_EDP                      0x6f814