On Mon, 2017-04-03 at 10:07 -0700, Jim Bride wrote:
> On SKL+ there is a bit in SRD_CTL that software is not supposed to
> modify, but we currently clobber that bit when we enable PSR. In
> order to preserve the value of that bit, go ahead and read SRD_CTL and
> do a field-wise setting of the various bits that we need to initialize
> before writing the register back out. Additionally, go ahead and
> explicitly disable single-frame update since we aren't currently
> supporting it.
>
> Cc: Rodrigo Vivi <[email protected]>
> Cc: Wayne Boyer <[email protected]>
>
> Signed-off-by: Jim Bride <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> drivers/gpu/drm/i915/intel_psr.c | 23 +++++++++++++++++++++--
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 11b12f4..54d39e4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3590,14 +3590,17 @@ enum {
> #define EDP_PSR_SKIP_AUX_EXIT (1<<12)
> #define EDP_PSR_TP1_TP2_SEL (0<<11)
> #define EDP_PSR_TP1_TP3_SEL (1<<11)
> +#define EDP_PSR_TP2_TP3_TIME_MASK (3<<8)
> #define EDP_PSR_TP2_TP3_TIME_500us (0<<8)
> #define EDP_PSR_TP2_TP3_TIME_100us (1<<8)
> #define EDP_PSR_TP2_TP3_TIME_2500us (2<<8)
> #define EDP_PSR_TP2_TP3_TIME_0us (3<<8)
> +#define EDP_PSR_TP1_TIME_MASK (0x3<<4)
> #define EDP_PSR_TP1_TIME_500us (0<<4)
> #define EDP_PSR_TP1_TIME_100us (1<<4)
> #define EDP_PSR_TP1_TIME_2500us (2<<4)
> #define EDP_PSR_TP1_TIME_0us (3<<4)
> +#define EDP_PSR_IDLE_FRAME_MASK (0xf<<0)
> #define EDP_PSR_IDLE_FRAME_SHIFT 0
>
> #define EDP_PSR_AUX_CTL
> _MMIO(dev_priv->psr_mmio_base + 0x10)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index c3780d0..a050859 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -280,17 +280,34 @@ static void intel_enable_source_psr1(struct intel_dp
> *intel_dp)
> * with the 5 or 6 idle patterns.
> */
> uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
> - uint32_t val = EDP_PSR_ENABLE;
> + uint32_t val = I915_READ(EDP_PSR_CTL);
>
> + val |= EDP_PSR_ENABLE;
> +
> + /* We always set the max sleep time to the maximum value, so
> + * no need to zero out the field first.
> + */
I believe it is better to zero out instead of adding a comment.
So we could play with max_sleep_time if needed.
Otherwise we shouldn't allow the flexible value here so we should create
a define EDP_PSR_MAX_SLEEP_TIME (0x1f << 20)
and here do a val |= EDP_PSR_MAX_SLEEP_TIME;
> val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
> +
> + val &= ~EDP_PSR_IDLE_FRAME_MASK;
> val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>
> + val &= ~EDP_PSR_MIN_LINK_ENTRY_TIME_MASK;
> if (IS_HASWELL(dev_priv))
> val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>
> - if (dev_priv->psr.link_standby)
> + if (dev_priv->psr.link_standby) {
> val |= EDP_PSR_LINK_STANDBY;
>
> + /* SFU should only be enabled with link standby, but for
> + * now we do not support it. */
> + val &= ~BDW_PSR_SINGLE_FRAME;
> + } else {
> + val &= ~EDP_PSR_LINK_STANDBY;
> + val &= ~BDW_PSR_SINGLE_FRAME;
> + }
> +
> + val &= ~EDP_PSR_TP1_TIME_MASK;
> if (dev_priv->vbt.psr.tp1_wakeup_time > 5)
> val |= EDP_PSR_TP1_TIME_2500us;
> else if (dev_priv->vbt.psr.tp1_wakeup_time > 1)
> @@ -300,6 +317,7 @@ static void intel_enable_source_psr1(struct intel_dp
> *intel_dp)
> else
> val |= EDP_PSR_TP1_TIME_0us;
>
> + val &= ~EDP_PSR_TP2_TP3_TIME_MASK;
> if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
> val |= EDP_PSR_TP2_TP3_TIME_2500us;
> else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1)
> @@ -309,6 +327,7 @@ static void intel_enable_source_psr1(struct intel_dp
> *intel_dp)
> else
> val |= EDP_PSR_TP2_TP3_TIME_0us;
>
> + val &= ~EDP_PSR_TP1_TP3_SEL;
> if (intel_dp_source_supports_hbr2(intel_dp) &&
> drm_dp_tps3_supported(intel_dp->dpcd))
> val |= EDP_PSR_TP1_TP3_SEL;
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
