Re: [Mesa-dev] [PATCH 2/3] radv: rename global extension properties structs

[Emil Velikov]

On 16 January 2017 at 14:16, Bas Nieuwenhuizen  wrote:
> On Mon, Jan 16, 2017 at 2:51 PM, Emil Velikov  
> wrote:
>> On 14 January 2017 at 02:31, Andres Rodriguez  wrote:
>>> On Fri, Jan 13, 2017 at 8:13 PM, Emil Velikov 
>>> wrote:

 On 13 January 2017 at 23:44, Andres Rodriguez  wrote:
 > All extension arrays are global, but only one of them refers to instance
 > extensions.
 >
 > The device extension array refers to extensions that are common across
 > all physical devices. This disctinction will be more imporant once we
 Typos: "distinction" and "important"

 > have dynamic extension support for devices.
 >
 I think that this and 3/3 are very good idea, but since RADV supports
 only one device I'm not sure that they're applicable, yet.
 Not too familiar with the RADV code so I might be off there.
>>>
>>>
>>> Besides differences in HW functionality, another use for this feature would
>>> be to expose an extension only if the software stack supports it.
>>>
>> Guess I was drooling too much over someone adding multiple devices
>> support for radv ;-)
>>
>>> Eg. something like:
>>>
>>> if (libdrm_version >= x && drm_version >= y)
>>> register_extension(...)
>>>
>>> This will come into play with some of the other patches on amd-gfx that
>>> you've helped me review :)
>>>
>> Yw. As you get to the respective work - please don't base it on libdrm
>> version. Please check that the kernel module is old enough either via
>> a) a module version check or b) -EINVAL as returned by the module
>> input validation. Former seems to be used by radeon/amdgpu userspace
>> while the latter by the i915 one.
>
> Using the kernel exported DRM version seems to be common practice for
> radeonsi and the amdgpu and radeon winsyses though?
Indeed that's the case. I kind of said[meant to say] the same :-]

> That version has
> been increased in both kernel drivers to indiciate new features too.
> libdrm version will probably need to be compile time though anyway.
>
I would not bother with the latter - just bump LIBDRM_AMDGPU_REQUIRED and enjoy.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] radv: rename global extension properties structs

[Bas Nieuwenhuizen]

On Mon, Jan 16, 2017 at 2:51 PM, Emil Velikov  wrote:
> On 14 January 2017 at 02:31, Andres Rodriguez  wrote:
>> On Fri, Jan 13, 2017 at 8:13 PM, Emil Velikov 
>> wrote:
>>>
>>> On 13 January 2017 at 23:44, Andres Rodriguez  wrote:
>>> > All extension arrays are global, but only one of them refers to instance
>>> > extensions.
>>> >
>>> > The device extension array refers to extensions that are common across
>>> > all physical devices. This disctinction will be more imporant once we
>>> Typos: "distinction" and "important"
>>>
>>> > have dynamic extension support for devices.
>>> >
>>> I think that this and 3/3 are very good idea, but since RADV supports
>>> only one device I'm not sure that they're applicable, yet.
>>> Not too familiar with the RADV code so I might be off there.
>>
>>
>> Besides differences in HW functionality, another use for this feature would
>> be to expose an extension only if the software stack supports it.
>>
> Guess I was drooling too much over someone adding multiple devices
> support for radv ;-)
>
>> Eg. something like:
>>
>> if (libdrm_version >= x && drm_version >= y)
>> register_extension(...)
>>
>> This will come into play with some of the other patches on amd-gfx that
>> you've helped me review :)
>>
> Yw. As you get to the respective work - please don't base it on libdrm
> version. Please check that the kernel module is old enough either via
> a) a module version check or b) -EINVAL as returned by the module
> input validation. Former seems to be used by radeon/amdgpu userspace
> while the latter by the i915 one.

