From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Souza, Jose" Subject: Re: [PATCH v4 8/9] drm/i915/psr: Set idle frames to maximum while getting pipe CRC Date: Mon, 4 Mar 2019 20:56:25 +0000 Message-ID: <5ae203b528cf8887e2a882d760bdf8d81cb13514.camel@intel.com> References: <20190302013456.24138-1-jose.souza@intel.com> <20190302013456.24138-8-jose.souza@intel.com> <139defa77975684317225ff22799831f21b59036.camel@intel.com> <04eb0cd26a9820e2109457c6b0a353742fb7ae1e.camel@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1124344984==" Return-path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3D4C989DD8 for ; Mon, 4 Mar 2019 20:56:28 +0000 (UTC) In-Reply-To: <04eb0cd26a9820e2109457c6b0a353742fb7ae1e.camel@intel.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "intel-gfx@lists.freedesktop.org" , "Pandiyan, Dhinakaran" List-Id: intel-gfx@lists.freedesktop.org --===============1124344984== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-7fhSDcxPgIw4QMMlwWqe" --=-7fhSDcxPgIw4QMMlwWqe Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2019-03-04 at 11:00 -0800, Dhinakaran Pandiyan wrote: > On Mon, 2019-03-04 at 10:40 -0800, Souza, Jose wrote: > > On Mon, 2019-03-04 at 10:31 -0800, Dhinakaran Pandiyan wrote: > > > On Fri, 2019-03-01 at 17:34 -0800, Jos=C3=A9 Roberto de Souza wrote: > > > > Increase the idle frames to activate PSR1 to avoid CRC > > > > timeouts, > > > > as > > > > soon as pipe CRC is enabled it will avoid PSR1 to activate but > > > > if > > > > PSR1 is activate before that, hardware goes to lower power > > > > states > > > > that inhibits CRC calculations causing CRC timeout errors in > > > > IGT > > > > tests. >=20 > Okay, so you are solving the case where PSR is already active when we > want CRCs. How does modifying the idle frame count help if PSR is > already active? Exactly this sporadicts timeout should be caused by this: - IGT test modify frontbuffer or do a flip - IGT execution is preempted and it takes more than X idle frames - PSR is activated - IGT test start CRC capture - CRC timeout So increasing the idle frames should reduce the scenario above. >=20 > The commit also says PSR does not become active if CRCs were already > enabled (idle count should not matter here). In that case, just > disabling and re-enabling should be good, isn't it? Or perhaps, just > doing a psr_flush() would do the same job? Good suggestion, psr_flush() should do the same job I will test and change to that. >=20 > > > > Cc: Dhinakaran Pandiyan > > > > Cc: Ville Syrj=C3=A4l=C3=A4 > > > > Signed-off-by: Jos=C3=A9 Roberto de Souza > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > > > drivers/gpu/drm/i915/intel_psr.c | 17 +++++++++++++++-- > > > > 2 files changed, 16 insertions(+), 2 deletions(-) > > > >=20 > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > > b/drivers/gpu/drm/i915/i915_drv.h > > > > index 453af7438e67..e336f758e481 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > > @@ -521,6 +521,7 @@ struct i915_psr { > > > > bool sink_not_reliable; > > > > bool irq_aux_error; > > > > u16 su_x_granularity; > > > > + bool crc_enabled; > > > > }; > > > > =20 > > > > enum intel_pch { > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > > > b/drivers/gpu/drm/i915/intel_psr.c > > > > index d3e3996551c6..b237d96db277 100644 > > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > > @@ -452,6 +452,16 @@ static void hsw_activate_psr1(struct > > > > intel_dp > > > > *intel_dp) > > > > * frames, we'll go with 9 frames for now > > > > */ > > > > idle_frames =3D max(idle_frames, dev_priv- > > > > >psr.sink_sync_latency > > > > + 1); > > > > + > > > > + /* > > > > + * Increase the idle frames to active PSR1 to avoid CRC > > > > timeouts, as > > > > + * soon as pipe CRC is enabled it will avoid PSR1 to > > > > activate > > > > but if > > > > + * PSR1 is activate before that, hardware goes to lower > > > > power > > > > states > > > > + * that inhibits CRC calculations. > > > > + */ > > > > + if (dev_priv->psr.crc_enabled) > > > > + idle_frames =3D 0xf; > > >=20 > > > My understanding was only the enable bit can be changed when PSR > > > is > > > enabled. Does this work? > >=20 > > That is true, the additional changes in intel_psr_update() will > > disable > > and then enable PSR with this new idle_frames. > >=20 > > > > + > > > > val |=3D idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; --=-7fhSDcxPgIw4QMMlwWqe 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/mOWkkFAlx9kPcACgkQVenbO/mO Wkl/tgf+JOhm3YmVUHK58C7jUIPe+Lr71HjzpnqkAbPxLZGCBLBX6ah6Tg68tHxm kB1/7X3wbdSw74WAjx4F1alpy3zxBDv5ewmEpQ92q74ybLfudP2uSSzGB/bWq303 voxs4Vsd/sRbImzPjMfba2IPwujqiuDuc/Jv960kcVy2nqchGuwhdv06KJ3vvOiw FWyu7Z3jkBQgpzCXKwKqolNt6iRTV3YjDRiKXTvV9AN6AAz0W2CvEbNatLcl+i3t /t5LRPSgsCnE8FtbgocosvqzlAPeikn5IaW2Wn2YPQdzj5oq/r/UVT3fD9Mi0aYR to7dgfID+4c8Um+eZRQnChBk2f/RsA== =CYqD -----END PGP SIGNATURE----- --=-7fhSDcxPgIw4QMMlwWqe-- --===============1124344984== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4 --===============1124344984==--