Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The i915 driver fails to perform full plane updates when source coordinates change (e.g., panning or cropping), leading to visual corruption. --
commit 020ae3c303979cf114157a7b95370725231ed90b Author: Thomas Zimmermann <[email protected]> drm/damage-helper: Remove old state from drm_atomic_helper_damage_merged() Nothing in drm_atomic_helper_damage_merged() requires the old plane state. Remove the parameter and mass-convert callers. Most callers now no longer require the old plane state in their plane's atomic_update helper. > diff --git a/drivers/gpu/drm/i915/display/intel_plane.c > b/drivers/gpu/drm/i915/display/intel_plane.c > index 2a52b36c646c7..3560e222a3eae 100644 > --- a/drivers/gpu/drm/i915/display/intel_plane.c > +++ b/drivers/gpu/drm/i915/display/intel_plane.c [ ... ] > @@ -346,7 +346,6 @@ static void intel_plane_clear_hw_state(struct > intel_plane_state *plane_state) > > static void > intel_plane_copy_uapi_plane_damage(struct intel_plane_state *new_plane_state, > - const struct intel_plane_state > *old_uapi_plane_state, > const struct intel_plane_state > *new_uapi_plane_state) > { > struct intel_display *display = to_intel_display(new_plane_state); > @@ -356,10 +355,9 @@ intel_plane_copy_uapi_plane_damage(struct > intel_plane_state *new_plane_state, > if (DISPLAY_VER(display) < 12) > return; > > - if (!drm_atomic_helper_damage_merged(&old_uapi_plane_state->uapi, > - &new_uapi_plane_state->uapi, > + if (!drm_atomic_helper_damage_merged(&new_uapi_plane_state->uapi, > damage)) [Severity: High] Does this code risk missing full plane updates when source coordinates change? It appears that with the removal of the old state, the fallback checking for source coordinate changes (like panning) was also removed from the damage iterators. The DRM core now expects drivers to call drm_atomic_helper_check_plane_damage() to set ignore_damage_clips = true when the source changes. Because i915 implements a custom atomic check phase in intel_atomic_check() that does not call drm_atomic_helper_check_planes() or drm_atomic_helper_check_plane_damage(), ignore_damage_clips remains unset. Will drm_atomic_helper_damage_merged() incorrectly return only partial damage here when the plane is panned, missing the full update? > - /* Incase helper fails, mark whole plane region as damage */ > + /* In case the helper fails, mark whole plane region as damage > */ > *damage = drm_plane_state_src(&new_uapi_plane_state->uapi); > } [ ... ] > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index e138982dc91f6..22bab263d85d4 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c [ ... ] > @@ -3018,8 +3018,7 @@ 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); > > - if (!drm_atomic_helper_damage_merged(&old_plane_state->uapi, > - &new_plane_state->uapi, > &damaged_area)) > + if (!drm_atomic_helper_damage_merged(&new_plane_state->uapi, > &damaged_area)) [Severity: High] Can this cause severe visual corruption for PSR2 selective fetch? If intel_psr2_sel_fetch_update() no longer forces a full update on source coordinate changes, it seems this might only fetch the small damaged area (such as a blinking cursor) while leaving the rest of the panned screen stale. > continue; > > damaged_area.y1 += new_plane_state->uapi.dst.y1 - src.y1; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
