diff mbox

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

Message ID 1371077699-30702-3-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter June 12, 2013, 10:54 p.m. UTC
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(-)

Comments

Ville Syrjala June 13, 2013, 7:57 a.m. UTC | #1
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
Daniel Vetter June 13, 2013, 10:01 a.m. UTC | #2
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@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
diff mbox

Patch

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));
 }
 
 static bool ironlake_compute_clocks(struct drm_crtc *crtc,