Re: [PATCH v4 06/22] cpufreq: amd: introduce a new amd pstate driver to support future processors

[Huang Rui <ray.huang@amd.com>]

On Thu, Nov 25, 2021 at 11:03:58PM +0800, Giovanni Gherdovich wrote:
> On Fri, 2021-11-19 at 18:30 +0800, Huang Rui wrote:
> > <snip>
> >
> > Performance Per Watt (PPW) Calculation:
> > 
> > The PPW calculation is referred by below paper:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsoftware.intel.com%2Fcontent%2Fdam%2Fdevelop%2Fexternal%2Fus%2Fen%2Fdocuments%2Fperformance-per-what-paper.pdf&amp;data=04%7C01%7Cray.huang%40amd.com%7C65f95f752ce142a5356d08d9b024d0de%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637734494579654964%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=6SzLLzQ1mRlbxdO1skcnrjulOmaqGWCrtDpIZToVjMM%3D&amp;reserved=0
> > 
> > Below formula is referred from below spec to measure the PPW:
> > 
> > (F / t) / P = F * t / (t * E) = F / E,
> > 
> > "F" is the number of frames per second.

This "F" should be the number of frames, not per second.

> > "P" is power measured in watts.
> > "E" is energy measured in joules.
> 
> Hello, I'd appreciate if you can remove the reference to the above paper and
> formula, because it is not really relevant to this context, and ends up being
> confusing.
> 
> It describes performance per watt tailored to graphics benchmarks, in the form
> of frames per joule. Nothing wrong with that, but it only works for tests that
> measure frames per second, and none of the tests below is of that type.

OK, actually, we would test 3D steam game benchmarks as well on our
processors with amd-pstate. :-)

And yes, below three benchmark tests are not related with "frame", but the
"MB" or "runs" of speedometer are actually another meaning of "frame" which
represent the performance data on each benchmark, right. That's is original
reason to put paper and formula here.

If you think it may confuse any guys, I can remove this in next version.

> 
> You have:
> 
> - tbench measures throughput (MB/sec)
> - gitsource, aka run the git test suite, measures elapsed time
> - speedometer, a web browser test that gives "runs per minute"
> 
> If you want performance per watt, you need to express your result as
> "operations per second", where "operations" is up to you to define. For
> tbench, one "operation" is moving a MB of data. For speedometer, one
> "operation" is one "run", as defined in the benchmark. Once you have op/sec
> (aka performance), divide by the average power measured over the entire
> duration of the benchmark.
> 
> In cases like gitsource, where you have elapsed_time as a result, performance
> per watt is 1 / (elapsed_time * average_power).
> 

The "operations" you mentioned here is another meaning of "frames" at the
paper in general. That's actual my previous purpose. :-)

> > We use the RAPL interface with "perf" tool to get the energy data of the
> > package power.
> > 
> > The data comparisons between amd-pstate and acpi-freq module are tested on
> > AMD Cezanne processor:
> > 
> > 1) TBench CPU benchmark:
> > 
> > +---------------------------------------------------------------------+
> > >                                                                     |
> > >               TBench (Performance Per Watt)                         |
> > >                                                    Higher is better |
> > +-------------------+------------------------+------------------------+
> > >                   |  Performance Per Watt  |  Performance Per Watt  |
> > >   Kernel Module   |       (Schedutil)      |       (Ondemand)       |
> > >                   |  Unit: MB / (s * J)    |  Unit: MB / (s * J)    |
> 
> The unit "MB / (s * J)" doesn't really work, it should be "MB / (sec * watt)".
> Can you double check that you divided the performance result by the average
> power? Same for the other tests.
> 
> It is also relevant to show performance, alongside with perf-per-watt.
> 

Aligned with you offline before, we will update the new performance per
watt data here with MB / Joule. Thanks to point it out.

Thanks,
Ray