Using the kernel exported DRM version seems to be common practice for
radeonsi and the amdgpu and radeon winsyses though? That version has
been increased in both kernel drivers to indiciate new features too.
libdrm version will probably need to be compile time though anyway.

- Bas
>
> Thanks
> Emil
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] radv: rename global extension properties structs

[Emil Velikov]

On 14 January 2017 at 02:31, Andres Rodriguez  wrote:
> On Fri, Jan 13, 2017 at 8:13 PM, Emil Velikov 
> wrote:
>>
>> On 13 January 2017 at 23:44, Andres Rodriguez  wrote:
>> > All extension arrays are global, but only one of them refers to instance
>> > extensions.
>> >
>> > The device extension array refers to extensions that are common across
>> > all physical devices. This disctinction will be more imporant once we
>> Typos: "distinction" and "important"
>>
>> > have dynamic extension support for devices.
>> >
>> I think that this and 3/3 are very good idea, but since RADV supports
>> only one device I'm not sure that they're applicable, yet.
>> Not too familiar with the RADV code so I might be off there.
>
>
> Besides differences in HW functionality, another use for this feature would
> be to expose an extension only if the software stack supports it.
>
Guess I was drooling too much over someone adding multiple devices
support for radv ;-)

> Eg. something like:
>
> if (libdrm_version >= x && drm_version >= y)
> register_extension(...)
>
> This will come into play with some of the other patches on amd-gfx that
> you've helped me review :)
>
Yw. As you get to the respective work - please don't base it on libdrm
version. Please check that the kernel module is old enough either via
a) a module version check or b) -EINVAL as returned by the module
input validation. Former seems to be used by radeon/amdgpu userspace
while the latter by the i915 one.

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] radv: rename global extension properties structs

[Andres Rodriguez]

On Fri, Jan 13, 2017 at 8:13 PM, Emil Velikov 
wrote:

> On 13 January 2017 at 23:44, Andres Rodriguez  wrote:
> > All extension arrays are global, but only one of them refers to instance
> > extensions.
> >
> > The device extension array refers to extensions that are common across
> > all physical devices. This disctinction will be more imporant once we
> Typos: "distinction" and "important"
>
> > have dynamic extension support for devices.
> >
> I think that this and 3/3 are very good idea, but since RADV supports
> only one device I'm not sure that they're applicable, yet.
> Not too familiar with the RADV code so I might be off there.
>

Besides differences in HW functionality, another use for this feature would
be to expose an extension only if the software stack supports it.

Eg. something like:

if (libdrm_version >= x && drm_version >= y)
register_extension(...)

This will come into play with some of the other patches on amd-gfx that
you've helped me review :)


>
> Emil
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] radv: rename global extension properties structs

[Emil Velikov]

On 13 January 2017 at 23:44, Andres Rodriguez  wrote:
> All extension arrays are global, but only one of them refers to instance
> extensions.
>
> The device extension array refers to extensions that are common across
> all physical devices. This disctinction will be more imporant once we
Typos: "distinction" and "important"

> have dynamic extension support for devices.
>
I think that this and 3/3 are very good idea, but since RADV supports
only one device I'm not sure that they're applicable, yet.
Not too familiar with the RADV code so I might be off there.

Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/3] radv: rename global extension properties structs

[Andres Rodriguez]

All extension arrays are global, but only one of them refers to instance
extensions.

The device extension array refers to extensions that are common across
all physical devices. This disctinction will be more imporant once we
have dynamic extension support for devices.

Signed-off-by: Andres Rodriguez 
---
 src/amd/vulkan/radv_device.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 99c56a4..e0991d4 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -147,7 +147,7 @@ radv_physical_device_finish(struct radv_physical_device 
*device)
device->ws->destroy(device->ws);
 }
 
