egl: Make EGL_EXT_device_drm optional.

Submitted by Mathias Fröhlich on June 28, 2019, 1:30 p.m.

Details

Message ID 29fc028310efa45775133ea3ecb3d73057e79b67.1561715595.git.Mathias.Froehlich@gmx.net
State New
Headers show
Series "egl: Make EGL_EXT_device_drm optional." ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Mathias Fröhlich June 28, 2019, 1:30 p.m.
From: Mathias Fröhlich <mathias.froehlich@web.de>

Hi,

Ilia mentioned that there are drm rendernode only drivers out there.
To support an egl device on those platforms, make the EGL_EXT_device_drm
device extension optional.

Please review
best and thanks

Mathias



Relax drm device requirement to just have a render node to create
an EGLDevice. Off screen rendering is just possible with a render
node without having a master device node available.
To enable the EGL_EXT_device_drm device extension also check for
the primary node.

Signed-off-by: Mathias Fröhlich <Mathias.Froehlich@web.de>
---
 src/egl/drivers/dri2/platform_device.c |  7 ++-----
 src/egl/main/egldevice.c               | 25 +++++++++++++++++++------
 src/egl/main/egldevice.h               |  3 +++
 3 files changed, 24 insertions(+), 11 deletions(-)

--
2.21.0

Patch hide | download patch | download mbox

diff --git a/src/egl/drivers/dri2/platform_device.c b/src/egl/drivers/dri2/platform_device.c
index 6b80a3869b3..1213fa89337 100644
--- a/src/egl/drivers/dri2/platform_device.c
+++ b/src/egl/drivers/dri2/platform_device.c
@@ -394,15 +394,12 @@  dri2_initialize_device(_EGLDriver *drv, _EGLDisplay *disp)
    disp->Device = dev;
    disp->DriverData = (void *) dri2_dpy;
    err = "DRI2: failed to load driver";
