From: Tvrtko Ursulin <tvrtko.ursu...@intel.com>

Having it as u64 was a confusing (but harmless) mistake.

Also add some asserts to make sure the internal field does not overflow
in the future.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: Ashutosh Dixit <ashutosh.di...@intel.com>
Cc: Umesh Nerlige Ramappa <umesh.nerlige.rama...@intel.com>
---
I am not entirely sure the __builtin_constant_p->BUILD_BUG_ON branch will
work with all compilers. Lets see...

Compile tested only.
---
 drivers/gpu/drm/i915/i915_pmu.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 7ece883a7d95..8736b3418f88 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -50,7 +50,7 @@ static u8 engine_event_instance(struct perf_event *event)
        return (event->attr.config >> I915_PMU_SAMPLE_BITS) & 0xff;
 }
 
-static bool is_engine_config(u64 config)
+static bool is_engine_config(const u64 config)
 {
        return config < __I915_PMU_OTHER(0);
 }
@@ -82,15 +82,28 @@ static unsigned int other_bit(const u64 config)
 
 static unsigned int config_bit(const u64 config)
 {
+       unsigned int bit;
+
        if (is_engine_config(config))
-               return engine_config_sample(config);
+               bit = engine_config_sample(config);
        else
-               return other_bit(config);
+               bit = other_bit(config);
+
+       if (__builtin_constant_p(config))
+               BUILD_BUG_ON(bit >
+                            BITS_PER_TYPE(typeof_member(struct i915_pmu,
+                                                        enable)) - 1);
+       else
+               WARN_ON_ONCE(bit >
+                            BITS_PER_TYPE(typeof_member(struct i915_pmu,
+                                                        enable)) - 1);
+
+       return bit;
 }
 
-static u64 config_mask(u64 config)
+static u32 config_mask(const u64 config)
 {
-       return BIT_ULL(config_bit(config));
+       return BIT(config_bit(config));
 }
 
 static bool is_engine_event(struct perf_event *event)
@@ -633,11 +646,10 @@ static void i915_pmu_enable(struct perf_event *event)
 {
        struct drm_i915_private *i915 =
                container_of(event->pmu, typeof(*i915), pmu.base);
+       const unsigned int bit = event_bit(event);
        struct i915_pmu *pmu = &i915->pmu;
        unsigned long flags;
-       unsigned int bit;
 
-       bit = event_bit(event);
        if (bit == -1)
                goto update;
 
@@ -651,7 +663,7 @@ static void i915_pmu_enable(struct perf_event *event)
        GEM_BUG_ON(bit >= ARRAY_SIZE(pmu->enable_count));
        GEM_BUG_ON(pmu->enable_count[bit] == ~0);
 
-       pmu->enable |= BIT_ULL(bit);
+       pmu->enable |= BIT(bit);
        pmu->enable_count[bit]++;
 
        /*
@@ -698,7 +710,7 @@ static void i915_pmu_disable(struct perf_event *event)
 {
        struct drm_i915_private *i915 =
                container_of(event->pmu, typeof(*i915), pmu.base);
-       unsigned int bit = event_bit(event);
+       const unsigned int bit = event_bit(event);
        struct i915_pmu *pmu = &i915->pmu;
        unsigned long flags;
 
@@ -734,7 +746,7 @@ static void i915_pmu_disable(struct perf_event *event)
         * bitmask when the last listener on an event goes away.
         */
        if (--pmu->enable_count[bit] == 0) {
-               pmu->enable &= ~BIT_ULL(bit);
+               pmu->enable &= ~BIT(bit);
                pmu->timer_enabled &= pmu_needs_timer(pmu, true);
        }
 
-- 
2.39.2

Reply via email to