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 <[email protected]> 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/[email protected]?part=1
