The patch does what it says.

Tested-by: Mika Kahola <mika.kah...@intel.com>
Reviewed-by: Mika Kahola <mika.kah...@intel.com>

On Thu, 2016-12-15 at 19:47 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> 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: sta...@vger.kernel.org
> Cc: Jonas Aaberg <c...@gmx.net>
> Tested-by: Jonas Aaberg <c...@gmx.net>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99086
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
>  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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to