[PATCH weston v2 3/3] compositor-drm: Support linux_dmabuf output for sprite planes without gbm

[Daniel Stone]

Hi,

On 3 October 2016 at 15:01, Derek Foreman <derekf at osg.samsung.com> wrote:
> On 03/10/16 06:03 AM, Fabien DESSENNE wrote:
>> 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.

Yes. The semantics are incredibly underspecified, and the drivers
diverge so much in implementation that we can never really have a
reliable implementation. If you don't have a vblank wait, then you
really need to do a pageflip of the primary plane as well to get the
event to drive things off. Or use the vblank callbacks, which is
really broken and racy. There's no way to ever fix that API properly.

>> 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.

Er, yeah.

>> 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.

Yes - I think the dmabuf patches from Tomohito-san are very useful,
and we should push those forward.

> I won't bother posting any patches to remove existing sprite functionality -
> it seems they wouldn't be well received. :)

Honestly, I've been leaning towards ripping them out as well.

Unfortunately a lot of things have come up this year which have kept
me from spending quality time with the atomic code, but I'm now back
on it and setting myself pretty aggressive targets to get new
revisions out there. It's long past due, and really needs to be
merged.

What held things up last time, is that I finally hit the breaking
point with drm_{output,sprite}::{current,next}. The entire setup is so
fragile that the only proper fix - which aligns nicely with the
kernel's atomic API - is to split things up into three state members:
one each for actually-on-screen-now,
submitted-to-kernel-but-not-there-yet, and in-flight. Doing this was a
lot of churn, and hit a lot of fragile points, especially in the
sprite code. Then when we're finished, we end up with two codepaths
for sprites. compositor-drm becomes pretty huge.

Given that we've had it not just disabled by default, but requiring
either patches or user intervention _at every run_ to enable, and that
the users (e.g. ST) here have competent atomic KMS drivers, I'd be
very much tempted to make my own life easier by removing it as a
prelude to the atomic patches, and building back up from there.

Anyway, I'll review these patches this week, and send out v(n+1) of
atomic next. Thanks!

Cheers,
Daniel