[Mesa-dev,15/19] mesa: hook up UUID queries for driver and device

Submitted by Andres Rodriguez on June 30, 2017, 11:03 p.m.

Details

Message ID 20170630230312.23213-16-andresx7@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Andres Rodriguez June 30, 2017, 11:03 p.m.
Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 src/mesa/main/dd.h                  |  6 ++++++
 src/mesa/main/get.c                 | 17 +++++++++++++++++
 src/mesa/main/version.c             | 16 ++++++++++++++++
 src/mesa/main/version.h             |  6 ++++++
 src/mesa/state_tracker/st_context.c | 10 ++++++++++
 5 files changed, 55 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
index 27c6efc..65bc97e 100644
--- a/src/mesa/main/dd.h
+++ b/src/mesa/main/dd.h
@@ -1108,6 +1108,12 @@  struct dd_function_table {
                               GLenum usage,
                               struct gl_buffer_object *bufObj);
 
+   /**
+    * Fill uuid with an unique identifier for this driver
+    *
+    * uuid must point to UUID_SIZE_EXT bytes of available memory
+    */
+   void (*GetUuid)(struct gl_context *ctx, char *uuid);
    /*@}*/
 
    /**
diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index 9f26ad1..bcbec1a 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -40,6 +40,7 @@ 
 #include "framebuffer.h"
 #include "samplerobj.h"
 #include "stencil.h"
+#include "version.h"
 
 /* This is a table driven implemetation of the glGet*v() functions.
  * The basic idea is that most getters just look up an int somewhere
@@ -832,6 +833,14 @@  find_custom_value(struct gl_context *ctx, const struct value_desc *d, union valu
 	 ctx->Texture.Unit[unit].CurrentTex[d->offset]->Name;
       break;
 
+   /* GL_EXT_external_objects */
+   case GL_DRIVER_UUID_EXT:
+      _mesa_get_driver_uuid(ctx, v->value_int_4);
+      break;
+   case GL_DEVICE_UUID_EXT:
+      _mesa_get_device_uuid(ctx, v->value_int_4);
+      break;
+
    /* GL_EXT_packed_float */
    case GL_RGBA_SIGNED_COMPONENTS_EXT:
       {
@@ -2491,6 +2500,14 @@  find_value_indexed(const char *func, GLenum pname, GLuint index, union value *v)
          goto invalid_value;
       v->value_int = ctx->Const.MaxComputeVariableGroupSize[index];
       return TYPE_INT;
+
+   /* GL_EXT_external_objects */
+   case GL_DRIVER_UUID_EXT:
+      _mesa_get_driver_uuid(ctx, v->value_int_4);
+      return TYPE_INT_4;
+   case GL_DEVICE_UUID_EXT:
+      _mesa_get_device_uuid(ctx, v->value_int_4);
+      return TYPE_INT_4;
    }
 
  invalid_enum:
diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
index 34f8bbb..402fc01 100644
--- a/src/mesa/main/version.c
+++ b/src/mesa/main/version.c
@@ -635,3 +635,19 @@  _mesa_compute_version(struct gl_context *ctx)
       break;
    }
 }
+
+
+void
+_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid)
+{
+   uuid[0] = 'M';
+   uuid[1] = 'E';
+   uuid[2] = 'S';
+   uuid[4] = 'A';
+}
+
+void
+_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid)
+{
+   ctx->Driver.GetUuid(ctx, (char*) uuid);
+}
diff --git a/src/mesa/main/version.h b/src/mesa/main/version.h
index ee7cb75..4cb5e5f 100644
--- a/src/mesa/main/version.h
+++ b/src/mesa/main/version.h
@@ -47,4 +47,10 @@  _mesa_override_gl_version(struct gl_context *ctx);
 extern void
 _mesa_override_glsl_version(struct gl_constants *consts);
 
+extern void
+_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid);
+
+extern void
+_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid);
+
 #endif /* VERSION_H */
diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
index a846be3..110805e 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -641,6 +641,15 @@  st_set_background_context(struct gl_context *ctx,
    smapi->set_background_context(&st->iface, queue_info);
 }
 
