Open Bug 1572697 (egl-linux-dmabuf) Opened 3 years ago Updated 5 days ago

[meta][Linux/EGL] Implement DMABuf backend

Categories

(Core :: Graphics: Layers, enhancement)

Desktop
Linux
enhancement

Tracking

()

People

(Reporter: stransky, Assigned: stransky)

References

(Depends on 3 open bugs)

Details

(Keywords: meta)

Attachments

(2 obsolete files)

Implement dma buf EGL texture backend on Wayland similar what we have on Android/Mac. Dma buf buffers can be shared with main/compositor process, can be bound as a render target or texture and can be located at GPU memory. The same dma buf buffer can be also used as HW overlay when it's attached to wl_surface/wl_subsurface as wl_buffer.

Attached patch proof of concept (obsolete) — Splinter Review

proof of concept, uses EGLImageTextureSource at WaylandDMABUFTextureHostOGL. Basically works with GL compositor (not webrender) and contains lot's of debuging code.

Depends on: 1552590
Attached patch dmabuf.patch (obsolete) — Splinter Review

There's a WIP I'd like to get a feedback for as there are some parts I'm unsure about them.

The patch implements WaylandDMABUFTextureClientOGL / WaylandDMABUFTextureHostOGL interfaces.
As a base surface is used WaylandDMABufSurface which provides those low level functionality:

  • map do CPU memory for direct access
  • create EGLImage on top of it
  • share across threads/processes (uses SurfaceDescriptorDMABuf)

The surface is serialized to SurfaceDescriptorDMABuf, WaylandDMABUFTextureClientOGL works with map only and WaylandDMABUFTextureHostOGL creates EGLImage on top of it. EGLImageTextureSource is used by WaylandDMABUFTextureHostOGL to store the EGLImage although the image is owned by underlying WaylandDMABufSurface.

There are some question I'd like to clarify:

  • Is it good to create EGLImage on the host side or shall it be created on Client side thus the surface will be serialized by ELGImage descriptor (and perhaps use EGLImageTextureHost?)
  • How the KHR_fence_sync works and can it be created on host side? (as the egl image is created on host side)
  • Is the extureData::Info correctly filed, especially hasSynchronization/canExposeMappedData? I need to call ReadUnlock() at WaylandDMABUFTextureHostOGL::UpdatedInternal() to compensate a lock at TextureClient::OnForwardedToHost().

Thanks!

Attachment #9086408 - Flags: feedback?(sotaro.ikeda.g)

:stransky, sorry, I do not have a time today. I am going to look into it tomorrow.

Hello.

As a user am I supposed to see a difference yet? For me so far playing a YT video full screen (QHD monitor) is slightly more energy intensive than without the patch, playback is still not as smooth as with XWayland.
Also the new patch introduces a lot of rendering issues of the YT website.

F.

