Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <29f6d4f0-9844-efcf-5fd9-03611580afd3@os.amperecomputing.com>
Date:   Tue, 4 Apr 2023 11:57:31 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.9.1
Subject: Re: [PATCH] cpufreq: CPPC: use 10ms delay instead of 2us to avoid
 high error
To:     Pierre Gondois <pierre.gondois@arm.com>,
        Viresh Kumar <viresh.kumar@linaro.org>
Cc:     rafael@kernel.org, scott@os.amperecomputing.com,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        vincent.guittot@linaro.org, lukasz.luba@arm.com,
        ionela.voinescu@arm.com
References: <20230328193846.8757-1-yang@os.amperecomputing.com>
 <20230330035612.ekh2lpqzohggg6uf@vireshk-i7>
 <b9e94d15-ee49-aa8c-e0fb-1243a91825cd@arm.com>
Content-Language: en-US
From:   Yang Shi <yang@os.amperecomputing.com>
In-Reply-To: <b9e94d15-ee49-aa8c-e0fb-1243a91825cd@arm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NEdCdUphV3dONzdmeEdGNFl6ZjZzeFFDcldSOEJ1K3QvTmJJT3pyTVJYenZO?=
 =?utf-8?B?dEc4REJJVVpLejV1VjBJVERnWW9aQ2k4VHovaWZqcW0yemhVL3U3T2tBZzIx?=
 =?utf-8?B?dGwrbWtYa2NNUXh2Rk1CNHdlZG42b2JVZzBwMEljbitnWVV5WkRwVGorNWVR?=
 =?utf-8?B?QU9XS0txUktlb0hsU0RnZWw1NnUxMCtSMkgxMU90ZURBKzhOa1VkeE9nOFFi?=
 =?utf-8?B?RWNPOEE4MlRBRUJHelBGdlJmd2lLeVBWMnZKVlEyV013UHhVYnVnZHZmZUt4?=
 =?utf-8?B?SmpyZkxvMlI2UWU0b1ZnOG5iYzYwVG1xTTlsY0V0VTdXcHZLdVJEMTN2cE9a?=
 =?utf-8?B?M01ZOWQ1TUY3NndUeVhLWGlaYVVES1d1cmdkWWg3QnVuNlVtTVdaakVTay9m?=
 =?utf-8?B?UXY5WE1HeFdzNTJwS2hNcjhQSGFhbTBCWFB4TWJ0K0tqQzd2QzZLK01DYUta?=
 =?utf-8?B?b3pVZGpuS2dqWWNGd1c0S2VMdC9VT3FpakxZZmFHSC9CZ25XVk1jSTRiMzEv?=
 =?utf-8?B?Ky9RaWNTV0ZySXhMSWVFOVhaQ0QrY3pvUktwSWdsU213MVVaTWlXbzF3K3hv?=
 =?utf-8?B?aG5naTRmR3FVWTJlcVQvTnRCT0pnTTliS2s4L0RBZDVDSFFmeVR4OUQ2bGdI?=
 =?utf-8?B?KzBsSzNJYXpTMWZ6bGhRb1N5akt3VGhjRmNsc3luMDljeWZBNEpjYjQvdGJR?=
 =?utf-8?B?MklBak5lUVhlV3ZSUEd5ZnNkcGFFeVhucVVRbWVWMjFLZ0FEczlFTjl0NGpu?=
 =?utf-8?B?Y2R1QXVod3htbWxEdkRkdlV6dWtMdUlrTXk2VUJkTDB4YXhYZ2FnLzNjQ21y?=
 =?utf-8?B?Y25QVXc0QTBQM2hEaUhxcmlqUk8veDQ4REZtSUhxVGNFZUdEbmJub1Z1MStX?=
 =?utf-8?B?VjY3aW5HMXY5N1hZQXFEajR0a0puNVg3SjhQWGZPUnpGckdWcHA4QkczVFYz?=
 =?utf-8?B?MGZXc1BCNExNenFzd3kxRHFCN2t0aDR4a1Y5cmJ4WjVCNVFqUDdQOS9lZldu?=
 =?utf-8?B?L0JMMjUyT2xRL0lkOXoxaE5xeWs2WFduZFVlQUJ3TXltdHZZeG5mY2NpcFhx?=
 =?utf-8?B?b2ZmM1IzejdCVW82WDBzbXUwZWhWSlFwd1ZyVXF3ZERQbXgwQ2JYSWpzc1ky?=
 =?utf-8?B?WnZaR2RXSEdSSUZua25ZWHo4bk5UNUFuWUlBLzVnU2tSREJjL05ISHlSYjdr?=
 =?utf-8?B?MHE2NVdNR0VQWERrZUNPVzl3WG5DY3JmeXcrbjRsMzdSNXZ0VTRUcUpWNjc3?=
 =?utf-8?B?RUxiWlllZUxwbzZxbGNJbEkwWHBqSnN4RjhtSUdIcmI3QkxoUWN4YzBJZStM?=
 =?utf-8?B?Njc3aExSZ2NyajZaeFF1aDBNRkFXMm5lc243cUVyUytOWVphTVpGWktkTWxQ?=
 =?utf-8?B?dHR0ODNDL0FqeWpUYTFKLy9ZSXRQRGc3SHNucThsRVR2MVF2MGxXblZzN3Bt?=
 =?utf-8?B?N0lXeUZ6ekhKY3UwZENmUWN3Z3Rud3RiK0pIYVA3YlRHdFlNRlRoamNHT0Ix?=
 =?utf-8?B?UEN1RElramJYTXJlczVkMnI5V1p4THJNNll2Z2VhSjBCd1dWSW1jVENHdkdw?=
 =?utf-8?B?YTdxSkVnU3YvVlRZaU9GM3VqNGFRZGkxZGtuNjZEQktMcUFwNUNjQ3d6MXF4?=
 =?utf-8?B?OHFuS2pIK0w3UU1VMTcwSjZtaG10cEpNelhZazhsdEFqelEzdFJVZnJxMkVP?=
 =?utf-8?B?K3NOK0h5SXZ6T0lLL3pxcERTYUNHc216TzZmc2tmUytiRFlVN0pvTXBtY3Bm?=
 =?utf-8?B?eUpTZlNUT21zQUxOajkwWFVMd3pOVWt5NmZmRkx5VTdsSnhaa2xsOXd2STl5?=
 =?utf-8?B?VVYrczFqOUZUbnAxb1JHMjBDK29rZ1N4T0czQnZvMGd0OGFkUkZ0V0VURENy?=
 =?utf-8?B?ajVvKzlxVEsyaWIxSHJqM2VZWnAzZDV3OEx6STdvZWw5VkdHUmo0YU84OGc3?=
 =?utf-8?B?bndoRnQzRGFVR1AzNEtUMjlycWlqUE5JMkZxbGRpNFlBQkZFTlMxSnB2YzBt?=
 =?utf-8?B?b3krYkFLQWYyZVN5aGtTQlBTNVJiYTI5VDhIRjNnZkdpekRiTmwyeEZsTHNZ?=
 =?utf-8?B?TU8yOG4xdjRBZ01TUW9Pdy9tRUZYRlA0eXRFYURTY3UxSHo0SEdFTTJ1clVS?=
 =?utf-8?B?aU5CbmRKQkRoSFNqUVZpdVd4UXRiektocm9zdGJFcEdwWHNQaTBVOUZ1cWlI?=
 =?utf-8?Q?BxJjLkL61okTsd/2wWwizTQ=3D?=
