From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Pandruvada, Srinivas" Subject: Re: [PATCH 1/2] cpufreq, intel_pstate, Fix limits->max_policy_pct rounding error Date: Fri, 20 Nov 2015 20:02:37 +0000 Message-ID: <1448049757.4914.3.camel@intel.com> References: <1448022757-7856-1-git-send-email-prarit@redhat.com> <1448022757-7856-2-git-send-email-prarit@redhat.com> <20151120131833.GD3957@ubuntu> <564F37C8.60307@redhat.com> <20151120151944.GE3957@ubuntu> <564F3FBA.6030806@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <564F3FBA.6030806@redhat.com> Content-Language: en-US Content-ID: Sender: linux-kernel-owner@vger.kernel.org To: "prarit@redhat.com" Cc: "Brown, Len" , "linux-kernel@vger.kernel.org" , "viresh.kumar@linaro.org" , "kristen@linux.intel.com" , "linux-pm@vger.kernel.org" , "rjw@rjwysocki.net" , "Yates, Alexandra" List-Id: linux-pm@vger.kernel.org T24gRnJpLCAyMDE1LTExLTIwIGF0IDEwOjQzIC0wNTAwLCBQcmFyaXQgQmhhcmdhdmEgd3JvdGU6 DQo+IA0KPiBPbiAxMS8yMC8yMDE1IDEwOjE5IEFNLCBWaXJlc2ggS3VtYXIgd3JvdGU6DQo+ID4g T24gMjAtMTEtMTUsIDEwOjEwLCBQcmFyaXQgQmhhcmdhdmEgd3JvdGU6DQo+ID4+Pj4gIAlsaW1p dHMtPm1heF9wb2xpY3lfcGN0ID0gY2xhbXBfdChpbnQsIGxpbWl0cy0+bWF4X3BvbGljeV9wY3Qs IDAgLCAxMDApOw0KPiA+Pj4NCj4gPj4+IEFuZCBwdXQgdGhpcyBhZnRlciB0aGUgbGF0ZXIgb25l ID8NCj4gPj4+DQo+ID4+Pj4gKwlsaW1pdHMtPm1heF9wb2xpY3lfcGN0ID0gRElWX1JPVU5EX1VQ KHBvbGljeS0+bWF4ICogMTAwLA0KPiA+Pj4+ICsJCQkJCSAgICAgIHBvbGljeS0+Y3B1aW5mby5t YXhfZnJlcSk7DQo+ID4+Pj4gIA0KPiA+Pj4+ICAJLyogTm9ybWFsaXplIHVzZXIgaW5wdXQgdG8g W21pbl9wb2xpY3lfcGN0LCBtYXhfcG9saWN5X3BjdF0gKi8NCj4gPj4+PiAgCWxpbWl0cy0+bWlu X3BlcmZfcGN0ID0gbWF4KGxpbWl0cy0+bWluX3BvbGljeV9wY3QsDQo+ID4+Pg0KPiA+Pj4gU3Vy ZSB5b3UgdGVzdGVkIGl0ICA/IDopDQo+ID4+DQo+ID4+IE9vcHMgLS0gYW5kIHllYWgsIHRlc3Rl ZC4gIEl0IHdvcmtzIGJlY2F1c2UgSSByZXdyaXRlIHRoZSB2YWx1ZSBvZg0KPiA+PiBtYXhfcG9s aWN5X3BjdCA6KS4gIEknbGwgcmVwb3N0IHNob3J0bHkuDQo+ID4gDQo+ID4gQnV0IHdlIGFyZW4n dCBkb2luZyBiZWxvdyBhbnltb3JlLCBkb2Vzbid0IHRoaXMgY2hhbmdlIHRoZQ0KPiA+IGNhbGN1 bGF0aW9ucyBhdCBhbGw/DQo+ID4gDQo+ID4gICAJbGltaXRzLT5tYXhfcG9saWN5X3BjdCA9IGNs YW1wX3QoaW50LCBsaW1pdHMtPm1heF9wb2xpY3lfcGN0LCAwICwgMTAwKTsNCj4gDQo+IFRoZSBj bGFtcCBvbmx5IGNvbmZpbmVzIHRoZSBtYXhfcG9saWN5IGJldHdlZW4gMCBhbmQgMTAwLiAgQUZB SUsgaXQgZG9lc24ndCBtYWtlDQo+IGFueSBjaGFuZ2UgdG90aGUgdmFsdWUgb2YgbGltaXRzLT5t YXhfcG9saWN5X3BjdCB1bmxlc3MgaXQgd2FzIG91dHNpZGUgb2YgdGhhdA0KPiByYW5nZS4NCj4g DQo+IFAuDQo+ID4gDQoNCldpdGggdGhlIGNoYW5nZXMgYmVsb3cgKGFzIHN1Z2dlc3RlZCBhYm92 ZSksIEkgZGlkIHRlc3RzLiBFeGNlcHQgdHdvDQpjYXNlcywgaXQgZGlkIGNvcnJlY3QuIFRob3Nl IHR3byBhcmUgaW4gdHVyYm8gcmFuZ2UsIHNvIEkgYW0gT0sgd2l0aA0KdGhhdC4gDQoNCg0KZGlm ZiAtLWdpdCBhL2RyaXZlcnMvY3B1ZnJlcS9pbnRlbF9wc3RhdGUuYw0KYi9kcml2ZXJzL2NwdWZy ZXEvaW50ZWxfcHN0YXRlLmMNCmluZGV4IGI3OGFiZTkuLmMzYmNjYTQgMTAwNjQ0DQotLS0gYS9k cml2ZXJzL2NwdWZyZXEvaW50ZWxfcHN0YXRlLmMNCisrKyBiL2RyaXZlcnMvY3B1ZnJlcS9pbnRl bF9wc3RhdGUuYw0KQEAgLTExMTEsOSArMTExMSw5IEBAIHN0YXRpYyBpbnQgaW50ZWxfcHN0YXRl X3NldF9wb2xpY3koc3RydWN0DQpjcHVmcmVxX3BvbGljeSAqcG9saWN5KQ0KIAlsaW1pdHMgPSAm cG93ZXJzYXZlX2xpbWl0czsNCiAJbGltaXRzLT5taW5fcG9saWN5X3BjdCA9IChwb2xpY3ktPm1p biAqIDEwMCkgLw0KcG9saWN5LT5jcHVpbmZvLm1heF9mcmVxOw0KIAlsaW1pdHMtPm1pbl9wb2xp Y3lfcGN0ID0gY2xhbXBfdChpbnQsIGxpbWl0cy0+bWluX3BvbGljeV9wY3QsIDAgLA0KMTAwKTsN Ci0JbGltaXRzLT5tYXhfcG9saWN5X3BjdCA9IChwb2xpY3ktPm1heCAqIDEwMCkgLw0KcG9saWN5 LT5jcHVpbmZvLm1heF9mcmVxOw0KKwlsaW1pdHMtPm1heF9wb2xpY3lfcGN0ID0gRElWX1JPVU5E X1VQKHBvbGljeS0+bWF4ICogMTAwLA0KKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgIHBvbGljeS0+Y3B1aW5mby5tYXhfZnJlcSk7DQogCWxpbWl0cy0+bWF4X3Bv bGljeV9wY3QgPSBjbGFtcF90KGludCwgbGltaXRzLT5tYXhfcG9saWN5X3BjdCwgMCAsDQoxMDAp Ow0KLQ0KIAkvKiBOb3JtYWxpemUgdXNlciBpbnB1dCB0byBbbWluX3BvbGljeV9wY3QsIG1heF9w b2xpY3lfcGN0XSAqLw0KIAlsaW1pdHMtPm1pbl9wZXJmX3BjdCA9IG1heChsaW1pdHMtPm1pbl9w b2xpY3lfcGN0LA0KIAkJCQkgICBsaW1pdHMtPm1pbl9zeXNmc19wY3QpOw0KQEAgLTExMzEsNyAr MTEzMSw3IEBAIHN0YXRpYyBpbnQgaW50ZWxfcHN0YXRlX3NldF9wb2xpY3koc3RydWN0DQpjcHVm cmVxX3BvbGljeSAqcG9saWN5KQ0KIAkJCQkgIGludF90b2ZwKDEwMCkpOw0KIAlsaW1pdHMtPm1h eF9wZXJmID0gZGl2X2ZwKGludF90b2ZwKGxpbWl0cy0+bWF4X3BlcmZfcGN0KSwNCiAJCQkJICBp bnRfdG9mcCgxMDApKTsNCi0NCisJbGltaXRzLT5tYXhfcGVyZiA9IHJvdW5kX3VwKGxpbWl0cy0+ bWF4X3BlcmYsIDgpOw0KIAlpZiAoaHdwX2FjdGl2ZSkNCiAJCWludGVsX3BzdGF0ZV9od3Bfc2V0 KCk7DQoNCg0KMzMwMCA6IE9LDQozMjAwIDogMSBsZXNzDQozMTAwIDogMSBsZXNzDQozMDAwIDog MSBsZXNzDQoyOTAwIDogT0sNCjI4MDAgOiBPSw0KMjcwMCA6IE9LDQoyNjAwIDogT0sNCjI1MDAg OiBPSw0KMjQwMCA6IE9LDQoyMzAwIDogT0sNCjIyMDAgOiBPSw0KMjEwMCA6IE9LDQoyMDAwIDog T0sNCjE5MDAgOiBPSw0KMTgwMCA6IE9LDQoxNzAwIDogT0sNCjE2MDAgOiBPSw0KMTUwMCA6IE9L DQoxNDAwIDogT0sNCjEzMDAgOiBPSw0KMTIwMCA6IE9LDQoxMTAwIDogT0sNCjEwMDAgOiBPSw0K OTAwICA6IE9LDQo4MDAgOiBPSw0KDQpUaGFua3MsDQpTcmluaXZhcw0K From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1163291AbbKTUCl (ORCPT ); Fri, 20 Nov 2015 15:02:41 -0500 Received: from mga02.intel.com ([134.134.136.20]:41334 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751428AbbKTUCj (ORCPT ); Fri, 20 Nov 2015 15:02:39 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,324,1444719600"; d="scan'208";a="856223759" From: "Pandruvada, Srinivas" To: "prarit@redhat.com" CC: "Brown, Len" , "linux-kernel@vger.kernel.org" , "viresh.kumar@linaro.org" , "kristen@linux.intel.com" , "linux-pm@vger.kernel.org" , "rjw@rjwysocki.net" , "Yates, Alexandra" Subject: Re: [PATCH 1/2] cpufreq, intel_pstate, Fix limits->max_policy_pct rounding error Thread-Topic: [PATCH 1/2] cpufreq, intel_pstate, Fix limits->max_policy_pct rounding error Thread-Index: AQHRI4+T2KWs1QhTd0qXMTHBhOp3NZ6lauaAgAAfIwCAAAK4AIAABsEAgABISYA= Date: Fri, 20 Nov 2015 20:02:37 +0000 Message-ID: <1448049757.4914.3.camel@intel.com> References: <1448022757-7856-1-git-send-email-prarit@redhat.com> <1448022757-7856-2-git-send-email-prarit@redhat.com> <20151120131833.GD3957@ubuntu> <564F37C8.60307@redhat.com> <20151120151944.GE3957@ubuntu> <564F3FBA.6030806@redhat.com> In-Reply-To: <564F3FBA.6030806@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.20.117] Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id tAKK2j6C003618 On Fri, 2015-11-20 at 10:43 -0500, Prarit Bhargava wrote: > > On 11/20/2015 10:19 AM, Viresh Kumar wrote: > > On 20-11-15, 10:10, Prarit Bhargava wrote: > >>>> limits->max_policy_pct = clamp_t(int, limits->max_policy_pct, 0 , 100); > >>> > >>> And put this after the later one ? > >>> > >>>> + limits->max_policy_pct = DIV_ROUND_UP(policy->max * 100, > >>>> + policy->cpuinfo.max_freq); > >>>> > >>>> /* Normalize user input to [min_policy_pct, max_policy_pct] */ > >>>> limits->min_perf_pct = max(limits->min_policy_pct, > >>> > >>> Sure you tested it ? :) > >> > >> Oops -- and yeah, tested. It works because I rewrite the value of > >> max_policy_pct :). I'll repost shortly. > > > > But we aren't doing below anymore, doesn't this change the > > calculations at all? > > > > limits->max_policy_pct = clamp_t(int, limits->max_policy_pct, 0 , 100); > > The clamp only confines the max_policy between 0 and 100. AFAIK it doesn't make > any change tothe value of limits->max_policy_pct unless it was outside of that > range. > > P. > > With the changes below (as suggested above), I did tests. Except two cases, it did correct. Those two are in turbo range, so I am OK with that. diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index b78abe9..c3bcca4 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -1111,9 +1111,9 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy) limits = &powersave_limits; limits->min_policy_pct = (policy->min * 100) / policy->cpuinfo.max_freq; limits->min_policy_pct = clamp_t(int, limits->min_policy_pct, 0 , 100); - limits->max_policy_pct = (policy->max * 100) / policy->cpuinfo.max_freq; + limits->max_policy_pct = DIV_ROUND_UP(policy->max * 100, + policy->cpuinfo.max_freq); limits->max_policy_pct = clamp_t(int, limits->max_policy_pct, 0 , 100); - /* Normalize user input to [min_policy_pct, max_policy_pct] */ limits->min_perf_pct = max(limits->min_policy_pct, limits->min_sysfs_pct); @@ -1131,7 +1131,7 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy) int_tofp(100)); limits->max_perf = div_fp(int_tofp(limits->max_perf_pct), int_tofp(100)); - + limits->max_perf = round_up(limits->max_perf, 8); if (hwp_active) intel_pstate_hwp_set(); 3300 : OK 3200 : 1 less 3100 : 1 less 3000 : 1 less 2900 : OK 2800 : OK 2700 : OK 2600 : OK 2500 : OK 2400 : OK 2300 : OK 2200 : OK 2100 : OK 2000 : OK 1900 : OK 1800 : OK 1700 : OK 1600 : OK 1500 : OK 1400 : OK 1300 : OK 1200 : OK 1100 : OK 1000 : OK 900 : OK 800 : OK Thanks, Srinivas {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I