[PATCH 4/5] drm/damage-helper: Do partial updates if framebuffer has not been changed

[Thomas Zimmermann]

Hi Ville

Am 20.09.22 um 16:31 schrieb Ville Syrjälä:
> On Tue, Sep 20, 2022 at 03:56:18PM +0200, Thomas Zimmermann wrote:
>> Set partial updates on a plane if the framebuffer has not been changed
>> on an atomic commit. If such a plane has damage clips, the driver will
>> use them; otherwise the update is effectively empty. Planes that change
>> their framebuffer still perform a full update.
> 
> I have a feeling you're sort of papering over some inefficient
> userspace that's asking updates on planes that don't actually
> need them. I'm not sure adding more code for that is a particularly
> great idea. Wouldn't it be better to just fix the userspace code?

Some more context might be in order:

The ast driver currently uses VRAM helpers, which leads to many 
problems; including blank screens from the low amount of video memory. 
The best solution is to switch SHMEM with BOs on system meory. The video 
memory is only the internal buffer for scanout. This update involves a 
mempcy from the BO to video memory.

Ast's hardware is really slow, so it makes sense to reduce the updates 
to video memory to a minimum. There's support for cursor planes, which 
really makes a difference here.

But doing any cursor-plane update (e.g., moving, animation) with SHMEM 
and the current damage helpers always results in a full-screen memcpy 
from the BO to the video memory for the primary plane. As the ast 
hardware is slow, performance goes down to a an estimated 5 frame per 
seconds. After moving the mouse, one can watch the mouse cursor follow 
along the screen for the next seconds. Userspace sends the movement 
information and DRM slowly processes them. The same can be observed for 
cursor animation.

The problem is that each change to the cursor plane results in an 
atomic_update call for the primary plane. Not having damage information 
for the plane just means 'update everything'. Not a problem if redrawing 
consists of reprogramming the scanout address. For a memcpy it's not 
feasible.

So can this be fixed in userspace? No realistically IMHO. I've seen this 
problem on Weston, Wayland-Gnome and X11-Gnome. And they are all 
problematic in their own way. (That's why there are multiple patches 
needed.) For example, X11 uses the legacy mouse ioctl, which one of the 
patches fixes. The other userspace needs the other heuristics. A 
potential userspace fix would mean to always set empty-damage 
information on all planes that don't get an update. And I don't consider 
X11 fixable TBH.

> 
> Any property change on the plane could need a full plane
> update as well (eg. some color mangement stuff etc.). So you'd
> have to keep adding exceptions to that list whenever new
> properties are added.

It's not about the occasional change of a property. It's the constant 
sluggish redraw when the interface is supposed to be snappy, such as 
mouse interaction.

> 
> And I think the semantics of the ioctl(s) has so far been that
> basically any page flip (whether or not it actually changes
> the fb) still ends up doing whatever flushing is needed to
> guarantee the fb contents are up to date on the screen (if
> someone sneaked in some front buffer rendering in between).
> Ie. you could even use basically a nop page flip in place
> of dirtyfb.

That's why full updates are still the default. Only in cases where it's 
save to avoid an update of unaffected planes, we do so.

I know that we don't give performance guarantees to userspace. But using 
cursor/overlay planes should be faster then not using them. I think 
that's a reasonable assumption for userspace to make.

> 
> Another thing the ioctls have always done is actually perform
> the commit whether anything changed or nor. That is, they
> still do all the same the same vblanky stuff and the commit
> takes the same amount of time. Not sure if your idea is
> to potentially short circuit the entire thing and make it
> take no time at all?

The patches don't change the overall commit logics. All they do is to 
set damage updates to a size of 0 if a plane reliably does not need an 
update. A driver's atomic_update still runs, but the damage iterator 
does not return any damaged areas.

Best regards
Thomas


> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220921/056153a0/attachment-0001.sig>