On Thu, 2018-11-29 at 15:10 -0800, Rodrigo Vivi wrote: > 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. The vbt idle_frames should not be used for PSR2 as PSR2 just wait for X frames(idle or not) to activate PSR2 so just rely in sink sync_latency should be enough but I can bring it back for safety. > > > + 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
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
