From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 315FCC433F5 for ; Tue, 15 Mar 2022 09:49:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346829AbiCOJuj (ORCPT ); Tue, 15 Mar 2022 05:50:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56914 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236295AbiCOJug (ORCPT ); Tue, 15 Mar 2022 05:50:36 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 343121AF24 for ; Tue, 15 Mar 2022 02:49:23 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 25ED01474; Tue, 15 Mar 2022 02:49:23 -0700 (PDT) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0BA6D3F66F; Tue, 15 Mar 2022 02:49:21 -0700 (PDT) Date: Tue, 15 Mar 2022 09:49:19 +0000 From: Sudeep Holla To: Leo Yan Cc: Ionela Voinescu , Sudeep Holla , Greg Kroah-Hartman , "Rafael J. Wysocki" , Vincent Guittot , Bryan O'Donoghue , linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 0/3] arch_topology: Correct CPU capacity scaling Message-ID: References: <20220313055512.248571-1-leo.yan@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Leo, On Mon, Mar 14, 2022 at 06:10:58PM +0000, Ionela Voinescu wrote: > Hi Leo, > > On Sunday 13 Mar 2022 at 13:55:09 (+0800), Leo Yan wrote: > > This patch set is to address issues for CPU capacity scaling. > > > > "capacity-dmips-mhz" property might be absent in all CPU nodes, and in > > another situation, DT might have inconsistent binding issue, e.g. some > > CPU nodes have "capacity-dmips-mhz" property and some nodes miss the > > property. Current code mixes these two cases and always rollback to CPU > > capacity 1024 for these two cases. > > Ideally the schema can be made to catch such issues. While I understand that it is work in progress, we can flag the error in the code to handle that. Rollback to 1024 seems correct default behaviour to me. > > Patches 01 and 02 in this set are used to distinguish the two different > > DT binding cases, and for the inconsistent binding issue, it rolls back > > to 1024 without CPU capacity scaling. > > > > Patch 03 is to handle the case for absenting "capacity-dmips-mhz" > > property in CPU nodes, the patch proceeds to do CPU capacity scaling based > > on CPU maximum capacity. Thus it can reflect the correct CPU capacity for > > Arm platforms with "fast" and "slow" clusters (CPUs in two clusters have > > the same raw capacity but with different maximum frequencies). > > NACK for the approach. Just fix the DT. > > In my opinion it's difficult to handle absent "capacity-dmips-mhz" > properties, as they can be a result of 3 scenarios: potential.. > 1. bug in DT > 2. unwillingness to fill this information in DT > 3. suggestion that we're dealing with CPUs with same u-arch > (same capacity-dmips-mhz) > > I'm not sure it's up to us to interpret suggestions in the code so I > believe treating missing information as error is the right choice, which > is how we're handling this now. > +1 for all the points above and are very much valid. > For 3. (and patch 03), isn't it easier to populate capacity-dmips-mhz to > the same value (say 1024) in DT? That is a clear message that we're > dealing with CPUs with the same u-arch. > Indeed. -- Regards, Sudeep