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