(In reply to Francois Guerraz from comment #4)

Hello.

As a user am I supposed to see a difference yet? For me so far playing a YT video full screen (QHD monitor) is slightly more energy intensive than without the patch, playback is still not as smooth as with XWayland.
Also the new patch introduces a lot of rendering issues of the YT website.

With the current setup you may not see any difference as some parts are missing, especially with video playback which is decoded to CPU memory right now.

(In reply to Martin Stránský [:stransky] from comment #2)

  • Is it good to create EGLImage on the host side or shall it be created on Client side thus the surface will be serialized by ELGImage descriptor (and perhaps use EGLImageTextureHost?)

It is ok to create EGLImage on the host side, since host side bind WaylandDMABufSurface to GL texture. If we want to bind WaylandDMABufSurface on client side for WebGL, we aslo need to create EGLImage on client side like SharedSurface_EGLImage::Create(). Though SharedSurface_EGLImage could be used only when client and compositor are in same process.
https://searchfox.org/mozilla-central/source/gfx/gl/SharedSurfaceEGL.cpp#20

(In reply to Martin Stránský [:stransky] from comment #2)

  • How the KHR_fence_sync works and can it be created on host side? (as the egl image is created on host side)

KHR_fence_sync is used to wait GPU task complete. It is used when client side upload data via GPU like WebGL. One example implementation is SharedSurface_EGLImage::ProducerReleaseImpl()
https://searchfox.org/mozilla-central/source/gfx/gl/SharedSurfaceEGL.cpp#95

But I do not know if cross process EGLSync is implemented on Wayland. Though cross process EGLSync exists on android. When WebGL operation happen in GPU process(Bug 1477756 ) and Wayland supports GPU process, it might possible to use simple EGLSync.

(In reply to Martin Stránský [:stransky] from comment #2)

  • Is the extureData::Info correctly filed, especially hasSynchronization/canExposeMappedData? I need to call ReadUnlock() at WaylandDMABUFTextureHostOGL::UpdatedInternal() to compensate a lock at TextureClient::OnForwardedToHost().

extureData::Info is filled correctly. Yea, hasSynchronization/canExposeMappedData usages not clear. hasSynchronization is set when TextureClient/TextureHost has internal synchronization, like keyed mutex on ID3D11Texture2D. canExposeMappedData seems to be true only when BorrowMappedData() is implemented. For now, only BufferTextureData implements it.

WaylandDMABUFTextureHostOGL does not need to call ReadUnlock() explicitly, since its buffer is directly bounded to GL texture. When TextureHost usage for composite was ended, ReadUnlock() is called automatically by TextureHost::UnbindTextureSource() and TextureSourceProvider::ReadUnlockTextures().

Comment on attachment 9086408 [details] [diff] [review]
dmabuf.patch

It looks good!
Attachment #9086408 - Flags: feedback?(sotaro.ikeda.g) → feedback+
Depends on: 1576898
Depends on: 1576904
Depends on: 1576910
Depends on: 1578380

(In reply to Sotaro Ikeda [:sotaro] from comment #8)

WaylandDMABUFTextureHostOGL does not need to call ReadUnlock() explicitly, since its buffer is directly bounded to GL texture.

When I don't call ReadUnlock() explicitly from WaylandDMABUFTextureHostOGL it leads to many warning messages:

[30375, Main Thread] WARNING: Attempt to Lock a texture that is being read by the compositor!: file /home/komat/tmp676-trunk-gtk3/src-wayland/gfx/layers/client/TextureClient.cpp, line 674

and the whole borowser is sluggish as it waits to read lock timeout.

When TextureHost usage for composite was ended, ReadUnlock() is called automatically by
TextureHost::UnbindTextureSource() and TextureSourceProvider::ReadUnlockTextures().

I don't know why but it's not sufficient. For instance there's a simple log from a composition cycle:

0x7f81eebf5580 Client Try Lock() mReadLock = 0x7f81f365c640 mIsReadLocked = 0 ****

  • Locked OK
    0x7f81eebf5580 Client UnLock mReadLock = 0x7f81f365c640 mIsReadLocked = 1 ****
  • Unlocked OK
    Client ReadLock() from OnForwardedToHost(), mUpdated = 1 mIsReadLocked = 0 ****
  • Locked OK
    AddCompositableRef 0x7f81f36d3350, count = 1
    TextureHost::SetReadLocked(), mReadLock = 0x7f81f365c6d0 mReadLocked = 0
  • Locked OK

AddCompositableRef 0x7f81f36d3350, count = 2
ReleaseCompositableRef 0x7f81f36d3350, count = 1

TextureSourceProvider::ReadUnlockTextures(), mUnlockAfterComposition = 0

  • read-lock is not unlocked as CompositableRef is still set, so mUnlockAfterComposition is empty.

0x7f81eebf5580 Client Try Lock() mReadLock = 0x7f81f365c640 mIsReadLocked = 0 ****

  • read lock failure (leads to timeout) and a warning:

[8967, Main Thread] WARNING: Attempt to Lock a texture that is being read by the compositor!: file /home/komat/tmp676-trunk-gtk3/src-wayland/gfx/layers/client/TextureClient.cpp, line 674

[..timeout..]

ReleaseCompositableRef 0x7f81f36d3350, count = 0
TextureHost::UnbindTextureSource(), mReadLocked = 1
Unlock after Composition

So looks like final ReleaseCompositableRef is called too late. When ReadUnlock() is called from WaylandDMABUFTextureHostOGL::UpdatedInternal() it's called right after TextureHost::SetReadLocked() and there's no timeout then.

Any idea what's wrong here?
Thanks.

Flags: needinfo?(sotaro.ikeda.g)

WaylandDMABUFTextureHostOGL owns a directly texture bounded buffer. Then we should not call ReadUnlock() until all CompositableRefs are released. If TextureHost is not directly texture bounded buffer, we could call ReadUnlock() like BufferTextureHost::UploadIfNeeded().
https://searchfox.org/mozilla-central/source/gfx/layers/composite/TextureHost.cpp#935

So, it seems that client side TextureClient usage seems not correct. For example, ContentClient::CreateContentClient() creates ContentClientSingleBuffered with OpenGL compositor on linux. But it should be ContentClientDoubleBuffered when DMABuf usage is enabled. This part needs to be changed.
https://searchfox.org/mozilla-central/source/gfx/layers/client/ContentClient.cpp#95

By the way, it is nice to update attachment 9086408 [details] [diff] [review], it could not be applied to latest m-c.

Flags: needinfo?(sotaro.ikeda.g)
Component: Graphics → Graphics: Layers
Depends on: 1582773

(In reply to Sotaro Ikeda [:sotaro] from comment #11)

WaylandDMABUFTextureHostOGL owns a directly texture bounded buffer. Then we should not call ReadUnlock() until all CompositableRefs are released. If TextureHost is not directly texture bounded buffer, we could call ReadUnlock() like BufferTextureHost::UploadIfNeeded().
https://searchfox.org/mozilla-central/source/gfx/layers/composite/TextureHost.cpp#935

So, it seems that client side TextureClient usage seems not correct. For example, ContentClient::CreateContentClient() creates ContentClientSingleBuffered with OpenGL compositor on linux. But it should be ContentClientDoubleBuffered when DMABuf usage is enabled. This part needs to be changed.
https://searchfox.org/mozilla-central/source/gfx/layers/client/ContentClient.cpp#95

That seems to be working, Thanks!

By the way, it is nice to update attachment 9086408 [details] [diff] [review], it could not be applied to latest m-c.

I already have a patch set updated to latest trunk and divided to separated patches. I'm just finishing with final tweaks and I'm going to submit them for review next week.

With dmabuf on, filling up this survery (https://qsurvey.mozilla.com/s3/533fee4ba7d5) I had crashes when ticking the boxes.

bp-d757b4a1-b3bf-42e2-a669-6bd3c0190925
bp-34f45a4e-a4cf-4dcb-8d7f-642f30190925

Depends on: 1583731
Summary: [Wayland] Implement Wayland DMABuf texture client/host → [Wayland] Implement Wayland DMABuf texture backend

(In reply to Francois Guerraz from comment #13)

With dmabuf on, filling up this survery (https://qsurvey.mozilla.com/s3/533fee4ba7d5) I had crashes when ticking the boxes.

bp-d757b4a1-b3bf-42e2-a669-6bd3c0190925
bp-34f45a4e-a4cf-4dcb-8d7f-642f30190925

gbm_bo_get_stride_for_plane() should not be called as we don't use modifiers. Also we need to disable dmabuf buffers for partial screen updates.

I noticed that CreateWLBuffer is called twice in WaylandDMABufSurface::Resize. First call is in Create method, second one is in CreateWLBuffer itself. It looks like a bug. Can anyone check it?

Depends on: 1586261
Depends on: 1586696
Summary: [Wayland] Implement Wayland DMABuf texture backend → [Wayland] Implement Wayland DMABuf backend
Depends on: 1588736
Depends on: 1588904
Depends on: 1588907
Alias: wayland-dmabuf
Depends on: 1589924

Note that I've seen a couple of OOMs (presumably, my computer crashed) with widget.wayland_dmabuf_backend.enabled, so you might want to keep an eye out for them.

Depends on: 1590832
Depends on: 1594688
Depends on: 1594692
Depends on: 1595770
Depends on: 1599016
Depends on: 1599393

Hello. On nightly, with the current state of implementation, are we supposed to "see" anything different when enabling the DMABuf backend?

(In reply to Francois Guerraz from comment #17)

Hello. On nightly, with the current state of implementation, are we supposed to "see" anything different when enabling the DMABuf backend?

Yes, you'll see slower composition and slightly higher CPU usage.

(In reply to Martin Stránský [:stransky] from comment #18)

(In reply to Francois Guerraz from comment #17)

Hello. On nightly, with the current state of implementation, are we supposed to "see" anything different when enabling the DMABuf backend?

Yes, you'll see slower composition and slightly higher CPU usage.

Ok. For me at the moment it makes no noticeable difference (I was hoping to be able to play 4k content smoothly like chrome does)

Depends on: 1608379
Depends on: 1608380
Depends on: 1608553

Tiny update: when I tried this some months ago on a new-ish Intel iGPU it was unbearably slow, but now it's not noticeably slower. That's some nice progress.

Depends on: 1608800
Depends on: 1610201
Depends on: 1613052
No longer depends on: egl-linux-vaapi
Depends on: 1613364
Depends on: 1614568
Depends on: 1617553
Depends on: 1618031
Depends on: 1619530
See Also: → 1580166
Depends on: 1619907
Depends on: 1620287
Depends on: 1620830
Depends on: 1622627
Depends on: 1624290
Depends on: 1624441
Depends on: 1624743
No longer depends on: 1624441
Depends on: 1625294
Depends on: 1627674
Depends on: 1629788
Depends on: 1631736
Depends on: 1634213
Depends on: 1634268
Depends on: 1634610
Depends on: 1638858
Depends on: 1619648
Depends on: 1645704
Depends on: 1645678
Depends on: 1645706
Depends on: 1645734
Depends on: 1646005
Depends on: 1646007
No longer depends on: 1646005
Depends on: 1645672
Depends on: 1652783
Depends on: 1654798
Alias: wayland-dmabuf → egl-linux-dmabuf
OS: Unspecified → Linux
Hardware: Unspecified → Desktop
See Also: → egl-linux-vaapi
Summary: [Wayland] Implement Wayland DMABuf backend → [Linux/EGL] Implement DMABuf backend
Depends on: 1655026
Depends on: 1655090
Depends on: 1655323
Depends on: 1655747
Depends on: 1655919
Depends on: 1655943
Depends on: 1656505
Depends on: 1657577
Depends on: 1657747
Depends on: 1658035
Depends on: 1658430
Depends on: 1659495
Depends on: 1659350
Depends on: 1661209
Depends on: 1659352
Depends on: 1661442
Depends on: 1662409
Depends on: 1662496
Depends on: 1661572
Depends on: 1664911
Depends on: 1667429
Depends on: 1668575
Depends on: 1666099
No longer depends on: 1666099
Depends on: 1670567
Depends on: 1670624
Depends on: 1677855
Blocks: 1684224
No longer blocks: 1684224
Depends on: 1684224

FYI: nvidia seems to get serious about dmabuf support (on Wayland, but probably equally on X11/EGL), see e.g. https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/587

Depends on: 1688851
No longer blocks: wayland
Depends on: 1695930
Depends on: 1696869
Depends on: 1696937
Depends on: 1699075
Depends on: 1699433
Depends on: 1700439
Depends on: 1710815
Depends on: 1718499
No longer depends on: 1710815
No longer depends on: 1684224
Depends on: 1730936
Depends on: 1735929
Depends on: 1736761
Depends on: 1580166
See Also: 1580166
Depends on: 1739924
Summary: [Linux/EGL] Implement DMABuf backend → [meta][Linux/EGL] Implement DMABuf backend
Attachment #9084272 - Attachment is obsolete: true
Attachment #9086408 - Attachment is obsolete: true
Depends on: 1746551
Depends on: 1749119
Depends on: 1750389
No longer depends on: 1750389
No longer depends on: 1735929
Depends on: 1735929
Depends on: 1776362
No longer depends on: 1776362
Depends on: 1776563
Depends on: 1776889
Severity: normal → S3
Depends on: 1801892
You need to log in before you can comment on or make changes to this bug.