Re: [PATCH 3/3] drm/i915: explicitly set up PIPECONF (and gamma table) on haswell

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jun 13, 2013 at 12:54:59AM +0200, Daniel Vetter wrote:
> Again we don't really support different settings, so don't let the
> BIOS sneak stuff through.
> 
> Since the motivation for this patch series is to ensure we have the
> correct gamma table mode selected also add the required write to the
> GAMMA_MODE register to select the 8bit legacy table.
> 
> And since I find lowercase letters in #defines offensive, also
> bikeshed those.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 6 +++---
>  drivers/gpu/drm/i915/intel_display.c | 7 ++++---
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 01e8783..8136b00 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3670,9 +3670,9 @@
>  #define _GAMMA_MODE_B		0x4ac80
>  #define GAMMA_MODE(pipe) _PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
>  #define GAMMA_MODE_MODE_MASK	(3 << 0)
> -#define GAMMA_MODE_MODE_8bit	(0 << 0)
> -#define GAMMA_MODE_MODE_10bit	(1 << 0)
> -#define GAMMA_MODE_MODE_12bit	(2 << 0)
> +#define GAMMA_MODE_MODE_8BIT	(0 << 0)
> +#define GAMMA_MODE_MODE_10BIT	(1 << 0)
> +#define GAMMA_MODE_MODE_12BIT	(2 << 0)
>  #define GAMMA_MODE_MODE_SPLIT	(3 << 0)
>  
>  /* interrupts */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4ca0273..e1184eb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5393,13 +5393,11 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
>  	enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
>  	uint32_t val;
>  
> -	val = I915_READ(PIPECONF(cpu_transcoder));
> +	val = 0;
>  
> -	val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
>  	if (intel_crtc->config.dither)
>  		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>  
> -	val &= ~PIPECONF_INTERLACE_MASK_HSW;
>  	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
>  		val |= PIPECONF_INTERLACED_ILK;
>  	else
> @@ -5407,6 +5405,9 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
>  
>  	I915_WRITE(PIPECONF(cpu_transcoder), val);
>  	POSTING_READ(PIPECONF(cpu_transcoder));
> +
> +	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
> +	POSTING_READ(GAMMA_MODE(intel_crtc->pipe));

Why the POSTING_READ()? In fact, why do we have any posting reads in
xxx_set_pipeconf()?

Otherwise, for the series:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  }
>  
>  static bool ironlake_compute_clocks(struct drm_crtc *crtc,
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC