2023-11-16 09:21:29

by Pekka Paalanen

[permalink] [raw]
Subject: Re: [PATCH 2/6] drm: Add drm_atomic_helper_buffer_damage_{iter_init, merged}() helpers

On Tue, 14 Nov 2023 17:05:12 +0100
Javier Martinez Canillas <[email protected]> wrote:

> Thomas Zimmermann <[email protected]> writes:
>
> > Hi
> >
> > Am 09.11.23 um 18:24 schrieb Javier Martinez Canillas:
>
> [...]
>
> >> struct drm_rect src;
> >> memset(iter, 0, sizeof(*iter));
> >> @@ -223,7 +224,8 @@ __drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> >> iter->plane_src.x2 = (src.x2 >> 16) + !!(src.x2 & 0xFFFF);
> >> iter->plane_src.y2 = (src.y2 >> 16) + !!(src.y2 & 0xFFFF);
> >>
> >> - if (!iter->clips || !drm_rect_equals(&state->src, &old_state->src)) {
> >> + if (!iter->clips || !drm_rect_equals(&state->src, &old_state->src) ||
> >> + (buffer_damage && old_state->fb != state->fb)) {
> >
> > I'd assume that this change effectivly disables damage handling. AFAICT
> > user space often does a page flip with a new framebuffer plus damage
> > data. Now, with each change of the framebuffer we ignore the damage
> > information. It's not a blocker as that's the behavior before 6.4, but
> > we should be aware of it.
> >
>
> Yes, which is the goal of this patch since page flip with a new framebuffer
> attached to a plane plus damage information can't be supported by drivers
> that do per-buffer uploads.
>
> This was causing some weston and wlroots to have flickering artifacts, due
> the framebuffers being changed since the last plane update.
>
> For now it was decided with Sima, Simon and Pekka that is the best we can
> do and the reason why I add a TODO in patch #6.
>

Hi all,

this made me thinking...

The per-buffer damage accumulation that would be needed is per
upload-buffer, not per KMS FB from userspace. So it should not make any
difference whether userspace flips to another FB or not, the damage
will need to be accumulated per upload-buffer anyway if the driver is
flipping upload-buffers, in order to make the upload-buffer fully
up-to-date.

Why was that not more broken than it already was?

Is there a fixed 1:1 relationship between userspace KMS FBs and the
driver upload-buffers?

Userspace is already taking care that the KMS FB is always fully
up-to-date, FWIW, so the kernel can certainly read areas outside of
FB_DAMAGE_CLIPS if it wants to.


Thanks,
pq


Attachments:
(No filename) (849.00 B)
OpenPGP digital signature