[Intel-gfx] [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 at 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 at intel.com>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Cc: John Harrison <john.c.harrison at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at 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 at 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