Re: [Mesa-dev] [PATCH 6/6] anv: do not open random render node(s)

[Jason Ekstrand]

I haven't reviewed in any sort of detail, but I'm fine with this change
FWIW.  I glanced at the drm entrypoints you're calling, and that's not code
we want to duplicate in anv.

On Fri, Dec 2, 2016 at 10:31 AM, Emil Velikov 
wrote:

> On 2 December 2016 at 17:33, Eric Engestrom 
> wrote:
> > On Friday, 2016-12-02 16:31:49 +, Emil Velikov wrote:
> >> From: Emil Velikov 
> >>
> >> drmGetDevices2() provides us with enough flexibility to build heuristics
> >> upon. Opening a random node on the other hand will wake up the device,
> >> regardless if it's the one we're intereseted or not.
> >
> > "interested"
> > (same in the previous patch)
> >
> >>
> >> Cc: Jason Ekstrand 
> >> Signed-off-by: Emil Velikov 
> >> ---
> >>  src/intel/vulkan/Makefile.am  |  3 ++-
> >>  src/intel/vulkan/anv_device.c | 53 ++
> +
> >>  2 files changed, 40 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/src/intel/vulkan/Makefile.am b/src/intel/vulkan/Makefile.am
> >> index df7645f..e309491 100644
> >> --- a/src/intel/vulkan/Makefile.am
> >> +++ b/src/intel/vulkan/Makefile.am
> >> @@ -66,7 +66,7 @@ AM_CPPFLAGS += \
> >>  endif
> >>
> >>  AM_CPPFLAGS += \
> >> - $(INTEL_CFLAGS) \
> >> + $(LIBDRM_CFLAGS) \
> >>   $(VALGRIND_CFLAGS) \
> >>   $(DEFINES)
> >>
> >> @@ -131,6 +131,7 @@ VULKAN_LIB_DEPS += \
> >>   $(top_builddir)/src/intel/isl/libisl.la \
> >>   $(top_builddir)/src/intel/blorp/libblorp.la \
> >>   $(PER_GEN_LIBS) \
> >> + $(LIBDRM_LIBS) \
> >>   $(PTHREAD_LIBS) \
> >>   $(DLOPEN_LIBS) \
> >>   -lm
> >
> > Unrelated -> separate patch?
> >
> It's related: libvulkan_intel.so does not pull libdrm.so as dependency
> (up-to this patch). As we use it below, we ought to add the link here.
> Otherwise we'll end up with interment breakage.
>
> Note: libvulkan_radeon.so already depends on libdrm, which is why the
> hunk is missing from that patch.
>
> >> diff --git a/src/intel/vulkan/anv_device.c
> b/src/intel/vulkan/anv_device.c
> >> index d594df7..9927ac2 100644
> >> --- a/src/intel/vulkan/anv_device.c
> >> +++ b/src/intel/vulkan/anv_device.c
> >> @@ -29,6 +29,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>  #include "anv_private.h"
> >>  #include "util/strtod.h"
> >> @@ -375,6 +376,40 @@ void anv_DestroyInstance(
> >> vk_free(>alloc, instance);
> >>  }
> >>
> >> +static VkResult
> >> +anv_enumerate_devices(struct anv_instance *instance)
> >> +{
> >> +   /* TODO: Check for more devices ? */
> >> +   drmDevicePtr devices[8];
> >> +   VkResult result = VK_SUCCESS;
> >> +   int max_devices;
> >> +
> >> +   max_devices = drmGetDevices2(0, devices, sizeof(devices));
> >> +   if (max_devices < 1)
> >> +  return VK_ERROR_INCOMPATIBLE_DRIVER;
> >> +
> >> +   for (unsigned i = 0; i < (unsigned)max_devices; i++) {
> >> +  if (devices[i]->available_nodes & 1 << DRM_NODE_RENDER &&
> >> +  devices[i]->bustype == DRM_BUS_PCI &&
> >> +  devices[i]->deviceinfo.pci->vendor_id == 0x8086) {
> >
> > Yay, magic values!
> > I feel like we should replace all those with PCI_VENDOR_INTEL or
> something.
> >
> We have another three instances of these for each Vulkan driver.
> Barring any objections I'll do that as cleanup on top ?
>
> 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 6/6] anv: do not open random render node(s)

[Emil Velikov]

On 2 December 2016 at 17:33, Eric Engestrom  wrote:
> On Friday, 2016-12-02 16:31:49 +, Emil Velikov wrote:
>> From: Emil Velikov 
>>
>> drmGetDevices2() provides us with enough flexibility to build heuristics
>> upon. Opening a random node on the other hand will wake up the device,
>> regardless if it's the one we're intereseted or not.
>
> "interested"
> (same in the previous patch)
>
>>
>> Cc: Jason Ekstrand 
>> Signed-off-by: Emil Velikov 
>> ---
>>  src/intel/vulkan/Makefile.am  |  3 ++-
>>  src/intel/vulkan/anv_device.c | 53 
>> +++
>>  2 files changed, 40 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/intel/vulkan/Makefile.am b/src/intel/vulkan/Makefile.am
>> index df7645f..e309491 100644
>> --- a/src/intel/vulkan/Makefile.am
>> +++ b/src/intel/vulkan/Makefile.am
>> @@ -66,7 +66,7 @@ AM_CPPFLAGS += \
>>  endif
>>
>>  AM_CPPFLAGS += \
>> - $(INTEL_CFLAGS) \
>> + $(LIBDRM_CFLAGS) \
>>   $(VALGRIND_CFLAGS) \
>>   $(DEFINES)
>>
>> @@ -131,6 +131,7 @@ VULKAN_LIB_DEPS += \
>>   $(top_builddir)/src/intel/isl/libisl.la \
>>   $(top_builddir)/src/intel/blorp/libblorp.la \
>>   $(PER_GEN_LIBS) \
>> + $(LIBDRM_LIBS) \
>>   $(PTHREAD_LIBS) \
>>   $(DLOPEN_LIBS) \
>>   -lm
>
> Unrelated -> separate patch?
>
It's related: libvulkan_intel.so does not pull libdrm.so as dependency
(up-to this patch). As we use it below, we ought to add the link here.
Otherwise we'll end up with interment breakage.

Note: libvulkan_radeon.so already depends on libdrm, which is why the
hunk is missing from that patch.

>> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
>> index d594df7..9927ac2 100644
>> --- a/src/intel/vulkan/anv_device.c
>> +++ b/src/intel/vulkan/anv_device.c
>> @@ -29,6 +29,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #include "anv_private.h"
>>  #include "util/strtod.h"
>> @@ -375,6 +376,40 @@ void anv_DestroyInstance(
>> vk_free(>alloc, instance);
>>  }
>>
>> +static VkResult
>> +anv_enumerate_devices(struct anv_instance *instance)
>> +{
>> +   /* TODO: Check for more devices ? */
>> +   drmDevicePtr devices[8];
>> +   VkResult result = VK_SUCCESS;
>> +   int max_devices;
>> +
>> +   max_devices = drmGetDevices2(0, devices, sizeof(devices));
>> +   if (max_devices < 1)
>> +  return VK_ERROR_INCOMPATIBLE_DRIVER;
>> +
>> +   for (unsigned i = 0; i < (unsigned)max_devices; i++) {
>> +  if (devices[i]->available_nodes & 1 << DRM_NODE_RENDER &&
>> +  devices[i]->bustype == DRM_BUS_PCI &&
>> +  devices[i]->deviceinfo.pci->vendor_id == 0x8086) {
>
> Yay, magic values!
> I feel like we should replace all those with PCI_VENDOR_INTEL or something.
>
We have another three instances of these for each Vulkan driver.
Barring any objections I'll do that as cleanup on top ?

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

Re: [Mesa-dev] [PATCH 6/6] anv: do not open random render node(s)

[Eric Engestrom]

On Friday, 2016-12-02 16:31:49 +, Emil Velikov wrote:
> From: Emil Velikov 
> 
> drmGetDevices2() provides us with enough flexibility to build heuristics
> upon. Opening a random node on the other hand will wake up the device,
> regardless if it's the one we're intereseted or not.

"interested"
(same in the previous patch)

> 
> Cc: Jason Ekstrand 
> Signed-off-by: Emil Velikov 
> ---
>  src/intel/vulkan/Makefile.am  |  3 ++-
>  src/intel/vulkan/anv_device.c | 53 
> +++
>  2 files changed, 40 insertions(+), 16 deletions(-)
> 
> diff --git a/src/intel/vulkan/Makefile.am b/src/intel/vulkan/Makefile.am
> index df7645f..e309491 100644
> --- a/src/intel/vulkan/Makefile.am
> +++ b/src/intel/vulkan/Makefile.am
> @@ -66,7 +66,7 @@ AM_CPPFLAGS += \
>  endif
>  
>  AM_CPPFLAGS += \
> - $(INTEL_CFLAGS) \
> + $(LIBDRM_CFLAGS) \
>   $(VALGRIND_CFLAGS) \
>   $(DEFINES)
>  
> @@ -131,6 +131,7 @@ VULKAN_LIB_DEPS += \
>   $(top_builddir)/src/intel/isl/libisl.la \
>   $(top_builddir)/src/intel/blorp/libblorp.la \
>   $(PER_GEN_LIBS) \
> + $(LIBDRM_LIBS) \
>   $(PTHREAD_LIBS) \
>   $(DLOPEN_LIBS) \
>   -lm

Unrelated -> separate patch?

> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index d594df7..9927ac2 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "anv_private.h"
>  #include "util/strtod.h"
> @@ -375,6 +376,40 @@ void anv_DestroyInstance(
> vk_free(>alloc, instance);
>  }
>  
> +static VkResult
> +anv_enumerate_devices(struct anv_instance *instance)
> +{
> +   /* TODO: Check for more devices ? */
> +   drmDevicePtr devices[8];
> +   VkResult result = VK_SUCCESS;
> +   int max_devices;
> +
> +   max_devices = drmGetDevices2(0, devices, sizeof(devices));
> +   if (max_devices < 1)
> +  return VK_ERROR_INCOMPATIBLE_DRIVER;
> +
> +   for (unsigned i = 0; i < (unsigned)max_devices; i++) {
> +  if (devices[i]->available_nodes & 1 << DRM_NODE_RENDER &&
> +  devices[i]->bustype == DRM_BUS_PCI &&
> +  devices[i]->deviceinfo.pci->vendor_id == 0x8086) {

Yay, magic values!
I feel like we should replace all those with PCI_VENDOR_INTEL or something.

Anyway, series is
Reviewed-by: Eric Engestrom 

> +
> + result = anv_physical_device_init(>physicalDevice,
> +instance,
> +devices[i]->nodes[DRM_NODE_RENDER]);
> + if (result != VK_ERROR_INCOMPATIBLE_DRIVER)
> +break;
> +  }
> +   }
> +
> +   if (result == VK_ERROR_INCOMPATIBLE_DRIVER)
> +  instance->physicalDeviceCount = 0;
> +   else if (result == VK_SUCCESS)
> +  instance->physicalDeviceCount = 1;
> +
> +   return result;
> +}
> +
> +
>  VkResult anv_EnumeratePhysicalDevices(
>  VkInstance  _instance,
>  uint32_t*   pPhysicalDeviceCount,
> @@ -384,22 +419,10 @@ VkResult anv_EnumeratePhysicalDevices(
> VkResult result;
>  
> if (instance->physicalDeviceCount < 0) {
> -  char path[20];
> -  for (unsigned i = 0; i < 8; i++) {
> - snprintf(path, sizeof(path), "/dev/dri/renderD%d", 128 + i);
> - result = anv_physical_device_init(>physicalDevice,
> -   instance, path);
> - if (result != VK_ERROR_INCOMPATIBLE_DRIVER)
> -break;
> -  }
> -
> -  if (result == VK_ERROR_INCOMPATIBLE_DRIVER) {
> - instance->physicalDeviceCount = 0;
> -  } else if (result == VK_SUCCESS) {
> - instance->physicalDeviceCount = 1;
> -  } else {
> +  result = anv_enumerate_devices(instance);
> +  if (result != VK_SUCCESS &&
> +  result != VK_ERROR_INCOMPATIBLE_DRIVER)
>   return result;
> -  }
> }
>  
> /* pPhysicalDeviceCount is an out parameter if pPhysicalDevices is NULL;
> -- 
> 2.10.2
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 6/6] anv: do not open random render node(s)

[Emil Velikov]

From: Emil Velikov 

drmGetDevices2() provides us with enough flexibility to build heuristics
upon. Opening a random node on the other hand will wake up the device,
regardless if it's the one we're intereseted or not.

Cc: Jason Ekstrand 
Signed-off-by: Emil Velikov 
---
 src/intel/vulkan/Makefile.am  |  3 ++-
 src/intel/vulkan/anv_device.c | 53 +++
 2 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/src/intel/vulkan/Makefile.am b/src/intel/vulkan/Makefile.am
index df7645f..e309491 100644
--- a/src/intel/vulkan/Makefile.am
+++ b/src/intel/vulkan/Makefile.am
@@ -66,7 +66,7 @@ AM_CPPFLAGS += \
 endif
 
 AM_CPPFLAGS += \
-   $(INTEL_CFLAGS) \
+   $(LIBDRM_CFLAGS) \
$(VALGRIND_CFLAGS) \
$(DEFINES)
 
@@ -131,6 +131,7 @@ VULKAN_LIB_DEPS += \
$(top_builddir)/src/intel/isl/libisl.la \
$(top_builddir)/src/intel/blorp/libblorp.la \
$(PER_GEN_LIBS) \
+   $(LIBDRM_LIBS) \
$(PTHREAD_LIBS) \
$(DLOPEN_LIBS) \
-lm
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index d594df7..9927ac2 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "anv_private.h"
 #include "util/strtod.h"
@@ -375,6 +376,40 @@ void anv_DestroyInstance(
vk_free(>alloc, instance);
 }
 
+static VkResult
+anv_enumerate_devices(struct anv_instance *instance)
+{
+   /* TODO: Check for more devices ? */
+   drmDevicePtr devices[8];
+   VkResult result = VK_SUCCESS;
+   int max_devices;
+
+   max_devices = drmGetDevices2(0, devices, sizeof(devices));
+   if (max_devices < 1)
+  return VK_ERROR_INCOMPATIBLE_DRIVER;
+
+   for (unsigned i = 0; i < (unsigned)max_devices; i++) {
+  if (devices[i]->available_nodes & 1 << DRM_NODE_RENDER &&
+  devices[i]->bustype == DRM_BUS_PCI &&
+  devices[i]->deviceinfo.pci->vendor_id == 0x8086) {
+
+ result = anv_physical_device_init(>physicalDevice,
+instance,
+devices[i]->nodes[DRM_NODE_RENDER]);
+ if (result != VK_ERROR_INCOMPATIBLE_DRIVER)
+break;
+  }
+   }
+
+   if (result == VK_ERROR_INCOMPATIBLE_DRIVER)
+  instance->physicalDeviceCount = 0;
+   else if (result == VK_SUCCESS)
+  instance->physicalDeviceCount = 1;
+
+   return result;
+}
+
+
 VkResult anv_EnumeratePhysicalDevices(
 VkInstance  _instance,
 uint32_t*   pPhysicalDeviceCount,
@@ -384,22 +419,10 @@ VkResult anv_EnumeratePhysicalDevices(
VkResult result;
 
if (instance->physicalDeviceCount < 0) {
-  char path[20];
-  for (unsigned i = 0; i < 8; i++) {
- snprintf(path, sizeof(path), "/dev/dri/renderD%d", 128 + i);
- result = anv_physical_device_init(>physicalDevice,
-   instance, path);
- if (result != VK_ERROR_INCOMPATIBLE_DRIVER)
-break;
-  }
-
-  if (result == VK_ERROR_INCOMPATIBLE_DRIVER) {
- instance->physicalDeviceCount = 0;
-  } else if (result == VK_SUCCESS) {
- instance->physicalDeviceCount = 1;
-  } else {
+  result = anv_enumerate_devices(instance);
+  if (result != VK_SUCCESS &&
+  result != VK_ERROR_INCOMPATIBLE_DRIVER)
  return result;
-  }
}
 
/* pPhysicalDeviceCount is an out parameter if pPhysicalDevices is NULL;
-- 
2.10.2

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