[Mesa-dev] [PATCH 2/2] egl: Don't add hardware device if there is no render node.

[Mathias Fröhlich]

On Monday, 10 June 2019 12:11:40 CEST Mathias Fröhlich wrote:
> Hi Emil,
> 
> On Friday, 7 June 2019 15:43:48 CEST Emil Velikov wrote:
> > On Thu, 6 Jun 2019 at 12:10, <Mathias.Froehlich at gmx.net> wrote:
> > >
> > > From: Mathias Fröhlich <Mathias.Froehlich at gmx.net>
> > >
> > > Do not offer a hardware drm backed egl device if no render node
> > > is available.
> > As far as I can see current implementation does _not_ add the DRM
> > device if its missing render node (and a primary one).
> > Looking at the change below, it's effectively making the primary node optional.
> > 
> > Hence the comment does not alight with the code - old and new. Can you
> > elaborate?
> 
> The currently pushed implementation asks drmGetDevices2 for a list of
> devices and all devices with either a render node or a master node or both
> are added as hardware device. This check is the bitmasking test with
> device->available_nodes that you have put near the top of _eglAddDRMDevice.
> So, if there is no render node the hardware device is added.
> Later on the filename of the render node as returned from
> _eglGetDRMDeviceRenderNode is opened which does not succeed in that case.
> egInitialize fails then which is not nice.
> 
> Past my change, a pure hardware device is not added if there is no render node.
> That is decided by this above mentioned bitmask test.
> 
> The codepath that adds devices via _eglAddDevice from all the platforms is
> untouched as this still uses the same bitmask as before.
> I did not check this code path specifically above the call to _eglAddDevice
> as this patch does not change the behavior of this case.
> 
> best
> 
> Mathias
> 
> > 
> > I have not thought exactly how primary node-less DRM will work out
> > esp. since the EGL_EXT_device_drm extension explicitly mentions one.

Emil,

ok, now I see. You also want the master node and bail out if that is not there.

The problem is that the current EGLDevice code *needs* a user accessible
render node to function, but does not enforce it to be there. It just says if we
have either a render node or a master node or both go ahead.

Your bitmaks test needs to read

int wanted_nodes =(1 << DRM_NODE_PRIMARY |
                                   1 << DRM_NODE_RENDER);
if ((device->available_nodes & wanted_nodes) != wanted_nodes)
   return -1;

if you want to have *both* bits set for a hardware device.
Means with your current pushed code the master node is 'optional' as well!!

From the use case that mostly drove the device extension
originally - put the EGL_EXT_device_drm extension away - do make sense
once you have a user accessible render node device file with a mesa driver
that the loader can access. No matter if there is a master device file and no matter
what (historic?) statement about a master node device file the
EGL_EXT_device_drm makes ...

Also, if the master node is returned from libdrm, it is returned from the
string query function. Is that sufficient?

Can we make the EGL_EXT_device_drm optional then? Means render node
accessible by user and driver name returned by libdrm results in an EGLdevice
ready to use.

best

Mathias

> > 
> > -Emil
> > 
> 
> 
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>