[PATCH] drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips

[PATCH] drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips

[Thomas Zimmermann]

Git commit 35ed38d58257 ("drm: Allow drivers to indicate the damage
helpers to ignore damage clips") introduced ignore_damage_clips to
selectively ignore damage clipping in certain framebuffer changes. The
mode-setting pipeline can disabled damage clippings for an atomic commit
by setting ignore_damage_clips in struct drm_plane_state. The atomic
commit will then do a full display update.

Although the i915 driver does not modify the flag, DRM's damage iterator
will soon rely on it. Calling drm_atomic_helper_check_plane_damage() right
before drm_atomic_helper_damage_merged() guarantees that it has the correct
state. The i915 driver does not do this elsewhere so far.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
Taken with minor updates to the commit description from the series at

  https://lore.kernel.org/dri-devel/20260610152505.260172-1-tzimmermann@suse.de/
---
 drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 92af21d823a3..5a155f60110f 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2949,6 +2949,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 		src = drm_plane_state_src(&new_plane_state->uapi);
 		drm_rect_fp_to_int(&src, &src);
 
+		/* Prepare plane-damage state before using it */
+		drm_atomic_helper_check_plane_damage(&state->base, &new_plane_state->uapi);
+
 		if (!drm_atomic_helper_damage_merged(&old_plane_state->uapi,
 						     &new_plane_state->uapi, &damaged_area))
 			continue;
-- 
2.54.0

Re: [PATCH] drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm_atomic_helper_check_plane_damage() is placed too late in the atomic check pipeline and conditionally bypassed, breaking the damage clipping contract during modesets.
--

commit 5c79bcf5010895b431b3c2135c7b80a5163d8e9f
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips

This commit introduces a call to drm_atomic_helper_check_plane_damage() in
intel_psr2_sel_fetch_update() to selectively ignore damage clipping in certain
framebuffer changes.

> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 92af21d823a37..5a155f60110fb 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
[ ... ]
> @@ -2949,6 +2949,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>  		src = drm_plane_state_src(&new_plane_state->uapi);
>  		drm_rect_fp_to_int(&src, &src);
>  
> +		/* Prepare plane-damage state before using it */
> +		drm_atomic_helper_check_plane_damage(&state->base, &new_plane_state->uapi);
> +
>  		if (!drm_atomic_helper_damage_merged(&old_plane_state->uapi,

[Severity: High]
Is this call to drm_atomic_helper_check_plane_damage() placed too late in the
atomic check pipeline?

By placing it here in intel_psr2_sel_fetch_update() (which runs during the CRTC
check phase), intel_plane_atomic_check() has already run and called
intel_plane_copy_uapi_plane_damage(). That means new_plane_state->damage was
already calculated using the unsanitized fb_damage_clips before this check
could clear them during a full modeset.

Additionally, this loop selectively skips planes on non-PSR2 CRTCs or planes
with changed visibility (e.g. via the continue statements earlier in the loop).
Doesn't this mean the damage state is left unsanitized for those skipped
planes, potentially causing visual corruption when restricted clip rectangles
are used instead of updating the full plane?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701130929.234695-1-tzimmermann@suse.de?part=1

Re: [PATCH] drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips

[Thomas Zimmermann]

Am 01.07.26 um 15:23 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] drm_atomic_helper_check_plane_damage() is placed too late in the atomic check pipeline and conditionally bypassed, breaking the damage clipping contract during modesets.

This is intentionally. We need check_plane_damage only for the call to 
damage iterators that comes next.

> --
>
> commit 5c79bcf5010895b431b3c2135c7b80a5163d8e9f
> Author: Thomas Zimmermann <tzimmermann@suse.de>
>
> drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips
>
> This commit introduces a call to drm_atomic_helper_check_plane_damage() in
> intel_psr2_sel_fetch_update() to selectively ignore damage clipping in certain
> framebuffer changes.
>
>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
>> index 92af21d823a37..5a155f60110fb 100644
>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> [ ... ]
>> @@ -2949,6 +2949,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>>   		src = drm_plane_state_src(&new_plane_state->uapi);
>>   		drm_rect_fp_to_int(&src, &src);
>>   
>> +		/* Prepare plane-damage state before using it */
>> +		drm_atomic_helper_check_plane_damage(&state->base, &new_plane_state->uapi);
>> +
>>   		if (!drm_atomic_helper_damage_merged(&old_plane_state->uapi,
> [Severity: High]
> Is this call to drm_atomic_helper_check_plane_damage() placed too late in the
> atomic check pipeline?
>
> By placing it here in intel_psr2_sel_fetch_update() (which runs during the CRTC
> check phase), intel_plane_atomic_check() has already run and called
> intel_plane_copy_uapi_plane_damage(). That means new_plane_state->damage was
> already calculated using the unsanitized fb_damage_clips before this check
> could clear them during a full modeset.
>
> Additionally, this loop selectively skips planes on non-PSR2 CRTCs or planes
> with changed visibility (e.g. via the continue statements earlier in the loop).
> Doesn't this mean the damage state is left unsanitized for those skipped
> planes, potentially causing visual corruption when restricted clip rectangles
> are used instead of updating the full plane?
>

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

Re: [PATCH] drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips

[Thomas Zimmermann]

gentle reminder to review this patch

Am 01.07.26 um 15:07 schrieb Thomas Zimmermann:
> Git commit 35ed38d58257 ("drm: Allow drivers to indicate the damage
> helpers to ignore damage clips") introduced ignore_damage_clips to
> selectively ignore damage clipping in certain framebuffer changes. The
> mode-setting pipeline can disabled damage clippings for an atomic commit
> by setting ignore_damage_clips in struct drm_plane_state. The atomic
> commit will then do a full display update.
>
> Although the i915 driver does not modify the flag, DRM's damage iterator
> will soon rely on it. Calling drm_atomic_helper_check_plane_damage() right
> before drm_atomic_helper_damage_merged() guarantees that it has the correct
> state. The i915 driver does not do this elsewhere so far.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> Taken with minor updates to the commit description from the series at
>
>    https://lore.kernel.org/dri-devel/20260610152505.260172-1-tzimmermann@suse.de/
> ---
>   drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 92af21d823a3..5a155f60110f 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2949,6 +2949,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>   		src = drm_plane_state_src(&new_plane_state->uapi);
>   		drm_rect_fp_to_int(&src, &src);
>   
> +		/* Prepare plane-damage state before using it */
> +		drm_atomic_helper_check_plane_damage(&state->base, &new_plane_state->uapi);
> +
>   		if (!drm_atomic_helper_damage_merged(&old_plane_state->uapi,
>   						     &new_plane_state->uapi, &damaged_area))
>   			continue;

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

Re: [PATCH] drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips

[Rodrigo Vivi]

On Wed, Jul 08, 2026 at 09:44:05AM +0200, Thomas Zimmermann wrote:
> gentle reminder to review this patch

Ville, could you please take a look on this?

Thomas, have you seen shashiko for this?
https://sashiko.dev/#/patchset/20260701130929.234695-1-tzimmermann%40suse.de

> 
> Am 01.07.26 um 15:07 schrieb Thomas Zimmermann:
> > Git commit 35ed38d58257 ("drm: Allow drivers to indicate the damage
> > helpers to ignore damage clips") introduced ignore_damage_clips to
> > selectively ignore damage clipping in certain framebuffer changes. The
> > mode-setting pipeline can disabled damage clippings for an atomic commit
> > by setting ignore_damage_clips in struct drm_plane_state. The atomic
> > commit will then do a full display update.
> > 
> > Although the i915 driver does not modify the flag, DRM's damage iterator
> > will soon rely on it. Calling drm_atomic_helper_check_plane_damage() right
> > before drm_atomic_helper_damage_merged() guarantees that it has the correct
> > state. The i915 driver does not do this elsewhere so far.
> > 
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> > ---
> > Taken with minor updates to the commit description from the series at
> > 
> >    https://lore.kernel.org/dri-devel/20260610152505.260172-1-tzimmermann@suse.de/
> > ---
> >   drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 92af21d823a3..5a155f60110f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2949,6 +2949,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> >   		src = drm_plane_state_src(&new_plane_state->uapi);
> >   		drm_rect_fp_to_int(&src, &src);
> > +		/* Prepare plane-damage state before using it */
> > +		drm_atomic_helper_check_plane_damage(&state->base, &new_plane_state->uapi);
> > +
> >   		if (!drm_atomic_helper_damage_merged(&old_plane_state->uapi,
> >   						     &new_plane_state->uapi, &damaged_area))
> >   			continue;
> 
> -- 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
> GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)
> 
> 

Re: [PATCH] drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips

[Thomas Zimmermann]

Hi

Am 08.07.26 um 22:11 schrieb Rodrigo Vivi:
> On Wed, Jul 08, 2026 at 09:44:05AM +0200, Thomas Zimmermann wrote:
>> gentle reminder to review this patch
> Ville, could you please take a look on this?
>
> Thomas, have you seen shashiko for this?
> https://sashiko.dev/#/patchset/20260701130929.234695-1-tzimmermann%40suse.de

I've seen it and replied on dri-devel. The rational is that it is kind 
of intentional.  I looked through i915's atomic_check to find a good 
common spot for _check_plane_damage(), but neither place seemed to fit 
well.  Hence as the call is only required for the damage iterator's 
state,(*) I put it right before where the damage iterator is being 
used.  But since you mentioned it, I'll take another look at the issue.


Best regards
Thomas

(*) I later want to streamline damage handling as in 
https://lore.kernel.org/dri-devel/20260610152505.260172-1-tzimmermann@suse.de/


>
>> Am 01.07.26 um 15:07 schrieb Thomas Zimmermann:
>>> Git commit 35ed38d58257 ("drm: Allow drivers to indicate the damage
>>> helpers to ignore damage clips") introduced ignore_damage_clips to
>>> selectively ignore damage clipping in certain framebuffer changes. The
>>> mode-setting pipeline can disabled damage clippings for an atomic commit
>>> by setting ignore_damage_clips in struct drm_plane_state. The atomic
>>> commit will then do a full display update.
>>>
>>> Although the i915 driver does not modify the flag, DRM's damage iterator
>>> will soon rely on it. Calling drm_atomic_helper_check_plane_damage() right
>>> before drm_atomic_helper_damage_merged() guarantees that it has the correct
>>> state. The i915 driver does not do this elsewhere so far.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>>> ---
>>> Taken with minor updates to the commit description from the series at
>>>
>>>     https://lore.kernel.org/dri-devel/20260610152505.260172-1-tzimmermann@suse.de/
>>> ---
>>>    drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
>>> index 92af21d823a3..5a155f60110f 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>>> @@ -2949,6 +2949,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>>>    		src = drm_plane_state_src(&new_plane_state->uapi);
>>>    		drm_rect_fp_to_int(&src, &src);
>>> +		/* Prepare plane-damage state before using it */
>>> +		drm_atomic_helper_check_plane_damage(&state->base, &new_plane_state->uapi);
>>> +
>>>    		if (!drm_atomic_helper_damage_merged(&old_plane_state->uapi,
>>>    						     &new_plane_state->uapi, &damaged_area))
>>>    			continue;
>> -- 
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
>> GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)
>>
>>

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

Re: [PATCH] drm/i915/display: Handle struct drm_plane_state.ignore_damage_clips

[Thomas Zimmermann]

another gentle reminder to give this patch a review

Am 01.07.26 um 15:07 schrieb Thomas Zimmermann:
> Git commit 35ed38d58257 ("drm: Allow drivers to indicate the damage
> helpers to ignore damage clips") introduced ignore_damage_clips to
> selectively ignore damage clipping in certain framebuffer changes. The
> mode-setting pipeline can disabled damage clippings for an atomic commit
> by setting ignore_damage_clips in struct drm_plane_state. The atomic
> commit will then do a full display update.
>
> Although the i915 driver does not modify the flag, DRM's damage iterator
> will soon rely on it. Calling drm_atomic_helper_check_plane_damage() right
> before drm_atomic_helper_damage_merged() guarantees that it has the correct
> state. The i915 driver does not do this elsewhere so far.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> Taken with minor updates to the commit description from the series at
>
>    https://lore.kernel.org/dri-devel/20260610152505.260172-1-tzimmermann@suse.de/
> ---
>   drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 92af21d823a3..5a155f60110f 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2949,6 +2949,9 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>   		src = drm_plane_state_src(&new_plane_state->uapi);
>   		drm_rect_fp_to_int(&src, &src);
>   
> +		/* Prepare plane-damage state before using it */
> +		drm_atomic_helper_check_plane_damage(&state->base, &new_plane_state->uapi);
> +
>   		if (!drm_atomic_helper_damage_merged(&old_plane_state->uapi,
>   						     &new_plane_state->uapi, &damaged_area))
>   			continue;

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