+static void
+st_get_uuid(struct gl_context *ctx, char *uuid)
+{
+   struct pipe_screen *screen = st_context(ctx)->pipe->screen;
+
+   assert(GL_UUID_SIZE_EXT <= PIPE_UUID_SIZE);
+   memcpy(uuid, screen->uuid, GL_UUID_SIZE_EXT);
+}
+
 void st_init_driver_functions(struct pipe_screen *screen,
                               struct dd_function_table *functions)
 {
@@ -687,4 +696,5 @@  void st_init_driver_functions(struct pipe_screen *screen,
    functions->UpdateState = st_invalidate_state;
    functions->QueryMemoryInfo = st_query_memory_info;
    functions->SetBackgroundContext = st_set_background_context;
+   functions->GetUuid = st_get_uuid;
 }

Comments

On June 30, 2017 4:04:31 PM Andres Rodriguez <andresx7@gmail.com> wrote:

> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> ---
>  src/mesa/main/dd.h                  |  6 ++++++
>  src/mesa/main/get.c                 | 17 +++++++++++++++++
>  src/mesa/main/version.c             | 16 ++++++++++++++++
>  src/mesa/main/version.h             |  6 ++++++
>  src/mesa/state_tracker/st_context.c | 10 ++++++++++
>  5 files changed, 55 insertions(+)
>
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 27c6efc..65bc97e 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -1108,6 +1108,12 @@ struct dd_function_table {
>                                GLenum usage,
>                                struct gl_buffer_object *bufObj);
>
> +   /**
> +    * Fill uuid with an unique identifier for this driver
> +    *
> +    * uuid must point to UUID_SIZE_EXT bytes of available memory
> +    */
> +   void (*GetUuid)(struct gl_context *ctx, char *uuid);
>     /*@}*/
>
>     /**
> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
> index 9f26ad1..bcbec1a 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -40,6 +40,7 @@
>  #include "framebuffer.h"
>  #include "samplerobj.h"
>  #include "stencil.h"
> +#include "version.h"
>
>  /* This is a table driven implemetation of the glGet*v() functions.
>   * The basic idea is that most getters just look up an int somewhere
> @@ -832,6 +833,14 @@ find_custom_value(struct gl_context *ctx, const struct 
> value_desc *d, union valu
>  	 ctx->Texture.Unit[unit].CurrentTex[d->offset]->Name;
>        break;
>
> +   /* GL_EXT_external_objects */
> +   case GL_DRIVER_UUID_EXT:
> +      _mesa_get_driver_uuid(ctx, v->value_int_4);
> +      break;
> +   case GL_DEVICE_UUID_EXT:
> +      _mesa_get_device_uuid(ctx, v->value_int_4);
> +      break;
> +
>     /* GL_EXT_packed_float */
>     case GL_RGBA_SIGNED_COMPONENTS_EXT:
>        {
> @@ -2491,6 +2500,14 @@ find_value_indexed(const char *func, GLenum pname, 
> GLuint index, union value *v)
>           goto invalid_value;
>        v->value_int = ctx->Const.MaxComputeVariableGroupSize[index];
>        return TYPE_INT;
> +
> +   /* GL_EXT_external_objects */
> +   case GL_DRIVER_UUID_EXT:
> +      _mesa_get_driver_uuid(ctx, v->value_int_4);
> +      return TYPE_INT_4;
> +   case GL_DEVICE_UUID_EXT:
> +      _mesa_get_device_uuid(ctx, v->value_int_4);
> +      return TYPE_INT_4;
>     }
>
>   invalid_enum:
> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
> index 34f8bbb..402fc01 100644
> --- a/src/mesa/main/version.c
> +++ b/src/mesa/main/version.c
> @@ -635,3 +635,19 @@ _mesa_compute_version(struct gl_context *ctx)
>        break;
>     }
>  }
> +
> +
> +void
> +_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid)
> +{
> +   uuid[0] = 'M';
> +   uuid[1] = 'E';
> +   uuid[2] = 'S';
> +   uuid[4] = 'A';

This isn't right.  The driver uuid is supposed to encompass not only the 
driver but also a version.  This is used to ensure that all of the magic 
between GL and Vulkan will "just work".  That magic includes whatever 
heuristics are used to choose image layout.  Since these may change from 
version to version, this should be some sort of version number and probably 
needs to be provided by the actual driver.

> +}
> +
> +void
> +_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid)
> +{
> +   ctx->Driver.GetUuid(ctx, (char*) uuid);
> +}
> diff --git a/src/mesa/main/version.h b/src/mesa/main/version.h
> index ee7cb75..4cb5e5f 100644
> --- a/src/mesa/main/version.h
> +++ b/src/mesa/main/version.h
> @@ -47,4 +47,10 @@ _mesa_override_gl_version(struct gl_context *ctx);
>  extern void
>  _mesa_override_glsl_version(struct gl_constants *consts);
>
> +extern void
> +_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid);
> +
> +extern void
> +_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid);
> +
>  #endif /* VERSION_H */
> diff --git a/src/mesa/state_tracker/st_context.c 
> b/src/mesa/state_tracker/st_context.c
> index a846be3..110805e 100644
> --- a/src/mesa/state_tracker/st_context.c
> +++ b/src/mesa/state_tracker/st_context.c
> @@ -641,6 +641,15 @@ st_set_background_context(struct gl_context *ctx,
>     smapi->set_background_context(&st->iface, queue_info);
>  }
>
> +static void
> +st_get_uuid(struct gl_context *ctx, char *uuid)
> +{
> +   struct pipe_screen *screen = st_context(ctx)->pipe->screen;
> +
> +   assert(GL_UUID_SIZE_EXT <= PIPE_UUID_SIZE);
> +   memcpy(uuid, screen->uuid, GL_UUID_SIZE_EXT);
> +}
> +
>  void st_init_driver_functions(struct pipe_screen *screen,
>                                struct dd_function_table *functions)
>  {
> @@ -687,4 +696,5 @@ void st_init_driver_functions(struct pipe_screen *screen,
>     functions->UpdateState = st_invalidate_state;
>     functions->QueryMemoryInfo = st_query_memory_info;
>     functions->SetBackgroundContext = st_set_background_context;
> +   functions->GetUuid = st_get_uuid;
>  }
> --
> 2.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 2017-06-30 07:21 PM, Jason Ekstrand wrote:
> On June 30, 2017 4:04:31 PM Andres Rodriguez <andresx7@gmail.com> wrote:
> 
>> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
>> ---
>>  src/mesa/main/dd.h                  |  6 ++++++
>>  src/mesa/main/get.c                 | 17 +++++++++++++++++
>>  src/mesa/main/version.c             | 16 ++++++++++++++++
>>  src/mesa/main/version.h             |  6 ++++++
>>  src/mesa/state_tracker/st_context.c | 10 ++++++++++
>>  5 files changed, 55 insertions(+)
>>
>> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
>> index 27c6efc..65bc97e 100644
>> --- a/src/mesa/main/dd.h
>> +++ b/src/mesa/main/dd.h
>> @@ -1108,6 +1108,12 @@ struct dd_function_table {
>>                                GLenum usage,
>>                                struct gl_buffer_object *bufObj);
>>
>> +   /**
>> +    * Fill uuid with an unique identifier for this driver
>> +    *
>> +    * uuid must point to UUID_SIZE_EXT bytes of available memory
>> +    */
>> +   void (*GetUuid)(struct gl_context *ctx, char *uuid);
>>     /*@}*/
>>
>>     /**
>> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
>> index 9f26ad1..bcbec1a 100644
>> --- a/src/mesa/main/get.c
>> +++ b/src/mesa/main/get.c
>> @@ -40,6 +40,7 @@
>>  #include "framebuffer.h"
>>  #include "samplerobj.h"
>>  #include "stencil.h"
>> +#include "version.h"
>>
>>  /* This is a table driven implemetation of the glGet*v() functions.
>>   * The basic idea is that most getters just look up an int somewhere
>> @@ -832,6 +833,14 @@ find_custom_value(struct gl_context *ctx, const 
>> struct value_desc *d, union valu
>>       ctx->Texture.Unit[unit].CurrentTex[d->offset]->Name;
>>        break;
>>
>> +   /* GL_EXT_external_objects */
>> +   case GL_DRIVER_UUID_EXT:
>> +      _mesa_get_driver_uuid(ctx, v->value_int_4);
>> +      break;
>> +   case GL_DEVICE_UUID_EXT:
>> +      _mesa_get_device_uuid(ctx, v->value_int_4);
>> +      break;
>> +
>>     /* GL_EXT_packed_float */
>>     case GL_RGBA_SIGNED_COMPONENTS_EXT:
>>        {
>> @@ -2491,6 +2500,14 @@ find_value_indexed(const char *func, GLenum 
>> pname, GLuint index, union value *v)
>>           goto invalid_value;
>>        v->value_int = ctx->Const.MaxComputeVariableGroupSize[index];
>>        return TYPE_INT;
>> +
>> +   /* GL_EXT_external_objects */
>> +   case GL_DRIVER_UUID_EXT:
>> +      _mesa_get_driver_uuid(ctx, v->value_int_4);
>> +      return TYPE_INT_4;
>> +   case GL_DEVICE_UUID_EXT:
>> +      _mesa_get_device_uuid(ctx, v->value_int_4);
>> +      return TYPE_INT_4;
>>     }
>>
>>   invalid_enum:
>> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
>> index 34f8bbb..402fc01 100644
>> --- a/src/mesa/main/version.c
>> +++ b/src/mesa/main/version.c
>> @@ -635,3 +635,19 @@ _mesa_compute_version(struct gl_context *ctx)
>>        break;
>>     }
>>  }
>> +
>> +
>> +void
>> +_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid)
>> +{
>> +   uuid[0] = 'M';
>> +   uuid[1] = 'E';
>> +   uuid[2] = 'S';
>> +   uuid[4] = 'A';
> 
> This isn't right.  The driver uuid is supposed to encompass not only the 
> driver but also a version.  This is used to ensure that all of the magic 
> between GL and Vulkan will "just work".  That magic includes whatever 
> heuristics are used to choose image layout.  Since these may change from 
> version to version, this should be some sort of version number and 
> probably needs to be provided by the actual driver.
> 

This is something I've been on the fence about for a while, mostly 
because the vulkan and gl driver for mesa come out of the same source 
base. It is very unlikely that an end user will have an incompatible 
version of mesa-gl and mesa-vulkan since they will probably be built 
from the same commit.

The possible users that could end up mixing and matching different 
versions of vulkan and gl are developers or testers. And, while I don't 
personally agree with having a mix-and-match of shared libraries, some 
people sometimes do it. And that is something that has always been "at 
your own risk". I didn't want to kill this use case with a version check.

Having said that, your suggestion is 100% according to spec. I'm just 
abusing the fact that mesa-gl and mesa-vk are release tagged together. 
It lets us be less restrictive for funky dev/test setups (and it lets me 
be a little lazy).

If we still want the version in there, that can be accommodated. If so, 
would the mesa version number be sufficient, or do we want something 
more specific?

Regards,
Andres

>> +}
>> +
>> +void
>> +_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid)
>> +{
>> +   ctx->Driver.GetUuid(ctx, (char*) uuid);
>> +}
>> diff --git a/src/mesa/main/version.h b/src/mesa/main/version.h
>> index ee7cb75..4cb5e5f 100644
>> --- a/src/mesa/main/version.h
>> +++ b/src/mesa/main/version.h
>> @@ -47,4 +47,10 @@ _mesa_override_gl_version(struct gl_context *ctx);
>>  extern void
>>  _mesa_override_glsl_version(struct gl_constants *consts);
>>
>> +extern void
>> +_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid);
>> +
>> +extern void
>> +_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid);
>> +
>>  #endif /* VERSION_H */
>> diff --git a/src/mesa/state_tracker/st_context.c 
>> b/src/mesa/state_tracker/st_context.c
>> index a846be3..110805e 100644
>> --- a/src/mesa/state_tracker/st_context.c
>> +++ b/src/mesa/state_tracker/st_context.c
>> @@ -641,6 +641,15 @@ st_set_background_context(struct gl_context *ctx,
>>     smapi->set_background_context(&st->iface, queue_info);
>>  }
>>
>> +static void
>> +st_get_uuid(struct gl_context *ctx, char *uuid)
>> +{
>> +   struct pipe_screen *screen = st_context(ctx)->pipe->screen;
>> +
>> +   assert(GL_UUID_SIZE_EXT <= PIPE_UUID_SIZE);
>> +   memcpy(uuid, screen->uuid, GL_UUID_SIZE_EXT);
>> +}
>> +
>>  void st_init_driver_functions(struct pipe_screen *screen,
>>                                struct dd_function_table *functions)
>>  {
>> @@ -687,4 +696,5 @@ void st_init_driver_functions(struct pipe_screen 
>> *screen,
>>     functions->UpdateState = st_invalidate_state;
>>     functions->QueryMemoryInfo = st_query_memory_info;
>>     functions->SetBackgroundContext = st_set_background_context;
>> +   functions->GetUuid = st_get_uuid;
>>  }
>> -- 
>> 2.9.3
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>
On June 30, 2017 5:58:24 PM Andres Rodriguez <andresx7@gmail.com> wrote:

>
>
> On 2017-06-30 07:21 PM, Jason Ekstrand wrote:
>> On June 30, 2017 4:04:31 PM Andres Rodriguez <andresx7@gmail.com> wrote:
>>
>>> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
>>> ---
>>>  src/mesa/main/dd.h                  |  6 ++++++
>>>  src/mesa/main/get.c                 | 17 +++++++++++++++++
>>>  src/mesa/main/version.c             | 16 ++++++++++++++++
>>>  src/mesa/main/version.h             |  6 ++++++
>>>  src/mesa/state_tracker/st_context.c | 10 ++++++++++
>>>  5 files changed, 55 insertions(+)
>>>
>>> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
>>> index 27c6efc..65bc97e 100644
>>> --- a/src/mesa/main/dd.h
>>> +++ b/src/mesa/main/dd.h
>>> @@ -1108,6 +1108,12 @@ struct dd_function_table {
>>>                                GLenum usage,
>>>                                struct gl_buffer_object *bufObj);
>>>
>>> +   /**
>>> +    * Fill uuid with an unique identifier for this driver
>>> +    *
>>> +    * uuid must point to UUID_SIZE_EXT bytes of available memory
>>> +    */
>>> +   void (*GetUuid)(struct gl_context *ctx, char *uuid);
>>>     /*@}*/
>>>
>>>     /**
>>> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
>>> index 9f26ad1..bcbec1a 100644
>>> --- a/src/mesa/main/get.c
>>> +++ b/src/mesa/main/get.c
>>> @@ -40,6 +40,7 @@
>>>  #include "framebuffer.h"
>>>  #include "samplerobj.h"
>>>  #include "stencil.h"
>>> +#include "version.h"
>>>
>>>  /* This is a table driven implemetation of the glGet*v() functions.
>>>   * The basic idea is that most getters just look up an int somewhere
>>> @@ -832,6 +833,14 @@ find_custom_value(struct gl_context *ctx, const
>>> struct value_desc *d, union valu
>>>       ctx->Texture.Unit[unit].CurrentTex[d->offset]->Name;
>>>        break;
>>>
>>> +   /* GL_EXT_external_objects */
>>> +   case GL_DRIVER_UUID_EXT:
>>> +      _mesa_get_driver_uuid(ctx, v->value_int_4);
>>> +      break;
>>> +   case GL_DEVICE_UUID_EXT:
>>> +      _mesa_get_device_uuid(ctx, v->value_int_4);
>>> +      break;
>>> +
>>>     /* GL_EXT_packed_float */
>>>     case GL_RGBA_SIGNED_COMPONENTS_EXT:
>>>        {
>>> @@ -2491,6 +2500,14 @@ find_value_indexed(const char *func, GLenum
>>> pname, GLuint index, union value *v)
>>>           goto invalid_value;
>>>        v->value_int = ctx->Const.MaxComputeVariableGroupSize[index];
>>>        return TYPE_INT;
>>> +
>>> +   /* GL_EXT_external_objects */
>>> +   case GL_DRIVER_UUID_EXT:
>>> +      _mesa_get_driver_uuid(ctx, v->value_int_4);
>>> +      return TYPE_INT_4;
>>> +   case GL_DEVICE_UUID_EXT:
>>> +      _mesa_get_device_uuid(ctx, v->value_int_4);
>>> +      return TYPE_INT_4;
>>>     }
>>>
>>>   invalid_enum:
>>> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
>>> index 34f8bbb..402fc01 100644
>>> --- a/src/mesa/main/version.c
>>> +++ b/src/mesa/main/version.c
>>> @@ -635,3 +635,19 @@ _mesa_compute_version(struct gl_context *ctx)
>>>        break;
>>>     }
>>>  }
>>> +
>>> +
>>> +void
>>> +_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid)
>>> +{
>>> +   uuid[0] = 'M';
>>> +   uuid[1] = 'E';
>>> +   uuid[2] = 'S';
>>> +   uuid[4] = 'A';
>>
>> This isn't right.  The driver uuid is supposed to encompass not only the
>> driver but also a version.  This is used to ensure that all of the magic
>> between GL and Vulkan will "just work".  That magic includes whatever
>> heuristics are used to choose image layout.  Since these may change from
>> version to version, this should be some sort of version number and
>> probably needs to be provided by the actual driver.
>>
>
> This is something I've been on the fence about for a while, mostly
> because the vulkan and gl driver for mesa come out of the same source
> base. It is very unlikely that an end user will have an incompatible
> version of mesa-gl and mesa-vulkan since they will probably be built
> from the same commit.
>
> The possible users that could end up mixing and matching different
> versions of vulkan and gl are developers or testers. And, while I don't
> personally agree with having a mix-and-match of shared libraries, some
> people sometimes do it. And that is something that has always been "at
> your own risk". I didn't want to kill this use case with a version check.

Killing that use-case is exactly why that query exists in the first place.  
Texture layout is a very complex operation with, at least for Intel, 
several arbitrary choices.  If any of that code changes, things are liable 
to break in strange, unpredictable, and hard to debug ways.  The individual 
developers may not know if a particular addrlib or ISL change will affect 
things so it's better off to assume that they all do.

Also, I don't think users trying to run mixed setups is as unlikely as you 
make it sound.  There are all sorts of people out there that will build a 
custom mesa if needed to play their game.  If they try to use that with 
their system mesa, they're in for trouble.

> Having said that, your suggestion is 100% according to spec. I'm just
> abusing the fact that mesa-gl and mesa-vk are release tagged together.
> It lets us be less restrictive for funky dev/test setups (and it lets me
> be a little lazy).
>
> If we still want the version in there, that can be accommodated. If so,
> would the mesa version number be sufficient, or do we want something
> more specific?

I think we probably want some sort of git she or build id.  This actually 
gets rather tricky because you can have breaking changes in your tree that 
aren't committed in git.  In the world of shader caching, we've solved this 
by using the sha1 build-id of the .so.  Unfortunately, this doesn't work 
for driver uuid because the Vulkan and GL drivers are in different .sos 
with different build-ids.  Ideally, for us, I'd like it to be some sort of 
sha1 hash of the ISL sources used to build the driver.  Ultimately, though, 
I think this is a back-end driver choice.and not something we want to 
enforce across all of mesa.

--Jason

> Regards,
> Andres
>
>>> +}
>>> +
>>> +void
>>> +_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid)
>>> +{
>>> +   ctx->Driver.GetUuid(ctx, (char*) uuid);
>>> +}
>>> diff --git a/src/mesa/main/version.h b/src/mesa/main/version.h
>>> index ee7cb75..4cb5e5f 100644
>>> --- a/src/mesa/main/version.h
>>> +++ b/src/mesa/main/version.h
>>> @@ -47,4 +47,10 @@ _mesa_override_gl_version(struct gl_context *ctx);
>>>  extern void
>>>  _mesa_override_glsl_version(struct gl_constants *consts);
>>>
>>> +extern void
>>> +_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid);
>>> +
>>> +extern void
>>> +_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid);
>>> +
>>>  #endif /* VERSION_H */
>>> diff --git a/src/mesa/state_tracker/st_context.c
>>> b/src/mesa/state_tracker/st_context.c
>>> index a846be3..110805e 100644
>>> --- a/src/mesa/state_tracker/st_context.c
>>> +++ b/src/mesa/state_tracker/st_context.c
>>> @@ -641,6 +641,15 @@ st_set_background_context(struct gl_context *ctx,
>>>     smapi->set_background_context(&st->iface, queue_info);
>>>  }
>>>
>>> +static void
>>> +st_get_uuid(struct gl_context *ctx, char *uuid)
>>> +{
>>> +   struct pipe_screen *screen = st_context(ctx)->pipe->screen;
>>> +
>>> +   assert(GL_UUID_SIZE_EXT <= PIPE_UUID_SIZE);
>>> +   memcpy(uuid, screen->uuid, GL_UUID_SIZE_EXT);
>>> +}
>>> +
>>>  void st_init_driver_functions(struct pipe_screen *screen,
>>>                                struct dd_function_table *functions)
>>>  {
>>> @@ -687,4 +696,5 @@ void st_init_driver_functions(struct pipe_screen
>>> *screen,
>>>     functions->UpdateState = st_invalidate_state;
>>>     functions->QueryMemoryInfo = st_query_memory_info;
>>>     functions->SetBackgroundContext = st_set_background_context;
>>> +   functions->GetUuid = st_get_uuid;
>>>  }
>>> --
>>> 2.9.3
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
On 2017-07-01 11:29 AM, Jason Ekstrand wrote:
> On June 30, 2017 5:58:24 PM Andres Rodriguez <andresx7@gmail.com> wrote:
> 
>>
>>
>> On 2017-06-30 07:21 PM, Jason Ekstrand wrote:
>>> On June 30, 2017 4:04:31 PM Andres Rodriguez <andresx7@gmail.com> wrote:
>>>
>>>> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
>>>> ---
>>>>  src/mesa/main/dd.h                  |  6 ++++++
>>>>  src/mesa/main/get.c                 | 17 +++++++++++++++++
>>>>  src/mesa/main/version.c             | 16 ++++++++++++++++
>>>>  src/mesa/main/version.h             |  6 ++++++
>>>>  src/mesa/state_tracker/st_context.c | 10 ++++++++++
>>>>  5 files changed, 55 insertions(+)
>>>>
>>>> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
>>>> index 27c6efc..65bc97e 100644
>>>> --- a/src/mesa/main/dd.h
>>>> +++ b/src/mesa/main/dd.h
>>>> @@ -1108,6 +1108,12 @@ struct dd_function_table {
>>>>                                GLenum usage,
>>>>                                struct gl_buffer_object *bufObj);
>>>>
>>>> +   /**
>>>> +    * Fill uuid with an unique identifier for this driver
>>>> +    *
>>>> +    * uuid must point to UUID_SIZE_EXT bytes of available memory
>>>> +    */
>>>> +   void (*GetUuid)(struct gl_context *ctx, char *uuid);
>>>>     /*@}*/
>>>>
>>>>     /**
>>>> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
>>>> index 9f26ad1..bcbec1a 100644
>>>> --- a/src/mesa/main/get.c
>>>> +++ b/src/mesa/main/get.c
>>>> @@ -40,6 +40,7 @@
>>>>  #include "framebuffer.h"
>>>>  #include "samplerobj.h"
>>>>  #include "stencil.h"
>>>> +#include "version.h"
>>>>
>>>>  /* This is a table driven implemetation of the glGet*v() functions.
>>>>   * The basic idea is that most getters just look up an int somewhere
>>>> @@ -832,6 +833,14 @@ find_custom_value(struct gl_context *ctx, const
>>>> struct value_desc *d, union valu
>>>>       ctx->Texture.Unit[unit].CurrentTex[d->offset]->Name;
>>>>        break;
>>>>
>>>> +   /* GL_EXT_external_objects */
>>>> +   case GL_DRIVER_UUID_EXT:
>>>> +      _mesa_get_driver_uuid(ctx, v->value_int_4);
>>>> +      break;
>>>> +   case GL_DEVICE_UUID_EXT:
>>>> +      _mesa_get_device_uuid(ctx, v->value_int_4);
>>>> +      break;
>>>> +
>>>>     /* GL_EXT_packed_float */
>>>>     case GL_RGBA_SIGNED_COMPONENTS_EXT:
>>>>        {
>>>> @@ -2491,6 +2500,14 @@ find_value_indexed(const char *func, GLenum
>>>> pname, GLuint index, union value *v)
>>>>           goto invalid_value;
>>>>        v->value_int = ctx->Const.MaxComputeVariableGroupSize[index];
>>>>        return TYPE_INT;
>>>> +
>>>> +   /* GL_EXT_external_objects */
>>>> +   case GL_DRIVER_UUID_EXT:
>>>> +      _mesa_get_driver_uuid(ctx, v->value_int_4);
>>>> +      return TYPE_INT_4;
>>>> +   case GL_DEVICE_UUID_EXT:
>>>> +      _mesa_get_device_uuid(ctx, v->value_int_4);
>>>> +      return TYPE_INT_4;
>>>>     }
>>>>
>>>>   invalid_enum:
>>>> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
>>>> index 34f8bbb..402fc01 100644
>>>> --- a/src/mesa/main/version.c
>>>> +++ b/src/mesa/main/version.c
>>>> @@ -635,3 +635,19 @@ _mesa_compute_version(struct gl_context *ctx)
>>>>        break;
>>>>     }
>>>>  }
>>>> +
>>>> +
>>>> +void
>>>> +_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid)
>>>> +{
>>>> +   uuid[0] = 'M';
>>>> +   uuid[1] = 'E';
>>>> +   uuid[2] = 'S';
>>>> +   uuid[4] = 'A';
>>>
>>> This isn't right.  The driver uuid is supposed to encompass not only the
>>> driver but also a version.  This is used to ensure that all of the magic
>>> between GL and Vulkan will "just work".  That magic includes whatever
>>> heuristics are used to choose image layout.  Since these may change from
>>> version to version, this should be some sort of version number and
>>> probably needs to be provided by the actual driver.
>>>
>>
>> This is something I've been on the fence about for a while, mostly
>> because the vulkan and gl driver for mesa come out of the same source
>> base. It is very unlikely that an end user will have an incompatible
>> version of mesa-gl and mesa-vulkan since they will probably be built
>> from the same commit.
>>
>> The possible users that could end up mixing and matching different
>> versions of vulkan and gl are developers or testers. And, while I don't
>> personally agree with having a mix-and-match of shared libraries, some
>> people sometimes do it. And that is something that has always been "at
>> your own risk". I didn't want to kill this use case with a version check.
> 
> Killing that use-case is exactly why that query exists in the first 
> place. Texture layout is a very complex operation with, at least for 
> Intel, several arbitrary choices.  If any of that code changes, things 
> are liable to break in strange, unpredictable, and hard to debug ways.  
> The individual developers may not know if a particular addrlib or ISL 
> change will affect things so it's better off to assume that they all do.
> 
> Also, I don't think users trying to run mixed setups is as unlikely as 
> you make it sound.  There are all sorts of people out there that will 
> build a custom mesa if needed to play their game.  If they try to use 
> that with their system mesa, they're in for trouble.
> 
>> Having said that, your suggestion is 100% according to spec. I'm just
>> abusing the fact that mesa-gl and mesa-vk are release tagged together.
>> It lets us be less restrictive for funky dev/test setups (and it lets me
>> be a little lazy).
>>
>> If we still want the version in there, that can be accommodated. If so,
>> would the mesa version number be sufficient, or do we want something
>> more specific?
> 
> I think we probably want some sort of git she or build id.  This 
> actually gets rather tricky because you can have breaking changes in 
> your tree that aren't committed in git.  In the world of shader caching, 
> we've solved this by using the sha1 build-id of the .so.  Unfortunately, 
> this doesn't work for driver uuid because the Vulkan and GL drivers are 
> in different .sos with different build-ids.  Ideally, for us, I'd like 
> it to be some sort of sha1 hash of the ISL sources used to build the 
> driver.  Ultimately, though, I think this is a back-end driver 
> choice.and not something we want to enforce across all of mesa.
> 

Cool. I'll plumb this down to gallium then. Thanks for the time to write 
this out.

Regards,
Andres

P.S. Happy Canada Day to everyone :)

