diff mbox series

xf86drm: add drmOpenByFB

Message ID 20200523154426.1088988-1-issor.oruam@gmail.com (mailing list archive)
State New, archived
Headers show
Series xf86drm: add drmOpenByFB | expand

Commit Message

Mauro Rossi May 23, 2020, 3:44 p.m. UTC
OpenByFB is introduced to overcome GPU driver loading order issue
on a device with multiple GPUs, e.g. Intel iGPU and Nvidia dGPU
where the first drmfb kernel module loaded will become device file
/dev/dri/card0 and the second will become /dev/dri/card1

The use case is to prefer Intel iGPU over dGPU, or viceversa,
in a deterministic and reliable manner.

OpenByFB function opens the DRM device with specified fb and node type,
thus enabling gralloc to open the correct device node of the primary fb,
regardless of the GPU driver loading order.

Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
---
 core-symbols.txt |  1 +
 xf86drm.c        | 42 ++++++++++++++++++++++++++++++++++++++++++
 xf86drm.h        |  1 +
 3 files changed, 44 insertions(+)

Comments

Simon Ser May 23, 2020, 9:36 p.m. UTC | #1
On Saturday, May 23, 2020 5:44 PM, Mauro Rossi <issor.oruam@gmail.com> wrote:

> OpenByFB is introduced to overcome GPU driver loading order issue
> on a device with multiple GPUs, e.g. Intel iGPU and Nvidia dGPU
> where the first drmfb kernel module loaded will become device file
> /dev/dri/card0 and the second will become /dev/dri/card1
>
> The use case is to prefer Intel iGPU over dGPU, or viceversa,
> in a deterministic and reliable manner.
>
> OpenByFB function opens the DRM device with specified fb and node type,
> thus enabling gralloc to open the correct device node of the primary fb,
> regardless of the GPU driver loading order.

"FB" is ambiguous here, because the other FB functions in this file
refer to DRM frame-buffer objects (as opposed to fbdev I believe?).
Daniel Vetter May 24, 2020, 6:53 p.m. UTC | #2
On Sat, May 23, 2020 at 5:44 PM Mauro Rossi <issor.oruam@gmail.com> wrote:
>
> OpenByFB is introduced to overcome GPU driver loading order issue
> on a device with multiple GPUs, e.g. Intel iGPU and Nvidia dGPU
> where the first drmfb kernel module loaded will become device file
> /dev/dri/card0 and the second will become /dev/dri/card1
>
> The use case is to prefer Intel iGPU over dGPU, or viceversa,
> in a deterministic and reliable manner.
>
> OpenByFB function opens the DRM device with specified fb and node type,
> thus enabling gralloc to open the correct device node of the primary fb,
> regardless of the GPU driver loading order.

The fbdev load ordering is as inconsistent/random as the dri node
loading. Well more so, because you might have random firmware fbdev
drivers hanging out there. Hence I'm not following how this solves
anything for your problem.

I think usually what userspace does it look at the boot_vga property
of the underlying device in sysfs, and prefer that one. Iirc (but
might be wrong) that's patched to essentially mean "primary display
device, the same the firmware used for boot-up screens". Either way I
think the proper way to solve this is you figure out matching rules
you care about using udev/sysfs, and then use that information to open
the right drm node. Opening by fbdev seems a most convoluted way to
get to a drm device (we're trying to deprecate fbdev outright as much
as possible), and you could even match for any specific device with
sysfs already if that's what you're doing using udev rules.
-Daniel


