On Thu, Jul 02, 2026 at 09:21:53AM +0200, Maarten Lankhorst wrote:
> This fixes a lockdep splat that occurs in the code that should be run
> with interrupts disabled. The uncore and DMC locks should not be taken
> and released repeatedly in a timing sensitive path.
>
> Signed-off-by: Maarten Lankhorst <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_de.h | 8 ++++++
> drivers/gpu/drm/i915/display/intel_vblank.c | 4 +--
> drivers/gpu/drm/i915/intel_uncore.h | 26 +++++++++++++------
> .../drm/xe/compat-i915-headers/intel_uncore.h | 7 +++++
> 4 files changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_de.h
> b/drivers/gpu/drm/i915/display/intel_de.h
> index 1029790194296..497a308322847 100644
> --- a/drivers/gpu/drm/i915/display/intel_de.h
> +++ b/drivers/gpu/drm/i915/display/intel_de.h
> @@ -65,6 +65,14 @@ intel_de_read64_2x32(struct intel_display *display,
> intel_reg_t reg)
> return (u64)upper << 32 | lower;
> }
>
> +static inline u64
> +intel_de_read64_2x32_fw(struct intel_display *display,
> + i915_reg_t lower_reg, i915_reg_t upper_reg)
> +{
> + return intel_uncore_read64_2x32_fw(__to_uncore(display),
> + lower_reg, upper_reg);
> +}
> +
> static inline void
> intel_de_posting_read(struct intel_display *display, intel_reg_t reg)
> {
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c
> b/drivers/gpu/drm/i915/display/intel_vblank.c
> index c0cc0a4c25dbe..5ca22899055d7 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -109,8 +109,8 @@ u32 i915_get_vblank_counter(struct drm_crtc *crtc)
> * we get a low value that's stable across two reads of the high
> * register.
> */
> - frame = intel_de_read64_2x32_volatile(display, PIPEFRAMEPIXEL(display,
> pipe),
> - PIPEFRAME(display, pipe));
> + frame = intel_de_read64_2x32_fw(display, PIPEFRAMEPIXEL(display, pipe),
> + PIPEFRAME(display, pipe));
This is the only user of intel_de_read64_2x32_volatile() and it
doesn't need any locking/wakelocks/etc so we should just convert
intel_de_read64_2x32_volatile() into intel_de_read64_2x32_volatile_fw().
And we should just skip the uncore dependency (so that we'll have one
less thing to worry about later) so I'd just implement it directly in
intel_de.[ch] in terms of intel_de_read_fw().
>
> pixel = frame & PIPE_PIXEL_MASK;
> frame = (frame >> PIPE_FRAME_LOW_SHIFT) & 0xffffff;
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h
> b/drivers/gpu/drm/i915/intel_uncore.h
> index fafc2ca9a2376..507398a562649 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -449,13 +449,28 @@ static inline void intel_uncore_rmw_fw(struct
> intel_uncore *uncore,
> intel_uncore_write_fw(uncore, reg, val);
> }
>
> +static inline u64
> +intel_uncore_read64_2x32_fw(struct intel_uncore *uncore,
> + i915_reg_t lower_reg, i915_reg_t upper_reg)
> +{
> + u32 upper, lower, old_upper, loop = 0;
> + upper = intel_uncore_read_fw(uncore, upper_reg);
> + do {
> + old_upper = upper;
> + lower = intel_uncore_read_fw(uncore, lower_reg);
> + upper = intel_uncore_read_fw(uncore, upper_reg);
> + } while (upper != old_upper && loop++ < 2);
> +
> + return (u64)upper << 32 | lower;
> +}
> +
> static inline u64
> intel_uncore_read64_2x32(struct intel_uncore *uncore,
> i915_reg_t lower_reg, i915_reg_t upper_reg)
> {
> - u32 upper, lower, old_upper, loop = 0;
> enum forcewake_domains fw_domains;
> unsigned long flags;
> + u64 ret;
>
> fw_domains = intel_uncore_forcewake_for_reg(uncore, lower_reg,
> FW_REG_READ);
> @@ -466,17 +481,12 @@ intel_uncore_read64_2x32(struct intel_uncore *uncore,
> spin_lock_irqsave(&uncore->lock, flags);
> intel_uncore_forcewake_get__locked(uncore, fw_domains);
>
> - upper = intel_uncore_read_fw(uncore, upper_reg);
> - do {
> - old_upper = upper;
> - lower = intel_uncore_read_fw(uncore, lower_reg);
> - upper = intel_uncore_read_fw(uncore, upper_reg);
> - } while (upper != old_upper && loop++ < 2);
> + ret = intel_uncore_read64_2x32_fw(uncore, lower_reg, upper_reg);
>
> intel_uncore_forcewake_put__locked(uncore, fw_domains);
> spin_unlock_irqrestore(&uncore->lock, flags);
>
> - return (u64)upper << 32 | lower;
> + return ret;
> }
>
> static inline int intel_uncore_write_and_verify(struct intel_uncore *uncore,
> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
> b/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
> index 08d7ab9336725..764bc94044537 100644
> --- a/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
> +++ b/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
> @@ -74,6 +74,13 @@ intel_uncore_read64_2x32(struct intel_uncore *uncore,
> return (u64)upper << 32 | lower;
> }
>
> +static inline u64
> +intel_uncore_read64_2x32_fw(struct intel_uncore *uncore,
> + i915_reg_t i915_lower_reg, i915_reg_t
> i915_upper_reg)
> +{
> + return intel_uncore_read64_2x32(uncore, i915_lower_reg, i915_upper_reg);
> +}
> +
> static inline void intel_uncore_posting_read(struct intel_uncore *uncore,
> i915_reg_t i915_reg)
> {
> --
> 2.53.0
--
Ville Syrjälä
Intel
