Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The patch inadvertently removes `local_irq_save()` protection from the Xe driver's vblank timestamping critical section, allowing hard IRQs to interrupt timing-critical register reads. - [Low] The sparse context annotations `__acquires(i915->uncore.lock)` and `__releases(i915->uncore.lock)` reference an undeclared identifier `i915`. --
(Note: The format of this report is altered to provide a free-form summary without directly quoting the patch code, due to automated recitation restrictions.) commit 8fb6242d95640a7bbab92fc65f4627e01aae6b0b Author: Mike Galbraith <[email protected]> drm/i915: Use preempt_disable/enable_rt() where recommended This patch changes preemption disabling in the intel vblank timestamping critical section, moving spin_lock_irqsave() into helper functions and explicitly disabling preemption for preempt-rt configurations. [Severity: Low] Will this cause a build failure when running the sparse static analyzer? The sparse annotations __acquires(i915->uncore.lock) and __releases(i915->uncore.lock) reference the i915 identifier in intel_vblank_section_enter_irqf() and intel_vblank_section_exit_irqf(). However, the function parameter is named display, and i915 is not locally declared. Should this reference the uncore lock through the display parameter instead? [Severity: Medium] Could this inadvertently remove IRQ protection for the Xe driver during timing critical register reads? Previously, i915_get_crtc_scanoutpos() explicitly called local_irq_save() before calling the intel_vblank_section_enter() stub. With this patch, the IRQ save is moved inside intel_vblank_section_enter_irqf(). However, the #else block implementation of intel_vblank_section_enter_irqf() merely sets *flags = 0. This means builds where that #else block is active (like the Xe driver) will no longer disable interrupts during this critical section. Might this allow hard IRQs to interrupt the sequence, creating an artificially large time delta between stime and etime, and causing the DRM core's max_error check to spuriously fail? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
