[PATCH xserver] edid: Prune duplicates after adding modes from DDC

[Michel Dänzer]

On 02/03/17 02:26 PM, Chen, Xiaogang wrote:
>> From: Michel Dänzer [mailto:michel at daenzer.net]
>> On 23/02/17 06:46 AM, Chen, Xiaogang wrote:
>>>> From: Michel Dänzer [mailto:michel at daenzer.net]
>>>>
>>>> Multiple calls to xf86EdidMonitorSet (which can be triggered e.g. by
>>>> running
>>>> xrandr) would potentially keep adding the same modes, causing the
>>>> Monitor-
>>>>> Modes list to keep growing larger and using up more memory.
>>>>
>>>> Fix this by calling xf86PruneDuplicateModes after adding the modes
>>>> returned by xf86DDCGetModes. This removes any newly added modes
>> which
>>>> were already in the Monitor->Modes list before, but keeps new modes
>>>> which weren't yet.
>>>>
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99521
>>>> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
>>>> ---
>>>>
>>>> Xiaogang / Jorge, does this fix the memory leak you're seeing?
>>>>
>>> [XCHEN] Hi Michel: I tested your patch with same script I used before
>>> (while true; do xrandr --verbose; done), the memory leak is still there.
>>> Actually the xf86PruneDuplicateModes() is called before at
>>> xf86DDCGetModes() which is called by xf86EdidMonitorSet().
>>
>> xf86DDCGetModes only calls xf86PruneDuplicateModes for the list of modes it
>> generates itself (note that xf86DDCGetModes isn't called only by
>> xf86EdidMonitorSet). The xf86PruneDuplicateModes call added by my patch
>> works on the combined Monitor->Modes list with the modes returned by
>> xf86DDCGetModes appended. The question now is why that call doesn't
>> actually eliminate the duplicate modes.
>>
>>
>>> I think the point here is: why we need to have these "mode"? I do not
>>> find these "mode" got used afterword.
>>
>> grep the tree for '->Modes'.
> [XCHEN] Those modes(generated by xf86DDCGetModes )are added at
> (xf86OutputPtr)output->scrn->monitor->modes. I searched the reference
> for it from the source tree, did not see those modes get freed. 
> It not seems mode duplicating issue, those modes need to be freed.

Running Xorg in valgrind --leak-check=full and exiting cleanly (e.g. via
Ctrl-Alt-Backspace), valgrind doesn't report a leak for the modes
allocated by xf86DDCGetModes, so they do get freed somewhere. The
problem really is that xf86EdidMonitorSet keeps adding the same modes to
the monitor's mode list, and xf86PruneDuplicateModes fails to eliminate
the duplicates for some reason.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer