Re: [PATCH 2/2] drm/i915: Fix scanline counter fixup on BDW

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Mar 11, 2014 at 12:58:46PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The display interrupts changed on BDW, so the current ILK-HSW specific
> code in ilk_pipe_in_vblank_locked() doesn't work there. Add the required
> bits for BDW, and while at it, change the existing code to use nicer
> looking vblank status bit macros.
> 
> Also remove the now stale __raw_i915_read16() definition which was
> left over from the failed gen2 ISR experiment.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73962
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 30 +++++++++++-------------------
>  1 file changed, 11 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d4c952d..ec9b8a4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -618,33 +618,25 @@ static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe)
>  
>  /* raw reads, only for fast reads of display block, no need for forcewake etc. */

fast reads in context with mmio transactions sounds like a fairly crazy
oxymoron. And our I915_READ functions should already dtrt wrt not doing
the forcewake dance for display registers, otherwise I'll consider it a
bug.

Care to throw a patch on top to remove these guys completely?

Thanks, Daniel

>  #define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs + (reg__))
> -#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)->regs + (reg__))
>  
>  static bool ilk_pipe_in_vblank_locked(struct drm_device *dev, enum pipe pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t status;
> -
> -	if (INTEL_INFO(dev)->gen < 7) {
> -		status = pipe == PIPE_A ?
> -			DE_PIPEA_VBLANK :
> -			DE_PIPEB_VBLANK;
> +	int reg;
> +
> +	if (INTEL_INFO(dev)->gen >= 8) {
> +		status = GEN8_PIPE_VBLANK;
> +		reg = GEN8_DE_PIPE_ISR(pipe);
> +	} else if (INTEL_INFO(dev)->gen >= 7) {
> +		status = DE_PIPE_VBLANK_IVB(pipe);
> +		reg = DEISR;
>  	} else {
> -		switch (pipe) {
> -		default:
> -		case PIPE_A:
> -			status = DE_PIPEA_VBLANK_IVB;
> -			break;
> -		case PIPE_B:
> -			status = DE_PIPEB_VBLANK_IVB;
> -			break;
> -		case PIPE_C:
> -			status = DE_PIPEC_VBLANK_IVB;
> -			break;
> -		}
> +		status = DE_PIPE_VBLANK(pipe);
> +		reg = DEISR;
>  	}
>  
> -	return __raw_i915_read32(dev_priv, DEISR) & status;
> +	return __raw_i915_read32(dev_priv, reg) & status;
>  }
>  
>  static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
> -- 
> 1.8.3.2
> 
> _______________________________________________
> 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