[PATCH 0/3] drm/i915: VLV color range stuff

[PATCH 0/3] drm/i915: VLV color range stuff

[ville.syrjala]

This is an attempt to make the RGB output color range selection work on VLV.

I also decided to refactor the <=gen4 PIPECONF setup code, so that it looks a
bit more like the the ILK/HSW codepaths.

So far this is only compile tested.

[PATCH 1/3] drm/i915: Introduce i9xx_set_pipeconf()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Extract the PIPECONF setup into i9xx_set_pipeconf(). This makes the
<=Gen4/VLV code follow the same pattern as the Gen5+ codepaths.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++----------------
 1 file changed, 68 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 298bc0c..dfaea15 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4514,6 +4514,71 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
 		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
 }
 
+static void i9xx_set_pipeconf(struct drm_crtc *crtc,
+			      const struct drm_display_mode *mode,
+			      const struct drm_display_mode *adjusted_mode,
+			      bool is_dp)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
+	u32 pipeconf;
+
+	/* setup pipeconf */
+	pipeconf = I915_READ(PIPECONF(pipe));
+
+	if (pipe == 0 && INTEL_INFO(dev)->gen < 4) {
+		/* Enable pixel doubling when the dot clock is > 90% of the (display)
+		 * core speed.
+		 *
+		 * XXX: No double-wide on 915GM pipe B. Is that the only reason for the
+		 * pipe == 0 check?
+		 */
+		if (mode->clock >
+		    dev_priv->display.get_display_clock_speed(dev) * 9 / 10)
+			pipeconf |= PIPECONF_DOUBLE_WIDE;
+		else
+			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
+	}
+
+	/* default to 8bpc */
+	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
+	if (is_dp && intel_crtc->config.dither) {
+		pipeconf |= PIPECONF_6BPC |
+			PIPECONF_DITHER_EN |
+			PIPECONF_DITHER_TYPE_SP;
+	}
+
+	if (IS_VALLEYVIEW(dev) &&
+	    intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP) &&
+	    intel_crtc->config.dither) {
+		pipeconf |= PIPECONF_6BPC |
+			PIPECONF_ENABLE |
+			I965_PIPECONF_ACTIVE;
+	}
+
+	if (HAS_PIPE_CXSR(dev)) {
+		if (intel_crtc->lowfreq_avail) {
+			DRM_DEBUG_KMS("enabling CxSR downclocking\n");
+			pipeconf |= PIPECONF_CXSR_DOWNCLOCK;
+		} else {
+			DRM_DEBUG_KMS("disabling CxSR downclocking\n");
+			pipeconf &= ~PIPECONF_CXSR_DOWNCLOCK;
+		}
+	}
+
+	pipeconf &= ~PIPECONF_INTERLACE_MASK;
+	if (!IS_GEN2(dev) &&
+	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
+		pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
+	else
+		pipeconf |= PIPECONF_PROGRESSIVE;
+
+	I915_WRITE(PIPECONF(pipe), pipeconf);
+	POSTING_READ(PIPECONF(pipe));
+}
+
 static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 			      int x, int y,
 			      struct drm_framebuffer *fb)
@@ -4528,7 +4593,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	int plane = intel_crtc->plane;
 	int refclk, num_connectors = 0;
 	intel_clock_t clock, reduced_clock;
-	u32 dspcntr, pipeconf;
+	u32 dspcntr;
 	bool ok, has_reduced_clock = false, is_sdvo = false;
 	bool is_lvds = false, is_tv = false, is_dp = false;
 	struct intel_encoder *encoder;
@@ -4605,9 +4670,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 				has_reduced_clock ? &reduced_clock : NULL,
 				num_connectors);
 
-	/* setup pipeconf */
-	pipeconf = I915_READ(PIPECONF(pipe));
-
 	/* Set up the display plane register */
 	dspcntr = DISPPLANE_GAMMA_ENABLE;
 
@@ -4618,58 +4680,9 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 			dspcntr |= DISPPLANE_SEL_PIPE_B;
 	}
 
