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

[Thomas Zimmermann]

Hi

Am 20.09.22 um 16:47 schrieb Daniel Stone:
> Hi,
> 
> On Tue, 20 Sept 2022 at 15:31, Ville Syrjälä 
> <ville.syrjala at linux.intel.com <mailto:ville.syrjala at linux.intel.com>> 
> wrote:
> 
>     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?
> 
> 
> I'm not sure why it would need to be 'fixed', when it's never been a 
> property of the atomic API that you must minimise updates. Weston does 
> this (dumps the full state in every time), and I know we're not the only 
> ones.

I've found a bug in one of the DRM helpers. It unconditionally adds the 
primary plane to the commit, which triggers the repaint. Fixing the bug 
resolves the problem for X11 and Wayland-Gnome.

> 
> 'Fixing' it would require doing a bunch of diffing in userspace, because 
> maintaining a delta and trying to unwind that delta across multiple 
> TEST_ONLY runs, isn't much fun. It's certainly a far bigger diff than 
> this patch.

But even with the fix applied, weston still wants to redraw the whole 
screen on every movement of the mouse cursor. The system is usable, so 
it's not a showstopper.

Still, weston should stop sending the primary plane's framebuffer on 
each cursor update. There's no update to the contents and the fix seems 
simple to do from userspace.

Best regards
Thomas

> 
>     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.
> 
> 
> Eh, it's just a blob ID comparison.
> 
>     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.
> 
>     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?
> 
> 
> I would expect it to always perform a commit, though.
> 
> Cheers,
> Daniel

-- 
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/20220929/29830ecc/attachment.sig>