AMD guys: commit messages?

[Emil Velikov]

On 8 December 2015 at 14:22, Christian König <deathsimple at vodafone.de> wrote:
> On 08.12.2015 15:04, Ilia Mirkin wrote:
>>
>> On Tue, Dec 8, 2015 at 8:43 AM, Ernst Sjöstrand <ernstp at gmail.com> wrote:
>>>
>>> Hello list!
>>>
>>> I lurk here and try to follow Mesa/DRI and most specifically Radeon
>>> driver development, report bugs, test new stuff and help get the bugs
>>> closed and so on...
>>>
>>> However I see that the commit messages for AMD/Radeon are often very
>>> unhelpful. They don't state the motivation behind the commits. Is this
>>> a optimization, a nice-to-have cleanup or does this actually fix
>>> something? What does this fix?
>>> Are there tests or bugreports related?
>>>
>>> Improving this could make it easier for new developers to start
>>> contributing in the long run also!
>>>
>>> Examples:
>>>
>>>
>>> http://cgit.freedesktop.org/mesa/mesa/commit/?id=d5a5dbd71f0e8756494809025ba2119efdf26373
>>>
>>> http://cgit.freedesktop.org/mesa/mesa/commit/?id=338d7bf0531a10d90db75ad333f7e0a31693641f
>>>
>>> http://cgit.freedesktop.org/mesa/mesa/commit/?id=4ebcf5194d98b47bd9e8a72b7418054708b14750
>>>
>>> This is also in the mesa dev guidelines, www.mesa3d.org/devinfo.html :
>>> "Patch fix is not clearly described. For example, a commit message of
>>> only a single line, no description of the bug, no mention of bugzilla,
>>> etc."
>>
>> So... what's the appropriate amount? Have every commit describe, in
>> detail, how the GPU works, how the driver works, and what little bit
>> of interaction is being changed? I'm not an AMD developer (I do hack
>> on nouveau though), but I basically get what all 3 of the above are
>> doing. The reason why you're having trouble is probably because you
>> don't know what the ingredients are -- what's a mip level, what's a
>> ring index, what's fence, what's a winsys, what's a "emit vertex", all
>> in the context of the relevant drivers. If you know what all these
>> things are, the above commits become much clearer. But having to
>> describe each of those things every time would ... not fly :)
>
>
> Yeah, exactly.
>
> I work for AMD but mostly on the kernel side and I wasn't involved in any of
> those patches, nor with the surrounding source code.
>
> But I do get just by reading the subject lines what all of those are about.
>
> So it's more about knowing the driver and the hardware a bit to understand
> what's going on here.
>
> While it often could be a bit more, describing everything in the commit
> message is way to much.
>
I might be biased yet it seems that the patches coming from AMD people
have the briefest of commit messages in whole of mesa. If this is a
bugfix one could mention the commit which introduces the issue (broken
from day 1, etc), if it's been tested one could mention the hardware
in question and etc.
For example in the case of "radeonsi: avoid stale state pointers" one
could mention if this fixes app X, and/or what are the possible side
effects.

That said I'm also guilty of keeping things brief on the odd occasion,
yet feel free to point out when things look funky.

Thanks
Emil