[Mesa-dev] [PATCH 00/10] egl: mixed bag of fixes/cleanups

[Daniel Stone]

Hi Emil,

On 29 August 2017 at 11:28, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 28 August 2017 at 14:50, Daniel Stone <daniel at fooishbar.org> wrote:
>>>   egl/wayland: plug leaks in dri2_wl_create_window_surface() error path
>>>   egl/wayland: polish object teardown in dri2_wl_destroy_surface
>>
>> Reviewed-by: Daniel Stone <daniels at collabora.com>
>>
> Guessing that the r-b stands for both patches?

Right you are.

>>>   egl: allow RGB565 formats in eglCreateWaylandBufferFromImageWL
>>
>> I've been avoiding trying to expand use of this API, in all honesty.
>>
> Any particular reason?
>
> Quick search on the webs + my system shows no users of the extension,
> apart from weston-nested.
> Even there it's not a hard requirement.
>
> Any ideas if the extension was ever used [elsewhere], should we
> consider deprecating it?

I haven't seen much use of it, myself. It's also only really useful
for wl_drm; I'd prefer to push zwp_linux_dmabuf instead. It also
relies on a more perfect proxying of wl_drm than we necessarily
actually implement.

>>>   egl: add juicy comment about WL_bind_wayland_display +
>>>     wl_drm/wl_dmabuf
>>
>> It depends on a couple of things, really. Firstly, on Mesa's general
>> feature deprecation schedule. Secondly, on external support
>> (Weston/Mutter/Enlightenment are fine, no support yet in
>> KWin/WLC/Smithay/...). Thirdly, on how cleanly we can separate it. If
>> we can break out all the BindWaylandDisplay code more than we already
>> have, it might be easier to maintain. And lastly, on NVIDIA: I believe
>> being a Streams provider requires BindWaylandDisplay, so we might need
>> that to be replaced before we can rip it out of Mesa.
>>
>> Having the comment expanded to cover these might be nice.
>
> Fully agree on all points, although I think the priority is in the opposite way.
> But above all does the extension make sense, or even yet, can it be
> implemented with wl_dmabuf?

The one thing it provides over and above wl_dmabuf is device
advertisement, to guide the implementation towards a particular GPU.
So maybe it makes sense to keep it just for that, purely for render
nodes.

Also VA-API still uses wl_drm, so we need to wean that away from it first.

>>>   egl/wayland: group wl_win specific code together
>>>   egl/wayland: make sure HAS_$FORMAT is set for wl_dmabuf
>>
>> Reviewed-by: Daniel Stone <daniels at collabora.com>
>>
> As above - I'm guessing the r-b stands for both patches?

Yep!

Cheers,
Daniel