On Wed, Sep 07, 2016 at 05:42:31PM -0700, Rodrigo Vivi wrote: > This reverts commit 1c80c25fb622973dd135878e98d172be20859049. > > There are panels that needs 4 idle frames before entering PSR, > but VBT is unproperly set. > > Also lately it was identified that idle frame count calculated at HW > can be off by 1, what makes the minimum of 2, at least. > > Without the current vbt+1 we are with the risk of having HW calculating > 0 idle frames and entering PSR when it shouldn't. Regardless the lack > of link training. > > Cc: Dominik Brodowski <li...@dominikbrodowski.net> > Cc: Jani Nikula <jani.nik...@intel.com> > Cc: Daniel Vetter <daniel.vet...@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com> > --- > drivers/gpu/drm/i915/intel_psr.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index 59a21c9..108ba1e 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -255,14 +255,14 @@ static void hsw_psr_enable_source(struct intel_dp > *intel_dp) > struct drm_i915_private *dev_priv = to_i915(dev); > > uint32_t max_sleep_time = 0x1f; > - /* Lately it was identified that depending on panel idle frame count > - * calculated at HW can be off by 1. So let's use what came > - * from VBT + 1. > - * There are also other cases where panel demands at least 4 > - * but VBT is not being set. To cover these 2 cases lets use > - * at least 5 when VBT isn't set to be on the safest side. > + /* > + * Let's respect VBT in case VBT asks a higher idle_frame value. > + * Let's use 6 as the minimum to cover all known cases including > + * the off-by-one issue that HW has in some cases. Also there are > + * cases where sink should be able to train > + * with the 5 or 6 idle patterns. > */ > - uint32_t idle_frames = dev_priv->vbt.psr.idle_frames + 1; > + uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
I don't really understand this logic compared to the explanations. Shouldn't we do something like 'idle_frames = max(4, psr.idle_frames) + 1;'? > uint32_t val = EDP_PSR_ENABLE; > > val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
