[06/15] drm/i915: Move VLV/CHV prepare_pll later
diff mbox

Message ID 1436388361-11130-7-git-send-email-ville.syrjala@linux.intel.com
State New
Headers show

Commit Message

Ville Syrjälä July 8, 2015, 8:45 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

With DPIO powergating active on CHV, we can't even access the DPIO PLL
registers until the lane power state overrides have been enabled. That
will happen from the encoder .pre_pll_enable() hook, so move
chv_prepare_pll() to happen after that point, which puts it just before
chv_enable_pll() actually.

Do the same for VLV to avoid accumulating weird differences between the
platforms. Both platforms seem happy with the new arrangement.

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

Comments

deepak.s@linux.intel.com Aug. 17, 2015, 4:36 a.m. UTC | #1
On 07/09/2015 02:15 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> With DPIO powergating active on CHV, we can't even access the DPIO PLL
> registers until the lane power state overrides have been enabled. That
> will happen from the encoder .pre_pll_enable() hook, so move
> chv_prepare_pll() to happen after that point, which puts it just before
> chv_enable_pll() actually.
>
> Do the same for VLV to avoid accumulating weird differences between the
> platforms. Both platforms seem happy with the new arrangement.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0473b38..666a236 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5979,13 +5979,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>   
>   	is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>   
> -	if (!is_dsi) {
> -		if (IS_CHERRYVIEW(dev))
> -			chv_prepare_pll(intel_crtc, intel_crtc->config);
> -		else
> -			vlv_prepare_pll(intel_crtc, intel_crtc->config);
> -	}
> -
>   	if (intel_crtc->config->has_dp_encoder)
>   		intel_dp_set_m_n(intel_crtc, M1_N1);
>   
> @@ -6009,10 +6002,13 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>   			encoder->pre_pll_enable(encoder);
>   
>   	if (!is_dsi) {
> -		if (IS_CHERRYVIEW(dev))
> +		if (IS_CHERRYVIEW(dev)) {
> +			chv_prepare_pll(intel_crtc, intel_crtc->config);
>   			chv_enable_pll(intel_crtc, intel_crtc->config);
> -		else
> +		} else {
> +			vlv_prepare_pll(intel_crtc, intel_crtc->config);
>   			vlv_enable_pll(intel_crtc, intel_crtc->config);
> +		}
>   	}
>   
>   	for_each_encoder_on_crtc(dev, crtc, encoder)
deepak.s@linux.intel.com Aug. 17, 2015, 4:39 a.m. UTC | #2
On 07/09/2015 02:15 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> With DPIO powergating active on CHV, we can't even access the DPIO PLL
> registers until the lane power state overrides have been enabled. That
> will happen from the encoder .pre_pll_enable() hook, so move
> chv_prepare_pll() to happen after that point, which puts it just before
> chv_enable_pll() actually.
>
> Do the same for VLV to avoid accumulating weird differences between the
> platforms. Both platforms seem happy with the new arrangement.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0473b38..666a236 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5979,13 +5979,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>   
>   	is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>   
> -	if (!is_dsi) {
> -		if (IS_CHERRYVIEW(dev))
> -			chv_prepare_pll(intel_crtc, intel_crtc->config);
> -		else
> -			vlv_prepare_pll(intel_crtc, intel_crtc->config);
> -	}
> -
>   	if (intel_crtc->config->has_dp_encoder)
>   		intel_dp_set_m_n(intel_crtc, M1_N1);
>   
> @@ -6009,10 +6002,13 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>   			encoder->pre_pll_enable(encoder);
>   
>   	if (!is_dsi) {
> -		if (IS_CHERRYVIEW(dev))
> +		if (IS_CHERRYVIEW(dev)) {
> +			chv_prepare_pll(intel_crtc, intel_crtc->config);
>   			chv_enable_pll(intel_crtc, intel_crtc->config);
> -		else
> +		} else {
> +			vlv_prepare_pll(intel_crtc, intel_crtc->config);
>   			vlv_enable_pll(intel_crtc, intel_crtc->config);
> +		}
>   	}
>   
>   	for_each_encoder_on_crtc(dev, crtc, encoder)
Looks fine to me
Reviewed-by: Deepak S <deepak.s@linux.intel.com>

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0473b38..666a236 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5979,13 +5979,6 @@  static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
 
-	if (!is_dsi) {
-		if (IS_CHERRYVIEW(dev))
-			chv_prepare_pll(intel_crtc, intel_crtc->config);
-		else
-			vlv_prepare_pll(intel_crtc, intel_crtc->config);
-	}
-
 	if (intel_crtc->config->has_dp_encoder)
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
@@ -6009,10 +6002,13 @@  static void valleyview_crtc_enable(struct drm_crtc *crtc)
 			encoder->pre_pll_enable(encoder);
 
 	if (!is_dsi) {
-		if (IS_CHERRYVIEW(dev))
+		if (IS_CHERRYVIEW(dev)) {
+			chv_prepare_pll(intel_crtc, intel_crtc->config);
 			chv_enable_pll(intel_crtc, intel_crtc->config);
-		else
+		} else {
+			vlv_prepare_pll(intel_crtc, intel_crtc->config);
 			vlv_enable_pll(intel_crtc, intel_crtc->config);
+		}
 	}
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)