-static const VkExtensionProperties global_extensions[] = {
+static const VkExtensionProperties instance_extensions[] = {
{
.extensionName = VK_KHR_SURFACE_EXTENSION_NAME,
.specVersion = 25,
@@ -172,7 +172,7 @@ static const VkExtensionProperties global_extensions[] = {
 #endif
 };
 
-static const VkExtensionProperties device_extensions[] = {
+static const VkExtensionProperties common_device_extensions[] = {
{
.extensionName = 
VK_KHR_SAMPLER_MIRROR_CLAMP_TO_EDGE_EXTENSION_NAME,
.specVersion = 1,
@@ -258,9 +258,9 @@ VkResult radv_CreateInstance(
 
for (uint32_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
bool found = false;
-   for (uint32_t j = 0; j < ARRAY_SIZE(global_extensions); j++) {
+   for (uint32_t j = 0; j < ARRAY_SIZE(instance_extensions); j++) {
if (strcmp(pCreateInfo->ppEnabledExtensionNames[i],
-  global_extensions[j].extensionName) == 0) {
+  instance_extensions[j].extensionName) == 0) {
found = true;
break;
}
@@ -697,9 +697,9 @@ VkResult radv_CreateDevice(
 
for (uint32_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
bool found = false;
-   for (uint32_t j = 0; j < ARRAY_SIZE(device_extensions); j++) {
+   for (uint32_t j = 0; j < ARRAY_SIZE(common_device_extensions); 
j++) {
if (strcmp(pCreateInfo->ppEnabledExtensionNames[i],
-  device_extensions[j].extensionName) == 0) {
+  common_device_extensions[j].extensionName) 
== 0) {
found = true;
break;
}
@@ -826,14 +826,14 @@ VkResult radv_EnumerateInstanceExtensionProperties(
VkExtensionProperties*  pProperties)
 {
if (pProperties == NULL) {
-   *pPropertyCount = ARRAY_SIZE(global_extensions);
+   *pPropertyCount = ARRAY_SIZE(instance_extensions);
return VK_SUCCESS;
}
 
-   *pPropertyCount = MIN2(*pPropertyCount, ARRAY_SIZE(global_extensions));
-   typed_memcpy(pProperties, global_extensions, *pPropertyCount);
+   *pPropertyCount = MIN2(*pPropertyCount, 
ARRAY_SIZE(instance_extensions));
+   typed_memcpy(pProperties, instance_extensions, *pPropertyCount);
 
-   if (*pPropertyCount < ARRAY_SIZE(global_extensions))
+   if (*pPropertyCount < ARRAY_SIZE(instance_extensions))
return VK_INCOMPLETE;
 
return VK_SUCCESS;
@@ -846,14 +846,14 @@ VkResult radv_EnumerateDeviceExtensionProperties(
VkExtensionProperties*  pProperties)
 {
if (pProperties == NULL) {
-   *pPropertyCount = ARRAY_SIZE(device_extensions);
+   *pPropertyCount = ARRAY_SIZE(common_device_extensions);
return VK_SUCCESS;
}
 
-   *pPropertyCount = MIN2(*pPropertyCount, ARRAY_SIZE(device_extensions));
-   typed_memcpy(pProperties, device_extensions, *pPropertyCount);
+   *pPropertyCount = MIN2(*pPropertyCount, 
ARRAY_SIZE(common_device_extensions));
+   typed_memcpy(pProperties, common_device_extensions, *pPropertyCount);
 
-   if (*pPropertyCount < ARRAY_SIZE(device_extensions))
+   if (*pPropertyCount < ARRAY_SIZE(common_device_extensions))
return VK_INCOMPLETE;
 
return VK_SUCCESS;
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/3] radv: rename global extension properties structs

[Andres Rodriguez]

All extension arrays are global, but only one of them refers to instance
extensions.

The device extension array refers to extensions that are common across
all physical devices. This disctinction will be more imporant once we
have dynamic extension support for devices.

Signed-off-by: Andres Rodriguez 
---
 src/amd/vulkan/radv_device.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index e8a91a3..5669fd7 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -147,7 +147,7 @@ radv_physical_device_finish(struct radv_physical_device 
*device)
device->ws->destroy(device->ws);
 }
 
-static const VkExtensionProperties global_extensions[] = {
+static const VkExtensionProperties instance_extensions[] = {
{
.extensionName = VK_KHR_SURFACE_EXTENSION_NAME,
.specVersion = 25,
@@ -172,7 +172,7 @@ static const VkExtensionProperties global_extensions[] = {
 #endif
 };
 
-static const VkExtensionProperties device_extensions[] = {
+static const VkExtensionProperties common_device_extensions[] = {
{
.extensionName = 
VK_KHR_SAMPLER_MIRROR_CLAMP_TO_EDGE_EXTENSION_NAME,
.specVersion = 1,
@@ -258,9 +258,9 @@ VkResult radv_CreateInstance(
 
for (uint32_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
bool found = false;
-   for (uint32_t j = 0; j < ARRAY_SIZE(global_extensions); j++) {
+   for (uint32_t j = 0; j < ARRAY_SIZE(instance_extensions); j++) {
if (strcmp(pCreateInfo->ppEnabledExtensionNames[i],
-  global_extensions[j].extensionName) == 0) {
+  instance_extensions[j].extensionName) == 0) {
found = true;
break;
}
@@ -697,9 +697,9 @@ VkResult radv_CreateDevice(
 
for (uint32_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
bool found = false;
-   for (uint32_t j = 0; j < ARRAY_SIZE(device_extensions); j++) {
+   for (uint32_t j = 0; j < ARRAY_SIZE(common_device_extensions); 
j++) {
if (strcmp(pCreateInfo->ppEnabledExtensionNames[i],
-  device_extensions[j].extensionName) == 0) {
+  common_device_extensions[j].extensionName) 
== 0) {
found = true;
break;
}
@@ -831,14 +831,14 @@ VkResult radv_EnumerateInstanceExtensionProperties(
VkExtensionProperties*  pProperties)
 {
if (pProperties == NULL) {
-   *pPropertyCount = ARRAY_SIZE(global_extensions);
+   *pPropertyCount = ARRAY_SIZE(instance_extensions);
return VK_SUCCESS;
}
 
-   *pPropertyCount = MIN2(*pPropertyCount, ARRAY_SIZE(global_extensions));
-   typed_memcpy(pProperties, global_extensions, *pPropertyCount);
+   *pPropertyCount = MIN2(*pPropertyCount, 
ARRAY_SIZE(instance_extensions));
+   typed_memcpy(pProperties, instance_extensions, *pPropertyCount);
 
-   if (*pPropertyCount < ARRAY_SIZE(global_extensions))
+   if (*pPropertyCount < ARRAY_SIZE(instance_extensions))
return VK_INCOMPLETE;
 
return VK_SUCCESS;
@@ -851,14 +851,14 @@ VkResult radv_EnumerateDeviceExtensionProperties(
VkExtensionProperties*  pProperties)
 {
if (pProperties == NULL) {
-   *pPropertyCount = ARRAY_SIZE(device_extensions);
+   *pPropertyCount = ARRAY_SIZE(common_device_extensions);
return VK_SUCCESS;
}
 
-   *pPropertyCount = MIN2(*pPropertyCount, ARRAY_SIZE(device_extensions));
-   typed_memcpy(pProperties, device_extensions, *pPropertyCount);
+   *pPropertyCount = MIN2(*pPropertyCount, 
ARRAY_SIZE(common_device_extensions));
+   typed_memcpy(pProperties, common_device_extensions, *pPropertyCount);
 
-   if (*pPropertyCount < ARRAY_SIZE(device_extensions))
+   if (*pPropertyCount < ARRAY_SIZE(common_device_extensions))
return VK_INCOMPLETE;
 
return VK_SUCCESS;
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev