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

[Daniel Vetter]

On Thu, Jun 13, 2013 at 10:57:52AM +0300, Ville Syrjälä wrote:
> 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 at 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()?

Cargo-culting at its best ;-) But I think cleaning that up is best left to
another series ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch