On 06/01/2013 05:56 PM, Viresh Kumar wrote: > On 31 May 2013 22:03, Stratos Karafotis <stratosk@xxxxxxxxxxxx> wrote: >> On 05/31/2013 11:51 AM, Viresh Kumar wrote: >>> I believe you should have removed other users of getavg() in a separate >>> patch and also cc'd relevant people so that you can some review comments >>> from them. >> >> I will split the patch in two. If it's OK, I will keep the removal of >> __cpufreq_driver_getavg in the original patch and move the clean up of >> APERF/MPERF support in a second patch. I will also cc relevant people. > > Even removal of __cpufreq_driver_getavg() should be done in a separate > patch, so that it can be reverted easily if required later. Thanks, Viresh. I will do the removal of that function in a seperate patch. Should I use a third patch for it? Or should I include it in the patch which will remove APERF/MPERF support? >>> "Proportional to load" means C * load, so why is "policy->max / 100" *the* right C? >> >> I think, finally(?) I see your point. The right C should be "policy->cpuinfo.max_freq / 100". > > Why are you changing it to cpuinfo.max_freq?? This is fixed once a driver is > initialized.. but user may request a lower max freq for a governor or policy. > Which is actually reflected in policy->max I believe. My initial thought is to have a linear function to calculate the target freq proportional to load: (I will use 'C' as the function's slope as Rafael used it) freq_target = C * load For simplicity, let's assume that load is between 0 and 1 as initially is calculated in governor. Ideally, for a load = 0, we should have freq_target = 0 and for load = 1, freq_target = cpuinfo.max So, the slope C = cpuinfo.max I think, it's matter of definition about what policy->min and policy->max can do. Should they change the slope C? Or only limit freq_target? I don't think that the policy->max (or min) should affect HOW (slope C) governor calculates freq_target but only limit the calculated result. Maybe, we could have separate tunables to a affect the slope C. If I'm wrong about the definition of policy->min, policy->max, I would change the code accordingly. > Over that why keeping following check is useful anymore? > > if (load_freq > od_tuners->up_threshold) > goto max. > > As, if load is over 95, then even policy->max * 95 / 100 will even give almost > the same freq. > I thought that too. But maybe user selects a lower value for up_threshold. (For example, up_threshold = 60). In my opinion, we have to keep up_theshold, to give the possibility to user to have max freq with small loads. Thanks for your comments! Stratos -- To unsubscribe from this list: send the line "unsubscribe cpufreq" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html