drm/i915: Do not put big intel_crtc_state on the stack
diff mbox

Message ID 1453217117-26125-1-git-send-email-tvrtko.ursulin@linux.intel.com
State New
Headers show

Commit Message

Tvrtko Ursulin Jan. 19, 2016, 3:25 p.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Having this on stack triggers the -Wframe-larger-than=1024 and
is not nice to put such big things on the kernel stack anyway.

This required a little bit of refactoring to handle the new
failure path from vlv_force_pll_on.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: John Harrison <john.c.harrison@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
Compile tested only!
---
 drivers/gpu/drm/i915/intel_display.c | 58 +++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_dp.c      |  8 +++--
 drivers/gpu/drm/i915/intel_drv.h     |  4 +--
 3 files changed, 45 insertions(+), 25 deletions(-)

Comments

Daniel Vetter Jan. 19, 2016, 8:22 p.m. UTC | #1
On Tue, Jan 19, 2016 at 03:25:17PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Having this on stack triggers the -Wframe-larger-than=1024 and
> is not nice to put such big things on the kernel stack anyway.
> 
> This required a little bit of refactoring to handle the new
> failure path from vlv_force_pll_on.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: John Harrison <john.c.harrison@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> Compile tested only!
> ---
>  drivers/gpu/drm/i915/intel_display.c | 58 +++++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_dp.c      |  8 +++--
>  drivers/gpu/drm/i915/intel_drv.h     |  4 +--
>  3 files changed, 45 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ccb3e3f47450..7bf18658c659 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7635,26 +7635,34 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>   * in cases where we need the PLL enabled even when @pipe is not going to
>   * be enabled.
>   */
> -void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> +int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
>  		      const struct dpll *dpll)
>  {
>  	struct intel_crtc *crtc =
>  		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> -	struct intel_crtc_state pipe_config = {
> -		.base.crtc = &crtc->base,
> -		.pixel_multiplier = 1,
> -		.dpll = *dpll,
> -	};
> +	struct intel_crtc_state *pipe_config;
> +
> +	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> +	if (!pipe_config)
> +		return -ENOMEM;

tbh I wouldn't bother with the return code here since the only caller
can't do anything about it anyway. But since this is a bit a bikeshed,
either way:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +
> +	pipe_config->base.crtc = &crtc->base;
> +	pipe_config->pixel_multiplier = 1;
> +	pipe_config->dpll = *dpll;
>  
>  	if (IS_CHERRYVIEW(dev)) {
> -		chv_compute_dpll(crtc, &pipe_config);
> -		chv_prepare_pll(crtc, &pipe_config);
> -		chv_enable_pll(crtc, &pipe_config);
> +		chv_compute_dpll(crtc, pipe_config);
> +		chv_prepare_pll(crtc, pipe_config);
> +		chv_enable_pll(crtc, pipe_config);
>  	} else {
> -		vlv_compute_dpll(crtc, &pipe_config);
> -		vlv_prepare_pll(crtc, &pipe_config);
> -		vlv_enable_pll(crtc, &pipe_config);
> +		vlv_compute_dpll(crtc, pipe_config);
> +		vlv_prepare_pll(crtc, pipe_config);
> +		vlv_enable_pll(crtc, pipe_config);
>  	}
> +
> +	kfree(pipe_config);
> +
> +	return 0;
>  }
>  
>  /**
> @@ -10828,7 +10836,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>  	struct drm_display_mode *mode;
> -	struct intel_crtc_state pipe_config;
> +	struct intel_crtc_state *pipe_config;
>  	int htot = I915_READ(HTOTAL(cpu_transcoder));
>  	int hsync = I915_READ(HSYNC(cpu_transcoder));
>  	int vtot = I915_READ(VTOTAL(cpu_transcoder));
> @@ -10839,6 +10847,12 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	if (!mode)
>  		return NULL;
>  
> +	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> +	if (!pipe_config) {
> +		kfree(mode);
> +		return NULL;
> +	}
> +
>  	/*
>  	 * Construct a pipe_config sufficient for getting the clock info
>  	 * back out of crtc_clock_get.
> @@ -10846,14 +10860,14 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	 * Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
>  	 * to use a real value here instead.
>  	 */
> -	pipe_config.cpu_transcoder = (enum transcoder) pipe;
> -	pipe_config.pixel_multiplier = 1;
> -	pipe_config.dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> -	pipe_config.dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> -	pipe_config.dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> -	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
> -
> -	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
> +	pipe_config->cpu_transcoder = (enum transcoder) pipe;
> +	pipe_config->pixel_multiplier = 1;
> +	pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> +	pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> +	pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> +	i9xx_crtc_clock_get(intel_crtc, pipe_config);
> +
> +	mode->clock = pipe_config->port_clock / pipe_config->pixel_multiplier;
>  	mode->hdisplay = (htot & 0xffff) + 1;
>  	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
>  	mode->hsync_start = (hsync & 0xffff) + 1;
> @@ -10865,6 +10879,8 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  
>  	drm_mode_set_name(mode);
>  
> +	kfree(pipe_config);
> +
>  	return mode;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 17612548c58d..e2bea710614f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -335,8 +335,12 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
>  		release_cl_override = IS_CHERRYVIEW(dev) &&
>  			!chv_phy_powergate_ch(dev_priv, phy, ch, true);
>  
> -		vlv_force_pll_on(dev, pipe, IS_CHERRYVIEW(dev) ?
> -				 &chv_dpll[0].dpll : &vlv_dpll[0].dpll);
> +		if (vlv_force_pll_on(dev, pipe, IS_CHERRYVIEW(dev) ?
> +				     &chv_dpll[0].dpll : &vlv_dpll[0].dpll)) {
> +			DRM_ERROR("Failed to force on pll for pipe %c!\n",
> +				  pipe_name(pipe));
> +			return;
> +		}
>  	}
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 059b46e22c31..538faaf4eeef 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1177,8 +1177,8 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>  struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
>  						struct intel_crtc_state *state);
>  
> -void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> -		      const struct dpll *dpll);
> +int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> +		     const struct dpll *dpll);
>  void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe);
>  
>  /* modesetting asserts */
> -- 
> 1.9.1
>
Tvrtko Ursulin Jan. 20, 2016, 10:14 a.m. UTC | #2
On 19/01/16 20:22, Daniel Vetter wrote:
> On Tue, Jan 19, 2016 at 03:25:17PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Having this on stack triggers the -Wframe-larger-than=1024 and
>> is not nice to put such big things on the kernel stack anyway.
>>
>> This required a little bit of refactoring to handle the new
>> failure path from vlv_force_pll_on.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: John Harrison <john.c.harrison@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>> Compile tested only!
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 58 +++++++++++++++++++++++-------------
>>   drivers/gpu/drm/i915/intel_dp.c      |  8 +++--
>>   drivers/gpu/drm/i915/intel_drv.h     |  4 +--
>>   3 files changed, 45 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index ccb3e3f47450..7bf18658c659 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7635,26 +7635,34 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>>    * in cases where we need the PLL enabled even when @pipe is not going to
>>    * be enabled.
>>    */
>> -void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
>> +int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
>>   		      const struct dpll *dpll)
>>   {
>>   	struct intel_crtc *crtc =
>>   		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
>> -	struct intel_crtc_state pipe_config = {
>> -		.base.crtc = &crtc->base,
>> -		.pixel_multiplier = 1,
>> -		.dpll = *dpll,
>> -	};
>> +	struct intel_crtc_state *pipe_config;
>> +
>> +	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
>> +	if (!pipe_config)
>> +		return -ENOMEM;
>
> tbh I wouldn't bother with the return code here since the only caller
> can't do anything about it anyway. But since this is a bit a bikeshed,
> either way:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks!

First version actually did not bother with returning the error, but then 
I thought someone will be guaranteed to complain. :)

