From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [Intel-gfx] [PATCH 17/19] drm/i915: Clear .last vblank count before drm_vblank_off() when sanitizing crtc state Date: Wed, 6 Aug 2014 15:30:17 +0200 Message-ID: <20140806133017.GL8727@phenom.ffwll.local> References: <1407325803-6944-1-git-send-email-ville.syrjala@linux.intel.com> <1407325803-6944-18-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wi0-f173.google.com (mail-wi0-f173.google.com [209.85.212.173]) by gabe.freedesktop.org (Postfix) with ESMTP id 7B1666E671 for ; Wed, 6 Aug 2014 06:30:06 -0700 (PDT) Received: by mail-wi0-f173.google.com with SMTP id f8so8816632wiw.12 for ; Wed, 06 Aug 2014 06:30:05 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1407325803-6944-18-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Wed, Aug 06, 2014 at 02:50:00PM +0300, ville.syrjala@linux.intel.com wro= te: > From: Ville Syrj=E4l=E4 > = > We call drm_vblank_off() during crtc sanitation to make sure the > software and hardware states agree. However drm_vblank_off() will > try to update the vblank timestamp and sequence number which lands > us in some trouble. > = > As the pipe is disabled the hardware frame counter query will > return 0. If the .last doesn't match the code will try to add the > difference to the user visible sequence number. During driver init > that's OK as .last =3D=3D 0, but during resume .last may be anything. > So we should make sure we don't try to apply the diff here by zeroing > .last beforehand. Otherwise there maybe be a random jump in the user > visible sequence number across suspend. > = > Signed-off-by: Ville Syrj=E4l=E4 > --- > drivers/gpu/drm/i915/intel_display.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > = > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index ae5f20d..c00bcd0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12918,8 +12918,11 @@ static void intel_sanitize_crtc(struct intel_crt= c *crtc) > /* restore vblank interrupts to correct state */ > if (crtc->active) > drm_vblank_on(dev, crtc->pipe); > - else > + else { > + /* avoid random jumps in seq/ts */ > + dev->vblank[crtc->pipe].last =3D 0; Should we have a drm_vblank_reset for this? I guess other drivers will have similar issues with "sorry, just lost it all" kind of transitions. Also this case should only happen when we enter the system resume with the pipes in dpms off state. In that case we should have sampled something in drm_vblank_off already and resampling doesn't look like a good idea. Do we instead need some protection in drm_vblank_off to avoid re-sampling? -Daniel > drm_vblank_off(dev, crtc->pipe); > + } > = > /* We need to sanitize the plane -> pipe mapping first because this will > * disable the crtc (and hence change the state) if it is wrong. Note > -- = > 1.8.5.5 > = > _______________________________________________ > 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