Review of attachment 311192 [details] [review]:

::: src/backends/native/meta-launcher.c
@@ +290,3 @@
   g_udev_enumerator_add_match_name (enumerator, "card*");
   g_udev_enumerator_add_match_tag (enumerator, "seat");
+  g_udev_enumerator_add_match_subsystem (enumerator, "drm");

unless i'm missing something, this shouldn't be necessary.
Note, "drm" is passed to g_udev_client_new().

@@ +328,2 @@
       pci_device = g_udev_device_get_parent_with_subsystem (dev, "pci", NULL);
+      if (pci_device) is_pci = TRUE;

I think I'd just scrap the booleans.  You're using the device variables as booleans anyway...

@@ +331,1 @@
+      platform_device = g_udev_device_get_parent_with_subsystem (dev, "platform", NULL);

probably should only do this if pci_device is NULL. (granted I guess in practice there won't ever be both)

@@ +334,2 @@
+      if (!is_platform && !is_pci)
+        continue;

I think it'd just scrap this too, the only thing it's bypassing is two condition checks, but in order to know it can do that, it has to check the same two conditions.

@@ +335,3 @@
+        continue;
+
+      if (is_pci)

so this would just be if (pci_device)

@@ +349,3 @@
+        }
+
+      if (is_platform)

and this one should just be: else if (platform_device)

@@ +354,3 @@
+          gboolean is_control = FALSE;
+
+          if (strstr(device_name, "control")) is_control = TRUE;

unless i'm missing something, this should never happen since we did

 g_udev_enumerator_add_match_name (enumerator, "card*");

above, right?

interesting, there's a similar (in spirit) patch for Xorg on list right now...

https://www.mail-archive.com/xorg-devel@lists.x.org/msg46448.html

(In reply to Ray Strode [halfline] from comment #1)
> Review of attachment 311192 [details] [review] [review]:
> @@ +331,1 @@
> +      platform_device = g_udev_device_get_parent_with_subsystem (dev,
> "platform", NULL);
> 
> probably should only do this if pci_device is NULL. (granted I guess in
> practice there won't ever be both)

Actually, this isn't true: Tegra combines a platform display controller (tegradrm) with a PCIE GPU (nouveau). But as we know, that's another can of worms. ;)

so actually it should be the other way around then? pci should only be tried if platform comes up empty?

Yeah, I'd guess that would be safer.

In Xserver world platform devices are checked first. PCI scan is done only if there is nothing on platform.

boot_vga stuff on aarch64 (xserver patch) will be handled on kernel side in same way as powerpc has it done.

Marcin though in the X server world platform devices are something different.  platform devices are devices where the driver uses logind to get the drm fd.  we do that in all cases with mutter on wayland.

prahal are you going to pick this back up?

Created attachment 315356 [details] [review]
launcher: Fix drm device detection for non pci devices

On Odroid U2 (exynos4412) the drm device is not bound to pci.
Open the detection to platform device of the drm subsystem, exclusive of
control devices.

so the above patch incorporates the changes I mentioned in the review. I haven't tried it yet.

Review of attachment 315356 [details] [review]:

::: src/backends/native/meta-launcher.c
@@ +339,3 @@
+          /* get value of boot_vga attribute or 0 if the device has no boot_vga */
+          boot_vga = g_udev_device_get_sysfs_attr_as_int (pci_device, "boot_vga");
+          g_object_unref (pci_device);

this unref and the one in the other side of the if are in the wrong place, so they'll leak

Created attachment 315357 [details] [review]
launcher: Fix drm device detection for non pci devices

On Odroid U2 (exynos4412) the drm device is not bound to pci.
Open the detection to platform device of the drm subsystem, exclusive of
control devices.

i had rob clark try this on one of his arm machines. seems to check out, so I'm going to push it.

Attachment 315357 [details] pushed as ca7c1d5 - launcher: Fix drm device detection for non pci devices