Re: [PATCH] drm/i915: workaround bad DSL readout v2

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: workaround bad DSL readout v2
Date: Tue, 22 Sep 2015 19:41:16 +0300	[thread overview]
Message-ID: <20150922164116.GS26517@intel.com> (raw)
In-Reply-To: <1441921133-3381-1-git-send-email-jbarnes@virtuousgeek.org>

On Thu, Sep 10, 2015 at 02:38:53PM -0700, Jesse Barnes wrote:
> On HSW at least (still testing other platforms, but should be harmless
> elsewhere), the DSL reg reads back as 0 when read around vblank start
> time.  This ends up confusing the atomic start/end checking code, since
> it causes the update to appear as if it crossed a frame count boundary.
> Avoid the problem by making sure we don't return scanline_offset from
> the get_crtc_scanline function.  In moving the code there, I add to add
> an additional delay since it could be called and have a legitimate 0
> result for some time (depending on the pixel clock).
> 
> v2: move hsw dsl read hack to get_crtc_scanline (Ville)
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=91579
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 90bc6c2..97e5d52 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -697,6 +697,27 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
>  		position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) & DSL_LINEMASK_GEN3;
>  
>  	/*
> +	 * On HSW, the DSL reg (0x70000) appears to return 0 if we
> +	 * read it right around the start of vblank. So try it again

Make that "just before the start of vblank". Otherwise I'm sure I'll
forget the details and start to think again about using ISR tricks.

> +	 * so we don't accidentally end up spanning a vblank frame
> +	 * increment, causing the pipe_update_end() code to squak at us.
> +	 */
> +	if (IS_HASWELL(dev) && !position) {
> +		int i, temp;
> +
> +		for (i = 0; i < 100; i++) {
> +			udelay(1);
> +			temp = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) &
> +				DSL_LINEMASK_GEN3;
> +			if (temp != position) {
> +				position = temp;
> +				goto out;

break is enough

A bit nasty this loop with irqs off, but it's the only way we can avoid
corrupted vblank timestamps from happening when the hardware is on the
fritz.

With the comment updatead and goto killed:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +			}
> +		}
> +	}
> +
> +out:
> +	/*
>  	 * See update_scanline_offset() for the details on the
>  	 * scanline_offset adjustment.
>  	 */
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx