[AMD Official Use Only - Internal Distribution Only] Hi Harry,
Checked Lee Starnes's patch. It does not fix the issue but does preserve the tradition of assigning the number of probed modes to aconnector->num_modes, which appears elsewhere in AMD code. I observed in some cases the memory leak is not reproducing (or not detected) even without mine or Lee's fix. Maybe that's why Lee thinks it may fix the leak. Another way to fix the issue is to not wipe out the probed list by calling INIT_LIST_HEAD() in amdgpu_dm_connector_ddc_get_modes(). But I am not entirely sure what other impact will that do. I am checking out how everyone is handling resume from S3 so would like some second opinions. Regards Regards Stylon Wang MTS Software Development Eng. | AMD Display Solution Team O +(886) 2-3789-3667 ext. 23667 C +(886) 921-897-142 ---------------------------------------------------------------------------------------------------------------------------------- 6F, 3, YuanCyu St (NanKang Software Park) Taipei, Taiwan Facebook<https://www.facebook.com/AMD> | Twitter<https://twitter.com/AMD> | amd.com<http://www.amd.com/> ________________________________ From: Deucher, Alexander <[email protected]> Sent: November 11, 2020 12:55 AM To: Wentland, Harry <[email protected]>; Wang, Chao-kai (Stylon) <[email protected]>; [email protected] <[email protected]> Cc: Kazlauskas, Nicholas <[email protected]> Subject: Re: [PATCH] drm/amd/display: Fix memory leaks in S3 resume [AMD Official Use Only - Internal Distribution Only] Ah, sorry, I missed that part of the patch. Alex ________________________________ From: Wentland, Harry <[email protected]> Sent: Tuesday, November 10, 2020 11:42 AM To: Deucher, Alexander <[email protected]>; Wang, Chao-kai (Stylon) <[email protected]>; [email protected] <[email protected]> Cc: Kazlauskas, Nicholas <[email protected]> Subject: Re: [PATCH] drm/amd/display: Fix memory leaks in S3 resume It's missing the "drm_connector_list_update" call which I assume is important. Stylon, can you review Lee Starnes's patch? Is the drm_connector_list_update call maybe not needed? Thanks, Harry On 2020-11-10 11:26 a.m., Deucher, Alexander wrote: [AMD Official Use Only - Internal Distribution Only] Lee Starnes just sent the exact same patch yesterday. Please review that one: https://patchwork.freedesktop.org/patch/399497/ Alex ________________________________ From: Wentland, Harry <[email protected]><mailto:[email protected]> Sent: Tuesday, November 10, 2020 9:27 AM To: Wang, Chao-kai (Stylon) <[email protected]><mailto:[email protected]>; [email protected]<mailto:[email protected]> <[email protected]><mailto:[email protected]> Cc: Kazlauskas, Nicholas <[email protected]><mailto:[email protected]>; Deucher, Alexander <[email protected]><mailto:[email protected]> Subject: Re: [PATCH] drm/amd/display: Fix memory leaks in S3 resume On 2020-11-10 2:49 a.m., Stylon Wang wrote: > EDID parsing in S3 resume pushes new display modes > to probed_modes list but doesn't consolidate to actual > mode list. This creates a race condition when > amdgpu_dm_connector_ddc_get_modes() re-initializes the > list head without walking the list and results in memory leak. > > Signed-off-by: Stylon Wang <[email protected]><mailto:[email protected]> Looks reasonable to me but haven't had a chance to understand whether this is the best solution. Acked-by: Harry Wentland <[email protected]><mailto:[email protected]> Harry > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > 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 0b6adf23d316..715e0bd489f8 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -2337,7 +2337,8 @@ void amdgpu_dm_update_connector_after_detect( > > drm_connector_update_edid_property(connector, > aconnector->edid); > - drm_add_edid_modes(connector, aconnector->edid); > + aconnector->num_modes = drm_add_edid_modes(connector, > aconnector->edid); > + drm_connector_list_update(connector); > > if (aconnector->dc_link->aux_mode) > drm_dp_cec_set_edid(&aconnector->dm_dp_aux.aux, >
_______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
