| Message ID | 20230330004103.1295413-9-umesh.nerlige.ramappa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add MTL PMU support for multi-gt | expand |
On 30/03/2023 01:41, Umesh Nerlige Ramappa wrote: > Split the event reading function into engine and other helpers. What, why and how please, third bit not being needed in this case. :) > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> > --- > drivers/gpu/drm/i915/i915_pmu.c | 93 ++++++++++++++++++--------------- > 1 file changed, 52 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index 40ce1dc00067..9bd9605d2662 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c > @@ -641,58 +641,69 @@ static u64 read_sample_us(struct i915_pmu *pmu, unsigned int gt_id, int sample) > return div_u64(read_sample(pmu, gt_id, sample), USEC_PER_SEC); > } > > -static u64 __i915_pmu_event_read(struct perf_event *event) > +static u64 __i915_pmu_event_read_engine(struct perf_event *event) > { > - struct drm_i915_private *i915 = > - container_of(event->pmu, typeof(*i915), pmu.base); > - struct i915_pmu *pmu = &i915->pmu; > + struct drm_i915_private *i915 = container_of(event->pmu, typeof(*i915), pmu.base); Liking the over 80 look, if you insist. :) > + u8 sample = engine_event_sample(event); > + struct intel_engine_cs *engine; > u64 val = 0; > > - if (is_engine_event(event)) { > - u8 sample = engine_event_sample(event); > - struct intel_engine_cs *engine; > - > - engine = intel_engine_lookup_user(i915, > - engine_event_class(event), > - engine_event_instance(event)); > + engine = intel_engine_lookup_user(i915, > + engine_event_class(event), > + engine_event_instance(event)); > > - if (drm_WARN_ON_ONCE(&i915->drm, !engine)) { > - /* Do nothing */ > - } else if (sample == I915_SAMPLE_BUSY && > - intel_engine_supports_stats(engine)) { > - ktime_t unused; > + if (drm_WARN_ON_ONCE(&i915->drm, !engine)) { > + /* Do nothing */ > + } else if (sample == I915_SAMPLE_BUSY && > + intel_engine_supports_stats(engine)) { > + ktime_t unused; > > - val = ktime_to_ns(intel_engine_get_busy_time(engine, > - &unused)); > - } else { > - val = engine->pmu.sample[sample].cur; > - } > + val = ktime_to_ns(intel_engine_get_busy_time(engine, > + &unused)); > } else { > - const unsigned int gt_id = config_gt_id(event->attr.config); > - const u64 config = config_counter(event->attr.config); > - > - switch (config) { > - case I915_PMU_ACTUAL_FREQUENCY: > - val = read_sample_us(pmu, gt_id, __I915_SAMPLE_FREQ_ACT); > - break; > - case I915_PMU_REQUESTED_FREQUENCY: > - val = read_sample_us(pmu, gt_id, __I915_SAMPLE_FREQ_REQ); > - break; > - case I915_PMU_INTERRUPTS: > - val = READ_ONCE(pmu->irq_count); > - break; > - case I915_PMU_RC6_RESIDENCY: > - val = get_rc6(i915->gt[gt_id]); > - break; > - case I915_PMU_SOFTWARE_GT_AWAKE_TIME: > - val = ktime_to_ns(intel_gt_get_awake_time(to_gt(i915))); > - break; > - } > + val = engine->pmu.sample[sample].cur; > } > > return val; > } > > +static u64 __i915_pmu_event_read_other(struct perf_event *event) > +{ > + struct drm_i915_private *i915 = container_of(event->pmu, typeof(*i915), pmu.base); > + const unsigned int gt_id = config_gt_id(event->attr.config); > + const u64 config = config_counter(event->attr.config); > + struct i915_pmu *pmu = &i915->pmu; > + u64 val = 0; > + > + switch (config) { > + case I915_PMU_ACTUAL_FREQUENCY: > + val = read_sample_us(pmu, gt_id, __I915_SAMPLE_FREQ_ACT); > + break; > + case I915_PMU_REQUESTED_FREQUENCY: > + val = read_sample_us(pmu, gt_id, __I915_SAMPLE_FREQ_REQ); > + break; > + case I915_PMU_INTERRUPTS: > + val = READ_ONCE(pmu->irq_count); > + break; > + case I915_PMU_RC6_RESIDENCY: > + val = get_rc6(i915->gt[gt_id]); > + break; > + case I915_PMU_SOFTWARE_GT_AWAKE_TIME: > + val = ktime_to_ns(intel_gt_get_awake_time(to_gt(i915))); > + break; > + } > + > + return val; > +} > + > +static u64 __i915_pmu_event_read(struct perf_event *event) > +{ > + if (is_engine_event(event)) > + return __i915_pmu_event_read_engine(event); > + else > + return __i915_pmu_event_read_other(event); > +} > + > static void i915_pmu_event_read(struct perf_event *event) > { > struct drm_i915_private *i915 = No real complaints - it is tidier and more readable. Just drop a note in the commit that is why and: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 40ce1dc00067..9bd9605d2662 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -641,58 +641,69 @@ static u64 read_sample_us(struct i915_pmu *pmu, unsigned int gt_id, int sample) return div_u64(read_sample(pmu, gt_id, sample), USEC_PER_SEC); } -static u64 __i915_pmu_event_read(struct perf_event *event) +static u64 __i915_pmu_event_read_engine(struct perf_event *event) { - struct drm_i915_private *i915 = - container_of(event->pmu, typeof(*i915), pmu.base); - struct i915_pmu *pmu = &i915->pmu; + struct drm_i915_private *i915 = container_of(event->pmu, typeof(*i915), pmu.base); + u8 sample = engine_event_sample(event); + struct intel_engine_cs *engine; u64 val = 0; - if (is_engine_event(event)) { - u8 sample = engine_event_sample(event); - struct intel_engine_cs *engine; - - engine = intel_engine_lookup_user(i915, - engine_event_class(event), - engine_event_instance(event)); + engine = intel_engine_lookup_user(i915, + engine_event_class(event), + engine_event_instance(event)); - if (drm_WARN_ON_ONCE(&i915->drm, !engine)) { - /* Do nothing */ - } else if (sample == I915_SAMPLE_BUSY && - intel_engine_supports_stats(engine)) { - ktime_t unused; + if (drm_WARN_ON_ONCE(&i915->drm, !engine)) { + /* Do nothing */ + } else if (sample == I915_SAMPLE_BUSY && + intel_engine_supports_stats(engine)) { + ktime_t unused; - val = ktime_to_ns(intel_engine_get_busy_time(engine, - &unused)); - } else { - val = engine->pmu.sample[sample].cur; - } + val = ktime_to_ns(intel_engine_get_busy_time(engine, + &unused)); } else { - const unsigned int gt_id = config_gt_id(event->attr.config); - const u64 config = config_counter(event->attr.config); - - switch (config) { - case I915_PMU_ACTUAL_FREQUENCY: - val = read_sample_us(pmu, gt_id, __I915_SAMPLE_FREQ_ACT); - break; - case I915_PMU_REQUESTED_FREQUENCY: - val = read_sample_us(pmu, gt_id, __I915_SAMPLE_FREQ_REQ); - break; - case I915_PMU_INTERRUPTS: - val = READ_ONCE(pmu->irq_count); - break; - case I915_PMU_RC6_RESIDENCY: - val = get_rc6(i915->gt[gt_id]); - break; - case I915_PMU_SOFTWARE_GT_AWAKE_TIME: - val = ktime_to_ns(intel_gt_get_awake_time(to_gt(i915))); - break; - } + val = engine->pmu.sample[sample].cur; } return val; } +static u64 __i915_pmu_event_read_other(struct perf_event *event) +{ + struct drm_i915_private *i915 = container_of(event->pmu, typeof(*i915), pmu.base); + const unsigned int gt_id = config_gt_id(event->attr.config); + const u64 config = config_counter(event->attr.config); + struct i915_pmu *pmu = &i915->pmu; + u64 val = 0; + + switch (config) { + case I915_PMU_ACTUAL_FREQUENCY: + val = read_sample_us(pmu, gt_id, __I915_SAMPLE_FREQ_ACT); + break; + case I915_PMU_REQUESTED_FREQUENCY: + val = read_sample_us(pmu, gt_id, __I915_SAMPLE_FREQ_REQ); + break; + case I915_PMU_INTERRUPTS: + val = READ_ONCE(pmu->irq_count); + break; + case I915_PMU_RC6_RESIDENCY: + val = get_rc6(i915->gt[gt_id]); + break; + case I915_PMU_SOFTWARE_GT_AWAKE_TIME: + val = ktime_to_ns(intel_gt_get_awake_time(to_gt(i915))); + break; + } + + return val; +} + +static u64 __i915_pmu_event_read(struct perf_event *event) +{ + if (is_engine_event(event)) + return __i915_pmu_event_read_engine(event); + else + return __i915_pmu_event_read_other(event); +} + static void i915_pmu_event_read(struct perf_event *event) { struct drm_i915_private *i915 =
Split the event reading function into engine and other helpers. Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> --- drivers/gpu/drm/i915/i915_pmu.c | 93 ++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 41 deletions(-)