-   if (_eglDeviceSupports(dev, _EGL_DEVICE_DRM)) {
+   if (!_eglDeviceIsSoftware(dev)) {
       if (!device_probe_device(disp))
          goto cleanup;
-   } else if (_eglDeviceSupports(dev, _EGL_DEVICE_SOFTWARE)) {
+   } else {
       if (!device_probe_device_sw(disp))
          goto cleanup;
-   } else {
-      _eglLog(_EGL_FATAL, "Driver bug: exposed device is neither DRM nor SOFTWARE one");
-      return EGL_FALSE;
    }

    if (!dri2_create_screen(disp)) {
diff --git a/src/egl/main/egldevice.c b/src/egl/main/egldevice.c
index 99d8a6c1886..70b596bc8ac 100644
--- a/src/egl/main/egldevice.c
+++ b/src/egl/main/egldevice.c
@@ -45,6 +45,8 @@  struct _egl_device {
    EGLBoolean MESA_device_software;
    EGLBoolean EXT_device_drm;

+   EGLBoolean is_software;
+
 #ifdef HAVE_LIBDRM
    drmDevicePtr device;
 #endif
@@ -69,8 +71,8 @@  _eglFiniDevice(void)
       dev = dev_list;
       dev_list = dev_list->Next;

+      assert(!_eglDeviceIsSoftware(dev));
 #ifdef HAVE_LIBDRM
-      assert(_eglDeviceSupports(dev, _EGL_DEVICE_DRM));
       drmFreeDevice(&dev->device);
 #endif
       free(dev);
@@ -98,6 +100,7 @@  _eglCheckDeviceHandle(EGLDeviceEXT device)
 _EGLDevice _eglSoftwareDevice = {
    .extensions = "EGL_MESA_device_software",
    .MESA_device_software = EGL_TRUE,
+   .is_software = EGL_TRUE,
 };

 #ifdef HAVE_LIBDRM
@@ -108,9 +111,8 @@  static int
 _eglAddDRMDevice(drmDevicePtr device, _EGLDevice **out_dev)
 {
    _EGLDevice *dev;
-   const int wanted_nodes = 1 << DRM_NODE_RENDER | 1 << DRM_NODE_PRIMARY;

-   if ((device->available_nodes & wanted_nodes) != wanted_nodes)
+   if ((device->available_nodes & 1 << DRM_NODE_RENDER) == 0)
       return -1;

    dev = _eglGlobal.DeviceList;
@@ -122,7 +124,7 @@  _eglAddDRMDevice(drmDevicePtr device, _EGLDevice **out_dev)
    while (dev->Next) {
       dev = dev->Next;

-      assert(_eglDeviceSupports(dev, _EGL_DEVICE_DRM));
+      assert(!_eglDeviceIsSoftware(dev));
       if (drmDevicesEqual(device, dev->device) != 0) {
          if (out_dev)
             *out_dev = dev;
@@ -138,8 +140,13 @@  _eglAddDRMDevice(drmDevicePtr device, _EGLDevice **out_dev)
    }

    dev = dev->Next;
-   dev->extensions = "EGL_EXT_device_drm";
-   dev->EXT_device_drm = EGL_TRUE;
+   if (device->available_nodes & (1 << DRM_NODE_PRIMARY)) {
+      dev->extensions = "EGL_EXT_device_drm";
+      dev->EXT_device_drm = EGL_TRUE;
+   } else {
+      dev->extensions = "";
+      dev->EXT_device_drm = EGL_FALSE;
+   }
    dev->device = device;

    if (out_dev)
@@ -188,6 +195,12 @@  out:
    return dev;
 }

+EGLBoolean
+_eglDeviceIsSoftware(_EGLDevice *dev)
+{
+   return dev->is_software;
+}
+
 EGLBoolean
 _eglDeviceSupports(_EGLDevice *dev, _EGLDeviceExtension ext)
 {
diff --git a/src/egl/main/egldevice.h b/src/egl/main/egldevice.h
index 83a47d5eacc..b75fd3ab12e 100644
--- a/src/egl/main/egldevice.h
+++ b/src/egl/main/egldevice.h
@@ -65,6 +65,9 @@  enum _egl_device_extension {

 typedef enum _egl_device_extension _EGLDeviceExtension;

+EGLBoolean
+_eglDeviceIsSoftware(_EGLDevice *dev);
+
 EGLBoolean
 _eglDeviceSupports(_EGLDevice *dev, _EGLDeviceExtension ext);


Comments

On 2019/06/28, Mathias.Froehlich@gmx.net wrote:
> From: Mathias Fröhlich <mathias.froehlich@web.de>
> 
> Hi,
> 
> Ilia mentioned that there are drm rendernode only drivers out there.
> To support an egl device on those platforms, make the EGL_EXT_device_drm
> device extension optional.
> 
Currently DRM core mandates that primary node is available, even for GPU
only devices.

A while back, we had a chat with kernel people why we do so. IIRC the
conclustion was that existing userspace will just work, since it already
must handle cases when you unplug your only monitor.

I think Ilia was having a pretty reasonable assumption, that's why I did
before digging deeper, as opposed to saying there _are_ such cases.

HTH
Emil
Emil,

Ok, I thought Ilja is a reference here. In this case forget that change!
Thanks!

best

Mathias

On Friday, 28 June 2019 18:26:07 CEST Emil Velikov wrote:
> On 2019/06/28, Mathias.Froehlich@gmx.net wrote:
> > From: Mathias Fröhlich <mathias.froehlich@web.de>
> > 
> > Hi,
> > 
> > Ilia mentioned that there are drm rendernode only drivers out there.
> > To support an egl device on those platforms, make the EGL_EXT_device_drm
> > device extension optional.
> > 
> Currently DRM core mandates that primary node is available, even for GPU
> only devices.
> 
> A while back, we had a chat with kernel people why we do so. IIRC the
> conclustion was that existing userspace will just work, since it already
> must handle cases when you unplug your only monitor.
> 
> I think Ilia was having a pretty reasonable assumption, that's why I did
> before digging deeper, as opposed to saying there _are_ such cases.
> 
> HTH
> Emil
>
It looks like I was mistaken -- a primary node is always allocated. Sorry!

On Sat, Jun 29, 2019 at 4:28 AM Mathias Fröhlich
<Mathias.Froehlich@gmx.net> wrote:
>
> Emil,
>
> Ok, I thought Ilja is a reference here. In this case forget that change!
> Thanks!
>
> best
>
> Mathias
>
> On Friday, 28 June 2019 18:26:07 CEST Emil Velikov wrote:
> > On 2019/06/28, Mathias.Froehlich@gmx.net wrote:
> > > From: Mathias Fröhlich <mathias.froehlich@web.de>
> > >
> > > Hi,
> > >
> > > Ilia mentioned that there are drm rendernode only drivers out there.
> > > To support an egl device on those platforms, make the EGL_EXT_device_drm
> > > device extension optional.
> > >
> > Currently DRM core mandates that primary node is available, even for GPU
> > only devices.
> >
> > A while back, we had a chat with kernel people why we do so. IIRC the
> > conclustion was that existing userspace will just work, since it already
> > must handle cases when you unplug your only monitor.
> >
> > I think Ilia was having a pretty reasonable assumption, that's why I did
> > before digging deeper, as opposed to saying there _are_ such cases.
> >
> > HTH
> > Emil
> >
>
>
>
>