Re: [PATCH v3 00/10] drm: Improve logic behind damage handling

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 27.05.26 um 16:46 schrieb Thomas Zimmermann:
> DRM clients can supply information on framebuffer areas to update as
> part of each page flip, called damage clipping rectangles. But DRM's
> processing of this information is inconsistent and prone to errors.
>
> - There are multiple fields and tests that decide if damage clips
> should be taken or ignored.
>
> - Sometimes damage clips are removed behind the back of the DRM client.
>
> - Atomic helpers evaluate damage clipping in the middle of the atomic
> check: after connectors and encoders, but before planes and CRTCs. Hence
> pipeline stages have an inconsistent view.
>
> - Which leads to drivers (ingenic) doing a re-evaluation if necessary.
>
> - Tests of plane source coordinates only happen during commits. At this
> point, the driver should already know if damage clips are to be taken or
> not. Because of this, some drivers (appletbdrm) might operate on incorrect
> damage information for their internal workings. This also leads to excessive
> use of the old plane state.
>
> Therefore go through DRM helpers and drivers and fix the logic.
>
> - Run all of the atomic checks with the damage information supplied by
> DRM clients. Afterwards evaluate plane and CRTC states on whether to
> take or ignore damage clips. Do all related tests in a single atomic
> helper.
>
> - Do not discard damage clips. Set ignore_damage_clips in struct
> drm_plane_state instead. This includes changes to plane source-coordinates.
> The damage iterator now only has to look at this flag to detect if it
> should use the damage clips.
>
> - Go over drivers and fix the damage handling in the plane's
> atomic_update helpers. Most drivers no longer need the old plane state
> in their update.
>
> - The appletbdrm driver requires a fix in how it uses damage information.
> Ingenic and vmwgfx can be simplified. These changes improve the drivers'
> code organization.
>
> - Kunit tests require some changes. Drop some obsolete tests and add a new
> one for ignore_damage_flags.
>
> Tested with bochs, mgag200, Kunit tests.
>
> v3:
> - fix error path in appletbdrm
> v2:
> - rebase on latest upstream
>
> Thomas Zimmermann (10):
>    drm/damage-helper: Do not alter damage clips on modeset, but ignore
>      them
>    drm/atomic-helpers: Evaluate plane damage after atomic_check
>    drm/ingenic: Remove calls to drm_atomic_helper_check_plane_damage()
>    drm/damage-helper: Test src coord in
>      drm_atomic_helper_check_plane_damage()
>    drm/appletbdrm: Allocate request/response buffers in begin_fb_access
>    drm/damage-helper: Remove old state from
>      drm_atomic_helper_damage_iter_init()
>    drm/damage-helper: Remove old state from
>      drm_atomic_helper_damage_merged()
>    drm/atomic_helper: Do not evaluate plane damage before atomic_check
>    drm/damage-helper: Rename state parameters in damage helpers
>    drm/vmwgfx: Remove unused field struct
>      vmwgfx_du_update_plane.old_state
>
>   drivers/gpu/drm/ast/ast_cursor.c              |   3 +-
>   drivers/gpu/drm/ast/ast_mode.c                |   2 +-
>   drivers/gpu/drm/drm_atomic_helper.c           |   6 +-
>   drivers/gpu/drm/drm_atomic_state_helper.c     |   1 +
>   drivers/gpu/drm/drm_damage_helper.c           |  44 ++--
>   drivers/gpu/drm/drm_fb_dma_helper.c           |   2 +-
>   drivers/gpu/drm/drm_mipi_dbi.c                |   3 +-
>   drivers/gpu/drm/gud/gud_pipe.c                |   3 +-
>   drivers/gpu/drm/hyperv/hyperv_drm_modeset.c   |   3 +-
>   drivers/gpu/drm/i915/display/intel_plane.c    |  11 +-
>   drivers/gpu/drm/i915/display/intel_psr.c      |   3 +-
>   drivers/gpu/drm/ingenic/ingenic-drm-drv.c     |   3 -
>   drivers/gpu/drm/ingenic/ingenic-ipu.c         |   8 +-
>   drivers/gpu/drm/mgag200/mgag200_mode.c        |   3 +-
>   drivers/gpu/drm/sitronix/st7571.c             |   3 +-
>   drivers/gpu/drm/sitronix/st7586.c             |   3 +-
>   drivers/gpu/drm/sitronix/st7920.c             |   3 +-
>   drivers/gpu/drm/solomon/ssd130x.c             |   9 +-
>   drivers/gpu/drm/sysfb/drm_sysfb_modeset.c     |   3 +-
>   .../gpu/drm/tests/drm_damage_helper_test.c    | 200 +++---------------
>   drivers/gpu/drm/tiny/appletbdrm.c             |  59 +++---
>   drivers/gpu/drm/tiny/bochs.c                  |   3 +-
>   drivers/gpu/drm/tiny/cirrus-qemu.c            |   2 +-
>   drivers/gpu/drm/tiny/gm12u320.c               |   2 +-
>   drivers/gpu/drm/tiny/ili9225.c                |   3 +-
>   drivers/gpu/drm/tiny/repaper.c                |   2 +-
>   drivers/gpu/drm/tiny/sharp-memory.c           |   3 +-
>   drivers/gpu/drm/udl/udl_modeset.c             |   3 +-
>   drivers/gpu/drm/virtio/virtgpu_plane.c        |   2 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |   5 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.h           |   2 -
>   drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c          |  12 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c          |  15 +-
>   include/drm/drm_damage_helper.h               |   9 +-
>   34 files changed, 123 insertions(+), 315 deletions(-)
>
>
> base-commit: 5fb5a9a63cf5ece68e0eeb6fa397da27712bccf0

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)