From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/4] drm/i915: Increase PSR Idle Frame to 2. Date: Thu, 4 Sep 2014 13:04:06 +0200 Message-ID: <20140904110406.GH15520@phenom.ffwll.local> References: <1409798999-1809-1-git-send-email-rodrigo.vivi@intel.com> <20140904075516.GB4193@intel.com> <20140904092916.GF15520@phenom.ffwll.local> <20140904100427.GD4193@intel.com> <20140904101819.GE4193@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wg0-f41.google.com (mail-wg0-f41.google.com [74.125.82.41]) by gabe.freedesktop.org (Postfix) with ESMTP id C6ECD6E284 for ; Thu, 4 Sep 2014 04:03:44 -0700 (PDT) Received: by mail-wg0-f41.google.com with SMTP id l18so10054689wgh.24 for ; Thu, 04 Sep 2014 04:03:43 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140904101819.GE4193@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org, Rodrigo Vivi List-Id: intel-gfx@lists.freedesktop.org On Thu, Sep 04, 2014 at 01:18:19PM +0300, Ville Syrj=E4l=E4 wrote: > On Thu, Sep 04, 2014 at 01:04:27PM +0300, Ville Syrj=E4l=E4 wrote: > > On Thu, Sep 04, 2014 at 11:29:16AM +0200, Daniel Vetter wrote: > > > On Thu, Sep 04, 2014 at 10:55:16AM +0300, Ville Syrj=E4l=E4 wrote: > > > > On Wed, Sep 03, 2014 at 10:49:56PM -0400, Rodrigo Vivi wrote: > > > > > With Software tracking we are going to PSR sooner than we should = and staying > > > > > with blank screens in many cases. > > > > > = > > > > > Using 2 identical frames to detect idleness is safier. > > > > = > > > > This idle frame detection still depends of FBC right? > > > > = > > > > I believe if we want to go for full sw tracking on HSW/BDW we need = to > > > > use the debug register to force PSR entry/exit. > > > = > > > Currently the sw tracking relies upon 1 additional full upload happen= ing > > > after the flush, which hopefully should magically happen if we have j= ust 1 > > > idle frame. > > > = > > > If we'd completely switch to sw tracking we'd need to set up a vblank > > > worker to disable psr after the next vblank, which would comlicate the > > > code I think. > > = > > vlv/chv have no hw tracking so if the current sw tracking can't deal > > with that then it would seem to need more work. > = > Hmm. Actually they seem to have a hw timer mode where we can program the > number of idle frames. I think idle here means "since the last plane > register frobbing" as there's no real modification tracking ala. FBC. > So maybe it can work roughly the same way as HSW in that regard. Essentially the primitive the current code needs (modulo bugs, which seem to still be) is to "upload one more full frame, then enter psr". If a lot of platforms can't do that themselves I guess we could wrap some helpers for them. But if there's some real sw tracking bug still, and Rodrigo's patch looks like this is still the case, we need to fix that ofc. -Daniel > = > > = > > > -Daniel > > > = > > > > = > > > > > = > > > > > Discovered and validated with refactored igt/kms_sink_psr_crc. > > > > > = > > > > > Signed-off-by: Rodrigo Vivi > > > > > --- > > > > > drivers/gpu/drm/i915/intel_dp.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > = > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i9= 15/intel_dp.c > > > > > index f79473b..a796831 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > > @@ -1813,7 +1813,7 @@ static void intel_edp_psr_enable_source(str= uct intel_dp *intel_dp) > > > > > struct drm_device *dev =3D dig_port->base.base.dev; > > > > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > > > > uint32_t max_sleep_time =3D 0x1f; > > > > > - uint32_t idle_frames =3D 1; > > > > > + uint32_t idle_frames =3D 2; > > > > > uint32_t val =3D 0x0; > > > > > const uint32_t link_entry_time =3D EDP_PSR_MIN_LINK_ENTRY_TIME_= 8_LINES; > > > > > bool only_standby =3D false; > > > > > -- = > > > > > 1.9.3 > > > > > = > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > = > > > > -- = > > > > Ville Syrj=E4l=E4 > > > > Intel OTC > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > = > > > -- = > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > = > > -- = > > Ville Syrj=E4l=E4 > > Intel OTC > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > = > -- = > Ville Syrj=E4l=E4 > Intel OTC -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch