xf86drm: continue after drmProcessPlatformDevice failure
diff mbox

Message ID 20170719153706.1535-1-gurchetansingh@chromium.org
State New
Headers show

Commit Message

Gurchetan Singh July 19, 2017, 3:37 p.m. UTC
On ChromeOS devices, readdir() processes the directory in
the following order:

-NAME-              -TYPE-
.                    n/a
..                   n/a
vgem                 n/a
card1           DRM_BUS_PLATFORM
renderD129      DRM_BUS_PLATFORM
card0             DRM_BUS_PCI
renderD128        DRM_BUS_PCI
controlD64        DRM_BUS_PCI

In drmGetDevices2, after drmProcessPlatformDevice fails for
/dev/dri/card1, we don't process the remaining directory entries.
As such, Vulkan fails to initialize since Mesa uses drmGetDevices2.
To fix this, continue if drmProcessPlatformDevice fails.
---
 xf86drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Emil Velikov July 20, 2017, 11:33 a.m. UTC | #1
On 19 July 2017 at 16:37, Gurchetan Singh <gurchetansingh@chromium.org> wrote:
> On ChromeOS devices, readdir() processes the directory in
> the following order:
>
> -NAME-              -TYPE-
> .                    n/a
> ..                   n/a
> vgem                 n/a
> card1           DRM_BUS_PLATFORM
> renderD129      DRM_BUS_PLATFORM
> card0             DRM_BUS_PCI
> renderD128        DRM_BUS_PCI
> controlD64        DRM_BUS_PCI
>
> In drmGetDevices2, after drmProcessPlatformDevice fails for
> /dev/dri/card1, we don't process the remaining directory entries.
> As such, Vulkan fails to initialize since Mesa uses drmGetDevices2.
> To fix this, continue if drmProcessPlatformDevice fails.

Added a fixes tag and addressed the host1x + usb instances.

Thank you!
Emil
Thierry Reding July 20, 2017, 1:47 p.m. UTC | #2
On Wed, Jul 19, 2017 at 08:37:06AM -0700, Gurchetan Singh wrote:
> On ChromeOS devices, readdir() processes the directory in
> the following order:
> 
> -NAME-              -TYPE-
> .                    n/a
> ..                   n/a
> vgem                 n/a
> card1           DRM_BUS_PLATFORM
> renderD129      DRM_BUS_PLATFORM
> card0             DRM_BUS_PCI
> renderD128        DRM_BUS_PCI
> controlD64        DRM_BUS_PCI
> 
> In drmGetDevices2, after drmProcessPlatformDevice fails for
> /dev/dri/card1, we don't process the remaining directory entries.
> As such, Vulkan fails to initialize since Mesa uses drmGetDevices2.
> To fix this, continue if drmProcessPlatformDevice fails.
> ---
>  xf86drm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I'm not sure this is a good fix. drmProcessPlatformDevice() is only
supposed to fail in fatal situations, such as out-of-memory. So the
reason it doesn't continue is that it doesn't make sense to. In the
case of out-of-memory situations, for example, the next allocation
is assumed to fail as well.

Now, reading the code again, maybe the reason it is failing is because
the code tries to look for OF_FULLNAME and OF_COMPATIBLE_N and may not
find them. I suspect that card1 and renderD129 are from a vgem device,
which is, as far as I can tell, the only case of a platform device
without an OF node.

In that case I think we should fix drmProcessPlatformDevice() to deal
with those cases instead (make the list of compatibles optional, and
fallback to some other mechanism to determine the name). Otherwise we
will be silently ignoring errors and skipping devices that we really
shouldn't.

Thierry

Patch
diff mbox

diff --git a/xf86drm.c b/xf86drm.c
index 879f85b6..50862f22 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3997,7 +3997,7 @@  int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
             ret = drmProcessPlatformDevice(&device, node, node_type, maj, min,
                                            devices != NULL, flags);
             if (ret)
-                goto free_devices;
+                continue;
 
             break;