[PATCH weston v2 3/3] compositor-drm: Support linux_dmabuf output for sprite planes without gbm
Derek Foreman
derekf at osg.samsung.com
Mon Oct 3 14:01:54 UTC 2016
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 <etom at igel.co.jp>
>>> ---
>>> 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
>> wayland-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list