[Mesa-dev] radv initial submission

[Emil Velikov]

On 4 October 2016 at 23:29, Dave Airlie <airlied at 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