Atomic KMS: enable bypass #3595
Conversation
|
|
||
| for (std::size_t i = 0; i < std::min(4zu, plane_descriptors.size()); i++) | ||
| { | ||
| bo_handles[i] = plane_descriptors[i].dma_buf; |
There was a problem hiding this comment.
This is where the problem lies; drmModeAddFB2 expects GEM handles, and you're giving it dmabuf file descriptors.
Here is the necessary DRM API for converting the two, along with an ominous warning about the sharp edges of that API.
Since I think we share the underlying FD with the RenderingPlatform (and so with EGL etc), I think we'll need to do the “everything goes through gbm” route. That means going through gbm_bo_import (with type = GBM_BO_IMPORT_FD_MODIFIER and flags = GBM_BO_USE_SCANOUT) and then pulling all the GEM handles out with gbm_bo_get_handle_for_plane (and then handling the lifetime of the gbm_bo, keeping a track of it, and destroying it when necessary)
RAOF
force-pushed
the
initial-atomic-kms-platform
branch
from
d172bfa to
6e64d62
Compare
tarek-y-ismail
force-pushed
the
atomic-kms-enable-bypass
branch
3 times, most recently
from
eeec45a to
3bd9e74
Compare
tarek-y-ismail
force-pushed
the
atomic-kms-enable-bypass
branch
from
a420ff3 to
8031474
Compare
|
Could reuse a rebase after we figure out why artifacts appear when bypass is active. |
Feel free to rebase the chain of PRs already. |
| for (std::size_t i = 0; i < std::min(MAX_PLANES, plane_descriptors.size()); i++) | ||
| gem_handles[i] = gbm_bo_get_handle_for_plane(gbm_bo, i).u32; | ||
|
|
||
| auto fb_id = std::shared_ptr<uint32_t>{ |
There was a problem hiding this comment.
As it stands, I think this code leaks a gbm_bo each time it's called? It calls gbm_bo_import, which will allocate a gbm_bo, but nothing every frees it.
Maybe import_gbm_bo should return a shared_ptr<gbm_bo> with cleanup functor, and that should in turn be captured by this shared_ptr?
There was a problem hiding this comment.
Hmm, I didn't know that, I thought we only got a (cached) handle to the buffer object (i.e. created once on the first import and reused later).
There was a problem hiding this comment.
Also, I've noticed that I import and destroy a buffer object every frame, this doesn't feel correct (unless the GBM code handles this). I think it might also be related to the artifacts showing in glmark2
tarek-y-ismail
force-pushed
the
initial-atomic-kms-platform
branch
from
24b446d to
0dc388b
Compare
tarek-y-ismail
force-pushed
the
atomic-kms-enable-bypass
branch
from
e252b73 to
bd7b097
Compare
`drmModeAddFB2` returns -2, which is `ENOENT` for some reason...
Removed some unused headers as well :)
…orted for scanout. The compositor code is expecting `framebuffer_for` to sometimes return `nullptr`; this is fine. What's *not* fine is passing a null `fb_id` to `AtomicKmsFbHandle` and then returning a *non*-null `Framebuffer` that's broken :)
…ng a buffer for bypass
tarek-y-ismail
force-pushed
the
atomic-kms-enable-bypass
branch
from
bd7b097 to
82cdf9c
Compare
|
@tarek-y-ismail what is the status of this PR? Clearly it needs conflicts resolved, but does it need more? |
@RAOF is investigating the artifacts. |
|
Small update: |
Apologies for the general suckiness of the code, will improve that once it actually works.
To test:
./build/bin/miral-kiosk --show-splash=off --platform-display-libs=mir:atomic-kms --platform-rendering-libs=mir:gbm-kms --cursor=null & sleep 2 glmark2-waylandNote that until hardware cursor support is merged, the cursor must be disabled when testing bypass (current implementation works only with one renderable)