grate-driver / xf86-video-opentegra

X.Org video driver for NVIDIA Tegra
Other
12 stars 9 forks source link

EXA: Implement accelerated compositing #30

Closed digetx closed 6 years ago

digetx commented 6 years ago

Basically it is a bunch of GR3D shaders written in grate's asm that perform the pixmaps blending. In general there is a noticeable performance improvement, although there is a room for improvements as well. Compositing passes rendercheck and cairo tests on T20 and T30. There is one caveat in regards to 3D, turned out that a Read-Done commands stream synchronization (waitcheck) is required before starting to render, otherwise there are glitches like a cropped text. I hope we could get waitchecks and BO reservations upstreamed ASAP as we require them more and more.

kusma commented 6 years ago

While I really like what this PR accomplishes, I have some reservations against the way this is done...

First of all, this forks the assembler in grate, which cause more maintenance work. Perhaps we could instead libify the code in grate, and depend on that instead?

Secondly, the hand written asm is a bit daunting... Did you really hand-write all these combinations? I'd kinda hope/expect this to be generated by some script that combines snippets, so it'd be possible to fix a problem in one place rather than ten...

kusma commented 6 years ago

Also, this seems to me to be a bit like a tegra-custom glamour + minimal OpenGL driver. Perhaps we should instead focus on getting mesa/OpenGL up to speed here, and simply use glamour? Or would that somehow be more limiting in some other way?

digetx commented 6 years ago

Having a dependency on grate is a way much more burden in my opinion. While it would be nicer for a development, it would be PITA for a regular everyday usage because somebody would have to package and maintain 2 packages now. I've started to make Ubuntu PPA with the idea to have fresh packages from grate (https://launchpad.net/~digetx/+archive/ubuntu/grate) and already suffered a lot with the packaging, I'll continue my effort after the current PR's will land and really would want to avoid dealing with another package.

The asm is handwritten 😨, but its writing isn't that bad as it may looks like. Once the first shader is written, others are derived from it. Much more painful was to figure out how blending should be done in general, I haven't dealt with it before and xrender has its own quirks. I don't know how we could generate all that with a script and hope we won't find bugs in the asm often.. although it's really not that much hassle to edit shaders individually as it seems.

While glamor is still TBD, we can have accelerated EXA today. It doesn't mean that we don't need glamor anymore at all, even moreover I think EXA accel gives a good overview of what is needed to be done for the glamor and is just a useful and practical application of the work we have done with the grate. I don't know about limitations that glamor could have compared to EXA, that's something to be explored.

digetx commented 6 years ago

Sooo? Any more thoughts / suggestions?

digetx commented 6 years ago

Updated. Huge commit is fragmented, some crud is removed and review comments addresed, also fixed couple bugs that I missed before somehow.

digetx commented 6 years ago

Nah, not ready yet. Seems there is memleak, I'll look at it tomorrow.

digetx commented 6 years ago

Memleaks are fixed, should be fine now.

digetx commented 6 years ago

Huh, seems automake is beaten finally.

digetx commented 6 years ago

Hmm, I've noticed that new Xorg's EXA compositiong option is in the Screen setion. It probably should be in Device section.

digetx commented 6 years ago

Although no, it is fine.

digetx commented 6 years ago

I decided to omit "do not accelerate small pixmaps" patch and added option that disables EXA entirely.

digetx commented 6 years ago

@kusma Do you have any more comments on this? If something bothers you in this PR, don't shy and tell ;)

I've been testing compositing for quite some time now and it may go bad. Like rendering of whole window may be corrupted with a trinagle pattern. Although causing a redraw of pixmap usually recovers it, but sometime window closing may be required (like I had with chromium browser several times). I'm pretty sure that's caused by the lack of waitchecking, so maybe for now would be reasonable to keep compositing disabled by default.

digetx commented 6 years ago

Don't worry, I'm not going to spend all time on EXA. Maybe just some :)

I'll look though code one more time, probably disable compositing by default and then apply this PR.

kusma commented 6 years ago

Sounds good to me :)