Re: [Intel-gfx] [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
Quoting Tvrtko Ursulin (2019-11-11 10:40:17) > > On 11/11/2019 09:43, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2019-11-11 09:11:03) > >> > >> On 09/11/2019 10:53, Chris Wilson wrote: > >>> We report "frequencies" (actual-frequency, requested-frequency) as the > >>> number of accumulated cycles so that the average frequency over that > >>> period may be determined by the user. This means the units we report to > >>> the user are Mcycles (or just M), not MHz. > >>> > >>> Signed-off-by: Chris Wilson > >>> Cc: Tvrtko Ursulin > >>> Cc: sta...@vger.kernel.org > >>> --- > >>>drivers/gpu/drm/i915/i915_pmu.c | 4 ++-- > >>>1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/i915_pmu.c > >>> b/drivers/gpu/drm/i915/i915_pmu.c > >>> index 4804775644bf..9b02be0ad4e6 100644 > >>> --- a/drivers/gpu/drm/i915/i915_pmu.c > >>> +++ b/drivers/gpu/drm/i915/i915_pmu.c > >>> @@ -908,8 +908,8 @@ create_event_attributes(struct i915_pmu *pmu) > >>>const char *name; > >>>const char *unit; > >>>} events[] = { > >>> - __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", > >>> "MHz"), > >>> - __event(I915_PMU_REQUESTED_FREQUENCY, > >>> "requested-frequency", "MHz"), > >>> + __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "M"), > >>> + __event(I915_PMU_REQUESTED_FREQUENCY, > >>> "requested-frequency", "M"), > >>>__event(I915_PMU_INTERRUPTS, "interrupts", NULL), > >>>__event(I915_PMU_RC6_RESIDENCY, "rc6-residency", "ns"), > >>>}; > >>> > >> > >> MHz was wrong yes. But is 'M' established or would 'Mcycles' be better? > > > > The only place where "cycles" pops up is in the perf ui, the other > > events that I thought were similar in nature are unitless. As the > > 'cycle' itself is not an SI base unit as it is a mere count. > > > > ~o~ I have no idea ~o~ > > But if the argument if that 'cycle' is not SI then neither is 'M'. So I > think I would prefer 'Mcycles'. Nevertheless, I guess a strange 'M' is > better than wrong 'MHz'. Yeah, I'm really tempted to say we should just remove the M as well but what a waste of bits! I think 'M' is understandable from context, whereas MHz was quite misleading in perf stat :) Still, if we ever get any feedback, it should be easy to fix :) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
On 11/11/2019 09:43, Chris Wilson wrote: Quoting Tvrtko Ursulin (2019-11-11 09:11:03) On 09/11/2019 10:53, Chris Wilson wrote: We report "frequencies" (actual-frequency, requested-frequency) as the number of accumulated cycles so that the average frequency over that period may be determined by the user. This means the units we report to the user are Mcycles (or just M), not MHz. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/i915_pmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 4804775644bf..9b02be0ad4e6 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -908,8 +908,8 @@ create_event_attributes(struct i915_pmu *pmu) const char *name; const char *unit; } events[] = { - __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "MHz"), - __event(I915_PMU_REQUESTED_FREQUENCY, "requested-frequency", "MHz"), + __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "M"), + __event(I915_PMU_REQUESTED_FREQUENCY, "requested-frequency", "M"), __event(I915_PMU_INTERRUPTS, "interrupts", NULL), __event(I915_PMU_RC6_RESIDENCY, "rc6-residency", "ns"), }; MHz was wrong yes. But is 'M' established or would 'Mcycles' be better? The only place where "cycles" pops up is in the perf ui, the other events that I thought were similar in nature are unitless. As the 'cycle' itself is not an SI base unit as it is a mere count. ~o~ I have no idea ~o~ But if the argument if that 'cycle' is not SI then neither is 'M'. So I think I would prefer 'Mcycles'. Nevertheless, I guess a strange 'M' is better than wrong 'MHz'. Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
Quoting Tvrtko Ursulin (2019-11-11 09:11:03) > > On 09/11/2019 10:53, Chris Wilson wrote: > > We report "frequencies" (actual-frequency, requested-frequency) as the > > number of accumulated cycles so that the average frequency over that > > period may be determined by the user. This means the units we report to > > the user are Mcycles (or just M), not MHz. > > > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > Cc: sta...@vger.kernel.org > > --- > > drivers/gpu/drm/i915/i915_pmu.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c > > b/drivers/gpu/drm/i915/i915_pmu.c > > index 4804775644bf..9b02be0ad4e6 100644 > > --- a/drivers/gpu/drm/i915/i915_pmu.c > > +++ b/drivers/gpu/drm/i915/i915_pmu.c > > @@ -908,8 +908,8 @@ create_event_attributes(struct i915_pmu *pmu) > > const char *name; > > const char *unit; > > } events[] = { > > - __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "MHz"), > > - __event(I915_PMU_REQUESTED_FREQUENCY, "requested-frequency", > > "MHz"), > > + __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "M"), > > + __event(I915_PMU_REQUESTED_FREQUENCY, "requested-frequency", > > "M"), > > __event(I915_PMU_INTERRUPTS, "interrupts", NULL), > > __event(I915_PMU_RC6_RESIDENCY, "rc6-residency", "ns"), > > }; > > > > MHz was wrong yes. But is 'M' established or would 'Mcycles' be better? The only place where "cycles" pops up is in the perf ui, the other events that I thought were similar in nature are unitless. As the 'cycle' itself is not an SI base unit as it is a mere count. ~o~ I have no idea ~o~ -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
On 09/11/2019 10:53, Chris Wilson wrote: We report "frequencies" (actual-frequency, requested-frequency) as the number of accumulated cycles so that the average frequency over that period may be determined by the user. This means the units we report to the user are Mcycles (or just M), not MHz. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/i915_pmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 4804775644bf..9b02be0ad4e6 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -908,8 +908,8 @@ create_event_attributes(struct i915_pmu *pmu) const char *name; const char *unit; } events[] = { - __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "MHz"), - __event(I915_PMU_REQUESTED_FREQUENCY, "requested-frequency", "MHz"), + __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "M"), + __event(I915_PMU_REQUESTED_FREQUENCY, "requested-frequency", "M"), __event(I915_PMU_INTERRUPTS, "interrupts", NULL), __event(I915_PMU_RC6_RESIDENCY, "rc6-residency", "ns"), }; MHz was wrong yes. But is 'M' established or would 'Mcycles' be better? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
We report "frequencies" (actual-frequency, requested-frequency) as the number of accumulated cycles so that the average frequency over that period may be determined by the user. This means the units we report to the user are Mcycles (or just M), not MHz. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/i915_pmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 4804775644bf..9b02be0ad4e6 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -908,8 +908,8 @@ create_event_attributes(struct i915_pmu *pmu) const char *name; const char *unit; } events[] = { - __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "MHz"), - __event(I915_PMU_REQUESTED_FREQUENCY, "requested-frequency", "MHz"), + __event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "M"), + __event(I915_PMU_REQUESTED_FREQUENCY, "requested-frequency", "M"), __event(I915_PMU_INTERRUPTS, "interrupts", NULL), __event(I915_PMU_RC6_RESIDENCY, "rc6-residency", "ns"), }; -- 2.24.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx