AMD guys: commit messages?

AMD guys: commit messages?

[Ernst Sjöstrand]

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."

Regards!
//Ernst
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: AMD guys: commit messages?

[Ilia Mirkin]

On Tue, Dec 8, 2015 at 8:43 AM, Ernst Sjöstrand <ernstp@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 :)

You can usually tell a cleanup/hypothetical fix apart from a real fix
by seeing if it (a) references a bugzilla, (b) mentions a commit it
fixes, or (c) cc'd to stable.

  -ilia
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: AMD guys: commit messages?

[Christian König]

On 08.12.2015 15:04, Ilia Mirkin wrote:
> On Tue, Dec 8, 2015 at 8:43 AM, Ernst Sjöstrand <ernstp@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.

Regards,
Christian.

>
> You can usually tell a cleanup/hypothetical fix apart from a real fix
> by seeing if it (a) references a bugzilla, (b) mentions a commit it
> fixes, or (c) cc'd to stable.
>
>    -ilia
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: AMD guys: commit messages?

[Emil Velikov]

Hi Ernst,

On 8 December 2015 at 13:43, Ernst Sjöstrand <ernstp@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."
>
I'm glad that I'm not the only person noticing these. I've asked a few
times for people to write commit messages, mostly to get a summary of
what the patch does, as opposed to why it does it. I'm seriously
considering about not dropping fixes for [mesa] stable if they're so
terse.

I hope this situation improves, because as you've noticed a-is it make
things actually harder for anyone now working on the relevant code on
daily basis. And I doubt AMD wants to alienate "outsiders" from
working on their driver(s).

Thanks
Emil

P.S. Fwiw mesa-dev might have been better suited for this.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: AMD guys: commit messages?

[Emil Velikov]

On 8 December 2015 at 14:22, Christian König <deathsimple@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@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: AMD guys: commit messages?

[Marek Olšák]

There are a few useful prerequisites for contributing to Mesa and
being efficient:

1) Know OpenGL and GLSL
Tutorials: http://ogldev.atspace.co.uk/
You need to know the API one way or another. Eventually, you want to
switch to the official OpenGL, GLSL, and extension specifications as
your primary source of information.

2) Know the graphics pipeline
Articles: https://fgiesen.wordpress.com/2011/07/09/a-trip-through-the-graphics-pipeline-2011-index/
I was surprised about how accurately Z/Stencil optimizations were
described in part 7. It seems pretty dense. Oh and BTW, it's about
DX11, but I see that as an advantage. The hardware documentation uses
DX11 naming conventions, so why not just get to use them now.

3) Know the Mesa code base
Knowing the code and the overall architecture is 50% of success. This
applies to any project.

4) Know the hardware registers and ISA
There is open documentation, but it doesn't contain everything. In
such case, the code is the documentation.

Marek

On Tue, Dec 8, 2015 at 2:43 PM, Ernst Sjöstrand <ernstp@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."
>
> Regards!
> //Ernst
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel