2023-11-15 13:16:50

by Javier Martinez Canillas

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

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 <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
- 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/
*/

/**
--
2.41.0


2023-11-16 12:06:53

by Thomas Zimmermann

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

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 <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
> - 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)


Attachments:
OpenPGP_signature.asc (855.00 B)
OpenPGP digital signature

2023-11-16 12:15:59

by Simon Ser

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

On Thursday, November 16th, 2023 at 13:06, Thomas Zimmermann <[email protected]> wrote:

> > + * 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

Typo: mentioned

> > + * 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.

Why would these concepts be specific to user-space? The kernel could
better handle buffer damage instead of forcing full damage, by doing
something similar to what user-space does.

Anyways:

Reviewed-by: Simon Ser <[email protected]>

2023-11-16 12:34:18

by Thomas Zimmermann

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

Hi

Am 16.11.23 um 13:14 schrieb Simon Ser:
> On Thursday, November 16th, 2023 at 13:06, Thomas Zimmermann <[email protected]> wrote:
>
>>> + * 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
>
> Typo: mentioned
>
>>> + * 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.
>
> Why would these concepts be specific to user-space? The kernel could
> better handle buffer damage instead of forcing full damage, by doing
> something similar to what user-space does.

The terms 'frame damage' and 'buffer damage' do not exist in the kernel.
The problem can be better described in wording that is common within the
context of the kernel drivers.

Best regards
Thomas

>
> Anyways:
>
> Reviewed-by: Simon Ser <[email protected]>

--
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)


Attachments:
OpenPGP_signature.asc (855.00 B)
OpenPGP digital signature

2023-11-16 15:25:15

by Pekka Paalanen

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

On Thu, 16 Nov 2023 13:34:07 +0100
Thomas Zimmermann <[email protected]> wrote:

> Hi
>
> Am 16.11.23 um 13:14 schrieb Simon Ser:
> > On Thursday, November 16th, 2023 at 13:06, Thomas Zimmermann <[email protected]> wrote:
> >
> >>> + * 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
> >
> > Typo: mentioned
> >
> >>> + * 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.
> >
> > Why would these concepts be specific to user-space? The kernel could
> > better handle buffer damage instead of forcing full damage, by doing
> > something similar to what user-space does.
>
> The terms 'frame damage' and 'buffer damage' do not exist in the kernel.
> The problem can be better described in wording that is common within the
> context of the kernel drivers.

What terms does the kernel use for these two different concepts of
damage?


Thanks,
pq


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

2023-11-17 11:48:23

by Thomas Zimmermann

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

Hi

Am 16.11.23 um 16:24 schrieb Pekka Paalanen:
> On Thu, 16 Nov 2023 13:34:07 +0100
> Thomas Zimmermann <[email protected]> wrote:
>
>> Hi
>>
>> Am 16.11.23 um 13:14 schrieb Simon Ser:
>>> On Thursday, November 16th, 2023 at 13:06, Thomas Zimmermann <[email protected]> wrote:
>>>
>>>>> + * 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
>>>
>>> Typo: mentioned
>>>
>>>>> + * 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.
>>>
>>> Why would these concepts be specific to user-space? The kernel could
>>> better handle buffer damage instead of forcing full damage, by doing
>>> something similar to what user-space does.
>>
>> The terms 'frame damage' and 'buffer damage' do not exist in the kernel.
>> The problem can be better described in wording that is common within the
>> context of the kernel drivers.
>
> What terms does the kernel use for these two different concepts of
> damage?

None AFAIK. Damage is relative to the plane's backing storage/upload
buffer. That's frame damage. We never needed a name for buffer damage as
we don't support it.

Best regards
Thomas

>
>
> Thanks,
> pq

--
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)


Attachments:
OpenPGP_signature.asc (855.00 B)
OpenPGP digital signature