[Mesa-dev] radv initial submission

[Emil Velikov]

Hi Dave,

On 4 October 2016 at 02:48, Dave Airlie <airlied at gmail.com> wrote:
> Hi all,
>
> I fully expect this won't make it to the list in one piece due to size.
> I've pushed the same patchset to
> https://github.com/airlied/mesa/tree/radv-submit
>
> This is an initial submission of the open source radv vulkan driver
> for AMD GPUs supported by the amdgpu driver. It is a project Bas and I
> started some months ago as an interesting learning experience and it's
> been great to see it get this far in such a short time.
>
> This driver is in no way to be considered conformant or called such.
> It has NOT passed the vulkan conformance suite or ever been submitted
> to Khronos. It prints a big warning on startup to this effect.
>
> In saying that, it runs quite well with Dota 2, Talos Principle and
> vkQuake in our testing so far.
>
> Future plans are to add support for various hw speedups (fast clears),
> missing things like spilling, and multiple queues along with finishing
> out things like geom/tess shaders. Also trying to reuse code between
> this and the radeonsi driver where possible. radv already reuses
> the LLVM compiler backend and the addrlib code.
>
> I've split things out into 4 patches, 3 just add code files to various
> places and number 4 intergrates the build system changes so they
> are easier to review.
>
It would have been great if the build/integration bits were properly
split, but I won't sweat too much over it.

That aside there's a few small suggestions:
 - please don't use pragma once
 - 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.

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

Well done and keep up the nice work :-)
Emil