Messages in this thread | | | Date | Tue, 14 Nov 2023 17:31:39 +0100 | Subject | Re: [PATCH 2/6] drm: Add drm_atomic_helper_buffer_damage_{iter_init,merged}() helpers | From | Thomas Zimmermann <> |
| |
Hi
Am 14.11.23 um 16:58 schrieb Javier Martinez Canillas: > Thomas Zimmermann <tzimmermann@suse.de> writes: > > Hello Thomas, > > Thanks a lot for your feedback. > >> 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@vger.kernel.org> # v6.4+ >>> Reported-by: nerdopolis <bluescreen_avenger@verizon.net> >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218115 >>> Suggested-by: Sima Vetter <daniel.vetter@ffwll.ch> >>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.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) >> >> I think it would be preferable to drop patches one and two and instead >> add this parameter directly to drm_atomic_helper_damage_iter_init() and >> drm_atomic_helper_damage_merged(). That's a bit of churn, but more >> readable code. >> > > Makes sense. I'll do that in v2.
Instead of modifying these function interfaces, it might be even better to introduce a state flag in struct drm_plane_state that you can modify in the plane's atomic_check helper. Something simple like this:
if (old_fb != new_fb) plane_state->ignore_damage_clips = true;
in the affected drivers/planes. In drm_atomic_helper_damage_iter_init() you can use it to generate a full update. This avoids the churn and is in line with the overall check/commit design of the DRM framework.
Best regards Thomas
> >> Best regards >> Thomas >> >
-- 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) [unhandled content-type:application/pgp-signature] | |