Re: [PATCH v2 4/5] drm/plane: Extend damage tracking kernel-doc

[Thomas Zimmermann]

Hi

Am 15.11.23 um 14:15 schrieb Javier Martinez Canillas:
> The "Damage Tracking Properties" section in the documentation doesn't have
> info about the two type of damage handling: frame damage vs buffer damage.
>
> Add it to the section and mention that helpers only support frame damage,
> and how drivers handling buffer damage can indicate that the damage clips
> should be ignored.
>
> Also add references to further documentation about the two damage types.
>
> Suggested-by: Simon Ser <contact@xxxxxxxxxxx>
> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
> ---
> - Dropped Simon Ser's Reviwed-by tag because the text changed to adapt
>    to the approach Thomas Zimmermann suggested for v2.
>
> (no changes since v1)
>
>   drivers/gpu/drm/drm_plane.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 24e7998d1731..3b1b8bca3065 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1442,6 +1442,26 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>    * Drivers implementing damage can use drm_atomic_helper_damage_iter_init() and
>    * drm_atomic_helper_damage_iter_next() helper iterator function to get damage
>    * rectangles clipped to &drm_plane_state.src.
> + *
> + * Note that there are two types of damage handling: frame damage and buffer
> + * damage. The type of damage handling implemented depends on a driver's upload
> + * target. Drivers implementing a per-plane or per-CRTC upload target need to
> + * handle frame damage while drivers implementing a per-buffer upload target
> + * need to handle buffer damage.
> + *
> + * The existing damage helpers only support the frame damage type, there is no
> + * buffer age support or similar damage accumulation algorithm implemented yet.
> + *
> + * Only drivers handling frame damage can use the mentiored damage helpers to
> + * iterate over the damaged regions. Drivers that handle buffer damage, need to
> + * set &struct drm_plane_state.ignore_damage_clips as an indication to
> + * drm_atomic_helper_damage_iter_init() that the damage clips should be ignored.
> + * In that case, the returned damage rectangle is the &drm_plane_state.src since
> + * a full plane update should happen.
> + *
> + * For more information about the two type of damage, see:
> + *     https://registry.khronos.org/EGL/extensions/KHR/EGL_KHR_swap_buffers_with_damage.txt
> + *     https://emersion.fr/blog/2019/intro-to-damage-tracking/

One thought you might want to consider.

These URLs are helpful. The only issue I have is that frame damage and 
buffer damage are user-space concepts. The kernel bug is that damage 
handling expects the backing storage/upload buffer not to change for a 
given plane. If the upload buffer changes between page flips, the new 
upload buffer has to be updated as a whole. Hence no damage handling then.

And then maybe say that the effects of frame damage and buffer damage in 
userspace illustrate this problem and give the URLs.

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)

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature