From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Souza, Jose" Subject: Re: [PATCH 8/9] drm/i915/psr: Set the right frames values Date: Fri, 30 Nov 2018 00:48:09 +0000 Message-ID: <052a8fe64c912ab5ecef1f477b73bf2bb0262b89.camel@intel.com> References: <20181127003710.18618-1-jose.souza@intel.com> <20181127003710.18618-8-jose.souza@intel.com> <20181129231057.GT8630@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1677863386==" Return-path: In-Reply-To: <20181129231057.GT8630@intel.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: "Vivi, Rodrigo" Cc: "intel-gfx@lists.freedesktop.org" , "Pandiyan, Dhinakaran" , "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org --===============1677863386== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-JGi8nxEyO9snJyZwEh0E" --=-JGi8nxEyO9snJyZwEh0E Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2018-11-29 at 15:10 -0800, Rodrigo Vivi wrote: > On Mon, Nov 26, 2018 at 04:37:09PM -0800, Jos=C3=A9 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. > >=20 > > 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. > >=20 > > Cc: Rodrigo Vivi > > Cc: Dhinakaran Pandiyan > > Signed-off-by: Jos=C3=A9 Roberto de Souza > > --- > > drivers/gpu/drm/i915/intel_psr.c | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > >=20 > > 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 =3D &dev_priv->psr; > > u32 val; > > =20 > > - /* 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 =3D max(6, dev_priv->vbt.psr.idle_frames); >=20 > 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. >=20 > > + val =3D EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + > > 1); > > =20 > > - idle_frames =3D max(idle_frames, psr->sink_sync_latency + 1); > > - val =3D EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames); > > + /* Avoid deep sleep as much as possible to avoid PSR2 idle > > state */ > > + val |=3D EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15); > > =20 > > /* 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) >=3D 10 || IS_GEMINILAKE(dev_priv)) > > val |=3D EDP_Y_COORDINATE_ENABLE; > > =20 > > - val |=3D EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + > > 1); > > - > > if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >=3D 0 && > > dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <=3D 50) > > val |=3D EDP_PSR2_TP2_TIME_50us; > > --=20 > > 2.19.2 > >=20 > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel --=-JGi8nxEyO9snJyZwEh0E Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEVNG051EijGa0MiaQVenbO/mOWkkFAlwAiMcACgkQVenbO/mO WkmOowf9HtrvZ9OQAp3hUFa3bUTgwXnOVGhLz/L/aqRq9lzn25cjEV53sr59umD0 9URdvlfckT0QUWaprOaTABrcWDt4jdVMABsrjLxEWjH40Zfqr9/u9birEK2RRTru HuFgXrqy2rAAn+v2alKabw+p/jFQSBCnv3BRozKuebEReTnaWRps96jOLhBx0skr TlvuKMr/pxapNvdWex+3u9YOndfnK9+F8H1ENz1OlrgoJ7GcgZBdu4ht2hSmDT38 pRl1fAQpXSjYTxovHuuAucvThfm8+1tSg633SGr+Z3a7dDmsX3b3n4d+7QV+VrI6 9jmPMWUjTwcjBiCBVSGiArJ4FWM23g== =xivo -----END PGP SIGNATURE----- --=-JGi8nxEyO9snJyZwEh0E-- --===============1677863386== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1677863386==--