Re: [PATCH v2 1/2] drm/i915/display: Reject flipq for pipe or timing changes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Dead code in intel_flipq_commit_is_eligible() due to redundant pipe/timing update checks.
--

commit 6779c5cb91fea91170f2e668dc4b0daaf17635f8
Author: Mika Kahola <mika.kahola@intel.com>

drm/i915/display: Reject flipq for pipe or timing changes

This commit introduces intel_flipq_commit_is_eligible() to check if a
commit can use flipq. It explicitly rejects pipe, M/N, and LRR updates in
addition to the existing exclusions.

> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index fc30a455bed36..a2f5c3005dec1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c

[ ... ]

> +static bool intel_flipq_commit_is_eligible(struct intel_atomic_state *state,
> +					   struct intel_crtc *crtc)
> +{
> +	struct intel_display *display = to_intel_display(state);
> +	const struct intel_crtc_state *new_crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +
> +	if (!intel_flipq_supported(display))
> +		return false;
> +
> +	if (intel_crtc_needs_modeset(new_crtc_state) ||
> +	    intel_crtc_needs_fastset(new_crtc_state) ||
> +	    intel_crtc_needs_color_update(new_crtc_state))
> +		return false;

[ ... ]

> +	/*
> +	 * Flipq is only suitable for simple queued plane updates that do
> +	 * not require additional pipe or timing programming.
> +	 */
> +	if (new_crtc_state->update_pipe ||
> +	    new_crtc_state->update_m_n ||
> +	    new_crtc_state->update_lrr)
> +		return false;

[Severity: Low]
Are these final checks reachable?

Since intel_crtc_needs_fastset() directly evaluates to
new_crtc_state->update_pipe, any commit where update_pipe is true will return
early from the first set of checks in this function.

Additionally, any commit requiring M/N or LRR changes is initially evaluated
as a full modeset. If the commit is later downgraded from a full modeset to a
fastset, new_crtc_state->update_pipe is unconditionally set to true.

Therefore, if update_m_n or update_lrr is true, the commit is guaranteed to
either be a full modeset (intel_crtc_needs_modeset() == true) or a fastset
(intel_crtc_needs_fastset() == true). This appears to make this final block of
code structurally unreachable.

Can these redundant checks be removed, or is there a specific case where they
can still be evaluated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902105448.809455-1-mika.kahola@intel.com?part=1