>
> Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
> ---
>  core-symbols.txt |  1 +
>  xf86drm.c        | 42 ++++++++++++++++++++++++++++++++++++++++++
>  xf86drm.h        |  1 +
>  3 files changed, 44 insertions(+)
>
> diff --git a/core-symbols.txt b/core-symbols.txt
> index 1ff4ecaa..6bf8c70d 100644
> --- a/core-symbols.txt
> +++ b/core-symbols.txt
> @@ -146,6 +146,7 @@ drmModeSetCursor2
>  drmModeSetPlane
>  drmMsg
>  drmOpen
> +drmOpenByFB
>  drmOpenControl
>  drmOpenOnce
>  drmOpenOnceWithType
> diff --git a/xf86drm.c b/xf86drm.c
> index b49d42f7..229a54bf 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -793,6 +793,48 @@ drm_public int drmOpenRender(int minor)
>      return drmOpenMinor(minor, 0, DRM_NODE_RENDER);
>  }
>
> +/**
> + * Open the DRM device with specified type of specified framebuffer.
> + *
> + * Looks up the associated DRM device with specified type of the
> + * specified framebuffer and opens it.
> + *
> + * \param fb the index of framebuffer.
> + * \param type the device node type to open, PRIMARY, CONTROL or RENDER
> + *
> + * \return a file descriptor on success, or a negative value on error.
> + *
> + */
> +drm_public int drmOpenByFB(int fb, int type)
> +{
> +#ifdef __linux__
> +    DIR *sysdir;
> +    struct dirent *ent;
> +    char buf[64];
> +    const char *name = drmGetMinorName(type);
> +    int fd = -1, len = strlen(name);
> +
> +    snprintf(buf, sizeof(buf), "/sys/class/graphics/fb%d/device/drm", fb);
> +    sysdir = opendir(buf);
> +    if (!sysdir)
> +        return -errno;
> +
> +    while ((ent = readdir(sysdir))) {
> +        if (!strncmp(ent->d_name, name, len)) {
> +            snprintf(buf, sizeof(buf), "%s/%s", DRM_DIR_NAME, ent->d_name);
> +            fd = open(buf, O_RDWR | O_CLOEXEC, 0);
> +            break;
> +        }
> +    }
> +
> +    closedir(sysdir);
> +    return fd;
> +#else
> +#warning "Missing implementation of drmOpenByFB"
> +    return -EINVAL;
> +#endif
> +}
> +
>  /**
>   * Free the version information returned by drmGetVersion().
>   *
> diff --git a/xf86drm.h b/xf86drm.h
> index 7b85079a..d45d696f 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -605,6 +605,7 @@ extern int           drmOpenWithType(const char *name, const char *busid,
>
>  extern int           drmOpenControl(int minor);
>  extern int           drmOpenRender(int minor);
> +extern int           drmOpenByFB(int fb, int type);
>  extern int           drmClose(int fd);
>  extern drmVersionPtr drmGetVersion(int fd);
>  extern drmVersionPtr drmGetLibVersion(int fd);
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Simon Ser May 24, 2020, 7:25 p.m. UTC | #3
On Sunday, May 24, 2020 8:53 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Sat, May 23, 2020 at 5:44 PM Mauro Rossi issor.oruam@gmail.com wrote:
>
> > OpenByFB is introduced to overcome GPU driver loading order issue
> > on a device with multiple GPUs, e.g. Intel iGPU and Nvidia dGPU
> > where the first drmfb kernel module loaded will become device file
> > /dev/dri/card0 and the second will become /dev/dri/card1
> > The use case is to prefer Intel iGPU over dGPU, or viceversa,
> > in a deterministic and reliable manner.
> > OpenByFB function opens the DRM device with specified fb and node type,
> > thus enabling gralloc to open the correct device node of the primary fb,
> > regardless of the GPU driver loading order.
>
> The fbdev load ordering is as inconsistent/random as the dri node
> loading. Well more so, because you might have random firmware fbdev
> drivers hanging out there. Hence I'm not following how this solves
> anything for your problem.
>
> I think usually what userspace does it look at the boot_vga property
> of the underlying device in sysfs, and prefer that one.

Yes. See [1] for an example of how to do this.

[1]: https://github.com/swaywm/wlroots/blob/5c942bd5972afee9a68cb15c14aa83b4b0aaf82d/backend/session/session.c#L331
Chih-Wei Huang May 28, 2020, 9:46 a.m. UTC | #4
Simon Ser <contact@emersion.fr> 於 2020年5月25日 週一 上午3:25寫道:
> On Sunday, May 24, 2020 8:53 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Sat, May 23, 2020 at 5:44 PM Mauro Rossi issor.oruam@gmail.com wrote:
> >
> > > OpenByFB is introduced to overcome GPU driver loading order issue
> > > on a device with multiple GPUs, e.g. Intel iGPU and Nvidia dGPU
> > > where the first drmfb kernel module loaded will become device file
> > > /dev/dri/card0 and the second will become /dev/dri/card1
> > > The use case is to prefer Intel iGPU over dGPU, or viceversa,
> > > in a deterministic and reliable manner.
> > > OpenByFB function opens the DRM device with specified fb and node type,
> > > thus enabling gralloc to open the correct device node of the primary fb,
> > > regardless of the GPU driver loading order.
> >
> > The fbdev load ordering is as inconsistent/random as the dri node
> > loading. Well more so, because you might have random firmware fbdev
> > drivers hanging out there. Hence I'm not following how this solves
> > anything for your problem.
> >
> > I think usually what userspace does it look at the boot_vga property
> > of the underlying device in sysfs, and prefer that one.

Thank you for the reply.
I'm not the expert so
I can't fully understand what you mean.
What does 'boot_vga' mean and how could it help our problem?

The main problem we're trying to solve is to
find the DRM device of the primary framebuffer (fb0).
I believe /sys/class/graphics/fb0/device/drm
is the correct one we could use.
At least it works on all devices I've tested
including nvidia optimus notebooks.
If there is a simpler way to get the DRM device of fb0
I'm happy to learn.

> Yes. See [1] for an example of how to do this.
>
> [1]: https://github.com/swaywm/wlroots/blob/5c942bd5972afee9a68cb15c14aa83b4b0aaf82d/backend/session/session.c#L331

Thank you for the example.
However, our target platform is Android and
Android doesn't have udev. So I'm afraid we can't use it.
Emil Velikov May 28, 2020, 12:38 p.m. UTC | #5
On Thu, 28 May 2020 at 10:46, /wrote:
>
> Simon Ser <contact@emersion.fr> 於 2020年5月25日 週一 上午3:25寫道:
> > On Sunday, May 24, 2020 8:53 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > On Sat, May 23, 2020 at 5:44 PM Mauro Rossi issor.oruam@gmail.com wrote:
> > >
> > > > OpenByFB is introduced to overcome GPU driver loading order issue
> > > > on a device with multiple GPUs, e.g. Intel iGPU and Nvidia dGPU
> > > > where the first drmfb kernel module loaded will become device file
> > > > /dev/dri/card0 and the second will become /dev/dri/card1
> > > > The use case is to prefer Intel iGPU over dGPU, or viceversa,
> > > > in a deterministic and reliable manner.
> > > > OpenByFB function opens the DRM device with specified fb and node type,
> > > > thus enabling gralloc to open the correct device node of the primary fb,
> > > > regardless of the GPU driver loading order.
> > >
> > > The fbdev load ordering is as inconsistent/random as the dri node
> > > loading. Well more so, because you might have random firmware fbdev
> > > drivers hanging out there. Hence I'm not following how this solves
> > > anything for your problem.
> > >
> > > I think usually what userspace does it look at the boot_vga property
> > > of the underlying device in sysfs, and prefer that one.
>
> Thank you for the reply.
> I'm not the expert so
> I can't fully understand what you mean.
> What does 'boot_vga' mean and how could it help our problem?
>
The boot_vga is a flag which indicating which VGA (graphics) device
the system booted from.

> The main problem we're trying to solve is to
> find the DRM device of the primary framebuffer (fb0).
> I believe /sys/class/graphics/fb0/device/drm
> is the correct one we could use.
> At least it works on all devices I've tested
> including nvidia optimus notebooks.
> If there is a simpler way to get the DRM device of fb0
> I'm happy to learn.
>
> > Yes. See [1] for an example of how to do this.
> >
> > [1]: https://github.com/swaywm/wlroots/blob/5c942bd5972afee9a68cb15c14aa83b4b0aaf82d/backend/session/session.c#L331
>
> Thank you for the example.
> However, our target platform is Android and
> Android doesn't have udev. So I'm afraid we can't use it.

Indeed - udev isn't as wide-spread as it could have been. A quick WIP
should be in your inbox.
It implements boot_vga support for drmDevice.

Do give it some test/love and resubmit once it works on your end.

-Emil
Pekka Paalanen May 29, 2020, 7:48 a.m. UTC | #6
On Thu, 28 May 2020 17:46:08 +0800
Chih-Wei Huang <cwhuang@linux.org.tw> wrote:

> The main problem we're trying to solve is to
> find the DRM device of the primary framebuffer (fb0).

Hi,

I would say that is a completely wrong starting point. Please, let
fbdev die in peace/pieces. Do not make any new code that relies on
fbdev existing.

Why do you think you want to follow the setup fbdev has? How do you
know fb0 is the device you want and not fb1? How do you guarantee that
fb0 is the device you want?

"It was right on where I tested it" is no guarantee if you do not
understand how it actually is chosen under the hood. If you know how it
is chosen under the hood, you can do the same yourself without relying
on deprecated stuff (fbdev).

It is fbdev that should follow the DRM setup, not pick a DRM device
based on fbdev.

People already mentioned looking at device properties via libudev API.
If you cannot have libudev (I believe it does not need udev daemon,
btw.), then you can look at the same information directly in sysfs. Use
the information about the DRM devices themselves from sysfs to decide
which one to pick, and never look at fbdev anything. Or polish the
patch Emil proposed if boot_vga indeed matches what you actually want
to find.

I think Daniel Vetter explained nicely what boot_vga means. Is your
problem that the device may not be a PCI device, but a platform device
for instance?

Display servers use heuristics, for example if no device has boot_vga,
then pick the platform device (since there usually is only one with KMS
capabilities). Here are couple of examples.

Weston wants a device with KMS capabilities, because it currently
doesn't support using more than one KMS device and it also doesn't
explicitly support a separate render device:
https://gitlab.freedesktop.org/wayland/weston/-/blob/8.0.0/libweston/backend-drm/drm.c#L2546-2631

Mutter is primarily looking for a hardware rendering capable device,
because it can use any number of KMS devices in addition to a rendering
device, separate or same device:
https://gitlab.gnome.org/GNOME/mutter/-/blob/3.37.1/src/backends/native/meta-renderer-native.c#L3904-3953

Neither is probably perfect, but they are totally better than picking
based on fb0.


Thanks,
pq
Alex Deucher May 29, 2020, 2:20 p.m. UTC | #7
On Fri, May 29, 2020 at 3:49 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Thu, 28 May 2020 17:46:08 +0800
> Chih-Wei Huang <cwhuang@linux.org.tw> wrote:
>
> > The main problem we're trying to solve is to
> > find the DRM device of the primary framebuffer (fb0).
>
> Hi,
>
> I would say that is a completely wrong starting point. Please, let
> fbdev die in peace/pieces. Do not make any new code that relies on
> fbdev existing.
>
> Why do you think you want to follow the setup fbdev has? How do you
> know fb0 is the device you want and not fb1? How do you guarantee that
> fb0 is the device you want?
>
> "It was right on where I tested it" is no guarantee if you do not
> understand how it actually is chosen under the hood. If you know how it
> is chosen under the hood, you can do the same yourself without relying
> on deprecated stuff (fbdev).
>
> It is fbdev that should follow the DRM setup, not pick a DRM device
> based on fbdev.
>
> People already mentioned looking at device properties via libudev API.
> If you cannot have libudev (I believe it does not need udev daemon,
> btw.), then you can look at the same information directly in sysfs. Use
> the information about the DRM devices themselves from sysfs to decide
> which one to pick, and never look at fbdev anything. Or polish the
> patch Emil proposed if boot_vga indeed matches what you actually want
> to find.
>
> I think Daniel Vetter explained nicely what boot_vga means. Is your
> problem that the device may not be a PCI device, but a platform device
> for instance?
>
> Display servers use heuristics, for example if no device has boot_vga,
> then pick the platform device (since there usually is only one with KMS
> capabilities). Here are couple of examples.
>
> Weston wants a device with KMS capabilities, because it currently
> doesn't support using more than one KMS device and it also doesn't
> explicitly support a separate render device:
> https://gitlab.freedesktop.org/wayland/weston/-/blob/8.0.0/libweston/backend-drm/drm.c#L2546-2631
>
> Mutter is primarily looking for a hardware rendering capable device,
> because it can use any number of KMS devices in addition to a rendering
> device, separate or same device:
> https://gitlab.gnome.org/GNOME/mutter/-/blob/3.37.1/src/backends/native/meta-renderer-native.c#L3904-3953
>
> Neither is probably perfect, but they are totally better than picking
> based on fb0.

As an example, on my system with multiple GPUs, the one that the bios
lights up is the last one Linux enumerates when loading the GPU
drivers.

Alex
diff mbox series

Patch

diff --git a/core-symbols.txt b/core-symbols.txt
index 1ff4ecaa..6bf8c70d 100644
--- a/core-symbols.txt
+++ b/core-symbols.txt
@@ -146,6 +146,7 @@  drmModeSetCursor2
 drmModeSetPlane
 drmMsg
 drmOpen
+drmOpenByFB
 drmOpenControl
 drmOpenOnce
 drmOpenOnceWithType
diff --git a/xf86drm.c b/xf86drm.c
index b49d42f7..229a54bf 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -793,6 +793,48 @@  drm_public int drmOpenRender(int minor)
     return drmOpenMinor(minor, 0, DRM_NODE_RENDER);
 }
 
+/**
+ * Open the DRM device with specified type of specified framebuffer.
+ *
+ * Looks up the associated DRM device with specified type of the
+ * specified framebuffer and opens it.
+ *
+ * \param fb the index of framebuffer.
+ * \param type the device node type to open, PRIMARY, CONTROL or RENDER
+ *
+ * \return a file descriptor on success, or a negative value on error.
+ *
+ */
+drm_public int drmOpenByFB(int fb, int type)
+{
+#ifdef __linux__
+    DIR *sysdir;
+    struct dirent *ent;
+    char buf[64];
+    const char *name = drmGetMinorName(type);
+    int fd = -1, len = strlen(name);
+
+    snprintf(buf, sizeof(buf), "/sys/class/graphics/fb%d/device/drm", fb);
+    sysdir = opendir(buf);
+    if (!sysdir)
+        return -errno;
+
+    while ((ent = readdir(sysdir))) {
+        if (!strncmp(ent->d_name, name, len)) {
+            snprintf(buf, sizeof(buf), "%s/%s", DRM_DIR_NAME, ent->d_name);
+            fd = open(buf, O_RDWR | O_CLOEXEC, 0);
+            break;
+        }
+    }
+
+    closedir(sysdir);
+    return fd;
+#else
+#warning "Missing implementation of drmOpenByFB"
+    return -EINVAL;
+#endif
+}
+
 /**
  * Free the version information returned by drmGetVersion().
  *
diff --git a/xf86drm.h b/xf86drm.h
index 7b85079a..d45d696f 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -605,6 +605,7 @@  extern int           drmOpenWithType(const char *name, const char *busid,
 
 extern int           drmOpenControl(int minor);
 extern int           drmOpenRender(int minor);
+extern int           drmOpenByFB(int fb, int type);
 extern int           drmClose(int fd);
 extern drmVersionPtr drmGetVersion(int fd);
 extern drmVersionPtr drmGetLibVersion(int fd);