On 03/10/16 06:03 AM, Fabien DESSENNE wrote:
On 09/30/2016 08:49 PM, Derek Foreman wrote:
On 30/09/16 04:28 AM, Tomohito Esaki wrote:
Multiplanar formats are supported by using drmModeAddFB2 and bypassing
gbm. If drmModeAddFB2 isn't available, the existing gbm bo import path
is used and multiplanar formats are unsupported.
I'm not sure we should be doing anything with the existing sprite code
at all (except perhaps removing it, though I'm sure that's an unpopular
point of view :) since it's not really viable without atomic mode
setting, and is currently disabled until someone uses a debug key to
enable it.
Pekka - I can't recall, is the atomic mode setting series going to build
on the current sprite stuff or blow it away and start over?
I'm wondering if we should just tear out the existing sprite code in the
meantime...
Thanks,
Derek
Hi Derek,
I do not know all the history of Weston, and I am not aware of all the
reasons that made the sprites "broken". I have always been told that it
was not a good idea to enable sprites because it was not working as
expected unless we have the atomic mode.
Sprites are broken because without atomic mode setting it's impossible
to update all active sprites, well, atomically.
There's no way to ensure they all change in the same screen refresh
interval - we may have tearing, or on some driver stacks I think we end
up with vblank waits during plane setting (I think this bit us with the
cursor plane on intel drivers a while ago, and the cursor plane was made
a special case to help...)
Since tearing is something wayland was intended to do away with
entirely, having an option to enable to bring it back seems like a bad
move in weston.
Anyway, with my limited background, i'd like to share my point of view:
Firstly, sprite works quite fine (with a bunch of fixes that I will be
glad to share once sprites are accepted by all) : I can have up to 3
sprites (inluding gstreamer video playback with linux-dmabuf) + cursor
working smoothly on two parallel outputs (extended desktop) So, sprites
are probably a bit broken, but not totally out of order. We just still
need to improve it.
Unfortunately there are things (the tearing) that are unimprovable
without atomic underneath, so I'm worried about the testability of
sprite changes without atomic mode setting landing first.
Secondly, it is a (very) long time now, that I am waiting for atomic. I
discussed a bit about it recently with Pekka and my understanding is
that because people are quite busy, atomic is not about to land in main
in a short time.
This truly is a shame, I'd hoped to see it by now as well.
So, for theses two reasons, I think that is a good idea to improve
sprites now : let's keep them marked as broken for the time being.
The improvements proposed by Tomohito will serve as a basis for sprites
in atomic (I can confirm that the atomic WIP branch does not blow away
the sprite part, the prepare_overlay_view is still there)
For what it worth, I have recently played with the atomic WIP branch,
ported it to 1.11, and enabled linux-dmabuf in sprites. Not 100% perfect
but it works. I know that I am not the only one who did this job
recently, so I guess that we shall not have so many problem to find
people to contribute to the atomic revival (with unbroken sprites inside)
My fingers are crossed - I see this (both atomic mode setting, and the
ability to put dmabuf buffers into planes) as incredibly important work too.
I won't bother posting any patches to remove existing sprite
functionality - it seems they wouldn't be well received. :)
Thanks,
Derek
BR
Fabien, a sprite fan ;)
Signed-off-by: Tomohito Esaki <[email protected]>
---
libweston/compositor-drm.c | 53 +++++++++++++++++++++++-----------------------
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index b15fa01..f0e6f7c 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1008,11 +1008,9 @@ page_flip_handler(int fd, unsigned int frame,
static uint32_t
drm_output_check_sprite_format(struct drm_sprite *s,
- struct weston_view *ev, struct gbm_bo *bo)
+ struct weston_view *ev, uint32_t format)
{
- uint32_t i, format;
-
- format = gbm_bo_get_format(bo);
+ uint32_t i;
if (format == GBM_FORMAT_ARGB8888) {
pixman_region32_t r;
@@ -1053,15 +1051,12 @@ drm_output_prepare_overlay_view(struct drm_output
*output,
struct drm_sprite *s;
struct linux_dmabuf_buffer *dmabuf;
int found = 0;
- struct gbm_bo *bo;
+ struct gbm_bo *bo = NULL;
pixman_region32_t dest_rect, src_rect;
pixman_box32_t *box, tbox;
uint32_t format;
wl_fixed_t sx1, sy1, sx2, sy2;
- if (b->gbm == NULL)
- return NULL;
-
if (viewport->buffer.transform != output->base.transform)
return NULL;
@@ -1101,15 +1096,9 @@ drm_output_prepare_overlay_view(struct drm_output
*output,
if (!found)
return NULL;
- if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource))) {
+ if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource)) &&
+ b->no_addfb2 && b->gbm) {
#ifdef HAVE_GBM_FD_IMPORT
- /* XXX: TODO:
- *
- * Use AddFB2 directly, do not go via GBM.
- * Add support for multiplanar formats.
- * Both require refactoring in the DRM-backend to
- * support a mix of gbm_bos and drmfbs.
- */
struct gbm_import_fd_data gbm_dmabuf = {
.fd = dmabuf->attributes.fd[0],
.width = dmabuf->attributes.width,
@@ -1126,22 +1115,32 @@ drm_output_prepare_overlay_view(struct drm_output
*output,
#else
return NULL;
#endif
- } else {
+ } else if (b->gbm) {
bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_WL_BUFFER,
buffer_resource, GBM_BO_USE_SCANOUT);
}
- if (!bo)
- return NULL;
- format = drm_output_check_sprite_format(s, ev, bo);
- if (format == 0) {
- gbm_bo_destroy(bo);
- return NULL;
- }
+ if (bo) {
+ format = drm_output_check_sprite_format(
+ s, ev, gbm_bo_get_format(bo));
+ if (format == 0)
+ return NULL;
+
+ s->next = drm_fb_get_from_bo(bo, b, format);
+ if (!s->next) {
+ gbm_bo_destroy(bo);
+ return NULL;
+ }
+ } else if (dmabuf) {
+ format = drm_output_check_sprite_format(
+ s, ev, dmabuf->attributes.format);
+ if (format == 0)
+ return NULL;
- s->next = drm_fb_get_from_bo(bo, b, format);
- if (!s->next) {
- gbm_bo_destroy(bo);
+ s->next = drm_fb_create_dmabuf(dmabuf, b, format);
+ if (!s->next)
+ return NULL;
+ } else {
return NULL;
}
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
