Messages in this thread | | | Subject | Re: [RFC PATCH 1/7] PM: Introduce em_pd_get_higher_freq() | From | Douglas Raillard <> | Date | Wed, 19 Jun 2019 17:08:37 +0100 |
| |
Hi Patrick,
On 5/16/19 2:22 PM, Patrick Bellasi wrote: > On 16-May 14:01, Quentin Perret wrote: >> On Thursday 16 May 2019 at 13:42:00 (+0100), Patrick Bellasi wrote: >>>> +static inline unsigned long em_pd_get_higher_freq(struct em_perf_domain *pd, >>>> + unsigned long min_freq, unsigned long cost_margin) >>>> +{ >>>> + unsigned long max_cost = 0; >>>> + struct em_cap_state *cs; >>>> + int i; >>>> + >>>> + if (!pd) >>>> + return min_freq; >>>> + >>>> + /* Compute the maximum allowed cost */ >>>> + for (i = 0; i < pd->nr_cap_states; i++) { >>>> + cs = &pd->table[i]; >>>> + if (cs->frequency >= min_freq) { >>>> + max_cost = cs->cost + (cs->cost * cost_margin) / 1024; >>> ^^^^ >>> ... end here we should probably better use SCHED_CAPACITY_SCALE >>> instead of hard-coding in values, isn't it? >> >> I'm not sure to agree. This isn't part of the scheduler per se, and the >> cost thing isn't in units of capacity, but in units of power, so I don't >> think SCHED_CAPACITY_SCALE is correct here. > > Right, I get the units do not match and it would not be elegant to use > it here... > >> But I agree these hard coded values (that one, and the 512 in one of the >> following patches) could use some motivation :-) > > ... ultimately SCHED_CAPACITY_SCALE is just SCHED_FIXEDPOINT_SCALE, > which is adimensional. Perhaps we should use that or yet another alias > for the same.
Would it be a good idea to use SCHED_FIXEDPOINT_SCALE in energy.c ? Since it's not part of the scheduler, maybe there is a scale covering a wider scope, or we can introduce a similar ENERGY_FIXEDPOINT_SCALE in energy_model.h.
>> Thanks, >> Quentin >
Thanks, Douglas
| |