On 1/28/19 6:59 AM, Michel Dänzer wrote:
> On 2019-01-22 7:28 p.m., sunpeng.li@xxxxxxx wrote:
>> From: David Francis <David.Francis@xxxxxxx>
>>
>> [Why]
>> amdgpu_dm_commit_planes was performing multi-plane
>> flips incorrectly:
>>
>> It waited for vblank once per flipped plane
>>
>> It prepared flip ISR and acquired the corresponding vblank ref
>> once per plane, although it closed ISR and put the ref once
>> per crtc
>>
>> It called into dc once per flipped plane, duplicating some work
>>
>> [How]
>> Wait for vblank, get vblank ref, prepare flip ISR, and call into
>> DC only once, and only if there is a pageflip
>>
>> Make freesync continue to update planes even if vrr information
>> has already been changed
>>
>> Signed-off-by: David Francis <David.Francis@xxxxxxx>
>> Reviewed-by: Harry Wentland <Harry.Wentland@xxxxxxx>
>> Acked-by: Leo Li <sunpeng.li@xxxxxxx>
>
> This commit introduced a memory leak, see the attached report from kmemleak.
>
>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index db060da..818a2a1 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -4987,8 +4837,23 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>> struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state);
>> struct dm_crtc_state *dm_old_crtc_state =
>> to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc));
>> - int planes_count = 0;
>> + int flip_count = 0, planes_count = 0, vpos, hpos;
>> unsigned long flags;
>> + struct amdgpu_bo *abo;
>> + uint64_t tiling_flags, dcc_address;
>> + struct dc_stream_status *stream_status;
>> + uint32_t target, target_vblank;
>> +
>> + struct {
>> + struct dc_surface_update surface_updates[MAX_SURFACES];
>> + struct dc_flip_addrs flip_addrs[MAX_SURFACES];
>> + struct dc_stream_update stream_update;
>> + } *flip;
>> +
>> + flip = kzalloc(sizeof(*flip), GFP_KERNEL);
>> +
>> + if (!flip)
>> + dm_error("Failed to allocate update bundles\n");
>
> I can't see where this memory is freed, maybe this is the leak?
Yeah, I don't think this or the other allocation in a later patch is
ever freed. Thanks for the heads up, I'll make a quick patch addressing
these.
Nicholas Kazlauskas
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