If it had more than one caller there would be some value in being able 
to report the error closer to the callsite, but like this is as you say.

However, being one of the rare patches which got a CI success I did not 
want to risk that and just merged it like it is. :)

Regards,

Tvrtko
Jani Nikula Jan. 21, 2016, 9:47 a.m. UTC | #3
On Tue, 19 Jan 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Having this on stack triggers the -Wframe-larger-than=1024 and
> is not nice to put such big things on the kernel stack anyway.
>
> This required a little bit of refactoring to handle the new
> failure path from vlv_force_pll_on.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: John Harrison <john.c.harrison@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> Compile tested only!
> ---
>  drivers/gpu/drm/i915/intel_display.c | 58 +++++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_dp.c      |  8 +++--
>  drivers/gpu/drm/i915/intel_drv.h     |  4 +--
>  3 files changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ccb3e3f47450..7bf18658c659 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7635,26 +7635,34 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>   * in cases where we need the PLL enabled even when @pipe is not going to
>   * be enabled.
>   */
> -void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> +int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
>  		      const struct dpll *dpll)
>  {
>  	struct intel_crtc *crtc =
>  		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> -	struct intel_crtc_state pipe_config = {
> -		.base.crtc = &crtc->base,
> -		.pixel_multiplier = 1,
> -		.dpll = *dpll,
> -	};
> +	struct intel_crtc_state *pipe_config;
> +
> +	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> +	if (!pipe_config)
> +		return -ENOMEM;
> +
> +	pipe_config->base.crtc = &crtc->base;
> +	pipe_config->pixel_multiplier = 1;
> +	pipe_config->dpll = *dpll;
>  
>  	if (IS_CHERRYVIEW(dev)) {
> -		chv_compute_dpll(crtc, &pipe_config);
> -		chv_prepare_pll(crtc, &pipe_config);
> -		chv_enable_pll(crtc, &pipe_config);
> +		chv_compute_dpll(crtc, pipe_config);
> +		chv_prepare_pll(crtc, pipe_config);
> +		chv_enable_pll(crtc, pipe_config);
>  	} else {
> -		vlv_compute_dpll(crtc, &pipe_config);
> -		vlv_prepare_pll(crtc, &pipe_config);
> -		vlv_enable_pll(crtc, &pipe_config);
> +		vlv_compute_dpll(crtc, pipe_config);
> +		vlv_prepare_pll(crtc, pipe_config);
> +		vlv_enable_pll(crtc, pipe_config);
>  	}
> +
> +	kfree(pipe_config);
> +
> +	return 0;
>  }
>  
>  /**
> @@ -10828,7 +10836,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>  	struct drm_display_mode *mode;
> -	struct intel_crtc_state pipe_config;
> +	struct intel_crtc_state *pipe_config;
>  	int htot = I915_READ(HTOTAL(cpu_transcoder));
>  	int hsync = I915_READ(HSYNC(cpu_transcoder));
>  	int vtot = I915_READ(VTOTAL(cpu_transcoder));
> @@ -10839,6 +10847,12 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	if (!mode)
>  		return NULL;
>  
> +	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);

GFP_TEMPORARY

BR,
Jani.

> +	if (!pipe_config) {
> +		kfree(mode);
> +		return NULL;
> +	}
> +
>  	/*
>  	 * Construct a pipe_config sufficient for getting the clock info
>  	 * back out of crtc_clock_get.
> @@ -10846,14 +10860,14 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  	 * Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
>  	 * to use a real value here instead.
>  	 */
> -	pipe_config.cpu_transcoder = (enum transcoder) pipe;
> -	pipe_config.pixel_multiplier = 1;
> -	pipe_config.dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> -	pipe_config.dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> -	pipe_config.dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> -	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
> -
> -	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
> +	pipe_config->cpu_transcoder = (enum transcoder) pipe;
> +	pipe_config->pixel_multiplier = 1;
> +	pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> +	pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> +	pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> +	i9xx_crtc_clock_get(intel_crtc, pipe_config);
> +
> +	mode->clock = pipe_config->port_clock / pipe_config->pixel_multiplier;
>  	mode->hdisplay = (htot & 0xffff) + 1;
>  	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
>  	mode->hsync_start = (hsync & 0xffff) + 1;
> @@ -10865,6 +10879,8 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>  
>  	drm_mode_set_name(mode);
>  
> +	kfree(pipe_config);
> +
>  	return mode;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 17612548c58d..e2bea710614f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -335,8 +335,12 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
>  		release_cl_override = IS_CHERRYVIEW(dev) &&
>  			!chv_phy_powergate_ch(dev_priv, phy, ch, true);
>  
> -		vlv_force_pll_on(dev, pipe, IS_CHERRYVIEW(dev) ?
> -				 &chv_dpll[0].dpll : &vlv_dpll[0].dpll);
> +		if (vlv_force_pll_on(dev, pipe, IS_CHERRYVIEW(dev) ?
> +				     &chv_dpll[0].dpll : &vlv_dpll[0].dpll)) {
> +			DRM_ERROR("Failed to force on pll for pipe %c!\n",
> +				  pipe_name(pipe));
> +			return;
> +		}
>  	}
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 059b46e22c31..538faaf4eeef 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1177,8 +1177,8 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>  struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
>  						struct intel_crtc_state *state);
>  
> -void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> -		      const struct dpll *dpll);
> +int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> +		     const struct dpll *dpll);
>  void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe);
>  
>  /* modesetting asserts */

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ccb3e3f47450..7bf18658c659 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7635,26 +7635,34 @@  static void chv_prepare_pll(struct intel_crtc *crtc,
  * in cases where we need the PLL enabled even when @pipe is not going to
  * be enabled.
  */
-void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
+int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
 		      const struct dpll *dpll)
 {
 	struct intel_crtc *crtc =
 		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
-	struct intel_crtc_state pipe_config = {
-		.base.crtc = &crtc->base,
-		.pixel_multiplier = 1,
-		.dpll = *dpll,
-	};
+	struct intel_crtc_state *pipe_config;
+
+	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
+	if (!pipe_config)
+		return -ENOMEM;
+
+	pipe_config->base.crtc = &crtc->base;
+	pipe_config->pixel_multiplier = 1;
+	pipe_config->dpll = *dpll;
 
 	if (IS_CHERRYVIEW(dev)) {
-		chv_compute_dpll(crtc, &pipe_config);
-		chv_prepare_pll(crtc, &pipe_config);
-		chv_enable_pll(crtc, &pipe_config);
+		chv_compute_dpll(crtc, pipe_config);
+		chv_prepare_pll(crtc, pipe_config);
+		chv_enable_pll(crtc, pipe_config);
 	} else {
-		vlv_compute_dpll(crtc, &pipe_config);
-		vlv_prepare_pll(crtc, &pipe_config);
-		vlv_enable_pll(crtc, &pipe_config);
+		vlv_compute_dpll(crtc, pipe_config);
+		vlv_prepare_pll(crtc, pipe_config);
+		vlv_enable_pll(crtc, pipe_config);
 	}
+
+	kfree(pipe_config);
+
+	return 0;
 }
 
 /**
@@ -10828,7 +10836,7 @@  struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 	struct drm_display_mode *mode;
-	struct intel_crtc_state pipe_config;
+	struct intel_crtc_state *pipe_config;
 	int htot = I915_READ(HTOTAL(cpu_transcoder));
 	int hsync = I915_READ(HSYNC(cpu_transcoder));
 	int vtot = I915_READ(VTOTAL(cpu_transcoder));
@@ -10839,6 +10847,12 @@  struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 	if (!mode)
 		return NULL;
 
+	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
+	if (!pipe_config) {
+		kfree(mode);
+		return NULL;
+	}
+
 	/*
 	 * Construct a pipe_config sufficient for getting the clock info
 	 * back out of crtc_clock_get.
@@ -10846,14 +10860,14 @@  struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 	 * Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
 	 * to use a real value here instead.
 	 */
