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 73891C433E0 for ; Thu, 9 Jul 2020 12:45:14 +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 3EC9E2076A for ; Thu, 9 Jul 2020 12:45:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="FWJICek9" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3EC9E2076A 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=LnvjfzDf4SFkQPqljbPY2MskIIlYUyF1zHn0b9z0w9g=; b=FWJICek9FMke65qJ1J/i1zpyE JNcLiq1ea2PZynAsrINUz24DcyRyCIBKK42C6z2C+FguLReltqwhi++rTuT3P7v/llTMIdvolDiQU NxPovlMBlA3cWfogvMvuNN3+NJvxwEoskwhrr88l3gBPTqWRO7lrWqJlt9sHiEO6qZp0qW/8CWK0s JYL/BxaDdksMM45IMPPKdJBN5YPl7kT9L4oFWsnJFUUo5rDNzoF9BzGSJhTox8NAUTvMdu03ACBrf 7qwj+s3VdADbQtLzfql9NAGAuYpsY19ndKUp/ju2kl7aMXgyx3vNQ1TPwXBXsGQKMguikUBKgeYjt f9xI6EDBQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jtVu9-0006ON-Ux; Thu, 09 Jul 2020 12:43:57 +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 1jtVu6-0006NN-GD for linux-arm-kernel@lists.infradead.org; Thu, 09 Jul 2020 12:43:56 +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 232D61FB; Thu, 9 Jul 2020 05:43:51 -0700 (PDT) Received: from localhost (unknown [10.1.198.53]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B892C3F71E; Thu, 9 Jul 2020 05:43:50 -0700 (PDT) Date: Thu, 9 Jul 2020 13:43:49 +0100 From: Ionela Voinescu To: Viresh Kumar Subject: Re: [RFC 0/3] cpufreq: cppc: Add support for frequency invariance Message-ID: <20200709124349.GA15342@arm.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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_084354_815202_3F896F8F X-CRM114-Status: GOOD ( 23.12 ) 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: Juri Lelli , Vincent Guittot , "Rafael J. Wysocki" , Peter Zijlstra , Catalin Marinas , linux-pm@vger.kernel.org, Sudeep Holla , "Rafael J. Wysocki" , linux-kernel@vger.kernel.org, Steven Rostedt , Ben Segall , Ingo Molnar , Mel Gorman , Greg Kroah-Hartman , Peter Puhov , Will Deacon , Dietmar Eggemann , linux-arm-kernel@lists.infradead.org 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 Hi Viresh, I'll put all my comments here for now, as they refer more to the design of the solution. I hope it won't be too repetitive compared to what we previously discussed offline. I understand you want to get additional points of view. On Thursday 09 Jul 2020 at 15:43:32 (+0530), Viresh Kumar wrote: > Hello, > > CPPC cpufreq driver is used for ARM servers and this patch series tries > to provide frequency invariance support for them. The same is also > provided using a specific hardware extension, known as AMU (Activity > Monitors Unit), but that is optional for platforms and at least few of > them don't have it. > > This patchset allows multiple parts of the kernel to provide the same > functionality, by registering with the topology core. > > This is tested with some hacks, as I didn't have access to the right > hardware, on the ARM64 hikey board to check the overall functionality > and that works fine. > > Ionela/Peter Puhov, it would be nice if you guys can give this a shot. > I believe the code is unnecessarily invasive for the functionality it tries to introduce and it does break existing functionality. - (1) From code readability and design point of view, this switching between an architectural method and a driver method complicates an already complicated situation. We already have code that chooses between a cpufreq-based method and a counter based method for frequency invariance. This would basically introduce a choice between a cpufreq-based method through arch_set_freq_scale(), an architectural counter-based method through arch_set_freq_tick(), and another cpufreq-based method that piggy-backs on the architectural arch_set_freq_tick(). As discussed offline, before I even try to begin accepting the possibility of this complicated mix, I would like to know why methods of obtaining the same thing by using the cpufreq arch_set_freq_scale() or even the more invasive wrapping of the counter read functions is not working. I believe those solutions would brings only a fraction of the complexity added through this set. - (2) For 1/3, the presence of AMU counters does not guarantee their usability for frequency invariance. I know you wanted to avoid the complications of AMUs being marked as supporting invariance after the cpufreq driver init function, but this breaks the scenario in which the maximum frequency is invalid. - (3) For 2/3, currently we support platforms that have partial support for AMUs, while this would not be supported here. The suggestions at (1) would give us this for free. While I'm keen on having invariance support for CPPC when lacking part or full support for AMUs, I think we should explore alternative designs. I can try to come up with some code suggestions, but it will take a few days as I have many other things in the air :). I'm happy to test this when the design is agreed on. Hope it helps, Ionela. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel