freqbench icon indicating copy to clipboard operation
freqbench copied to clipboard

Change base_power calculation to prevent too low values

Open Pokemetti opened this issue 2 years ago • 3 comments

The base_power is currently calculated as the minimum value of the power measurements. This value can sometimes be lower than the actual average base power draw, resulting in too low base_power and thus too high per-frequency power and too low efficiency results. This can be seen on some results already, especially the proposed results of the SM7325. This patch changes the calculation of the base_power from being the smallest measurement (min) to the average measurement (median). I've tested this on my Samsung Galaxy A5 (2017) and it worked fine for me.

Pokemetti avatar Jun 07 '22 15:06 Pokemetti

This does make sense, but it might increase the risk of negative power readings for low frequencies (which has already been an issue on some Exynos devices). It may be worth recalculating existing devices based on results.json to make sure that isn't a major issue.

kdrag0n avatar Jun 15 '22 06:06 kdrag0n

I understand that concern, but I think that the issue lies somewhere else. I might test more on my Exynos 7880 sometime soon, maybe I can find a cheap other Exynos phone to test too.

Pokemetti avatar Jun 15 '22 09:06 Pokemetti

I've now run freqbench on my A52s with the SD778G (SM7325) and I get similar results to the already proposed ones, even when using idle_power, so this doesn't seem to be the fix for that problem.

Pokemetti avatar Aug 14 '22 14:08 Pokemetti

Seems like a good idea then, thanks for checking. Ideally we'd recalculate existing results to reflect this change as results.json contains all the data needed to do that, but it's fine for now.

kdrag0n avatar Aug 16 '22 13:08 kdrag0n