[platforms/drm] Use a shared pointer for gbm_surface
ClosedPublic
Actions

Authored by graesslin on Oct 5 2017, 5:05 PM.

Details

Reviewers

romangg
davidedmundson

Group Reviewers

KWin
Plasma

Commits

R108:47343fb8f759: [platforms/drm] Use a shared pointer for gbm_surface

Summary

The gbm_surface is owned by the EglGbmBackend, but it's not the only one
using it. The DrmSurfaceBuffer is also using it and needs it to destroy
the gbm_bo. Now this can become a problem in the following situation:

a page flip is still pending
the EglGbmBackend destroys the gbm_surface

-> when the page flip happens the DrmSurfaceBuffer will try to destroy
the gbm_bo and crash as the gbm_surface is no longer valid. This
situation can happen when switching screens or when switching compositing
backend (OpenGL 2 -> OpenGL 3).

To address this problem a class GbmSurface is added which wrapps the
gbm_surface pointer. The EglGbmBackend creates and holds a shared pointer
to the GbmSurface and passes that one to the DrmSurfaceBuffer. So when
cleaning up the gbm_surface only the shared pointer is reset and in case
the DrmSurfaceBuffer still needs it, it can access it without problems.

BUG: 385372
FIXED-IN: 5.11.0

Test Plan

Not yet

Diff Detail

Repository

R108 KWin

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

graesslin created this revision.Oct 5 2017, 5:05 PM

Restricted Application added a project: KWin. · View Herald TranscriptOct 5 2017, 5:05 PM

Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript

I have no idea why git think I copied from drm_buffer_gbm.h|cpp. I created the files from scratch.

FIXED-IN: 5.11.0

I'm afraid it won't be. Tag was today.

Patch makes, sense potentially makes that GBM sharing patch cleaner too..

davidedmundson accepted this revision.Oct 5 2017, 6:27 PM

This revision is now accepted and ready to land.Oct 5 2017, 6:27 PM

Added test case for new class

Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptOct 5 2017, 7:33 PM

In D8152#152540, @davidedmundson wrote:

FIXED-IN: 5.11.0

I'm afraid it won't be. Tag was today.

Yeah, I already noticed. Basically I arc diffed, opened up my mails and saw that it was tagged.

Closed by commit R108:47343fb8f759: [platforms/drm] Use a shared pointer for gbm_surface (authored by graesslin). · Explain WhyOct 9 2017, 7:06 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

		Path
M		autotests/CMakeLists.txt (7 lines)
A	M	autotests/test_gbm_surface.cpp (119 lines)
M		plugins/platforms/drm/CMakeLists.txt (2 lines)
M		plugins/platforms/drm/drm_backend.h (5 lines)
M		plugins/platforms/drm/drm_backend.cpp (2 lines)
P		plugins/platforms/drm/drm_buffer_gbm.h (8 lines) Copied to multiple locations: plugins/platforms/drm/gbm_surface.cpp plugins/platforms/drm/gbm_surface.h
M		plugins/platforms/drm/drm_buffer_gbm.cpp (11 lines)
M		plugins/platforms/drm/egl_gbm_backend.h (5 lines)
M		plugins/platforms/drm/egl_gbm_backend.cpp (11 lines)
P		plugins/platforms/drm/gbm_surface.h (40 lines) Copied from plugins/platforms/drm/drm_buffer_gbm.h
P		plugins/platforms/drm/gbm_surface.cpp (54 lines) Copied from plugins/platforms/drm/drm_buffer_gbm.h

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	20373	c27f6dc		Oct 5 2017, 5:05 PM	★	★
Diff 2	20375	31b5b7f	Added test case for new class	Oct 5 2017, 7:33 PM	★	★
Diff 3	20532	31b5b7f	R108:47343fb8f75909f6491c0534004df56ee1e53737	Oct 5 2017, 7:32 PM	★	★