Stable Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
@ 2019-11-09 10:53 Chris Wilson
  2019-11-11  9:11 ` [Intel-gfx] " Tvrtko Ursulin
  0 siblings, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2019-11-09 10:53 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson, Tvrtko Ursulin, stable

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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: stable@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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
  2019-11-09 10:53 [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles Chris Wilson
@ 2019-11-11  9:11 ` " Tvrtko Ursulin
  2019-11-11  9:43   ` Chris Wilson
  0 siblings, 1 reply; 5+ messages in thread
From: Tvrtko Ursulin @ 2019-11-11  9:11 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: stable


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 <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: stable@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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
  2019-11-11  9:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2019-11-11  9:43   ` Chris Wilson
  2019-11-11 10:40     ` Tvrtko Ursulin
  0 siblings, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2019-11-11  9:43 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: stable

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 <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: stable@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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
  2019-11-11  9:43   ` Chris Wilson
@ 2019-11-11 10:40     ` Tvrtko Ursulin
  2019-11-11 11:59       ` Chris Wilson
  0 siblings, 1 reply; 5+ messages in thread
From: Tvrtko Ursulin @ 2019-11-11 10:40 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: stable


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 <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: stable@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 <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles
  2019-11-11 10:40     ` Tvrtko Ursulin
@ 2019-11-11 11:59       ` Chris Wilson
  0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2019-11-11 11:59 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: stable

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 <chris@chris-wilson.co.uk>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>> Cc: stable@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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-09 10:53 [PATCH] drm/i915/pmu: "Frequency" is reported as accumulated cycles Chris Wilson
2019-11-11  9:11 ` [Intel-gfx] " Tvrtko Ursulin
2019-11-11  9:43   ` Chris Wilson
2019-11-11 10:40     ` Tvrtko Ursulin
2019-11-11 11:59       ` Chris Wilson

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git