Re: [PATCH v2] drm/i915/pmu: Use raw_spin_lock where ever needed in i915 pmu

[Vodapalli, Ravi Kumar]

On 11/4/2025 9:16 PM, Ravi Kumar Vodapalli wrote:
Currently i915 pmu driver gets registered with linux perf
subsystem with its callback functions implemented, it is
using spin_lock for synchronization where ever is needed.
linux perf subsystem in some instance is using raw_spin_locks
for synchronization and calls the i915 pmu callback functions with
raw_spin_lock held.

The issue is
When PREEMPT_RT is configured in kernel config the normal spin_lock
behaves as mutex lock and when these are called with raw_spin_lock
held race condition can occur.So in the path of the raw_spin_lock
held convert spin_lock into raw_spin_lock where ever is needed.

v2:
- Fix build error below
   drivers/gpu/drm/i915/i915_pmu.c:233:31: error: passing argument 1
   of ‘_raw_spin_lock_irqsave’ from incompatible pointer type
   [-Werror=incompatible-pointer-types]
   233 |         raw_spin_lock_irqsave(&pmu->lock, flags);
       |                               ^~~~~~~~~~

Here one more spin_lock is present in path of raw_spin_lock
in drivers/gpu/drm/i915/i915_pmu.c
__i915_pmu_event_read()--> get_rc6()-->__get_rc6()-->
intel_rc6_residency_ns()-->spin_lock_irqsave(&uncore->lock, flags);

Here spin lock is taken with "uncore->lock" variable, converting 
thislock to

raw_spin_lock is not straight forward because at many places in code this
lock is used.

drivers/gpu/drm/i915/gt/intel_rc6.c
drivers/gpu/drm/i915/gt/intel_workarounds.c
drivers/gpu/drm/i915/intel_uncore.



Signed-off-by: Ravi Kumar Vodapalli<ravi.kumar.vodapa...@intel.com>
---
  drivers/gpu/drm/i915/i915_pmu.c | 22 +++++++++++-----------
  drivers/gpu/drm/i915/i915_pmu.h |  2 +-
  2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 5bc696bfbb0f..d760ec44a98c 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -230,7 +230,7 @@ static u64 get_rc6(struct intel_gt *gt)
                intel_gt_pm_put_async(gt, wakeref);
        }
  
-	spin_lock_irqsave(&pmu->lock, flags);
+       raw_spin_lock_irqsave(&pmu->lock, flags);
  
  	if (wakeref) {
                store_sample(pmu, gt_id, __I915_SAMPLE_RC6, val);
@@ -251,7 +251,7 @@ static u64 get_rc6(struct intel_gt *gt)
        else
                store_sample(pmu, gt_id, __I915_SAMPLE_RC6_LAST_REPORTED, val);
  
-	spin_unlock_irqrestore(&pmu->lock, flags);
+       raw_spin_unlock_irqrestore(&pmu->lock, flags);
  
  	return val;
  }
@@ -302,7 +302,7 @@ void i915_pmu_gt_parked(struct intel_gt *gt)
        if (!pmu->registered)
                return;
  
-	spin_lock_irq(&pmu->lock);
+       raw_spin_lock_irq(&pmu->lock);
  
  	park_rc6(gt);
  
@@ -314,7 +314,7 @@ void i915_pmu_gt_parked(struct intel_gt *gt)
        if (pmu->unparked == 0)
                pmu->timer_enabled = false;
  
-	spin_unlock_irq(&pmu->lock);
+       raw_spin_unlock_irq(&pmu->lock);
  }
  
  void i915_pmu_gt_unparked(struct intel_gt *gt)
@@ -324,7 +324,7 @@ void i915_pmu_gt_unparked(struct intel_gt *gt)
        if (!pmu->registered)
                return;
  
-	spin_lock_irq(&pmu->lock);
+       raw_spin_lock_irq(&pmu->lock);
  
  	/*
         * Re-enable sampling timer when GPU goes active.
@@ -334,7 +334,7 @@ void i915_pmu_gt_unparked(struct intel_gt *gt)
  
  	pmu->unparked |= BIT(gt->info.id);
  
-	spin_unlock_irq(&pmu->lock);
+       raw_spin_unlock_irq(&pmu->lock);
  }
  
  static void
@@ -740,7 +740,7 @@ static void i915_pmu_enable(struct perf_event *event)
        if (bit == -1)
                goto update;
  
-	spin_lock_irqsave(&pmu->lock, flags);
+       raw_spin_lock_irqsave(&pmu->lock, flags);
  
  	/*
         * Update the bitmask of enabled events and increment
@@ -782,7 +782,7 @@ static void i915_pmu_enable(struct perf_event *event)
                engine->pmu.enable_count[sample]++;
        }
  
-	spin_unlock_irqrestore(&pmu->lock, flags);
+       raw_spin_unlock_irqrestore(&pmu->lock, flags);
  
  update:
        /*
@@ -803,7 +803,7 @@ static void i915_pmu_disable(struct perf_event *event)
        if (bit == -1)
                return;
  
-	spin_lock_irqsave(&pmu->lock, flags);
+       raw_spin_lock_irqsave(&pmu->lock, flags);
  
  	if (is_engine_event(event)) {
                u8 sample = engine_event_sample(event);
@@ -836,7 +836,7 @@ static void i915_pmu_disable(struct perf_event *event)
                pmu->timer_enabled &= pmu_needs_timer(pmu);
        }
  
-	spin_unlock_irqrestore(&pmu->lock, flags);
+       raw_spin_unlock_irqrestore(&pmu->lock, flags);
  }
  
  static void i915_pmu_event_start(struct perf_event *event, int flags)
@@ -1154,7 +1154,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
        };
        int ret = -ENOMEM;
  
-	spin_lock_init(&pmu->lock);
+       raw_spin_lock_init(&pmu->lock);
        hrtimer_setup(&pmu->timer, i915_sample, CLOCK_MONOTONIC, 
HRTIMER_MODE_REL);
        init_rc6(pmu);
  
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index 5826cc81858c..52d4b602310a 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -71,7 +71,7 @@ struct i915_pmu {
        /**
         * @lock: Lock protecting enable mask and ref count handling.
         */
-       spinlock_t lock;
+       raw_spinlock_t lock;
        /**
         * @unparked: GT unparked mask.
         */