| Message ID | 20170718143618.12254-5-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17-07-18 15:36:08, Tvrtko Ursulin wrote: >From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >As elsewhere in the code we have to decouple the binary >engine identifiers for easier maintenance. > >Also the sampler mask was incorrect in the timer callback. > I don't quite understand the point of this patch... can you perhaps embellish the commit message? >Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >--- > drivers/gpu/drm/i915/i915_pmu.c | 44 ++++++++++++++++++++++++++++++++++++----- > 1 file changed, 39 insertions(+), 5 deletions(-) > >diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c >index f03ddad44da6..9a8208dba7a9 100644 >--- a/drivers/gpu/drm/i915/i915_pmu.c >+++ b/drivers/gpu/drm/i915/i915_pmu.c >@@ -10,6 +10,25 @@ > #define RING_MASK 0xffffffff > #define RING_MAX 32 > >+#define ENGINE_SAMPLE_MASK (0xf) >+#define ENGINE_SAMPLE_BITS (4) >+ >+static const unsigned int engine_map[I915_NUM_ENGINES] = { >+ [RCS] = I915_SAMPLE_RCS, >+ [BCS] = I915_SAMPLE_BCS, >+ [VCS] = I915_SAMPLE_VCS, >+ [VCS2] = I915_SAMPLE_VCS2, >+ [VECS] = I915_SAMPLE_VECS, >+}; >+ >+static const unsigned int user_engine_map[I915_NUM_ENGINES] = { >+ [I915_SAMPLE_RCS] = RCS, >+ [I915_SAMPLE_BCS] = BCS, >+ [I915_SAMPLE_VCS] = VCS, >+ [I915_SAMPLE_VCS2] = VCS2, >+ [I915_SAMPLE_VECS] = VECS, >+}; >+ > static void engines_sample(struct drm_i915_private *dev_priv) > { > struct intel_engine_cs *engine; >@@ -26,9 +45,16 @@ static void engines_sample(struct drm_i915_private *dev_priv) > return; > > for_each_engine(engine, dev_priv, id) { >+ unsigned int user_engine = engine_map[id]; > u32 val; > >- if ((dev_priv->pmu.enable & (0x7 << (4*id))) == 0) >+ if (WARN_ON_ONCE(id >= ARRAY_SIZE(engine_map))) >+ continue; >+ else >+ user_engine = engine_map[id]; >+ >+ if (!(dev_priv->pmu.enable & >+ (ENGINE_SAMPLE_MASK << (ENGINE_SAMPLE_BITS * user_engine)))) > continue; > > if (i915_seqno_passed(intel_engine_get_seqno(engine), >@@ -112,6 +138,11 @@ static int engine_event_init(struct perf_event *event) > int engine = event->attr.config >> 2; > int sample = event->attr.config & 3; > >+ if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map))) >+ return -ENOENT; >+ else >+ engine = user_engine_map[engine]; >+ > switch (sample) { > case I915_SAMPLE_QUEUED: > case I915_SAMPLE_BUSY: >@@ -125,9 +156,6 @@ static int engine_event_init(struct perf_event *event) > return -ENOENT; > } > >- if (engine >= I915_NUM_ENGINES) >- return -ENOENT; >- > if (!i915->engine[engine]) > return -ENODEV; > >@@ -369,7 +397,13 @@ static void i915_pmu_event_read(struct perf_event *event) > if (event->attr.config < 32) { > int engine = event->attr.config >> 2; > int sample = event->attr.config & 3; >- val = i915->engine[engine]->pmu_sample[sample]; >+ >+ if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map))) { >+ /* Do nothing */ >+ } else { >+ engine = user_engine_map[engine]; >+ val = i915->engine[engine]->pmu_sample[sample]; >+ } > } else switch (event->attr.config) { > case I915_PMU_ACTUAL_FREQUENCY: > val = i915->pmu.sample[__I915_SAMPLE_FREQ_ACT]; >-- >2.9.4 >
On 25/07/2017 02:18, Ben Widawsky wrote: > On 17-07-18 15:36:08, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> As elsewhere in the code we have to decouple the binary >> engine identifiers for easier maintenance. >> >> Also the sampler mask was incorrect in the timer callback. >> > > I don't quite understand the point of this patch... can you perhaps > embellish > the commit message? It is to decouple the ABI namespace from the driver internal enumeration of engines and so allow the latter to be refactored in the future, and at the same time fix the fist patch to index engines other than RCS correctly. I did not bother with smart commit message for the first few patches since they really should be squashed with the first one from Chris. Regards, Tvrtko >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> --- >> drivers/gpu/drm/i915/i915_pmu.c | 44 >> ++++++++++++++++++++++++++++++++++++----- >> 1 file changed, 39 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_pmu.c >> b/drivers/gpu/drm/i915/i915_pmu.c >> index f03ddad44da6..9a8208dba7a9 100644 >> --- a/drivers/gpu/drm/i915/i915_pmu.c >> +++ b/drivers/gpu/drm/i915/i915_pmu.c >> @@ -10,6 +10,25 @@ >> #define RING_MASK 0xffffffff >> #define RING_MAX 32 >> >> +#define ENGINE_SAMPLE_MASK (0xf) >> +#define ENGINE_SAMPLE_BITS (4) >> + >> +static const unsigned int engine_map[I915_NUM_ENGINES] = { >> + [RCS] = I915_SAMPLE_RCS, >> + [BCS] = I915_SAMPLE_BCS, >> + [VCS] = I915_SAMPLE_VCS, >> + [VCS2] = I915_SAMPLE_VCS2, >> + [VECS] = I915_SAMPLE_VECS, >> +}; >> + >> +static const unsigned int user_engine_map[I915_NUM_ENGINES] = { >> + [I915_SAMPLE_RCS] = RCS, >> + [I915_SAMPLE_BCS] = BCS, >> + [I915_SAMPLE_VCS] = VCS, >> + [I915_SAMPLE_VCS2] = VCS2, >> + [I915_SAMPLE_VECS] = VECS, >> +}; >> + >> static void engines_sample(struct drm_i915_private *dev_priv) >> { >> struct intel_engine_cs *engine; >> @@ -26,9 +45,16 @@ static void engines_sample(struct drm_i915_private >> *dev_priv) >> return; >> >> for_each_engine(engine, dev_priv, id) { >> + unsigned int user_engine = engine_map[id]; >> u32 val; >> >> - if ((dev_priv->pmu.enable & (0x7 << (4*id))) == 0) >> + if (WARN_ON_ONCE(id >= ARRAY_SIZE(engine_map))) >> + continue; >> + else >> + user_engine = engine_map[id]; >> + >> + if (!(dev_priv->pmu.enable & >> + (ENGINE_SAMPLE_MASK << (ENGINE_SAMPLE_BITS * user_engine)))) >> continue; >> >> if (i915_seqno_passed(intel_engine_get_seqno(engine), >> @@ -112,6 +138,11 @@ static int engine_event_init(struct perf_event >> *event) >> int engine = event->attr.config >> 2; >> int sample = event->attr.config & 3; >> >> + if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map))) >> + return -ENOENT; >> + else >> + engine = user_engine_map[engine]; >> + >> switch (sample) { >> case I915_SAMPLE_QUEUED: >> case I915_SAMPLE_BUSY: >> @@ -125,9 +156,6 @@ static int engine_event_init(struct perf_event >> *event) >> return -ENOENT; >> } >> >> - if (engine >= I915_NUM_ENGINES) >> - return -ENOENT; >> - >> if (!i915->engine[engine]) >> return -ENODEV; >> >> @@ -369,7 +397,13 @@ static void i915_pmu_event_read(struct perf_event >> *event) >> if (event->attr.config < 32) { >> int engine = event->attr.config >> 2; >> int sample = event->attr.config & 3; >> - val = i915->engine[engine]->pmu_sample[sample]; >> + >> + if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map))) { >> + /* Do nothing */ >> + } else { >> + engine = user_engine_map[engine]; >> + val = i915->engine[engine]->pmu_sample[sample]; >> + } >> } else switch (event->attr.config) { >> case I915_PMU_ACTUAL_FREQUENCY: >> val = i915->pmu.sample[__I915_SAMPLE_FREQ_ACT]; >> -- >> 2.9.4 >> >
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index f03ddad44da6..9a8208dba7a9 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -10,6 +10,25 @@ #define RING_MASK 0xffffffff #define RING_MAX 32 +#define ENGINE_SAMPLE_MASK (0xf) +#define ENGINE_SAMPLE_BITS (4) + +static const unsigned int engine_map[I915_NUM_ENGINES] = { + [RCS] = I915_SAMPLE_RCS, + [BCS] = I915_SAMPLE_BCS, + [VCS] = I915_SAMPLE_VCS, + [VCS2] = I915_SAMPLE_VCS2, + [VECS] = I915_SAMPLE_VECS, +}; + +static const unsigned int user_engine_map[I915_NUM_ENGINES] = { + [I915_SAMPLE_RCS] = RCS, + [I915_SAMPLE_BCS] = BCS, + [I915_SAMPLE_VCS] = VCS, + [I915_SAMPLE_VCS2] = VCS2, + [I915_SAMPLE_VECS] = VECS, +}; + static void engines_sample(struct drm_i915_private *dev_priv) { struct intel_engine_cs *engine; @@ -26,9 +45,16 @@ static void engines_sample(struct drm_i915_private *dev_priv) return; for_each_engine(engine, dev_priv, id) { + unsigned int user_engine = engine_map[id]; u32 val; - if ((dev_priv->pmu.enable & (0x7 << (4*id))) == 0) + if (WARN_ON_ONCE(id >= ARRAY_SIZE(engine_map))) + continue; + else + user_engine = engine_map[id]; + + if (!(dev_priv->pmu.enable & + (ENGINE_SAMPLE_MASK << (ENGINE_SAMPLE_BITS * user_engine)))) continue; if (i915_seqno_passed(intel_engine_get_seqno(engine), @@ -112,6 +138,11 @@ static int engine_event_init(struct perf_event *event) int engine = event->attr.config >> 2; int sample = event->attr.config & 3; + if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map))) + return -ENOENT; + else + engine = user_engine_map[engine]; + switch (sample) { case I915_SAMPLE_QUEUED: case I915_SAMPLE_BUSY: @@ -125,9 +156,6 @@ static int engine_event_init(struct perf_event *event) return -ENOENT; } - if (engine >= I915_NUM_ENGINES) - return -ENOENT; - if (!i915->engine[engine]) return -ENODEV; @@ -369,7 +397,13 @@ static void i915_pmu_event_read(struct perf_event *event) if (event->attr.config < 32) { int engine = event->attr.config >> 2; int sample = event->attr.config & 3; - val = i915->engine[engine]->pmu_sample[sample]; + + if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map))) { + /* Do nothing */ + } else { + engine = user_engine_map[engine]; + val = i915->engine[engine]->pmu_sample[sample]; + } } else switch (event->attr.config) { case I915_PMU_ACTUAL_FREQUENCY: val = i915->pmu.sample[__I915_SAMPLE_FREQ_ACT];