[PATCH 15/20] drm/amd/display: Reduce stack size of commit_planes_to_stream

[andrey]

On 2017-10-16 07:51 PM, Harry Wentland wrote:
> On 2017-10-16 06:51 PM, andrey wrote:
>>
>> On 2017-10-16 04:31 PM, Harry Wentland wrote:
>>> This function likes to blow 1024 stack size when something is
>>> added to the addr struct. For now just dynamically allocate.
>>>
>>> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
>>> Reviewed-by: Tony Cheng <Tony.Cheng at amd.com>
>>> Acked-by: Harry Wentland <Harry.Wentland at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/display/dc/core/dc.c | 19 +++++++++++++------
>>>    1 file changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
>>> index ff6bd384aece..7a0593d4ca62 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
>>> @@ -926,9 +926,9 @@ bool dc_commit_planes_to_stream(
>>>            struct dc_state *state)
>>>    {
>>>        struct dc_surface_update updates[MAX_SURFACES];
>> Why this one is left on the stack then ?
> This one is just tracking a bunch of pointers to more flip_addr, plane_info,
> and scaling_info structs, so it's quite small.
>
> This whole function is quite ugly and eventually needs to go anyways, along with
> that whole dc_surface_update struct.

Maybe put some comment with TODO explainig  this, cause it's confusing 
why 3 structures are allocated dynamically while one is on the stack.

Andrey

>
> Harry
>
>> Thanks,
>> Andrey
>>
>>> -    struct dc_flip_addrs flip_addr[MAX_SURFACES];
>>> -    struct dc_plane_info plane_info[MAX_SURFACES];
>>> -    struct dc_scaling_info scaling_info[MAX_SURFACES];
>>> +    struct dc_flip_addrs *flip_addr;
>>> +    struct dc_plane_info *plane_info;
>>> +    struct dc_scaling_info *scaling_info;
>>>        int i;
>>>        struct dc_stream_update *stream_update =
>>>                kzalloc(sizeof(struct dc_stream_update), GFP_KERNEL);
>>> @@ -938,10 +938,14 @@ bool dc_commit_planes_to_stream(
>>>            return false;
>>>        }
>>>    +    flip_addr = kcalloc(MAX_SURFACES, sizeof(struct dc_flip_addrs),
>>> +                GFP_KERNEL);
>>> +    plane_info = kcalloc(MAX_SURFACES, sizeof(struct dc_plane_info),
>>> +                 GFP_KERNEL);
>>> +    scaling_info = kcalloc(MAX_SURFACES, sizeof(struct dc_scaling_info),
>>> +                   GFP_KERNEL);
>>> +
>>>        memset(updates, 0, sizeof(updates));
>>> -    memset(flip_addr, 0, sizeof(flip_addr));
>>> -    memset(plane_info, 0, sizeof(plane_info));
>>> -    memset(scaling_info, 0, sizeof(scaling_info));
>>>          stream_update->src = dc_stream->src;
>>>        stream_update->dst = dc_stream->dst;
>>> @@ -980,6 +984,9 @@ bool dc_commit_planes_to_stream(
>>>                new_plane_count,
>>>                dc_stream, stream_update, plane_states, state);
>>>    +    kfree(flip_addr);
>>> +    kfree(plane_info);
>>> +    kfree(scaling_info);
>>>        kfree(stream_update);
>>>        return true;
>>>    }