diff mbox

[04/18] drm/i915: don't call Haswell PCH code when we can't or don't need

Message ID 1351024208-3489-5-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Oct. 23, 2012, 8:29 p.m. UTC
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

On Ironlake we have one PCH transcoder and FDI per pipe, so we know
that if ironlake_crtc_driving_pch returns false we can disable the PCH
transcoder and we also know that when we disable the crtc we can also
disable the PCH transcoder.

On Haswell there is only 1 PCH transcoder and FDI and they can be used
by any CRTC. So if for one specific crtc haswell_crtc_driving_pch
returns false we can't assert anything about the state of the PCH
transcoder or the FDI link without checking if any other CRTC is using
the PCH.

So on this commit remove the "assert_fdi_{t,r}x_disabled" form
haswell_crtc_enable and also only disable FDI and the PCH transcoder
if the port being disabled was actually a PCH port (we only have one
port using PCH: the VGA port).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

Comments

Jani Nikula Oct. 25, 2012, 12:18 p.m. UTC | #1
On Tue, 23 Oct 2012, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> On Ironlake we have one PCH transcoder and FDI per pipe, so we know
> that if ironlake_crtc_driving_pch returns false we can disable the PCH
> transcoder and we also know that when we disable the crtc we can also
> disable the PCH transcoder.
>
> On Haswell there is only 1 PCH transcoder and FDI and they can be used
> by any CRTC. So if for one specific crtc haswell_crtc_driving_pch
> returns false we can't assert anything about the state of the PCH
> transcoder or the FDI link without checking if any other CRTC is using
> the PCH.
>
> So on this commit remove the "assert_fdi_{t,r}x_disabled" form
> haswell_crtc_enable and also only disable FDI and the PCH transcoder
> if the port being disabled was actually a PCH port (we only have one
> port using PCH: the VGA port).

I first wrote a message saying this is wrong, then revisited the patch,
and finally reached the same conclusions as you...

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


>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0c4e9c5..67c9472 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3286,12 +3286,8 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  
>  	is_pch_port = haswell_crtc_driving_pch(crtc);
>  
> -	if (is_pch_port) {
> +	if (is_pch_port)
>  		ironlake_fdi_pll_enable(intel_crtc);
> -	} else {
> -		assert_fdi_tx_disabled(dev_priv, pipe);
> -		assert_fdi_rx_disabled(dev_priv, pipe);
> -	}
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		if (encoder->pre_enable)
> @@ -3433,10 +3429,13 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
> +	bool is_pch_port;
>  
>  	if (!intel_crtc->active)
>  		return;
>  
> +	is_pch_port = haswell_crtc_driving_pch(crtc);
> +
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		encoder->disable(encoder);
>  
> @@ -3463,14 +3462,12 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  		if (encoder->post_disable)
>  			encoder->post_disable(encoder);
>  
> -	ironlake_fdi_disable(crtc);
> -
> -	intel_disable_transcoder(dev_priv, pipe);
> -
> -	/* disable PCH DPLL */
> -	intel_disable_pch_pll(intel_crtc);
> -
> -	ironlake_fdi_pll_disable(intel_crtc);
> +	if (is_pch_port) {
> +		ironlake_fdi_disable(crtc);
> +		intel_disable_transcoder(dev_priv, pipe);
> +		intel_disable_pch_pll(intel_crtc);
> +		ironlake_fdi_pll_disable(intel_crtc);
> +	}
>  
>  	intel_crtc->active = false;
>  	intel_update_watermarks(dev);
> -- 
> 1.7.11.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0c4e9c5..67c9472 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3286,12 +3286,8 @@  static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	is_pch_port = haswell_crtc_driving_pch(crtc);
 
-	if (is_pch_port) {
+	if (is_pch_port)
 		ironlake_fdi_pll_enable(intel_crtc);
-	} else {
-		assert_fdi_tx_disabled(dev_priv, pipe);
-		assert_fdi_rx_disabled(dev_priv, pipe);
-	}
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->pre_enable)
@@ -3433,10 +3429,13 @@  static void haswell_crtc_disable(struct drm_crtc *crtc)
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
+	bool is_pch_port;
 
 	if (!intel_crtc->active)
 		return;
 
+	is_pch_port = haswell_crtc_driving_pch(crtc);
+
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->disable(encoder);
 
@@ -3463,14 +3462,12 @@  static void haswell_crtc_disable(struct drm_crtc *crtc)
 		if (encoder->post_disable)
 			encoder->post_disable(encoder);
 
-	ironlake_fdi_disable(crtc);
-
-	intel_disable_transcoder(dev_priv, pipe);
-
-	/* disable PCH DPLL */
-	intel_disable_pch_pll(intel_crtc);
-
-	ironlake_fdi_pll_disable(intel_crtc);
+	if (is_pch_port) {
+		ironlake_fdi_disable(crtc);
+		intel_disable_transcoder(dev_priv, pipe);
+		intel_disable_pch_pll(intel_crtc);
+		ironlake_fdi_pll_disable(intel_crtc);
+	}
 
 	intel_crtc->active = false;
 	intel_update_watermarks(dev);