-	if (pipe == 0 && INTEL_INFO(dev)->gen < 4) {
-		/* Enable pixel doubling when the dot clock is > 90% of the (display)
-		 * core speed.
-		 *
-		 * XXX: No double-wide on 915GM pipe B. Is that the only reason for the
-		 * pipe == 0 check?
-		 */
-		if (mode->clock >
-		    dev_priv->display.get_display_clock_speed(dev) * 9 / 10)
-			pipeconf |= PIPECONF_DOUBLE_WIDE;
-		else
-			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
-	}
-
-	/* default to 8bpc */
-	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
-	if (is_dp) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-				    PIPECONF_DITHER_EN |
-				    PIPECONF_DITHER_TYPE_SP;
-		}
-	}
-
-	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) {
-		if (intel_crtc->config.dither) {
-			pipeconf |= PIPECONF_6BPC |
-					PIPECONF_ENABLE |
-					I965_PIPECONF_ACTIVE;
-		}
-	}
-
 	DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe == 0 ? 'A' : 'B');
 	drm_mode_debug_printmodeline(mode);
 
-	if (HAS_PIPE_CXSR(dev)) {
-		if (intel_crtc->lowfreq_avail) {
-			DRM_DEBUG_KMS("enabling CxSR downclocking\n");
-			pipeconf |= PIPECONF_CXSR_DOWNCLOCK;
-		} else {
-			DRM_DEBUG_KMS("disabling CxSR downclocking\n");
-			pipeconf &= ~PIPECONF_CXSR_DOWNCLOCK;
-		}
-	}
-
-	pipeconf &= ~PIPECONF_INTERLACE_MASK;
-	if (!IS_GEN2(dev) &&
-	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
-		pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
-	else
-		pipeconf |= PIPECONF_PROGRESSIVE;
-
 	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
 
 	/* pipesrc and dspsize control the size that is scaled from,
@@ -4680,8 +4693,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 		   (mode->hdisplay - 1));
 	I915_WRITE(DSPPOS(plane), 0);
 
-	I915_WRITE(PIPECONF(pipe), pipeconf);
-	POSTING_READ(PIPECONF(pipe));
+	i9xx_set_pipeconf(crtc, mode, adjusted_mode, is_dp);
+
 	intel_enable_pipe(dev_priv, pipe, false);
 
 	intel_wait_for_vblank(dev, pipe);
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Set PIPECONF color range bit on Valleyview

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

VLV has the color range selection bit in the PIPECONF register.
Configure it appropriately.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dfaea15..e49d86a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4575,6 +4575,13 @@ static void i9xx_set_pipeconf(struct drm_crtc *crtc,
 	else
 		pipeconf |= PIPECONF_PROGRESSIVE;
 
+	if (IS_VALLEYVIEW(dev)) {
+		if (intel_crtc->config.limited_color_range)
+			pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
+		else
+			pipeconf &= ~PIPECONF_COLOR_RANGE_SELECT;
+	}
+
 	I915_WRITE(PIPECONF(pipe), pipeconf);
 	POSTING_READ(PIPECONF(pipe));
 }
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Don't use the HDMI port color range bit on Valleyview

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

VLV docs still list the the color range selection bit for the HDMI
ports, but for DP ports it has been repurposed.

I have no idea whether the HDMI color range selection bit still works
on VLV, but since we now have to use the PIPECONF color range bit for
DP, we might as well do the same for HDMI.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index b206a0d..ee4a8da 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -602,7 +602,7 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
 	u32 hdmi_val;
 
 	hdmi_val = SDVO_ENCODING_HDMI;
-	if (!HAS_PCH_SPLIT(dev))
+	if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev))
 		hdmi_val |= intel_hdmi->color_range;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
 		hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH;
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Don't use the HDMI port color range bit on Valleyview

[Jani Nikula]

On Tue, 02 Apr 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VLV docs still list the the color range selection bit for the HDMI
> ports, but for DP ports it has been repurposed.
>
> I have no idea whether the HDMI color range selection bit still works
> on VLV, but since we now have to use the PIPECONF color range bit for
> DP, we might as well do the same for HDMI.

I don't have any additional information on this. The patch does what it
says above, so for that:

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index b206a0d..ee4a8da 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -602,7 +602,7 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
>  	u32 hdmi_val;
>  
>  	hdmi_val = SDVO_ENCODING_HDMI;
> -	if (!HAS_PCH_SPLIT(dev))
> +	if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev))
>  		hdmi_val |= intel_hdmi->color_range;
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
>  		hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH;
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Introduce i9xx_set_pipeconf()

[Jani Nikula]

On Tue, 02 Apr 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Extract the PIPECONF setup into i9xx_set_pipeconf(). This makes the
> <=Gen4/VLV code follow the same pattern as the Gen5+ codepaths.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++----------------
>  1 file changed, 68 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 298bc0c..dfaea15 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4514,6 +4514,71 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
>  		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
>  }
>  
> +static void i9xx_set_pipeconf(struct drm_crtc *crtc,
> +			      const struct drm_display_mode *mode,
> +			      const struct drm_display_mode *adjusted_mode,
> +			      bool is_dp)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	int pipe = intel_crtc->pipe;
> +	u32 pipeconf;
> +
> +	/* setup pipeconf */
> +	pipeconf = I915_READ(PIPECONF(pipe));
> +
> +	if (pipe == 0 && INTEL_INFO(dev)->gen < 4) {
> +		/* Enable pixel doubling when the dot clock is > 90% of the (display)
> +		 * core speed.
> +		 *
> +		 * XXX: No double-wide on 915GM pipe B. Is that the only reason for the
> +		 * pipe == 0 check?
> +		 */
> +		if (mode->clock >
> +		    dev_priv->display.get_display_clock_speed(dev) * 9 / 10)
> +			pipeconf |= PIPECONF_DOUBLE_WIDE;
> +		else
> +			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
> +	}
> +
> +	/* default to 8bpc */
> +	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> +	if (is_dp && intel_crtc->config.dither) {
> +		pipeconf |= PIPECONF_6BPC |
> +			PIPECONF_DITHER_EN |
> +			PIPECONF_DITHER_TYPE_SP;
> +	}
> +
> +	if (IS_VALLEYVIEW(dev) &&
> +	    intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP) &&
> +	    intel_crtc->config.dither) {
> +		pipeconf |= PIPECONF_6BPC |
> +			PIPECONF_ENABLE |
> +			I965_PIPECONF_ACTIVE;
> +	}
> +
> +	if (HAS_PIPE_CXSR(dev)) {
> +		if (intel_crtc->lowfreq_avail) {
> +			DRM_DEBUG_KMS("enabling CxSR downclocking\n");
> +			pipeconf |= PIPECONF_CXSR_DOWNCLOCK;
> +		} else {
> +			DRM_DEBUG_KMS("disabling CxSR downclocking\n");
> +			pipeconf &= ~PIPECONF_CXSR_DOWNCLOCK;
> +		}
> +	}
> +
> +	pipeconf &= ~PIPECONF_INTERLACE_MASK;
> +	if (!IS_GEN2(dev) &&
> +	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
> +		pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
> +	else
> +		pipeconf |= PIPECONF_PROGRESSIVE;
> +
> +	I915_WRITE(PIPECONF(pipe), pipeconf);
> +	POSTING_READ(PIPECONF(pipe));
> +}
> +
>  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  			      int x, int y,
>  			      struct drm_framebuffer *fb)
> @@ -4528,7 +4593,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	int plane = intel_crtc->plane;
>  	int refclk, num_connectors = 0;
>  	intel_clock_t clock, reduced_clock;
> -	u32 dspcntr, pipeconf;
> +	u32 dspcntr;
>  	bool ok, has_reduced_clock = false, is_sdvo = false;
>  	bool is_lvds = false, is_tv = false, is_dp = false;
>  	struct intel_encoder *encoder;
> @@ -4605,9 +4670,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  				has_reduced_clock ? &reduced_clock : NULL,
>  				num_connectors);
>  
> -	/* setup pipeconf */
> -	pipeconf = I915_READ(PIPECONF(pipe));
> -
>  	/* Set up the display plane register */
>  	dspcntr = DISPPLANE_GAMMA_ENABLE;
>  
> @@ -4618,58 +4680,9 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  			dspcntr |= DISPPLANE_SEL_PIPE_B;
>  	}
>  
> -	if (pipe == 0 && INTEL_INFO(dev)->gen < 4) {
> -		/* Enable pixel doubling when the dot clock is > 90% of the (display)
> -		 * core speed.
> -		 *
> -		 * XXX: No double-wide on 915GM pipe B. Is that the only reason for the
> -		 * pipe == 0 check?
> -		 */
> -		if (mode->clock >
> -		    dev_priv->display.get_display_clock_speed(dev) * 9 / 10)
> -			pipeconf |= PIPECONF_DOUBLE_WIDE;
> -		else
> -			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
> -	}
> -
> -	/* default to 8bpc */
> -	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> -	if (is_dp) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -				    PIPECONF_DITHER_EN |
> -				    PIPECONF_DITHER_TYPE_SP;
> -		}
> -	}
> -
> -	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -					PIPECONF_ENABLE |
> -					I965_PIPECONF_ACTIVE;
> -		}
> -	}
> -
>  	DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe == 0 ? 'A' : 'B');
>  	drm_mode_debug_printmodeline(mode);
>  
> -	if (HAS_PIPE_CXSR(dev)) {
> -		if (intel_crtc->lowfreq_avail) {
> -			DRM_DEBUG_KMS("enabling CxSR downclocking\n");
> -			pipeconf |= PIPECONF_CXSR_DOWNCLOCK;
> -		} else {
> -			DRM_DEBUG_KMS("disabling CxSR downclocking\n");
> -			pipeconf &= ~PIPECONF_CXSR_DOWNCLOCK;
> -		}
> -	}
> -
> -	pipeconf &= ~PIPECONF_INTERLACE_MASK;
> -	if (!IS_GEN2(dev) &&
> -	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
> -		pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
> -	else
> -		pipeconf |= PIPECONF_PROGRESSIVE;
> -
>  	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
>  
>  	/* pipesrc and dspsize control the size that is scaled from,
> @@ -4680,8 +4693,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  		   (mode->hdisplay - 1));
>  	I915_WRITE(DSPPOS(plane), 0);
>  
> -	I915_WRITE(PIPECONF(pipe), pipeconf);
> -	POSTING_READ(PIPECONF(pipe));
> +	i9xx_set_pipeconf(crtc, mode, adjusted_mode, is_dp);
> +
>  	intel_enable_pipe(dev_priv, pipe, false);
>  
>  	intel_wait_for_vblank(dev, pipe);
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Set PIPECONF color range bit on Valleyview

[Jani Nikula]

On Tue, 02 Apr 2013, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VLV has the color range selection bit in the PIPECONF register.
> Configure it appropriately.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dfaea15..e49d86a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4575,6 +4575,13 @@ static void i9xx_set_pipeconf(struct drm_crtc *crtc,
>  	else
>  		pipeconf |= PIPECONF_PROGRESSIVE;
>  
> +	if (IS_VALLEYVIEW(dev)) {
> +		if (intel_crtc->config.limited_color_range)
> +			pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
> +		else
> +			pipeconf &= ~PIPECONF_COLOR_RANGE_SELECT;
> +	}
> +
>  	I915_WRITE(PIPECONF(pipe), pipeconf);
>  	POSTING_READ(PIPECONF(pipe));
>  }
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Introduce i9xx_set_pipeconf()

[Daniel Vetter]

On Wed, Apr 03, 2013 at 01:08:16PM +0300, Jani Nikula wrote:
> On Tue, 02 Apr 2013, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Extract the PIPECONF setup into i9xx_set_pipeconf(). This makes the
> > <=Gen4/VLV code follow the same pattern as the Gen5+ codepaths.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next with a pretty massive bikeshed (extended conflict
resolutions ftw), thanks for the patch.
-Daniel

> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++----------------
> >  1 file changed, 68 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 298bc0c..dfaea15 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4514,6 +4514,71 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
> >  		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
> >  }
> >  
> > +static void i9xx_set_pipeconf(struct drm_crtc *crtc,
> > +			      const struct drm_display_mode *mode,
> > +			      const struct drm_display_mode *adjusted_mode,
> > +			      bool is_dp)
> > +{
> > +	struct drm_device *dev = crtc->dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +	int pipe = intel_crtc->pipe;
> > +	u32 pipeconf;
> > +
> > +	/* setup pipeconf */
> > +	pipeconf = I915_READ(PIPECONF(pipe));
> > +
> > +	if (pipe == 0 && INTEL_INFO(dev)->gen < 4) {
> > +		/* Enable pixel doubling when the dot clock is > 90% of the (display)
> > +		 * core speed.
> > +		 *
> > +		 * XXX: No double-wide on 915GM pipe B. Is that the only reason for the
> > +		 * pipe == 0 check?
> > +		 */
> > +		if (mode->clock >
> > +		    dev_priv->display.get_display_clock_speed(dev) * 9 / 10)
> > +			pipeconf |= PIPECONF_DOUBLE_WIDE;
> > +		else
> > +			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
> > +	}
> > +
> > +	/* default to 8bpc */
> > +	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> > +	if (is_dp && intel_crtc->config.dither) {
> > +		pipeconf |= PIPECONF_6BPC |
> > +			PIPECONF_DITHER_EN |
> > +			PIPECONF_DITHER_TYPE_SP;
> > +	}
> > +
> > +	if (IS_VALLEYVIEW(dev) &&
> > +	    intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP) &&
> > +	    intel_crtc->config.dither) {
> > +		pipeconf |= PIPECONF_6BPC |
> > +			PIPECONF_ENABLE |
> > +			I965_PIPECONF_ACTIVE;
> > +	}
> > +
> > +	if (HAS_PIPE_CXSR(dev)) {
> > +		if (intel_crtc->lowfreq_avail) {
> > +			DRM_DEBUG_KMS("enabling CxSR downclocking\n");
> > +			pipeconf |= PIPECONF_CXSR_DOWNCLOCK;
> > +		} else {
> > +			DRM_DEBUG_KMS("disabling CxSR downclocking\n");
> > +			pipeconf &= ~PIPECONF_CXSR_DOWNCLOCK;
> > +		}
> > +	}
> > +
> > +	pipeconf &= ~PIPECONF_INTERLACE_MASK;
> > +	if (!IS_GEN2(dev) &&
> > +	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
> > +		pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
> > +	else
> > +		pipeconf |= PIPECONF_PROGRESSIVE;
> > +
> > +	I915_WRITE(PIPECONF(pipe), pipeconf);
> > +	POSTING_READ(PIPECONF(pipe));
> > +}
> > +
> >  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> >  			      int x, int y,
> >  			      struct drm_framebuffer *fb)
> > @@ -4528,7 +4593,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> >  	int plane = intel_crtc->plane;
> >  	int refclk, num_connectors = 0;
> >  	intel_clock_t clock, reduced_clock;
> > -	u32 dspcntr, pipeconf;
> > +	u32 dspcntr;
> >  	bool ok, has_reduced_clock = false, is_sdvo = false;
> >  	bool is_lvds = false, is_tv = false, is_dp = false;
> >  	struct intel_encoder *encoder;
> > @@ -4605,9 +4670,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> >  				has_reduced_clock ? &reduced_clock : NULL,
> >  				num_connectors);
> >  
> > -	/* setup pipeconf */
> > -	pipeconf = I915_READ(PIPECONF(pipe));
> > -
> >  	/* Set up the display plane register */
> >  	dspcntr = DISPPLANE_GAMMA_ENABLE;
> >  
> > @@ -4618,58 +4680,9 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> >  			dspcntr |= DISPPLANE_SEL_PIPE_B;
> >  	}
> >  
> > -	if (pipe == 0 && INTEL_INFO(dev)->gen < 4) {
> > -		/* Enable pixel doubling when the dot clock is > 90% of the (display)
> > -		 * core speed.
> > -		 *
> > -		 * XXX: No double-wide on 915GM pipe B. Is that the only reason for the
> > -		 * pipe == 0 check?
> > -		 */
> > -		if (mode->clock >
> > -		    dev_priv->display.get_display_clock_speed(dev) * 9 / 10)
> > -			pipeconf |= PIPECONF_DOUBLE_WIDE;
> > -		else
> > -			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
> > -	}
> > -
> > -	/* default to 8bpc */
> > -	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> > -	if (is_dp) {
> > -		if (intel_crtc->config.dither) {
> > -			pipeconf |= PIPECONF_6BPC |
> > -				    PIPECONF_DITHER_EN |
> > -				    PIPECONF_DITHER_TYPE_SP;
> > -		}
> > -	}
> > -
> > -	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)) {
> > -		if (intel_crtc->config.dither) {
> > -			pipeconf |= PIPECONF_6BPC |
> > -					PIPECONF_ENABLE |
> > -					I965_PIPECONF_ACTIVE;
> > -		}
> > -	}
> > -
> >  	DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe == 0 ? 'A' : 'B');
> >  	drm_mode_debug_printmodeline(mode);
> >  
> > -	if (HAS_PIPE_CXSR(dev)) {
> > -		if (intel_crtc->lowfreq_avail) {
> > -			DRM_DEBUG_KMS("enabling CxSR downclocking\n");
> > -			pipeconf |= PIPECONF_CXSR_DOWNCLOCK;
> > -		} else {
> > -			DRM_DEBUG_KMS("disabling CxSR downclocking\n");
> > -			pipeconf &= ~PIPECONF_CXSR_DOWNCLOCK;
> > -		}
> > -	}
> > -
> > -	pipeconf &= ~PIPECONF_INTERLACE_MASK;
> > -	if (!IS_GEN2(dev) &&
> > -	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
> > -		pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
> > -	else
> > -		pipeconf |= PIPECONF_PROGRESSIVE;
> > -
> >  	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
> >  
> >  	/* pipesrc and dspsize control the size that is scaled from,
> > @@ -4680,8 +4693,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> >  		   (mode->hdisplay - 1));
> >  	I915_WRITE(DSPPOS(plane), 0);
> >  
> > -	I915_WRITE(PIPECONF(pipe), pipeconf);
> > -	POSTING_READ(PIPECONF(pipe));
> > +	i9xx_set_pipeconf(crtc, mode, adjusted_mode, is_dp);
> > +
> >  	intel_enable_pipe(dev_priv, pipe, false);
> >  
> >  	intel_wait_for_vblank(dev, pipe);
> > -- 
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/3] drm/i915: Don't use the HDMI port color range bit on Valleyview

[Daniel Vetter]

On Wed, Apr 03, 2013 at 12:33:31PM +0300, Jani Nikula wrote:
> On Tue, 02 Apr 2013, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > VLV docs still list the the color range selection bit for the HDMI
> > ports, but for DP ports it has been repurposed.
> >
> > I have no idea whether the HDMI color range selection bit still works
> > on VLV, but since we now have to use the PIPECONF color range bit for
> > DP, we might as well do the same for HDMI.
> 
> I don't have any additional information on this. The patch does what it
> says above, so for that:
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Patches 2/3 also queued for -next, thanks for the patch.
-Daniel
> 
> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index b206a0d..ee4a8da 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -602,7 +602,7 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
> >  	u32 hdmi_val;
> >  
> >  	hdmi_val = SDVO_ENCODING_HDMI;
> > -	if (!HAS_PCH_SPLIT(dev))
> > +	if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev))
> >  		hdmi_val |= intel_hdmi->color_range;
> >  	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
> >  		hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH;
> > -- 
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/3] drm/i915: Introduce i9xx_set_pipeconf()

[Daniel Vetter]

On Wed, Apr 03, 2013 at 07:06:01PM +0200, Daniel Vetter wrote:
> On Wed, Apr 03, 2013 at 01:08:16PM +0300, Jani Nikula wrote:
> > On Tue, 02 Apr 2013, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Extract the PIPECONF setup into i9xx_set_pipeconf(). This makes the
> > > <=Gen4/VLV code follow the same pattern as the Gen5+ codepaths.
> > 
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> Queued for -next with a pretty massive bikeshed (extended conflict
> resolutions ftw), thanks for the patch.

I've changed my mind after trying to rebase my current pipe_config stuff
on top of latest dinq - I've noticed that I've written this exact patch,
but in slightly different colors:

https://patchwork.kernel.org/patch/2173951/

The slightly different colors resulted in rebase hell in my pipe_config
branch, so I've ditched your patch here and exchanged it with mine -
rebasing your two patches on top was much easier ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch