From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com ([134.134.136.31]:33785 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751680AbdEQJuU (ORCPT ); Wed, 17 May 2017 05:50:20 -0400 Message-ID: <1495014573.6603.76.camel@intel.com> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Workaround VLV/CHV DSI scanline counter hardware fail From: Mika Kahola Reply-To: mika.kahola@intel.com To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org Cc: Jonas Aaberg , stable@vger.kernel.org Date: Wed, 17 May 2017 12:49:33 +0300 In-Reply-To: <20161215174734.28779-1-ville.syrjala@linux.intel.com> References: <20161215174734.28779-1-ville.syrjala@linux.intel.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: The patch does what it says. Tested-by: Mika Kahola Reviewed-by: Mika Kahola On Thu, 2016-12-15 at 19:47 +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä > > The scanline counter is bonkers on VLV/CHV DSI. The scanline counter > increment is not lined up with the start of vblank like it is on > every other platform and output type. This causes problems for > both the vblank timestamping and atomic update vblank evasion. > > On my FFRD8 machine at least, the scanline counter increment > happens about 1/3 of a scanline ahead of the start of vblank (which > is where all register latching happens still). That means we can't > trust the scanline counter to tell us whether we're in vblank or not > while we're on that particular line. In order to keep vblank > timestamping in working condition when called from the vblank irq, > we'll leave scanline_offset at one, which means that the entire > line containing the start of vblank is considered to be inside > the vblank. > > For the vblank evasion we'll need to consider that entire line > to be bad, since we can't tell whether the registers already > got latched or not. And we can't actually use the start of vblank > interrupt to get us past that line as the interrupt would fire > too soon, and then we'd up waiting for the next start of vblank > instead. One way around that would using the frame start > interrupt instead since that wouldn't fire until the next > scanline, but that would require some bigger changes in the > interrupt code. So for simplicity we'll just poll until we get > past the bad line. > > v2: Adjust the comments a bit > > Cc: stable@vger.kernel.org > Cc: Jonas Aaberg > Tested-by: Jonas Aaberg > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99086 > Signed-off-by: Ville Syrjälä > --- >  drivers/gpu/drm/i915/intel_display.c |  9 +++++++++ >  drivers/gpu/drm/i915/intel_sprite.c  | 21 +++++++++++++++++++++ >  2 files changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 9cc5dbfc314b..159b2eb9766b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13852,6 +13852,15 @@ static void update_scanline_offset(struct > intel_crtc *crtc) >    * type. For DP ports it behaves like most other platforms, > but on HDMI >    * there's an extra 1 line difference. So we need to add two > instead of >    * one to the value. > +  * > +  * On VLV/CHV DSI the scanline counter would appear to > increment > +  * approx. 1/3 of a scanline before start of vblank. > Unfortunately > +  * that means we can't tell whether we're in vblank or not > while > +  * we're on that particular line. We must still set > scanline_offset > +  * to 1 so that the vblank timestamps come out correct when > we query > +  * the scanline counter from within the vblank interrupt > handler. > +  * However if queried just before the start of vblank we'll > get an > +  * answer that's slightly in the future. >    */ >   if (IS_GEN2(dev_priv)) { >   const struct drm_display_mode *adjusted_mode = > &crtc->config->base.adjusted_mode; > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > b/drivers/gpu/drm/i915/intel_sprite.c > index 7031bc733d97..fb0e0d8e893a 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -81,10 +81,13 @@ int intel_usecs_to_scanlines(const struct > drm_display_mode *adjusted_mode, >   */ >  void intel_pipe_update_start(struct intel_crtc *crtc) >  { > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >   const struct drm_display_mode *adjusted_mode = &crtc- > >config->base.adjusted_mode; >   long timeout = msecs_to_jiffies_timeout(1); >   int scanline, min, max, vblank_start; >   wait_queue_head_t *wq = drm_crtc_vblank_waitqueue(&crtc- > >base); > + bool need_vlv_dsi_wa = (IS_VALLEYVIEW(dev_priv) || > IS_CHERRYVIEW(dev_priv)) && > + intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DSI); >   DEFINE_WAIT(wait); >   >   vblank_start = adjusted_mode->crtc_vblank_start; > @@ -136,6 +139,24 @@ void intel_pipe_update_start(struct intel_crtc > *crtc) >   >   drm_crtc_vblank_put(&crtc->base); >   > + /* > +  * On VLV/CHV DSI the scanline counter would appear to > +  * increment approx. 1/3 of a scanline before start of > vblank. > +  * The registers still get latched at start of vblank > however. > +  * This means we must not write any registers on the first > +  * line of vblank (since not the whole line is actually in > +  * vblank). And unfortunately we can't use the interrupt to > +  * wait here since it will fire too soon. We could use the > +  * frame start interrupt instead since it will fire after > the > +  * critical scanline, but that would require more changes > +  * in the interrupt code. So for now we'll just do the nasty > +  * thing and poll for the bad scanline to pass us by. > +  * > +  * FIXME figure out if BXT+ DSI suffers from this as well > +  */ > + while (need_vlv_dsi_wa && scanline == vblank_start) > + scanline = intel_get_crtc_scanline(crtc); > + >   crtc->debug.scanline_start = scanline; >   crtc->debug.start_vbl_time = ktime_get(); >   crtc->debug.start_vbl_count = > intel_crtc_get_vblank_counter(crtc); -- Mika Kahola - Intel OTC