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.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,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 610BDC433E0 for ; Thu, 9 Jul 2020 10:42:11 +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 2EAA3206DF for ; Thu, 9 Jul 2020 10:42:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="nvGpnjRO"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="qUIf+u3S" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2EAA3206DF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org 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=xgi7JtoEZsI6JMkPVnnukJu9ldco9d18hqxSHE8a/g4=; b=nvGpnjROly7pBEjh7ObHJ+RXk 1f9Bj2/sufbOU7websxAznMPFZyIOXeVR2ZZfERcLJqwFyql4habHNEhfDWQVMpu3zLRW/xekIOdc J7VfayglVWh1nHqDHRbukNHFsWj7AVTelGHkttatQbjpqembIIz0l84WdQrvvL2PRpqfncY5x9b0s 0V3txFUT44hklEWRmToAmWFg4XCUknIceK0DOyTXZuydudVwTQszk5/N9w93SA//Yakc+Vsvc45lq W8GcjNFjE5uuxr003TL1q4u6mY17ZT5ZRRZf57jxtHdtxdw6HFU75Gs7iYxMP6kXO3xhen3rZIrcp pOkqhdeKA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jtTz5-00007R-6V; Thu, 09 Jul 2020 10:40:55 +0000 Received: from mail-pl1-x643.google.com ([2607:f8b0:4864:20::643]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jtTz2-00006d-Td for linux-arm-kernel@lists.infradead.org; Thu, 09 Jul 2020 10:40:54 +0000 Received: by mail-pl1-x643.google.com with SMTP id o1so688720plk.1 for ; Thu, 09 Jul 2020 03:40:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=PWDALEc4aQBpiiDKJrZSu3AoHjPZ9+ketgO5D69yJ4o=; b=qUIf+u3SRF46tLGrPgQitW39s6nP+cSuEl8+w9cbKJlOP4lZjfL51qDpLPxu9+m0YQ qpB2BpjxNi2ZOve7gBwnul1AB9PZ742MHbmny/HTo3GyPI5x7sIY1SxSnka8mkQsKFXy Xhala6QVDw5SGFta7ZJR4f1acI3y8bIKCrl6wBc3HHbTKR5RsXXNzjdnQu9sfbTo13/4 epP3emPrL1Ft9m2e+WCMdtlfD5HVKK+eEv48FIs5f44ItRVEz40VEF9t4uh/9wrrql7M Z1wJI02wiR2fhi6oNqJu3v4NOxtUBLKFaGaf9TgVr9xgMP9rZzJNTxGAd5rcrAJ70VP/ sbHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=PWDALEc4aQBpiiDKJrZSu3AoHjPZ9+ketgO5D69yJ4o=; b=RQ/FX31ppO+HrIauxqhhuRAZXHEA49n7uH7SC/u7XNBMfiyvkrKs7lokSNu6YiWa9p KejJRyOhF0g2qip+m02UINAwn0rgPeIIm/SukDZoZgfxAWDQR9bLUvCDImwcuUQu2VBA nsByEbUcsHNULcMFG+kBc+00I6WjwsfVYvkomctDUQ9Zl2edPWcWO13JhIISlGaWvK7T Z4yRH6IRmakcPcdm5hgR8gt8zK02S2pw7HQ5T/F2QMAdynjnmnv7gf4Iywkkx7Vuu/0Z URhjNrwMIL2KMFdV7NJQGh6uE6K5kKfx/mmoVRJZipP/KN6sygfcEQ7AxItaQ18Waq57 /BrQ== X-Gm-Message-State: AOAM531Bn1pBxiYN0TCgctuTM3Bl6Q3PCLKErEYlbcLI4AXsAtMZJ9RZ NWGnnLk9Xsi1UR1nHvU1hizGwyGIsLw= X-Google-Smtp-Source: ABdhPJwCac281N4fXD18A96QQ4/Sj302QYu9LR9of9M4u+V8oOtEVbHUwoh2ALSx2vnR4eSJcx3REA== X-Received: by 2002:a17:90a:17e4:: with SMTP id q91mr13955267pja.61.1594291250844; Thu, 09 Jul 2020 03:40:50 -0700 (PDT) Received: from localhost ([122.172.40.201]) by smtp.gmail.com with ESMTPSA id ci23sm2197063pjb.29.2020.07.09.03.40.49 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Jul 2020 03:40:50 -0700 (PDT) Date: Thu, 9 Jul 2020 16:10:48 +0530 From: Viresh Kumar To: Ionela Voinescu Subject: Re: [PATCH] arm64: topology: Don't support AMU without cpufreq Message-ID: <20200709104048.emwuquj2qkyascb3@vireshk-i7> References: <20200709101734.GB5623@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200709101734.GB5623@arm.com> User-Agent: NeoMutt/20180716-391-311a52 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200709_064053_243213_8A362F9A X-CRM114-Status: GOOD ( 24.89 ) 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 On 09-07-20, 11:17, Ionela Voinescu wrote: > On Thursday 09 Jul 2020 at 12:22:45 (+0530), Viresh Kumar wrote: > > cpumask_set_cpu(cpu, valid_cpus); > > - have_policy |= enable_policy_freq_counters(cpu, valid_cpus); > > + update_amu_fie_cpus(cpu, valid_cpus); > > I see this as two different pieces of functionality: > - (1) validate_cpu_freq_invariance_counters(cpu) has the job of validating > the CPU support, including max_freq_hz. > - (2) enable_policy_freq_counters() has the job to restrict AMU enablement > for the CPUs in a policy if all CPUs in the policy support AMUs. > > So both of them, separately, support the case of !CONFIG_CPU_FREQ. > > > } > > > > - /* > > - * If we are not restricted by cpufreq policies, we only enable > > - * the use of the AMU feature for FIE if all CPUs support AMU. > > - * Otherwise, enable_policy_freq_counters has already enabled > > - * policy cpus. > > - */ > > - if (!have_policy && cpumask_equal(valid_cpus, cpu_present_mask)) > > This is meant to have the following logic: if for some reason we're not > restricted by policies (according to 2), but all AMU validation was > successful (according to 1), there is no reason not to enable fully AMU > enabled frequency invariance. > > 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 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 ? 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