[PATCH] drm/i915: Do not put big intel_crtc_state on the stack

[PATCH] drm/i915: Do not put big intel_crtc_state on the stack

[Tvrtko Ursulin]

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);
+	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

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

✓ Fi.CI.BAT: success for drm/i915: Do not put big intel_crtc_state on the stack

[Patchwork]

== Summary ==

Built on 20c388faff9d8c41ab27e825c685526561b892a2 drm-intel-nightly: 2016y-01m-19d-13h-31m-46s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (bdw-nuci7) UNSTABLE
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-b:
                pass       -> SKIP       (bdw-nuci7)

bdw-nuci7        total:140  pass:129  dwarn:1   dfail:0   fail:0   skip:10 
bsw-nuc-2        total:143  pass:117  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc          total:143  pass:125  dwarn:3   dfail:0   fail:0   skip:15 
hsw-brixbox      total:143  pass:136  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:143  pass:139  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:143  pass:102  dwarn:3   dfail:0   fail:0   skip:38 
skl-i5k-2        total:143  pass:133  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps      total:143  pass:124  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220t        total:143  pass:124  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1223/

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

Re: [PATCH] drm/i915: Do not put big intel_crtc_state on the stack

[Daniel Vetter]

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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not put big intel_crtc_state on the stack

[Tvrtko Ursulin]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not put big intel_crtc_state on the stack

[Jani Nikula]

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 */

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx