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.4 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,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 2BFD0C433DF for ; Fri, 10 Jul 2020 03:00:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 04B4820720 for ; Fri, 10 Jul 2020 03:00:46 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="GETUc2u3" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726832AbgGJDAg (ORCPT ); Thu, 9 Jul 2020 23:00:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50564 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726509AbgGJDAg (ORCPT ); Thu, 9 Jul 2020 23:00:36 -0400 Received: from mail-pl1-x634.google.com (mail-pl1-x634.google.com [IPv6:2607:f8b0:4864:20::634]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 257B6C08C5DC for ; Thu, 9 Jul 2020 20:00:36 -0700 (PDT) Received: by mail-pl1-x634.google.com with SMTP id p1so1654945pls.4 for ; Thu, 09 Jul 2020 20:00:36 -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=x2Q65EzY9A+4WJ3/3xgyKC2pLQLOauk7fOikDEnLLU4=; b=GETUc2u39p7zUsL72Oq0XwZQlEW4m73isgdfB6vVv1Ss0Y1ukG1UVuZuCfEU2GjUjf iDhzCNv+q8CYc3fum11i2xf1arEQIpeaxepR5LKM9HT2hqaeNWhYGHxXnm+wh/fODrhx d9MpgNGBhu+C78hGnOu/oOn4aJ5j8YQywSglNskU5la3PTJg2E7DVpQcQ938EgjOIxGx 7z++Zzk3H/qgdsNdOJGgja44weWdUbethNcvkBhQnxDV9zKEwHq05O2Ovy0YPno0sJPH ATYqPf+4jPueD+Z4NNE1FXoR4CCkfL5XWXkYj/FdWKIrC50uzI3a4z2+4wGDaBVZfdyI Ttpw== 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=x2Q65EzY9A+4WJ3/3xgyKC2pLQLOauk7fOikDEnLLU4=; b=Ep4N5tkr6aaYquSZKLWtNK2IZjpeZ4eCogeBR0dNaGnnLvVks42/QNbyA3ey3wW/6Y DbqZCPxQeBSLbSSKAahS7jAYIrC27NSM2DIrTbF2O03tsRYhX8FTmLamNZLZhlWqaFLW fVESGRnCNSsRC5Sso2p9V9cb3+HKSdgoQeIkeNMg702sZwYdDCQo4Nrj/RZ78PkikFlZ tDmrIn+YwL8/tssFP7czuDMQb2yxwN1Y5E41FOCmij5H1e8+ppmmw7cJ5rhO09wKhYVK AcoDUKvj5uWFlha0PppyaAhVXMsSbOv4igrsglDsZ/KxUElfQh2l95plsZPKbuHG/K4I VInw== X-Gm-Message-State: AOAM530hLSJHI4/rF+aw7MP6dNYCUL6pucL1kBUGtSoGI8jVHPfhVPYy nJ1PcCdOwD9QCYbYzDX+YnfCuujQQM8= X-Google-Smtp-Source: ABdhPJzJbs8HynvxExYfLaLpk6jM+CJiUTM1pLP8kT49ZfV7ziWx8HZYwK+knJKvvfM5wbheEJB6SA== X-Received: by 2002:a17:902:aa0c:: with SMTP id be12mr58339011plb.45.1594350035374; Thu, 09 Jul 2020 20:00:35 -0700 (PDT) Received: from localhost ([122.172.34.142]) by smtp.gmail.com with ESMTPSA id y17sm4141720pfe.30.2020.07.09.20.00.33 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Jul 2020 20:00:34 -0700 (PDT) Date: Fri, 10 Jul 2020 08:30:32 +0530 From: Viresh Kumar To: Ionela Voinescu Cc: Ben Segall , Catalin Marinas , Dietmar Eggemann , Greg Kroah-Hartman , Ingo Molnar , Juri Lelli , Mel Gorman , Peter Zijlstra , "Rafael J. Wysocki" , "Rafael J. Wysocki" , Steven Rostedt , Sudeep Holla , Vincent Guittot , Will Deacon , Peter Puhov , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org Subject: Re: [RFC 0/3] cpufreq: cppc: Add support for frequency invariance Message-ID: <20200710030032.3yq3lqqybhy5m744@vireshk-i7> References: <20200709124349.GA15342@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200709124349.GA15342@arm.com> User-Agent: NeoMutt/20180716-391-311a52 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Thanks for the quick reply Ionela. On 09-07-20, 13:43, Ionela Voinescu wrote: > 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. Not necessarily, I knew you would be one of the major reviewers here :) I posted so you don't need to review in private anymore and then the code is somewhat updated since the previous time. > On Thursday 09 Jul 2020 at 15:43:32 (+0530), Viresh Kumar wrote: > 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(). I agree. > 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() The problem is same as that was in case of x86, we don't know the real frequency the CPU may be running at and we need something that fires up periodically in a guaranteed way to capture the freq-scale. Though I am thinking now if we can trust the target_index() helper and keep updating the freq-scale based on the delta between last call to it and the latest call. I am not sure if it will be sufficient. > or even the more invasive wrapping of the > counter read functions is not working. I am not sure I understood this one. > - (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. Is that really a scenario ? i.e. Invalid maximum frequency ? Why would that ever happen ? And I am not sure if this breaks anything which already exists, because all we are doing in this case now is not registering cppc for FI, which should be fine. > - (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. As both were counter based mechanisms, I thought it would be better and more consistent if only one of them is picked. Though partial support of AMUs would still work without the CPPC driver. -- viresh 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 9A096C433DF for ; Fri, 10 Jul 2020 03:02:42 +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 5F95420720 for ; Fri, 10 Jul 2020 03:02:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="NLDQSHrG"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="GETUc2u3" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5F95420720 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=WU2VG6Zd02vVVC9cFGT8TwoINCE/Xx+JLGDAR62f4ls=; b=NLDQSHrGwOA1CtPQWT2MdP6lj jNX85nfHhYHTiOqwP8fW+Lb3UqAhT9FyghXpRV9gWPAuWVHntwu5jjEeL7ascScs+qveDy3LBjBHC AOOBWXkdQ9+hKS897PY+ychjEhAeB7tY5hY6UXtWI/hERXNRudQkVgDuxK89d+ub7sQxsIqnAJVSl yZPK5AQt5eRDq1sUT0wbi3LI6q2M/A4BBVn+uznUzYRQAetc8GJkZKGnAZYsBxsPV3ZcgMX2+NNdO 3PU7OAJ9Sdv9n0PEzEBX06HhvZWDn2VXRe7p72beRutKHmNpFcHRywk6M6shv2z3WpqhQ5ej2eXid 9XT8TCf4A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jtjHI-0007kT-4c; Fri, 10 Jul 2020 03:00:44 +0000 Received: from mail-pl1-x631.google.com ([2607:f8b0:4864:20::631]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jtjHE-0007ja-4p for linux-arm-kernel@lists.infradead.org; Fri, 10 Jul 2020 03:00:41 +0000 Received: by mail-pl1-x631.google.com with SMTP id k5so1639138plk.13 for ; Thu, 09 Jul 2020 20:00:37 -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=x2Q65EzY9A+4WJ3/3xgyKC2pLQLOauk7fOikDEnLLU4=; b=GETUc2u39p7zUsL72Oq0XwZQlEW4m73isgdfB6vVv1Ss0Y1ukG1UVuZuCfEU2GjUjf iDhzCNv+q8CYc3fum11i2xf1arEQIpeaxepR5LKM9HT2hqaeNWhYGHxXnm+wh/fODrhx d9MpgNGBhu+C78hGnOu/oOn4aJ5j8YQywSglNskU5la3PTJg2E7DVpQcQ938EgjOIxGx 7z++Zzk3H/qgdsNdOJGgja44weWdUbethNcvkBhQnxDV9zKEwHq05O2Ovy0YPno0sJPH ATYqPf+4jPueD+Z4NNE1FXoR4CCkfL5XWXkYj/FdWKIrC50uzI3a4z2+4wGDaBVZfdyI Ttpw== 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=x2Q65EzY9A+4WJ3/3xgyKC2pLQLOauk7fOikDEnLLU4=; b=M9C5rcavLKMZ5zgRynqrcDY2Mwav4MFuqeLwAaQ8fy/vUYSam4qM140SpswX7mYMjM Vb+zwJYV3ygM2Mz6rX7L8lEoXfXmScyd7bM0FyU70XxaAns0zoGS35PQHHO3X0GRxu5W LoK29ibQcn2DoxhXzth1csDFDky6dyzgGB0LnNnrp4bHdY+hjKJlnTG9Y/wZXhFPEdj0 wvySlUF7kS6gOeUANO1Hmzk18V2UUKWupGamAv9NEna5YYam1NBHSof95n49Ku6Hk1rJ J7LLz/9dXkk81WRV+Imoxvjpunt6ui0IYaTA6xT84L1FD5h4aT4b/W/41LD5IYoH0xjj WLDQ== X-Gm-Message-State: AOAM530GSO+OYUAot2Pe9mLtKinGcQsScSMHt3HXN5z/s1VGPA6LJvyW PQSzfrsZTjjrechEElTYejlZuA== X-Google-Smtp-Source: ABdhPJzJbs8HynvxExYfLaLpk6jM+CJiUTM1pLP8kT49ZfV7ziWx8HZYwK+knJKvvfM5wbheEJB6SA== X-Received: by 2002:a17:902:aa0c:: with SMTP id be12mr58339011plb.45.1594350035374; Thu, 09 Jul 2020 20:00:35 -0700 (PDT) Received: from localhost ([122.172.34.142]) by smtp.gmail.com with ESMTPSA id y17sm4141720pfe.30.2020.07.09.20.00.33 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Jul 2020 20:00:34 -0700 (PDT) Date: Fri, 10 Jul 2020 08:30:32 +0530 From: Viresh Kumar To: Ionela Voinescu Subject: Re: [RFC 0/3] cpufreq: cppc: Add support for frequency invariance Message-ID: <20200710030032.3yq3lqqybhy5m744@vireshk-i7> References: <20200709124349.GA15342@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200709124349.GA15342@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_230040_249896_6EF9DC52 X-CRM114-Status: GOOD ( 24.22 ) 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 Thanks for the quick reply Ionela. On 09-07-20, 13:43, Ionela Voinescu wrote: > 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. Not necessarily, I knew you would be one of the major reviewers here :) I posted so you don't need to review in private anymore and then the code is somewhat updated since the previous time. > On Thursday 09 Jul 2020 at 15:43:32 (+0530), Viresh Kumar wrote: > 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(). I agree. > 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() The problem is same as that was in case of x86, we don't know the real frequency the CPU may be running at and we need something that fires up periodically in a guaranteed way to capture the freq-scale. Though I am thinking now if we can trust the target_index() helper and keep updating the freq-scale based on the delta between last call to it and the latest call. I am not sure if it will be sufficient. > or even the more invasive wrapping of the > counter read functions is not working. I am not sure I understood this one. > - (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. Is that really a scenario ? i.e. Invalid maximum frequency ? Why would that ever happen ? And I am not sure if this breaks anything which already exists, because all we are doing in this case now is not registering cppc for FI, which should be fine. > - (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. As both were counter based mechanisms, I thought it would be better and more consistent if only one of them is picked. Though partial support of AMUs would still work without the CPPC driver. -- viresh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel