* AMD guys: commit messages?
@ 2015-12-08 13:43 Ernst Sjöstrand
2015-12-08 14:04 ` Ilia Mirkin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ernst Sjöstrand @ 2015-12-08 13:43 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: AMD guys: commit messages? 2015-12-08 13:43 AMD guys: commit messages? Ernst Sjöstrand @ 2015-12-08 14:04 ` Ilia Mirkin 2015-12-08 14:22 ` Christian König 2015-12-08 15:10 ` Emil Velikov 2015-12-13 13:07 ` Marek Olšák 2 siblings, 1 reply; 6+ messages in thread From: Ilia Mirkin @ 2015-12-08 14:04 UTC (permalink / raw) To: Ernst Sjöstrand; +Cc: dri-devel@lists.freedesktop.org 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: AMD guys: commit messages? 2015-12-08 14:04 ` Ilia Mirkin @ 2015-12-08 14:22 ` Christian König 2015-12-08 15:20 ` Emil Velikov 0 siblings, 1 reply; 6+ messages in thread From: Christian König @ 2015-12-08 14:22 UTC (permalink / raw) To: Ilia Mirkin, Ernst Sjöstrand; +Cc: dri-devel@lists.freedesktop.org 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: AMD guys: commit messages? 2015-12-08 14:22 ` Christian König @ 2015-12-08 15:20 ` Emil Velikov 0 siblings, 0 replies; 6+ messages in thread From: Emil Velikov @ 2015-12-08 15:20 UTC (permalink / raw) To: Christian König; +Cc: dri-devel@lists.freedesktop.org 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: AMD guys: commit messages? 2015-12-08 13:43 AMD guys: commit messages? Ernst Sjöstrand 2015-12-08 14:04 ` Ilia Mirkin @ 2015-12-08 15:10 ` Emil Velikov 2015-12-13 13:07 ` Marek Olšák 2 siblings, 0 replies; 6+ messages in thread From: Emil Velikov @ 2015-12-08 15:10 UTC (permalink / raw) To: Ernst Sjöstrand; +Cc: ML dri-devel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: AMD guys: commit messages? 2015-12-08 13:43 AMD guys: commit messages? Ernst Sjöstrand 2015-12-08 14:04 ` Ilia Mirkin 2015-12-08 15:10 ` Emil Velikov @ 2015-12-13 13:07 ` Marek Olšák 2 siblings, 0 replies; 6+ messages in thread From: Marek Olšák @ 2015-12-13 13:07 UTC (permalink / raw) To: Ernst Sjöstrand; +Cc: dri-devel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-13 13:07 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-08 13:43 AMD guys: commit messages? Ernst Sjöstrand 2015-12-08 14:04 ` Ilia Mirkin 2015-12-08 14:22 ` Christian König 2015-12-08 15:20 ` Emil Velikov 2015-12-08 15:10 ` Emil Velikov 2015-12-13 13:07 ` Marek Olšák
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.