[Mesa-dev] [PATCH] anv: Set driver version to Mesa version

[Bas Nieuwenhuizen]

On Mon, Jun 5, 2017 at 5:40 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Mon, Jun 5, 2017 at 7:48 AM, Alex Smith <asmith at feralinteractive.com>
> wrote:
>>
>> As already done by RADV, this code is lifted straight from there.
>
>
> I think this is a good idea but I'm very confused by the code.
>
>>
>> Signed-off-by: Alex Smith <asmith at feralinteractive.com>
>> ---
>>  src/intel/vulkan/anv_device.c | 24 +++++++++++++++++++++++-
>>  1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
>> index aacd07f..8090eea 100644
>> --- a/src/intel/vulkan/anv_device.c
>> +++ b/src/intel/vulkan/anv_device.c
>> @@ -754,6 +754,28 @@ void anv_GetPhysicalDeviceFeatures2KHR(
>>     }
>>  }
>>
>> +static uint32_t anv_get_driver_version()
>
>
> First off, it would be good to put this in src/vulkan/

Agreed.

>
>>
>> +{
>> +   const char *minor_string = strchr(VERSION, '.');
>> +   const char *patch_string = minor_string ? strchr(minor_string + 1,
>> ','): NULL;
>
>
> Why are we searching for "," and not "." here?

Looks like I made a typo, nice find!
>
>>
>> +   int major = atoi(VERSION);
>> +   int minor = minor_string ? atoi(minor_string + 1) : 0;
>> +   int patch = patch_string ? atoi(patch_string + 1) : 0;
>> +   if (strstr(VERSION, "devel")) {
>> +      if (patch == 0) {
>> +         patch = 99;
>
>
> Is this really what we want?  When I checkout the mesa-17.0.0 tag and
> configure, I see this in config.log:
>
> #define VERSION "17.0.0"
>
> This code looks like it would call that 16.99.99 which is a version that
> never existed.  Perhaps the better thing to do is to look for "-something"
> at the end of the string, assert that patch == 0, and do the decrement.
> That should catch -rcN as well as -devel.
>
> Dave?  Bas?

Note the strstr, which checks for devel, so with 17.0.0, that
decrement shouldn't happen. I'm open to generalizing this, I mostly
did this for getting a sense of what people used on vulkan.gpuinfo.org
and so the rc's weren't really a priority.

- Bas

>
> --Jason
>
>>
>> +         if (minor == 0) {
>> +            minor = 99;
>> +            --major;
>> +         } else
>> +            --minor;
>> +      } else
>> +         --patch;
>> +   }
>> +   uint32_t version = VK_MAKE_VERSION(major, minor, patch);
>> +   return version;
>> +}
>> +
>>  void anv_GetPhysicalDeviceProperties(
>>      VkPhysicalDevice                            physicalDevice,
>>      VkPhysicalDeviceProperties*                 pProperties)
>> @@ -886,7 +908,7 @@ void anv_GetPhysicalDeviceProperties(
>>
>>     *pProperties = (VkPhysicalDeviceProperties) {
>>        .apiVersion = VK_MAKE_VERSION(1, 0, 42),
>> -      .driverVersion = 1,
>> +      .driverVersion = anv_get_driver_version(),
>>        .vendorID = 0x8086,
>>        .deviceID = pdevice->chipset_id,
>>        .deviceType = VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU,
>> --
>> 2.9.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>