Re: [PATCH 02/14] drm/i915: DSL_LINEMASK is 12 bits only on gen2

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, May 04, 2012 at 05:18:14PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Gen3+ is 13 bits (12:0), and on gen2 only 12 (11:0). For both the high
> bits are marked reserved, read-only so continue to mask them. Bit 31
> is not reserved and has a meaning.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Queued for -next, thanks for the patch. While reading through the code
I've noticed that there are other places where we get this wrong. In the
crt load detect code we don't even bother with properly masking, and in
the precise vblank timestamp code we always use the gen3+ mask. That code
in addition doesn't properly handle the lack of the PIPEDSL register on
ilk+. Can I volunteer you to look into that?

Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |    3 ++-
>  drivers/gpu/drm/i915/intel_display.c |   11 ++++++++---
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7bc407a..8da0b40 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2475,7 +2475,8 @@
>  
>  /* Pipe A */
>  #define _PIPEADSL		0x70000
> -#define   DSL_LINEMASK		0x00000fff
> +#define   DSL_LINEMASK_GEN2	0x00000fff
> +#define   DSL_LINEMASK_GEN3	0x00001fff
>  #define _PIPEACONF		0x70008
>  #define   PIPECONF_ENABLE	(1<<31)
>  #define   PIPECONF_DISABLE	0
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e1716be..613f871 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -871,15 +871,20 @@ void intel_wait_for_pipe_off(struct drm_device *dev, int pipe)
>  			     100))
>  			DRM_DEBUG_KMS("pipe_off wait timed out\n");
>  	} else {
> -		u32 last_line;
> +		u32 last_line, line_mask;
>  		int reg = PIPEDSL(pipe);
>  		unsigned long timeout = jiffies + msecs_to_jiffies(100);
>  
> +		if (IS_GEN2(dev))
> +			line_mask = DSL_LINEMASK_GEN2;
> +		else
> +			line_mask = DSL_LINEMASK_GEN3;
> +
>  		/* Wait for the display line to settle */
>  		do {
> -			last_line = I915_READ(reg) & DSL_LINEMASK;
> +			last_line = I915_READ(reg) & line_mask;
>  			mdelay(5);
> -		} while (((I915_READ(reg) & DSL_LINEMASK) != last_line) &&
> +		} while (((I915_READ(reg) & line_mask) != last_line) &&
>  			 time_after(timeout, jiffies));
>  		if (time_after(jiffies, timeout))
>  			DRM_DEBUG_KMS("pipe_off wait timed out\n");
> -- 
> 1.7.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48