Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use gbm_bo_create_with_modifiers + don't encode if capture fails #220

Merged
merged 2 commits into from
Aug 9, 2023

Conversation

nowrep
Copy link
Contributor

@nowrep nowrep commented Aug 6, 2023

No description provided.

gbm_bo_create will return DRM_FORMAT_MOD_INVALID and that may
not be importable on compositor side with some formats.
Fallback to gbm_bo_create for GPUs without modifiers support.
buffer.bo = gbm_bo_create(gbm_device, buffer.width,
buffer.height, format, GBM_BO_USE_LINEAR | GBM_BO_USE_RENDERING);
const uint64_t modifier = 0; // DRM_FORMAT_MOD_LINEAR
buffer.bo = gbm_bo_create_with_modifiers(gbm_device, buffer.width,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind explaining why we try with_modifiers first, when should one use the two functions for allocation here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gbm_bo_create_with_modifiers is always preferable when supported (kernel supports modifiers). There are however still some GPUs without modifiers support, and with those the gbm_bo_create_with_modifiers will fail and we have to fallback to gbm_bo_create.

gbm_bo_create should work everywhere, but it will return DRM_FORMAT_MOD_INVALID which may be problematic - for example Vulkan doesn't allow importing dmabufs with invalid modifier.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, interesting. Do you happen to have a link to documentation explaining these things? My google-foo has failed me this time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know of any documentation explaining this, sorry. Most of my understanding comes from studying code.

Similar logic in wlroots https://gitlab.freedesktop.org/wlroots/wlroots/-/blob/master/render/allocator/gbm.c#L92

Actually the dmabuf screencopy code is based on wlroots example which uses gbm_bo_create only and up until now I didn't know it will always return invalid modifier. For some reason I expected it to return some (best?) modifier if kernel supports modifiers, and I didn't confirm it because it seemed to just work. Also gbm documentation doesn't mention it either.

Copy link
Owner

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@ammen99 ammen99 merged commit 4c62832 into ammen99:master Aug 9, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants