| 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
| ||
|
Message-ID: <6e663e37-b735-47f7-a841-fa0f93fdddaf@suse.de>
Date: Tue, 14 Nov 2023 16:49:31 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Javier Martinez Canillas <javierm@...hat.com>,
linux-kernel@...r.kernel.org
Cc: dri-devel@...ts.freedesktop.org, Gerd Hoffmann <kraxel@...hat.com>,
nerdopolis <bluescreen_avenger@...izon.net>,
Pekka Paalanen <pekka.paalanen@...labora.com>,
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
Hi
Am 09.11.23 um 18:24 schrieb Javier Martinez Canillas:
> To be used by drivers that do per-buffer (e.g: virtio-gpu) uploads (rather
> than per-plane uploads), since these type of drivers need to handle buffer
> damages instead of frame damages.
>
> The drm_atomic_helper_buffer_damage_iter_init() has the same logic than
> drm_atomic_helper_damage_iter_init() but it also takes into account if the
> framebuffer attached to plane's state has changed since the last update.
>
> And the drm_atomic_helper_buffer_damage_merged() is just a version of the
> drm_atomic_helper_damage_merged() helper, but it uses the iter_init helper
> that is mentioned above.
>
> Fixes: 01f05940a9a7 ("drm/virtio: Enable fb damage clips property for the primary plane")
> Cc: <stable@...r.kernel.org> # v6.4+
> Reported-by: nerdopolis <bluescreen_avenger@...izon.net>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218115
> Suggested-by: Sima Vetter <daniel.vetter@...ll.ch>
> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
> ---
>
> drivers/gpu/drm/drm_damage_helper.c | 79 ++++++++++++++++++++++++++---
> include/drm/drm_damage_helper.h | 7 +++
> 2 files changed, 80 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
> index aa2325567918..b72062c9d31c 100644
> --- a/drivers/gpu/drm/drm_damage_helper.c
> +++ b/drivers/gpu/drm/drm_damage_helper.c
> @@ -204,7 +204,8 @@ EXPORT_SYMBOL(drm_atomic_helper_dirtyfb);
> static void
> __drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> const struct drm_plane_state *old_state,
> - const struct drm_plane_state *state)
> + const struct drm_plane_state *state,
> + bool buffer_damage)
> {
> 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.
Best regards
Thomas
> iter->clips = NULL;
> iter->num_clips = 0;
> iter->full_update = true;
> @@ -243,6 +245,10 @@ __drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> * update). Currently this iterator returns full plane src in case plane src
> * changed but that can be changed in future to return damage.
> *
> + * Note that this helper is for drivers that do per-plane uploads and expect
> + * to handle frame damages. Drivers that do per-buffer uploads instead should
> + * use @drm_atomic_helper_buffer_damage_iter_init() that handles buffer damages.
> + *
> * For the case when plane is not visible or plane update should not happen the
> * first call to iter_next will return false. Note that this helper use clipped
> * &drm_plane_state.src, so driver calling this helper should have called
> @@ -253,10 +259,37 @@ drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> const struct drm_plane_state *old_state,
> const struct drm_plane_state *state)
> {
> - __drm_atomic_helper_damage_iter_init(iter, old_state, state);
> + __drm_atomic_helper_damage_iter_init(iter, old_state, state, false);
> }
> EXPORT_SYMBOL(drm_atomic_helper_damage_iter_init);
>
> +/**
> + * drm_atomic_helper_buffer_damage_iter_init - Initialize the buffer damage iterator.
> + * @iter: The iterator to initialize.
> + * @old_state: Old plane state for validation.
> + * @state: Plane state from which to iterate the damage clips.
> + *
> + * Initialize an iterator, which clips buffer damage
> + * &drm_plane_state.fb_damage_clips to plane &drm_plane_state.src. This iterator
> + * returns full plane src in case buffer damage is not present because user-space
> + * didn't sent, driver discarded it (it want to do full plane update) or the plane
> + * @state has an attached framebuffer that is different than the one in @state (it
> + * has changed since the last plane update).
> + *
> + * For the case when plane is not visible or plane update should not happen the
> + * first call to iter_next will return false. Note that this helper use clipped
> + * &drm_plane_state.src, so driver calling this helper should have called
> + * drm_atomic_helper_check_plane_state() earlier.
> + */
> +void
> +drm_atomic_helper_buffer_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> + const struct drm_plane_state *old_state,
> + const struct drm_plane_state *state)
> +{
> + __drm_atomic_helper_damage_iter_init(iter, old_state, state, true);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_buffer_damage_iter_init);
> +
> /**
> * drm_atomic_helper_damage_iter_next - Advance the damage iterator.
> * @iter: The iterator to advance.
> @@ -301,7 +334,8 @@ EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next);
>
> static bool __drm_atomic_helper_damage_merged(const struct drm_plane_state *old_state,
> struct drm_plane_state *state,
> - struct drm_rect *rect)
> + struct drm_rect *rect,
> + bool buffer_damage)
> {
> struct drm_atomic_helper_damage_iter iter;
> struct drm_rect clip;
> @@ -312,7 +346,7 @@ static bool __drm_atomic_helper_damage_merged(const struct drm_plane_state *old_
> rect->x2 = 0;
> rect->y2 = 0;
>
> - drm_atomic_helper_damage_iter_init(&iter, old_state, state);
> + __drm_atomic_helper_damage_iter_init(&iter, old_state, state, buffer_damage);
> drm_atomic_for_each_plane_damage(&iter, &clip) {
> rect->x1 = min(rect->x1, clip.x1);
> rect->y1 = min(rect->y1, clip.y1);
> @@ -336,6 +370,10 @@ static bool __drm_atomic_helper_damage_merged(const struct drm_plane_state *old_
> * For details see: drm_atomic_helper_damage_iter_init() and
> * drm_atomic_helper_damage_iter_next().
> *
> + * Note that this helper is for drivers that do per-plane uploads and expect
> + * to handle frame damages. Drivers that do per-buffer uploads instead should
> + * use @drm_atomic_helper_buffer_damage_merged() that handles buffer damages.
> + *
> * Returns:
> * True if there is valid plane damage otherwise false.
> */
> @@ -343,6 +381,35 @@ bool drm_atomic_helper_damage_merged(const struct drm_plane_state *old_state,
> struct drm_plane_state *state,
> struct drm_rect *rect)
> {
> - return __drm_atomic_helper_damage_merged(old_state, state, rect);
> + return __drm_atomic_helper_damage_merged(old_state, state, rect, false);
> }
> EXPORT_SYMBOL(drm_atomic_helper_damage_merged);
> +
> +/**
> + * drm_atomic_helper_buffer_damage_merged - Merged buffer damage
> + * @old_state: Old plane state for validation.
> + * @state: Plane state from which to iterate the damage clips.
> + * @rect: Returns the merged buffer damage rectangle
> + *
> + * This function merges any valid buffer damage clips into one rectangle and
> + * returns it in @rect. It checks if the framebuffers attached to @old_state
> + * and @state are the same. If that is not the case then the returned damage
> + * rectangle is the &drm_plane_state.src, since a full update should happen.
> + *
> + * Note that &drm_plane_state.fb_damage_clips == NULL in plane state means that
> + * full plane update should happen. It also ensure helper iterator will return
> + * &drm_plane_state.src as damage.
> + *
> + * For details see: drm_atomic_helper_buffer_damage_iter_init() and
> + * drm_atomic_helper_damage_iter_next().
> + *
> + * Returns:
> + * True if there is valid buffer damage otherwise false.
> + */
> +bool drm_atomic_helper_buffer_damage_merged(const struct drm_plane_state *old_state,
> + struct drm_plane_state *state,
> + struct drm_rect *rect)
> +{
> + return __drm_atomic_helper_damage_merged(old_state, state, rect, true);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_buffer_damage_merged);
> diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
> index effda42cce31..328bb249d68f 100644
> --- a/include/drm/drm_damage_helper.h
> +++ b/include/drm/drm_damage_helper.h
> @@ -74,11 +74,18 @@ void
> drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> const struct drm_plane_state *old_state,
> const struct drm_plane_state *new_state);
> +void
> +drm_atomic_helper_buffer_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
> + const struct drm_plane_state *old_state,
> + const struct drm_plane_state *new_state);
> bool
> drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter *iter,
> struct drm_rect *rect);
> bool drm_atomic_helper_damage_merged(const struct drm_plane_state *old_state,
> struct drm_plane_state *state,
> struct drm_rect *rect);
> +bool drm_atomic_helper_buffer_damage_merged(const struct drm_plane_state *old_state,
> + struct drm_plane_state *state,
> + struct drm_rect *rect);
>
> #endif
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (841 bytes)
Powered by blists - more mailing lists