Re: [PATCH 2/2] drm/i915/psr: Clear stale sel fetch enable bits on sel fetch disable

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces `clear_psr2_sel_fetch` to clear stale hardware bits when selective fetch is disabled, but immediately returns early, skipping the addition of planes to the atomic state. This causes the fix to fail during non-modeset transitions.
--

commit 6d95f584528dbe05e1cd976db461a45c72fb5319
Author: Nemesa Garg <nemesa.garg@intel.com>
drm/i915/psr: Clear stale sel fetch enable bits on sel fetch disable

This commit attempts to clear stale selective fetch enable bits by tracking
the transition in clear_psr2_sel_fetch and having the plane/cursor arm paths
write 0 to the registers. It updates intel_psr2_sel_fetch_update() to record
when selective fetch is turned off.

Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8739

[ ... ]
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index f490beb66629e..872e253db1786 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
[ ... ]
> @@ -2901,6 +2903,19 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>  	bool full_update = false, su_area_changed;
>  	int i, ret;
>  
> +	/*
> +	 * Selective fetch is not always usable, for instance it is dropped
> +	 * while pipe CRC is active. The planes keep their selective fetch
> +	 * enable bit set in hardware over that, and a plane disabled while
> +	 * selective fetch is off never gets the bit cleared. Once selective
> +	 * fetch comes back the hardware would resume fetching for a plane that
> +	 * is no longer enabled and keep its DDB range reserved, so have the
> +	 * plane update drop the bit for every plane of the pipe as selective
> +	 * fetch is turned off.
> +	 */
> +	crtc_state->clear_psr2_sel_fetch = old_crtc_state->enable_psr2_sel_fetch &&
> +		!crtc_state->enable_psr2_sel_fetch;
> +
>  	if (!crtc_state->enable_psr2_sel_fetch)
>  		return 0;

[Severity: High]
Will this early return skip adding unmodified planes to the atomic state
during non-modeset transitions?

During a non-modeset atomic commit (like a fastset triggered by color
management or CDCLK updates) that disables PSR2 selective fetch, the core
drm_atomic_helper_check_modeset() function does not automatically add all
planes to the state.

When intel_psr2_sel_fetch_update() detects this transition, it sets
clear_psr2_sel_fetch to true, but then immediately returns 0 here because
enable_psr2_sel_fetch is false.

Because the function returns before reaching the
drm_atomic_add_affected_planes() call further down, unmodified planes are
not added to the atomic state. Could this cause their plane arm callbacks to
be skipped entirely, leaving the hardware bits active and potentially causing
the display corruption this patch was intended to fix?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909110332.3528029-1-nemesa.garg@intel.com?part=2