On 4 October 2016 at 23:29, Dave Airlie <airl...@gmail.com> wrote: >> It would have been great if the build/integration bits were properly >> split, but I won't sweat too much over it. > > I thought they were, all the bits outside src/amd are in this patch really. > > I suppose I could move the Makefile.am and Makefile.sources into this > patch to make it easier to review. > > I'm going to squash merge all of this anyways. > >> >> That aside there's a few small suggestions: >> - please don't use pragma once > > why not? lots of mesa uses it, 134 instances in my tree now. > 134 is less that 10% of the actual headers we have around ;-)
There was a discussion, which you've already found, plus there's a piece that's missing. Some files must be included only by a certain header(s). Having the ifndef/define guards allows us to correctly guard/check things. >> - whenever possible reference the source where file X/Y is based on. >> - (patch 3/4) add a comment that libvulkan-test.la is currently >> unused and/or comment it out. >> - (patch 3/4) drop empty noinst_HEADERS >> - (patch 3/4) please have all the sources (.c and .h) sorted >> alphabetically in Makefile.sources - ls + sed helps a lot. >> - (patch 3/4) add a reference, and perhaps keep it in a radv >> README/TODO file, about the latest anv commit the driver is based >> upon. >> It'll be worth when porting newer anv changes. > > So far it isn't really based on an anv commit, so I'm not sure there;'s much > value in that, really only the WSI code is a possibility for sharing, and > possibly not even that. I don't see us merging stuff across and back really > ever. > Coincidentally I was looking at the WSI patches ;-) I am _not_ suggesting consolidation at all, but having a ref point as one wants to port new ANV idioms/code/other to RADV. If you think there won't be anything that can be used as a base/inspiration so be it. >> As a follow up we might want to check that the common headers don't >> get overwritten at install stage (factor out the vulkan_include* >> and/or move to reuse the Khronos ones). > > This would be a problem with anv now wouldn't it? > Sort of... it depends on a number of factors. Thanks again, Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