-	pipe_config.cpu_transcoder = (enum transcoder) pipe;
-	pipe_config.pixel_multiplier = 1;
-	pipe_config.dpll_hw_state.dpll = I915_READ(DPLL(pipe));
-	pipe_config.dpll_hw_state.fp0 = I915_READ(FP0(pipe));
-	pipe_config.dpll_hw_state.fp1 = I915_READ(FP1(pipe));
-	i9xx_crtc_clock_get(intel_crtc, &pipe_config);
-
-	mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
+	pipe_config->cpu_transcoder = (enum transcoder) pipe;
+	pipe_config->pixel_multiplier = 1;
+	pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(pipe));
+	pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(pipe));
+	pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(pipe));
+	i9xx_crtc_clock_get(intel_crtc, pipe_config);
+
+	mode->clock = pipe_config->port_clock / pipe_config->pixel_multiplier;
 	mode->hdisplay = (htot & 0xffff) + 1;
 	mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
 	mode->hsync_start = (hsync & 0xffff) + 1;
@@ -10865,6 +10879,8 @@  struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 
 	drm_mode_set_name(mode);
 
+	kfree(pipe_config);
+
 	return mode;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 17612548c58d..e2bea710614f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -335,8 +335,12 @@  vlv_power_sequencer_kick(struct intel_dp *intel_dp)
 		release_cl_override = IS_CHERRYVIEW(dev) &&
 			!chv_phy_powergate_ch(dev_priv, phy, ch, true);
 
-		vlv_force_pll_on(dev, pipe, IS_CHERRYVIEW(dev) ?
-				 &chv_dpll[0].dpll : &vlv_dpll[0].dpll);
+		if (vlv_force_pll_on(dev, pipe, IS_CHERRYVIEW(dev) ?
+				     &chv_dpll[0].dpll : &vlv_dpll[0].dpll)) {
+			DRM_ERROR("Failed to force on pll for pipe %c!\n",
+				  pipe_name(pipe));
+			return;
+		}
 	}
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 059b46e22c31..538faaf4eeef 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1177,8 +1177,8 @@  void assert_shared_dpll(struct drm_i915_private *dev_priv,
 struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
 						struct intel_crtc_state *state);
 
-void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
-		      const struct dpll *dpll);
+int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
+		     const struct dpll *dpll);
 void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe);
 
 /* modesetting asserts */