Thomas Zimmermann <tzimmermann@xxxxxxx> writes:
> Hi
>
> Am 14.11.23 um 16:58 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmermann@xxxxxxx> 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@xxxxxxxxxxxxxxx> # v6.4+
>>>> Reported-by: nerdopolis <bluescreen_avenger@xxxxxxxxxxx>
>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218115
>>>> Suggested-by: Sima Vetter <daniel.vetter@xxxxxxxx>
>>>> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
>>>> ---
>>>>
>>>> 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.
>
Thanks. That indeed seems more aligned with the rest of the DRM framework.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
