On Mon, Nov 26, 2018 at 04:37:09PM -0800, José Roberto de Souza wrote: > EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP() was being set with the number of > frames that it should wait to enter PSR, what is wrong. > Here it is setting this field with the highest value to avoid PSR2 > exits frequently, as when HW exit deep sleep it needs to go to idle > state causing a PSR exit for then waiting a few frames before > activate PSR2 again. > This will result in more power saving as the sleep state also provide > some power savings by doing selective updates instead of full screen > updates. > > About EDP_PSR2_FRAMES_BEFORE_ACTIVATE() it is the number of frames > (not idle frames) that PSR2 hardware will wait to activate PSR2, so > lets keep using the sink sync latency. > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_psr.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index ba7bbe3f8df2..6fd793fec5e9 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -482,13 +482,13 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) > struct i915_psr *psr = &dev_priv->psr; > u32 val; > > - /* Let's use 6 as the minimum to cover all known cases including the > - * off-by-one issue that HW has in some cases. > + /* sink_sync_latency of 8 means source has to wait for more than 8 > + * frames, we'll go with 9 frames for now > */ > - int idle_frames = max(6, dev_priv->vbt.psr.idle_frames); Too many changes in a single patch that I couldn't understand why we are removing the minimal of 6 that was our safe net. > + val = EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1); > > - idle_frames = max(idle_frames, psr->sink_sync_latency + 1); > - val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames); > + /* Avoid deep sleep as much as possible to avoid PSR2 idle state */ > + val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15); > > /* FIXME: selective update is probably totally broken because it doesn't > * mesh at all with our frontbuffer tracking. And the hw alone isn't > @@ -497,8 +497,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) > if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > val |= EDP_Y_COORDINATE_ENABLE; > > - val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1); > - > if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 && > dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50) > val |= EDP_PSR2_TP2_TIME_50us; > -- > 2.19.2 > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
