Thanks Mathias,
On 28 August 2018 at 08:31, Mathias Fröhlich <mathias.froehl...@gmx.net> wrote:
> Hi Emil,
>
> On Friday, 3 August 2018 14:44:16 CEST Emil Velikov wrote:
>> Hi all,
>>
>> This series implements the following extensions:
>> - EGL_EXT_device_base
>> - EGL_MESA_device_software
>> - EGL_EXT_device_drm
>> - EGL_platform_device
>>
>> As you know the APIs are used to enumerate, select and use EGLDevices.
>> The second extension (proposed by Ajax) defines a 'software' device,
>> alongside the existing DRM ones.
>>
>> While there are many usecases for this work, my primary interest is
>> allowing device selection, wrt testing. To achieve the goal, we would
>> need to finalise EGL_EXT_explicit_device (also proposed by Ajax).
>>
>> Piglit patches will be sent out shortly.
>>
>> Any feedback is greatly appreciated.
>
> From a higher point of view the approach looks good.
>
> To sum up, you basically associate an _EGLDevice with each _EGLDisplay
> where the _EGLDevice only contains the meta information where to
> find the device and the _EGLDisplay later contains open file descriptors
> to work with ...
>
It's the other way around - I associate each EGLDIsplay with a given EGLDevice.
The EGL_EXT_device_enumeration spec removed that (imho) bogus relation:
"Because the EGLDeviceEXT is a property of <dpy>, any use of an
associated EGLDeviceEXT after <dpy> has been terminated gives
undefined results. Querying an EGL_DEVICE_EXT from <dpy> after a
call to eglTerminate() (and subsequent re-initialization) may
return a different value."
> Nevertheless, running the tests and proof of concept programs that I used
> back in the day brought up some questions and one crash.
>
> At first the Crash: The attached eglcontext-pbuffer.c which goes the pbuffer
> approach
> instead of going surfaceless just dumps core in pbuffer creation using
> the patch series. I believe that it is legal what the program does, but may
> be you want to double check that too.
>
Off the top of my head, I think that's due to eglCreatePbufferSurface
instead of eglCreatePlatformPbufferSurfaceEXT.
I think we could wire that legacy API up, although the whole thing is
_really_ fiddly.
See my piglit patch 85e3b32b3260184853ec20b938574c26a0f39dd8 for some details.
> Then if I use the patch series on an account that has no DISPLAY set and no
> 'display server' running, eglInitialize fails in device_probe_device due to
> at first
> opening the '.../card0' device and bailing out when this does not work.
> Means the current patch series goes via opening the primary node which
> shall not be accessible by everyone and from that derives the rendernode
> device which is finally used for _EGLDisplay initialization.
> Can we alternatively go directly to the rendernode in some way?
>
I think we could. Can you elaborate a bit more or provide an example
on the topic though.
I'm quite curious what's happening here - is there no card0 node,
open() fails, other?
> For patch #7, can you explain why dri already provides the right format?
> It's probably correct, but I am missing some bits of that context creation
> big picture to give a review.
>
The format used when to create a surface is passed to the driver,
which then calls back the loader with the exact same one.
Admittedly there's a ton of conversion back and forth (within the
driver) so it's not always clear.
Pretty much every other platform respects the provided format, hence
my cleanup patches ;-)
That said, I'm perfectly fine with pulling those patches (and updating
platform_device.c) if it will make things easier.
> And finally, in patch #2 you mention that you want to avoid larger patches
> and the announced extensions are not yet working as expected.
> May be you can announce the extensions in a separate patch that follows
> all the implementation patches? That probably helps people that want to
> bisect something using piglit.
>
It's actually patch 1/10 which "enables" the extensions.
You're right though - we could merge the 1/10 boilerplate with 2/10
and enable the extensions once the SW _and_ DRM extensions have
landed.
Might even a) split the helpers from 3/10 into a prep patch and b) add
the DRM _eglFindDevice instances of 3/10 into the DRM patch - 4/10.
Another thing that comes to mind is ... minimise the first device is
always SW assumption.
That would involve adding a couple of _eglDeviceSupports(dev,
_EGL_DEVICE_DRM) calls. It should make things less magical and clearer
for the next EGL device extension.
FTR swrast with platform_device is still WIP - I've been working
fixing up swrast recently. It requires a ton of refactoring and fixes
;-)
> thanks for approaching this topic!
>
Thank you for helping out with review and questions.
I'll try to find time and respin the series tomorrow.
Cheers
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
