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=-7.0 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,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 37E5CC433DF for ; Tue, 4 Aug 2020 10:30:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 338D322CA1 for ; Tue, 4 Aug 2020 10:30:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728343AbgHDKaA (ORCPT ); Tue, 4 Aug 2020 06:30:00 -0400 Received: from foss.arm.com ([217.140.110.172]:42282 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726056AbgHDK37 (ORCPT ); Tue, 4 Aug 2020 06:29:59 -0400 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 B3DEF1FB; Tue, 4 Aug 2020 03:29:58 -0700 (PDT) Received: from [10.37.12.45] (unknown [10.37.12.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 123F83F718; Tue, 4 Aug 2020 03:29:56 -0700 (PDT) Subject: Re: [PATCH 0/4] CPUFreq statistics retrieved by drivers To: Viresh Kumar Cc: Sudeep Holla , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, cristian.marussi@arm.com, rjw@rjwysocki.net References: <20200729151208.27737-1-lukasz.luba@arm.com> <20200730085333.qubrsv7ufqninihd@vireshk-mac-ubuntu> <20200730091014.GA13158@bogus> <3b3a56e9-29ec-958f-fb3b-c689a9389d2f@arm.com> <20200804053502.35d3x3vnb3mggtqs@vireshk-mac-ubuntu> From: Lukasz Luba Message-ID: Date: Tue, 4 Aug 2020 11:29:54 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20200804053502.35d3x3vnb3mggtqs@vireshk-mac-ubuntu> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 8/4/20 6:35 AM, Viresh Kumar wrote: > On 30-07-20, 10:36, Lukasz Luba wrote: >> On 7/30/20 10:10 AM, Sudeep Holla wrote: >>> On Thu, Jul 30, 2020 at 02:23:33PM +0530, Viresh Kumar wrote: >>>> On 29-07-20, 16:12, Lukasz Luba wrote: >>>>> The existing CPUFreq framework does not tracks the statistics when the >>>>> 'fast switch' is used or when firmware changes the frequency independently >>>>> due to e.g. thermal reasons. However, the firmware might track the frequency >>>>> changes and expose this to the kernel. >>>>> >>>>> This patch set aims to introduce CPUfreq statistics gathered by firmware >>>>> and retrieved by CPUFreq driver. It would require a new API functions >>>>> in the CPUFreq, which allows to poke drivers to get these stats. >>>>> >>>>> The needed CPUFreq infrastructure is in patch 1/4, patch 2/4 extends >>>>> ARM SCMI protocol layer, patches 3/4, 4/4 modify ARM SCMI CPUFreq driver. >>>> >>>> Are you doing this for the fast switch case or because your platform >>>> actually runs at frequencies which may be different from what cpufreq >>>> core has requested ? >>>> >>> >>> I think so. >> >> For both cases, but fast switch is major and present. Thermal is not >> currently implemented in SCP FW, but might be in future. > > Okay, lets simplify things a bit and merge things slowly upstream and merge only > what is required right now. > > IIUC, the only concern right now is to capture stats with fast switch ? Maybe we > can do something else in that case and brainstorm a bit.. Correct, the fast switch is the only concern right now and not tracked. We could fill in that information with statistics data from firmware with a cpufreq driver help. I could make the if from patch 1/4 covering narrowed case, when fast switch is present, check for drivers stats. Something like: -----------8<------------------------------------------------------------ if (policy->fast_switch_enabled) if (policy->has_driver_stats) return cpufreq_stats_present_driver_data(policy, buf); else return 0; -------------->8---------------------------------------------------------- > >>>> I am also not sure what these tables should represent, what the >>>> cpufreq core has decided for the CPUs or the frequencies we actually >>>> run at, as these two can be very different for example if the hardware >>>> runs at frequencies which don't match exactly to what is there in the >>>> freq table. I believe these are rather to show what cpufreq and its >>>> governors are doing with the CPUs. >>>> >>> >>> Exactly, I raised similar point in internal discussion and asked Lukasz >>> to take up the same on the list. I assume it was always what cpufreq >>> requested rather than what was delivered. So will we break the userspace >>> ABI if we change that is the main question. >> >> Thank you for confirmation. If that is the mechanism for tracking what >> cpufreq governors are doing with the CPUs, then is clashes with >> presented data in FW memory, because firmware is the governor. > > Why is firmware the governor here ? Aren't you talking about the simple fast > switch case only ? I used a term 'governor' for the firmware because it makes the final set for the frequency. It (FW) should respect the frequency value set using the fast switch. I don't know how other firmware (e.g. Intel) treats this fast switch value or if they even expose FW stats, though. You can read about this statistics region in [1] at: 4.5.5 Performance domain statistics shared memory region > > Over that, I think this cpufreq stats information isn't parsed by any tool right > now and tweaking it a bit won't hurt anyone (like if we start capturing things a > bit differently). So we may not want to worry about breaking userspace ABI here, > if what we are looking to do is the right thing to do. So, there is some hope... IMHO it would be better to have this cpufreq stats in normal location, rather then in scmi debugfs. > >>>> Over that I would like the userspace stats to work exactly as the way >>>> they work right now, i.e. capture all transitions from one freq to >>>> other, not just time-in-state. Also resetting of the stats from >>>> userspace for example. All allocation and printing of the data must be >>>> done from stats core, the only thing which the driver would do at the >>>> end is updating the stats structure and nothing more. Instead of >>>> reading all stats from the firmware, it will be much easier if you can >>>> just get the information from the firmware whenever there is a >>>> frequency switch and then we can update the stats the way it is done >>>> right now. And that would be simple. >>>> >>> >>> Good point, but notifications may not be lightweight. If that is no good, >>> alternatively, I suggested to keep these firmware stats in a separate >>> debugfs. Thoughts ? >> >> I agree that notifications might not be lightweight. > > I am not sure what notifications are we talking about here. There is a notification mechanism described in the SCMI spec [1] at 4.5.4 Notifications. We were referring to that mechanism. > >> Furthermore I think >> this still clashes with the assumption that cpufreq governor decisions >> are tracked in these statistics, not the firmware decision. >> >> In this case I think we would have to create debugfs. >> Sudeep do you think these debugfs should be exposed from the protocol >> layer: >> drivers/firmware/arm_scmi/perf.c >> or maybe from the cpufreq scmi driver? I would probably be safer to have >> it in the cpufreq driver because we have scmi_handle there. > > For the CPUs it would be better if we can keep things in cpufreq only, lets see > how we go about it. > If that would be only ARM SCMI debugfs directory, then we would like to keep it in the scmi. We could re-use the code for devfreq (GPU) device, which is also exposed as performance domain. Thank you Viresh for your comments. Regards, Lukasz [1] https://static.docs.arm.com/den0056/b/DEN0056B_System_Control_and_Management_Interface_v2_0.pdf 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=-7.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,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 B5B96C433E0 for ; Tue, 4 Aug 2020 10:31:28 +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 9BE9B2065C for ; Tue, 4 Aug 2020 10:31:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="t5GRyum4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9BE9B2065C 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-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=jTMZ+Yv0QOEz0mZ9Oqm6bQJ2da4XtsWj/NpaTvRzkaQ=; b=t5GRyum4Ya+cHGHrbf3o+ERdE uJPIEwWIg6/BIdqRacBgTVqu3m6DN7GPaZwfg1Op3BTBJmt3Lgo5XLmBWiDpSVfXTpKujXcEOGOJ/ xPfxQD8nUsZm3GYhasOQ/C+aeoq6OqV4GT+4JUsTDJLp4JuVjP+8hbg5mKvawg6dOLDfbbuQujW+r 3RDeW9Mw8raK1UFx/BdqjA/B74DINsovTEQMe4uRR780V8AiKhRsVD7h20qt+U7PnHEPHhQ3Q32je 5rbhpq7cC3h8amiliCgy7wga2uv8RzVd4kXSNyClopruvgE3O2f64ChlWYI8xqE+q1jiU2y0kB+Ke 8QogPZKZA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k2uCq-0007bI-2g; Tue, 04 Aug 2020 10:30:04 +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 1k2uCn-0007a9-2z for linux-arm-kernel@lists.infradead.org; Tue, 04 Aug 2020 10:30:02 +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 B3DEF1FB; Tue, 4 Aug 2020 03:29:58 -0700 (PDT) Received: from [10.37.12.45] (unknown [10.37.12.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 123F83F718; Tue, 4 Aug 2020 03:29:56 -0700 (PDT) Subject: Re: [PATCH 0/4] CPUFreq statistics retrieved by drivers To: Viresh Kumar References: <20200729151208.27737-1-lukasz.luba@arm.com> <20200730085333.qubrsv7ufqninihd@vireshk-mac-ubuntu> <20200730091014.GA13158@bogus> <3b3a56e9-29ec-958f-fb3b-c689a9389d2f@arm.com> <20200804053502.35d3x3vnb3mggtqs@vireshk-mac-ubuntu> From: Lukasz Luba Message-ID: Date: Tue, 4 Aug 2020 11:29:54 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20200804053502.35d3x3vnb3mggtqs@vireshk-mac-ubuntu> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200804_063001_271431_E187787A X-CRM114-Status: GOOD ( 38.86 ) 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: linux-pm@vger.kernel.org, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Sudeep Holla , cristian.marussi@arm.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 8/4/20 6:35 AM, Viresh Kumar wrote: > On 30-07-20, 10:36, Lukasz Luba wrote: >> On 7/30/20 10:10 AM, Sudeep Holla wrote: >>> On Thu, Jul 30, 2020 at 02:23:33PM +0530, Viresh Kumar wrote: >>>> On 29-07-20, 16:12, Lukasz Luba wrote: >>>>> The existing CPUFreq framework does not tracks the statistics when the >>>>> 'fast switch' is used or when firmware changes the frequency independently >>>>> due to e.g. thermal reasons. However, the firmware might track the frequency >>>>> changes and expose this to the kernel. >>>>> >>>>> This patch set aims to introduce CPUfreq statistics gathered by firmware >>>>> and retrieved by CPUFreq driver. It would require a new API functions >>>>> in the CPUFreq, which allows to poke drivers to get these stats. >>>>> >>>>> The needed CPUFreq infrastructure is in patch 1/4, patch 2/4 extends >>>>> ARM SCMI protocol layer, patches 3/4, 4/4 modify ARM SCMI CPUFreq driver. >>>> >>>> Are you doing this for the fast switch case or because your platform >>>> actually runs at frequencies which may be different from what cpufreq >>>> core has requested ? >>>> >>> >>> I think so. >> >> For both cases, but fast switch is major and present. Thermal is not >> currently implemented in SCP FW, but might be in future. > > Okay, lets simplify things a bit and merge things slowly upstream and merge only > what is required right now. > > IIUC, the only concern right now is to capture stats with fast switch ? Maybe we > can do something else in that case and brainstorm a bit.. Correct, the fast switch is the only concern right now and not tracked. We could fill in that information with statistics data from firmware with a cpufreq driver help. I could make the if from patch 1/4 covering narrowed case, when fast switch is present, check for drivers stats. Something like: -----------8<------------------------------------------------------------ if (policy->fast_switch_enabled) if (policy->has_driver_stats) return cpufreq_stats_present_driver_data(policy, buf); else return 0; -------------->8---------------------------------------------------------- > >>>> I am also not sure what these tables should represent, what the >>>> cpufreq core has decided for the CPUs or the frequencies we actually >>>> run at, as these two can be very different for example if the hardware >>>> runs at frequencies which don't match exactly to what is there in the >>>> freq table. I believe these are rather to show what cpufreq and its >>>> governors are doing with the CPUs. >>>> >>> >>> Exactly, I raised similar point in internal discussion and asked Lukasz >>> to take up the same on the list. I assume it was always what cpufreq >>> requested rather than what was delivered. So will we break the userspace >>> ABI if we change that is the main question. >> >> Thank you for confirmation. If that is the mechanism for tracking what >> cpufreq governors are doing with the CPUs, then is clashes with >> presented data in FW memory, because firmware is the governor. > > Why is firmware the governor here ? Aren't you talking about the simple fast > switch case only ? I used a term 'governor' for the firmware because it makes the final set for the frequency. It (FW) should respect the frequency value set using the fast switch. I don't know how other firmware (e.g. Intel) treats this fast switch value or if they even expose FW stats, though. You can read about this statistics region in [1] at: 4.5.5 Performance domain statistics shared memory region > > Over that, I think this cpufreq stats information isn't parsed by any tool right > now and tweaking it a bit won't hurt anyone (like if we start capturing things a > bit differently). So we may not want to worry about breaking userspace ABI here, > if what we are looking to do is the right thing to do. So, there is some hope... IMHO it would be better to have this cpufreq stats in normal location, rather then in scmi debugfs. > >>>> Over that I would like the userspace stats to work exactly as the way >>>> they work right now, i.e. capture all transitions from one freq to >>>> other, not just time-in-state. Also resetting of the stats from >>>> userspace for example. All allocation and printing of the data must be >>>> done from stats core, the only thing which the driver would do at the >>>> end is updating the stats structure and nothing more. Instead of >>>> reading all stats from the firmware, it will be much easier if you can >>>> just get the information from the firmware whenever there is a >>>> frequency switch and then we can update the stats the way it is done >>>> right now. And that would be simple. >>>> >>> >>> Good point, but notifications may not be lightweight. If that is no good, >>> alternatively, I suggested to keep these firmware stats in a separate >>> debugfs. Thoughts ? >> >> I agree that notifications might not be lightweight. > > I am not sure what notifications are we talking about here. There is a notification mechanism described in the SCMI spec [1] at 4.5.4 Notifications. We were referring to that mechanism. > >> Furthermore I think >> this still clashes with the assumption that cpufreq governor decisions >> are tracked in these statistics, not the firmware decision. >> >> In this case I think we would have to create debugfs. >> Sudeep do you think these debugfs should be exposed from the protocol >> layer: >> drivers/firmware/arm_scmi/perf.c >> or maybe from the cpufreq scmi driver? I would probably be safer to have >> it in the cpufreq driver because we have scmi_handle there. > > For the CPUs it would be better if we can keep things in cpufreq only, lets see > how we go about it. > If that would be only ARM SCMI debugfs directory, then we would like to keep it in the scmi. We could re-use the code for devfreq (GPU) device, which is also exposed as performance domain. Thank you Viresh for your comments. Regards, Lukasz [1] https://static.docs.arm.com/den0056/b/DEN0056B_System_Control_and_Management_Interface_v2_0.pdf _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel