linux-kernel - Re: [PATCH 2/6] drm: Add drm_atomic_helper_buffer_damage_{iter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20231116112052.16d4d234.pekka.paalanen@collabora.com>
Date:   Thu, 16 Nov 2023 11:20:52 +0200
From:   Pekka Paalanen <pekka.paalanen@...labora.com>
To:     Javier Martinez Canillas <javierm@...hat.com>
Cc:     Thomas Zimmermann <tzimmermann@...e.de>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        Gerd Hoffmann <kraxel@...hat.com>,
        nerdopolis <bluescreen_avenger@...izon.net>,
        Bilal Elmoussaoui <belmouss@...hat.com>,
        Maxime Ripard <mripard@...nel.org>, stable@...r.kernel.org,
        Sima Vetter <daniel.vetter@...ll.ch>,
        Erico Nunes <nunes.erico@...il.com>
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 <javierm@...hat.com> wrote:

> Thomas Zimmermann <tzimmermann@...e.de> 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

Content of type "application/pgp-signature" skipped