Jason Ekstrand <ja...@jlekstrand.net> writes:

>> +   if (instance->enabled_extensions.KHR_display) {
>> +      master_fd = open(path, O_RDWR | O_CLOEXEC);
>>
>
> Is this supposed to be opening primary_path instead?

Yes, and this section of code needs to occur before anv_init_wsi.

I appear to have skipped testing this path on ANV and only tested it on
RADV -- RADV has the code in the right order. Thanks for catching this;
sorry I messed up and didn't test it.

> This could just be
>
> if (anv_gem_get_param(master_fd, I915_PARAM_CHIPSET_ID) == 0) {
>    close(master_fd);
>    master_fd = -1;
> }
>
> No need to type out all that IOCTL stuff.

Thanks, that's lots shorter (RADV doesn't appear to have a similar helper).

Here's an amendment to the proposed patch which fixes the bug and
switches to the simpler detection method.

From f4dac824a2566367cc3c66e1eda27fe4aaf64543 Mon Sep 17 00:00:00 2001
From: Keith Packard <kei...@keithp.com>
Date: Thu, 14 Jun 2018 11:31:20 -0700
Subject: [PATCH] anv: Open DRM master before initializing WSI layer. Close on
 device_finish.

The DRM master FD is passed to the WSI layer during initialization, so
we need to open the device slightly earlier in the function.

Also, close the DRM master FD when the driver is being shut down.

v2:
	Use anv_gem_get_param to detect working master_fd

Signed-off-by: Keith Packard <kei...@keithp.com>
---
 src/intel/vulkan/anv_device.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index b3c6d1a8722..3507a91810f 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -437,36 +437,32 @@ anv_physical_device_init(struct anv_physical_device *device,
    if (result != VK_SUCCESS)
       goto fail;
 
-   result = anv_init_wsi(device);
-   if (result != VK_SUCCESS) {
-      ralloc_free(device->compiler);
-      goto fail;
-   }
-
-   anv_physical_device_get_supported_extensions(device,
-                                                &device->supported_extensions);
-
    if (instance->enabled_extensions.KHR_display) {
-      master_fd = open(path, O_RDWR | O_CLOEXEC);
+      master_fd = open(primary_path, O_RDWR | O_CLOEXEC);
       if (master_fd >= 0) {
          /* prod the device with a GETPARAM call which will fail if
           * we don't have permission to even render on this device
           */
-         drm_i915_getparam_t gp;
-         memset(&gp, '\0', sizeof(gp));
-         int devid = 0;
-         gp.param = I915_PARAM_CHIPSET_ID;
-         gp.value = &devid;
-         int ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-         if (ret < 0) {
+         if (anv_gem_get_param(master_fd, I915_PARAM_CHIPSET_ID) == 0) {
             close(master_fd);
             master_fd = -1;
          }
       }
    }
+   device->master_fd = master_fd;
+
+   result = anv_init_wsi(device);
+   if (result != VK_SUCCESS) {
+      ralloc_free(device->compiler);
+      goto fail;
+   }
+
+   anv_physical_device_get_supported_extensions(device,
+                                                &device->supported_extensions);
+
 
    device->local_fd = fd;
-   device->master_fd = master_fd;
+
    return VK_SUCCESS;
 
 fail:
@@ -482,6 +478,8 @@ anv_physical_device_finish(struct anv_physical_device *device)
    anv_finish_wsi(device);
    ralloc_free(device->compiler);
    close(device->local_fd);
+   if (device->master_fd >= 0)
+      close(device->master_fd);
 }
 
 static void *
-- 
2.17.1

-- 
-keith

Attachment: signature.asc
Description: PGP signature

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

Reply via email to