> --Jason
> 
>> Regards,
>> Andres
>>
>>>> +}
>>>> +
>>>> +void
>>>> +_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid)
>>>> +{
>>>> +   ctx->Driver.GetUuid(ctx, (char*) uuid);
>>>> +}
>>>> diff --git a/src/mesa/main/version.h b/src/mesa/main/version.h
>>>> index ee7cb75..4cb5e5f 100644
>>>> --- a/src/mesa/main/version.h
>>>> +++ b/src/mesa/main/version.h
>>>> @@ -47,4 +47,10 @@ _mesa_override_gl_version(struct gl_context *ctx);
>>>>  extern void
>>>>  _mesa_override_glsl_version(struct gl_constants *consts);
>>>>
>>>> +extern void
>>>> +_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid);
>>>> +
>>>> +extern void
>>>> +_mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid);
>>>> +
>>>>  #endif /* VERSION_H */
>>>> diff --git a/src/mesa/state_tracker/st_context.c
>>>> b/src/mesa/state_tracker/st_context.c
>>>> index a846be3..110805e 100644
>>>> --- a/src/mesa/state_tracker/st_context.c
>>>> +++ b/src/mesa/state_tracker/st_context.c
>>>> @@ -641,6 +641,15 @@ st_set_background_context(struct gl_context *ctx,
>>>>     smapi->set_background_context(&st->iface, queue_info);
>>>>  }
>>>>
>>>> +static void
>>>> +st_get_uuid(struct gl_context *ctx, char *uuid)
>>>> +{
>>>> +   struct pipe_screen *screen = st_context(ctx)->pipe->screen;
>>>> +
>>>> +   assert(GL_UUID_SIZE_EXT <= PIPE_UUID_SIZE);
>>>> +   memcpy(uuid, screen->uuid, GL_UUID_SIZE_EXT);
>>>> +}
>>>> +
>>>>  void st_init_driver_functions(struct pipe_screen *screen,
>>>>                                struct dd_function_table *functions)
>>>>  {
>>>> @@ -687,4 +696,5 @@ void st_init_driver_functions(struct pipe_screen
>>>> *screen,
>>>>     functions->UpdateState = st_invalidate_state;
>>>>     functions->QueryMemoryInfo = st_query_memory_info;
>>>>     functions->SetBackgroundContext = st_set_background_context;
>>>> +   functions->GetUuid = st_get_uuid;
>>>>  }
>>>> -- 
>>>> 2.9.3
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>>
> 
>
On 01/07/17 09:58 AM, Andres Rodriguez wrote:
> On 2017-06-30 07:21 PM, Jason Ekstrand wrote:
>> On June 30, 2017 4:04:31 PM Andres Rodriguez <andresx7@gmail.com> wrote:
>>
>>> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
>>> index 34f8bbb..402fc01 100644
>>> --- a/src/mesa/main/version.c
>>> +++ b/src/mesa/main/version.c
>>> @@ -635,3 +635,19 @@ _mesa_compute_version(struct gl_context *ctx)
>>>        break;
>>>     }
>>>  }
>>> +
>>> +
>>> +void
>>> +_mesa_get_driver_uuid(struct gl_context *ctx, GLint *uuid)
>>> +{
>>> +   uuid[0] = 'M';
>>> +   uuid[1] = 'E';
>>> +   uuid[2] = 'S';
>>> +   uuid[4] = 'A';
>>
>> This isn't right.  The driver uuid is supposed to encompass not only
>> the driver but also a version.  This is used to ensure that all of the
>> magic between GL and Vulkan will "just work".  That magic includes
>> whatever heuristics are used to choose image layout.  Since these may
>> change from version to version, this should be some sort of version
>> number and probably needs to be provided by the actual driver.
>>
> 
> This is something I've been on the fence about for a while, mostly
> because the vulkan and gl driver for mesa come out of the same source
> base.

In addition to the issues raised by Jason, radv isn't the only Vulkan
driver for AMD GPUs.
>
> This is something I've been on the fence about for a while, mostly
> because the vulkan and gl driver for mesa come out of the same source
> base.

In addition to the issues raised by Jason, radv isn't the only Vulkan
driver for AMD GPUs.


I don't think Andres patch should be made jump through hoops for a driver
that isn't open source. AMD are free to provide patches to support this use
case but asking anyone outside AMD to care for a piece of software that
isn't
open source, isn't a very nice thing to do.

From reading Jason's requests it does look like just leave it to the
drivers to
sort out is the best option, and for radeonsi/radv we could go with a static
string as we at least share addrlib in this world. Beyond that I think AMD
would
need to propose later patch for interop with the pro driver.

Dave.
On Wed, Jul 5, 2017 at 4:50 PM, Dave Airlie <airlied@gmail.com> wrote:

>
>
> >
> > This is something I've been on the fence about for a while, mostly
> > because the vulkan and gl driver for mesa come out of the same source
> > base.
>
> In addition to the issues raised by Jason, radv isn't the only Vulkan
> driver for AMD GPUs.
>
>
> I don't think Andres patch should be made jump through hoops for a driver
> that isn't open source. AMD are free to provide patches to support this use
> case but asking anyone outside AMD to care for a piece of software that
> isn't
> open source, isn't a very nice thing to do.
>
> From reading Jason's requests it does look like just leave it to the
> drivers to
> sort out is the best option,
>

Thank you.  I think that's the best option too.


> and for radeonsi/radv we could go with a static
> string as we at least share addrlib in this world. Beyond that I think AMD
> would
> need to propose later patch for interop with the pro driver.
>

But what if addrlib changes? :-)  If you have any arbitrary decisions in
surface layout that are made using a heuristic, then you need to at least
make this some sort of addrlib version.  Now I'm just trolling...