X-OriginatorOrg: os.amperecomputing.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c4b9731a-b8bd-4c2b-b10a-08db353e743e
X-MS-Exchange-CrossTenant-AuthSource: BL3PR01MB7027.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2023 18:57:35.9041
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: PRr1VgAoyau82t5JiP0ZnAU2QEsGnp/B1dHAe0Tz0NJYAtv3dySFxIr2HUbjlYhpYXrkje/svYysHGOGvJ2wYbk7pQx6utfl1Ev1NLyVyXc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY3PR01MB6516
X-Spam-Status: No, score=-2.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,
        SPF_HELO_PASS,SPF_PASS autolearn=unavailable autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 3/31/23 2:53 AM, Pierre Gondois wrote:
> Hello,
>
> On 3/30/23 05:56, Viresh Kumar wrote:
>> + few folks.
>>
>> On 28-03-23, 12:38, Yang Shi wrote:
>>> When testing CPPC cpufreq on our platform, we noticed the error may 
>>> be quite
>>> high and the high error may happen quite often.  For example, on a 
>>> platform
>>> with a maximum frequency of 2.8GHz when the CPUs were fully loaded 
>>> (100% load),
>>> we saw cpuinfo_cur_freq may show 4GHz, it means the error is > 40%.  
>>> And the
>>> high error (> 1%) happened 256 times out of 2127 samples (sampled 
>>> every 3
>>> seconds) in an approximate 2hrs test.
>>>
>>> We tried to enlarge the delay, and tested with 100us, 1ms and 10ms.  
>>> The
>>> below is the results.
>>>
>>> 100us:
>>> The highest error is 4GHz, 22 times out of 3623 samples
>>>
>>> 1ms:
>>> The highest error is 3.3GHz, 3 times out of 2814 samples
>>>
>>> 10ms:
>>> No high error anymore
>>>
>>> Increase the measurement delay in cppc_cpufreq_get_rate to 10ms to 
>>> avoid
>>> high measurement errors.
>>>
>>> Signed-off-by: Yang Shi <yang@os.amperecomputing.com>
>>> ---
>>>   drivers/cpufreq/cppc_cpufreq.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c 
>>> b/drivers/cpufreq/cppc_cpufreq.c
>>> index 022e3555407c..c2bf65448d3d 100644
>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>> @@ -851,7 +851,7 @@ static unsigned int 
>>> cppc_cpufreq_get_rate(unsigned int cpu)
>>>       if (ret)
>>>           return ret;
>>>   -    udelay(2); /* 2usec delay between sampling */
>>> +    mdelay(10); /* 10msec delay between sampling */
>>>         ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t1);
>>>       if (ret)
>>> -- 
>>> 2.39.2
>>
>
> Just 2 considerations:
> -
> When using the schedutil governor, frequencies should be updated with
> a period of cppc_cpufreq_get_transition_delay_us().
> This period should be 1ms if CPPC doesn't rely on PCC channels, otherwise
> the value depends on the PCC channel (cf. cppc_get_transition_latency()).
>
> If the evaluation duration for the perf/ref counters is higher than this
> period, I think this would mean that multiple frequency update would 
> happen
> while trying to evaluate the current frequency of a CPU.
>
> -
> There is a TimeWindowRegister field in CPPC (cf. enum 
> cppc_regs::TIME_WINDOW
> and ACPI 6.5 s8.4.6.1.2.5 "Time Window Register") that should 
> approximately
> match what this patch aims to solve.
> """
> When Autonomous Selection is enabled, values written to the Time 
> Window Register are ignored. Reads of the Time
> Window register indicate minimum length of time (in ms) between 
> successive reads of the platform’s performance
> counters.
> """
> The only issue being that we should be in the case where Autonomous 
> Selection
> is disabled, where the description of the register is different.
>

Thanks for the points. IIUC, the delay should be limited by the two 
factors. So it should be max(cppc_cpufreq_get_transition_delay_us(), 
Time Winder Register)?

> Regards,
> Pierre