what about the approach suggested by Chris and acked by Jani?
will we simply ignore? or did you discussed with them on irc?
On Tue, Jul 25, 2017 at 9:48 AM, Jim Bride <jim.bride@xxxxxxxxxxxxxxx> 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.
>
> v2: * Do a field-wise init on EDP_PSR_MAX_SLEEP_TIME even though we
> always set it to the max value. (Rodrigo)
> * Rebase
> v3-v4: * Rebase
>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Cc: Wayne Boyer <wayne.boyer@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Signed-off-by: Jim Bride <jim.bride@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 4 ++++
> drivers/gpu/drm/i915/intel_psr.c | 21 +++++++++++++++++++--
> 2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c712d01..3e62429 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3789,18 +3789,22 @@ enum {
> #define EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES (1<<25)
> #define EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES (2<<25)
> #define EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES (3<<25)
> +#define EDP_PSR_MAX_SLEEP_TIME_MASK (0x1f<<20)
> #define EDP_PSR_MAX_SLEEP_TIME_SHIFT 20
> #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 559f1ab..132987b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -280,17 +280,32 @@ 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;
> +
> + val &= ~EDP_PSR_MAX_SLEEP_TIME_MASK;
> 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 +315,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 +325,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;
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
