[Intel-gfx] [PATCH v5 3/3] drm/i915: Make PSR registers relative to transcoders
Dhinakaran Pandiyan
dhinakaran.pandiyan at intel.com
Sat Jun 15 04:27:18 UTC 2019
"drm/i915/psr" in the subject.
On Sat, 2019-04-20 at 13:55 -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.
>
> For BDW+ psr_mmio_base is nothing more than TRANSCODER_EDP_OFFSET +
> 0x800 and using it makes more difficult for people with an PSR
> register address or PSR register name from from BSpec as i915 also
> don't match the BSpec names.
> For HSW psr_mmio_base is _DDI_BUF_CTL_A + 0x800 and PSR registers are
> only available in DDIA.
>
> Other reason to make relative to transcoder is that since BDW every
> transcoder have PSR registers, so in theory it should be possible to
> have PSR enabled in a non-eDP transcoder.
>
> So for BDW+ we can use _TRANS2() to get the register offset of any
> PSR register in any transcoder while for HSW we have _HSW_PSR_ADJ
> that will calculate the register offset for the single PSR instance,
> noting that we are already guarded about trying to enable PSR in other
> port than DDIA on HSW by the 'if (dig_port->base.port != PORT_A)' in
> intel_psr_compute_config(), this check should only be valid for HSW
> and will be changed in future.
> PSR2 registers and PSR_EVENT was added after Haswell so that is why
> _PSR_ADJ() is not used in some macros.
>
> The only registers that can not be relative to transcoder are
> PSR_IMR and PSR_IIR that are not relative to anything, so keeping it
> hardcoded.
>
> Also removing BDW_EDP_PSR_BASE from GVT because it is not used as it
> is the only PSR register that GVT have.
>
> v5:
> - Macros changed to be more explicit about HSW (Dhinakaran)
> - Squashed with the patch that added the tran parameter to the
> macros (Dhinakaran)
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
> drivers/gpu/drm/i915/gvt/handlers.c | 1 -
> drivers/gpu/drm/i915/i915_debugfs.c | 18 +++++----
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/i915_reg.h | 57 ++++++++++++++++++++---------
> drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++------------
> 5 files changed, 83 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index 18f01eeb2510..749e3e4204f2 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -2776,7 +2776,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);
Change this to _SRD_CTL_EDP to keep the change non-functional? Any case, we'll
need an ack to modify this.
> MMIO_D(_MMIO(0x66c00), D_BDW_PLUS);
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5823ffb17821..2a0f5871e9a8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2470,7 +2470,7 @@ psr_source_status(struct drm_i915_private *dev_priv,
> struct seq_file *m)
> "BUF_ON",
> "TG_ON"
> };
> - val = I915_READ(EDP_PSR2_STATUS);
> + val = I915_READ(EDP_PSR2_STATUS(dev_priv->psr.transcoder));
> status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> EDP_PSR2_STATUS_STATE_SHIFT;
> if (status_val < ARRAY_SIZE(live_status))
> @@ -2486,7 +2486,7 @@ psr_source_status(struct drm_i915_private *dev_priv,
> struct seq_file *m)
> "SRDOFFACK",
> "SRDENT_ON",
> };
> - val = I915_READ(EDP_PSR_STATUS);
> + val = I915_READ(EDP_PSR_STATUS(dev_priv->psr.transcoder));
> status_val = (val & EDP_PSR_STATUS_STATE_MASK) >>
> EDP_PSR_STATUS_STATE_SHIFT;
> if (status_val < ARRAY_SIZE(live_status))
> @@ -2529,10 +2529,10 @@ static int i915_edp_psr_status(struct seq_file *m,
> void *data)
> goto unlock;
>
> if (psr->psr2_enabled) {
> - val = I915_READ(EDP_PSR2_CTL);
> + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
> enabled = val & EDP_PSR2_ENABLE;
> } else {
> - val = I915_READ(EDP_PSR_CTL);
> + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder));
> enabled = val & EDP_PSR_ENABLE;
> }
> seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> @@ -2545,7 +2545,8 @@ static int i915_edp_psr_status(struct seq_file *m, void
> *data)
> * SKL+ Perf counter is reset to 0 everytime DC state is entered
> */
> if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> - val = I915_READ(EDP_PSR_PERF_CNT) & EDP_PSR_PERF_CNT_MASK;
> + val = I915_READ(EDP_PSR_PERF_CNT(dev_priv->psr.transcoder));
> + val &= EDP_PSR_PERF_CNT_MASK;
> seq_printf(m, "Performance counter: %u\n", val);
> }
>
> @@ -2563,8 +2564,11 @@ static int i915_edp_psr_status(struct seq_file *m, void
> *data)
> * Reading all 3 registers before hand to minimize crossing a
> * frame boundary between register reads
> */
> - for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += 3)
> - su_frames_val[frame / 3] =
> I915_READ(PSR2_SU_STATUS(frame));
> + for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += 3) {
> + val = I915_READ(PSR2_SU_STATUS(dev_priv->psr.transcoder,
> + frame));
> + su_frames_val[frame / 3] = val;
> + }
>
> seq_puts(m, "Frame:\tPSR2 SU blocks:\n");
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dc74d33c20aa..89f2401e80a4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -508,6 +508,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 +1535,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 31163415479d..572a9f661d60 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4212,10 +4212,19 @@ enum {
> #define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC)
> #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)
> +/*
> + * HSW+ eDP PSR registers
> + *
> + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + 0x800) with just
> one
> + * instance of it
> + */
> +#define _HSW_EDP_PSR_BASE 0x64800
> +#define _SRD_CTL_A 0x60800
/* BDW+ */
> +#define _SRD_CTL_EDP 0x6F800
> +#define _HSW_PSR_ADJ(reg) ((reg) - _SRD_CTL_A +
> _HSW_EDP_PSR_BASE)
> +#define _PSR_ADJ(tran, reg) (IS_HASWELL(dev_priv) ?
> _HSW_PSR_ADJ(reg) : _TRANS2(tran, reg))
> +#define _MMIO_PSR_ADJ(tran, reg) _MMIO(_PSR_ADJ(tran, reg))
nit: We can skip this macro
> +#define EDP_PSR_CTL(tran) _MMIO_PSR_ADJ(tran, _SRD_CTL_A)
#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(tran, _SRD_CTL_A))
It doesn't do much other than adding another layer.
> #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 */
> @@ -4252,16 +4261,22 @@ enum {
> #define EDP_PSR_TRANSCODER_A_SHIFT 8
> #define EDP_PSR_TRANSCODER_EDP_SHIFT 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(tran) _MMIO_PSR_ADJ(tran,
> _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(tran, i) _MMIO(_PSR_ADJ(tran,
> _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
The documentation recommends using lower case hex numbers.
> +#define EDP_PSR_STATUS(tran) _MMIO_PSR_ADJ(tran,
> _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)
> @@ -4286,10 +4301,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(tran) _MMIO_PSR_ADJ(tran,
> _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(tran) _MMIO_PSR_ADJ(tran,
> _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)
> @@ -4297,7 +4317,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(tran) _MMIO_TRANS2(tran, _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+ */
> @@ -4320,7 +4342,7 @@ enum {
> #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(tran) _MMIO_TRANS2(tran,
> _PSR_EVENT_TRANS_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)
> @@ -4338,15 +4360,16 @@ 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(tran) _MMIO_TRANS2(tran,
> _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(frame) (_PSR2_SU_STATUS((frame) / 3))
> +#define _PSR2_SU_STATUS_A 0x60914
> +#define _PSR2_SU_STATUS_EDP 0x6F914
> +#define _PSR2_SU_STATUS(tran, index) _MMIO(_TRANS2(tran, _PSR2_SU_STATUS_A) +
> (index) * 4)
> +#define PSR2_SU_STATUS(tran, frame) (_PSR2_SU_STATUS(tran, (frame) / 3))
> #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10)
> #define PSR2_SU_STATUS_MASK(frame) (0x3ff << PSR2_SU_STATUS_SHIFT(frame))
> #define PSR2_SU_STATUS_FRAMES 8
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 963663ba0edf..fe98041819ec 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
Now that we track the transcoder, intel_psr_irq_control() can be modified
to enable debug interrupts only that transcoder, can be done as a follow up.
> @@ -399,7 +399,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
>
> BUILD_BUG_ON(sizeof(aux_msg) > 20);
> for (i = 0; i < sizeof(aux_msg); i += 4)
> - I915_WRITE(EDP_PSR_AUX_DATA(i >> 2),
> + I915_WRITE(EDP_PSR_AUX_DATA(dev_priv->psr.transcoder, i >> 2),
> intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
>
> aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
> @@ -410,7 +410,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
>
> /* Select only valid bits for SRD_AUX_CTL */
> aux_ctl &= psr_aux_mask;
> - I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl);
> + I915_WRITE(EDP_PSR_AUX_CTL(dev_priv->psr.transcoder), aux_ctl);
> }
>
> static void intel_psr_enable_sink(struct intel_dp *intel_dp)
> @@ -500,8 +500,9 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp)
> if (INTEL_GEN(dev_priv) >= 8)
> val |= EDP_PSR_CRC_ENABLE;
>
> - val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK;
> - I915_WRITE(EDP_PSR_CTL, val);
> + val |= (I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) &
> + EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK);
> + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val);
> }
>
> static void hsw_activate_psr2(struct intel_dp *intel_dp)
> @@ -537,9 +538,9 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
> * 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);
> + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), 0);
>
> - I915_WRITE(EDP_PSR2_CTL, val);
> + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val);
> }
>
> static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> @@ -658,8 +659,8 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>
> if (INTEL_GEN(dev_priv) >= 9)
> - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> + WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)) &
> EDP_PSR2_ENABLE);
> + WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) &
> EDP_PSR_ENABLE);
> WARN_ON(dev_priv->psr.active);
> lockdep_assert_held(&dev_priv->psr.lock);
>
/*
* HSW spec explicitly says PSR is tied to port A.
* BDW+ platforms with DDI implementation of PSR have different
* PSR registers per transcoder and we only implement transcoder EDP
* ones. Since by Display design transcoder EDP is tied to port A
* we can safely escape based on the port A.
*/
if (dig_port->base.port != PORT_A) {
DRM_DEBUG_KMS("PSR condition failed: Port not supported\n");
return;
}
Now that we allow PSR on transcoders that support it, we can change this to
if (IS_HASWELL() && cpu_transcoder != TRANSCODER_EDP)
return;
This can be a follow up work.
> @@ -729,7 +730,7 @@ static void intel_psr_enable_source(struct intel_dp
> *intel_dp,
> if (INTEL_GEN(dev_priv) < 11)
> mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;
>
> - I915_WRITE(EDP_PSR_DEBUG, mask);
> + I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), mask);
> }
>
> static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> @@ -742,6 +743,7 @@ 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;
>
> DRM_DEBUG_KMS("Enabling PSR%s\n",
> dev_priv->psr.psr2_enabled ? "2" : "1");
> @@ -791,20 +793,27 @@ static void intel_psr_exit(struct drm_i915_private
> *dev_priv)
> u32 val;
>
> if (!dev_priv->psr.active) {
> - if (INTEL_GEN(dev_priv) >= 9)
> - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> + if (INTEL_GEN(dev_priv) >= 9) {
> + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
> + WARN_ON(val & EDP_PSR2_ENABLE);
> + }
> +
> + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder));
> + WARN_ON(val & EDP_PSR_ENABLE);
> +
> return;
> }
>
> if (dev_priv->psr.psr2_enabled) {
> - val = I915_READ(EDP_PSR2_CTL);
> + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder));
> WARN_ON(!(val & EDP_PSR2_ENABLE));
> - I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
> + val &= ~EDP_PSR2_ENABLE;
> + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val);
> } else {
> - val = I915_READ(EDP_PSR_CTL);
> + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder));
> WARN_ON(!(val & EDP_PSR_ENABLE));
> - I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
> + val &= ~EDP_PSR_ENABLE;
> + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val);
> }
> dev_priv->psr.active = false;
> }
> @@ -826,10 +835,10 @@ static void intel_psr_disable_locked(struct intel_dp
> *intel_dp)
> intel_psr_exit(dev_priv);
>
> if (dev_priv->psr.psr2_enabled) {
> - psr_status = EDP_PSR2_STATUS;
> + psr_status = EDP_PSR2_STATUS(dev_priv->psr.transcoder);
> psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> } else {
> - psr_status = EDP_PSR_STATUS;
> + psr_status = EDP_PSR_STATUS(dev_priv->psr.transcoder);
> psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> }
>
> @@ -956,7 +965,8 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state
> *new_crtc_state,
> * defensive enough to cover everything.
> */
>
> - return __intel_wait_for_register(&dev_priv->uncore, EDP_PSR_STATUS,
> + return __intel_wait_for_register(&dev_priv->uncore,
> + EDP_PSR_STATUS(dev_priv-
> >psr.transcoder),
> EDP_PSR_STATUS_STATE_MASK,
> EDP_PSR_STATUS_STATE_IDLE, 2, 50,
> out_value);
> @@ -972,10 +982,10 @@ static bool __psr_wait_for_idle_locked(struct
> drm_i915_private *dev_priv)
> return false;
>
> if (dev_priv->psr.psr2_enabled) {
> - reg = EDP_PSR2_STATUS;
> + reg = EDP_PSR2_STATUS(dev_priv->psr.transcoder);
> mask = EDP_PSR2_STATUS_STATE_MASK;
> } else {
> - reg = EDP_PSR_STATUS;
> + reg = EDP_PSR_STATUS(dev_priv->psr.transcoder);
> mask = EDP_PSR_STATUS_STATE_MASK;
> }
>
> @@ -1206,9 +1216,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;
Right below this, we have
val = I915_READ(EDP_PSR_IIR);
val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
if (val) {
DRM_DEBUG_KMS("PSR interruption error set\n");
dev_priv->psr.sink_not_reliable = true;
}
The workaround checks only eDP transcoders, fix it to check all transcoders? We
are still going to be assume only one eDP sink is connected, but it can be
driven by any transcoder.
>
More information about the Intel-gfx
mailing list