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 X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 27B02C433DF for ; Thu, 9 Jul 2020 12:47:52 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E57C42076A for ; Thu, 9 Jul 2020 12:47:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="cmQreIEF" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E57C42076A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=LK7CM/dD/8IlsZ7Bd69ll9LGfp5BNUZiBp2QZqbAd5w=; b=cmQreIEFc0E3U/ON8UTPR5QsR UX0eWq4m4wxxORTN8xPni3e1ctG3cAYndLIN1OzqNeRjesYz4gU/bHYOmfsVMWlpvO15l+gHwMfM6 WD+tXit2+Fv+iNRzibnDiGdTHnZ0lj1VCxG1kOTU6leOj4SCJpKjlejHRdvtp59MUyYrgrIdF/LD4 DkipBkHFOy5wD7aOny5lTLjScmR9dIyUZFnmX+5Nw7ZFh88FT+3qNM5d+Fd5L27E+BC5QzzUPbThQ RfrUtU3etqdixEBdcDT+bwZuPLiGQwDANTOTFjbqf1MnmwvWrJYDdwjPtN+jN1/vme+ioqAaeZGqO yHlvNbAsQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jtVwh-0006kd-Ns; Thu, 09 Jul 2020 12:46:35 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jtVwf-0006k4-3c for linux-arm-kernel@lists.infradead.org; Thu, 09 Jul 2020 12:46:34 +0000 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 4BD421045; Thu, 9 Jul 2020 05:46:32 -0700 (PDT) Received: from localhost (unknown [10.1.198.53]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E1B093F71E; Thu, 9 Jul 2020 05:46:31 -0700 (PDT) Date: Thu, 9 Jul 2020 13:46:30 +0100 From: Ionela Voinescu To: Viresh Kumar Subject: Re: [PATCH] arm64: topology: Don't support AMU without cpufreq Message-ID: <20200709124630.GB15342@arm.com> References: <20200709101734.GB5623@arm.com> <20200709104048.emwuquj2qkyascb3@vireshk-i7> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200709104048.emwuquj2qkyascb3@vireshk-i7> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200709_084633_262610_6BF83841 X-CRM114-Status: GOOD ( 29.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Vincent Guittot Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hey, On Thursday 09 Jul 2020 at 16:10:48 (+0530), Viresh Kumar wrote: [..] > > I agree that this happening is a cornercase and a reason for which > > cpufreq_get_hw_max_freq() was made weak. If some platform has entirely > > firmware driven frequency control, but it enables CONFIG_CPU_FREQ > > (as is the default) and it defines its own cpufreq_get_hw_max_freq(), > > it could benefit from AMU use. > > > > So I did believe it was best for these checks to be decoupled, for this > > reason, and potential other reasons in the future, involving more > > decoupling from cpufreq. > > > > I do have code in progress to clean the overall interaction between > > cpufreq and AMUs, started at [1]. Bear with me on this, it is all > > connected :). > > Of course I missed few things here. > > - I didn't realize that cpufreq_get_hw_max_freq() is defined weak :( > > I understand that we want to support everything that is possible, > but there is no need to support cases which we may never have > actually. We have seen code going in the kernel, which no one ever > ends up using. > > Do we see a case in near future where someone is going to override > this weak implementation ? If we don't have an actual target for it > at the moment, then we should probably remove the weak attribute and > simplify the code. > I saw this case during FVP testing, although I acknowledge the 'virtual' part of that platform [1]. But allowing this does enable AMU testing on an AEM FVP. While I completely understand the reasoning behind avoiding to introduce large changes for small corner-case gains, the arguments for this support was: - (1) AMUs are a new feature and it will take some time until we see the real usecases. That's always the case with early support for a feature - we want to add it early to enable its use and testing, but it will take some time to establish the true usecases. - (2) It literally needed 2 lines of code + the weak cpufreq function to support this. > - I understood earlier that, we don't pick up AMU support unless all > CPUs of a policy are supported by AMUs, but forgot that later while > writing the patch. What is the thing with AMUs? Why would some > platform add it only for some CPUs out of a policy ? Do we have such > platforms already or in queue ? > Given that I can't guarantee what hardware will or won't do, and given that AMUs are an optional feature, I controlled the only thing I could: the software :). By not making assumptions about the hardware, I ensured that the code does not break the interaction between cpufreq use or AMU use for frequency invariance. This will be nicer in the new code as the control will be at CPU level, rather than policy level. [1] https://developer.arm.com/tools-and-software/simulation-models/fixed-virtual-platforms Regards, Ionela. > Lets discuss more after we have settled on the first point here. > > Thanks for review Ionela. > > -- > viresh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel