Hi Am 21.09.22 um 12:18 schrieb Ville Syrjälä: [...]
Though I don't really know if a there is software relying on that behaviuor. I suppose one idea could be to keep that behaviour for the legacy ioctls but not for atomic. Ee. any fb directly specified in a legacy setcrtc/setplane/pageflip is always considered fully damaged. But including an the sameAssuming the specified fb to be damaged seems like a non-issue to me. The problem is with the other framebuffers: if userspace sends us a CURSOR_MOVE ioctl, we can safely assume the cursor fb to be damaged. But we don't want the primary plane's fb to be damaged. Or else we'd redraw the full primary plane.fb in the atomic ioctl does not imply damage. That would mean atomic has to rely on specifying damage explicitly, and any userspace that doesn't do that will be broken.The problem again is not in the damage information on planes that legitimately need a redraw. It's all the other planes that are being redrawn as well. Or is there some scenario that I don't see?I thought we're talking about eg. a cursor update that also includes the primary plane because apparently userspace is lazy. I think what you're saying is that currently there is no damage information for the primary plane so you're attempting to infer it based on whether the fb property changed or not.
Correct.
And what I was saying is that IIRC historically specifying the same fb again has still implied full damage. Changing that behaviour may or may not break exising userspace.
I cannot answer the question. The three cases I've seen at least worked with the new semantics. I think Daniel once mentioned that we already expect userspace to call the DIRTYFB ioctl after changing a framebuffer's content.
Or we could introduce a client cap for it I guess, but that would require (minimal) userspace changes.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.What I gathered is that you seemed to only consider the fb contents as something that needs damage handling. That is not the case for stuff like eDP PSR and DSI command mode displays where we need to upload a new full frame whenever also the non-damaged fb contents would get transformed by the hardware on the way to the remote frambuffer. That would mean any change in eg. scanout coordinates, color management, etc.There is support for changing scanout coordinates in drm_atomic_helper_damage_iter_init() in patch 2. In general, maybe the heuristic needs to be stricter to only handle updates that only change damage. For now, the problem only happens after converting ast to SHMEM. All the other SHMEM-based drivers use a single plane where the problem doesn't happen; or only reprogram the scanout address, which is fast. If the damage-handling logic imposes problems on other drivers, some of it could possibly be moved into ast itself.Maybe we have two clearly separate classes of uses case: 1. devices where only damage to the fb contents matter (eg. some kind of shadow fb that is the same size as the real fb). 2. devices where everything about the scanout process matters (eg. PSR) ? Maybe there should be helpers to deal with just the first case, and then some more helpers (or just driver code) to pile the rest on top as well when needed?
Makes sense.I know that these plane-state are not good style, but with them in place, much of the heuristic could be moved from drm_atomic_helper_check_plane_damage() into the driver if necessary.
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
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
