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=-5.3 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 085ABC433DF for ; Mon, 19 Oct 2020 08:50:56 +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 7EC3A22268 for ; Mon, 19 Oct 2020 08:50:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Ln81epcM" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7EC3A22268 Authentication-Results: mail.kernel.org; dmarc=fail (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: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=B+1eqyzlg+8hDmCJU46DrvieXmQdg8MjZD/iJyjpAHE=; b=Ln81epcMv+9pegzm0AKcnVSNG Cr1h24adz10pqXOK1DZU8OQxfHp2+QNjeUw7Z7hByDqctyzXMz3KucF4NO4ijYqSdsLbY5fBfXgZh mXmUXY/Wcsa5zyub1A/YsAb9z+7gxOlOAjhcYk5DMddo42IQ3pm7dNK0S0rI51dVlRzDxiCZNMLRl jkKJLjZ9vKaEgtlh9X+DIPVYBCLgazfbuDXX2s7jksj3NigGRVQ/N+fK4f9024LGXbj/cap4alu0j PqludzHfRSmynTrKoJD/TEryqe9RAh9OS1/U5nLxK1ikR2Byea2LBU6Rcafmkh6RiCqqixHhr8+Ab KLUZyKOlg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kUQr1-0001Tg-Gl; Mon, 19 Oct 2020 08:49:19 +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 1kUQqy-0001Rw-95 for linux-arm-kernel@lists.infradead.org; Mon, 19 Oct 2020 08:49:17 +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 A8E85101E; Mon, 19 Oct 2020 01:49:13 -0700 (PDT) Received: from [10.57.14.99] (unknown [10.57.14.99]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 84C043F66E; Mon, 19 Oct 2020 01:49:11 -0700 (PDT) Subject: Re: [PATCH v2 2/2] [RFC] CPUFreq: Add support for cpu-perf-dependencies To: Viresh Kumar References: <20200924095347.32148-3-nicola.mazzucato@arm.com> <20201006071909.3cgz7i5v35dgnuzn@vireshk-i7> <2417d7b5-bc58-fa30-192c-e5991ec22ce0@arm.com> <20201008110241.dcyxdtqqj7slwmnc@vireshk-i7> <20201008150317.GB20268@arm.com> <56846759-e3a6-9471-827d-27af0c3d410d@arm.com> <20201009053921.pkq4pcyrv4r7ylzu@vireshk-i7> <20201012154915.GD16519@bogus> <20201012165219.GA3573@arm.com> <17819d4d-9e7e-9a38-4227-d0d10a0749f1@arm.com> <20201014042531.r7iykzygkvmpsqck@vireshk-i7> From: Nicola Mazzucato Message-ID: <503af305-77a4-964a-ed17-8df8b4e3a546@arm.com> Date: Mon, 19 Oct 2020 09:50:33 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <20201014042531.r7iykzygkvmpsqck@vireshk-i7> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201019_044916_432305_6BCAEBBD X-CRM114-Status: GOOD ( 32.50 ) 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: devicetree@vger.kernel.org, linux-pm@vger.kernel.org, vireshk@kernel.org, daniel.lezcano@linaro.org, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org, Sudeep Holla , chris.redpath@arm.com, Ionela Voinescu , morten.rasmussen@arm.com, Lukasz Luba 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, thank you for your suggestion on using 'opp-shared'. I think it could work for most of the cases we explained earlier. Summarising, there are two parts of this entire proposal: 1) where/how to get the information: now we are focusing on taking advantage of 'opp-shared' within an empty opp table 2) and how/where this information will be consumed Further details below: 1) a CPUFreq driver that takes the OPPs from firmware, can call dev_pm_opp_of_get_sharing_cpus like you suggested. When doing so, a provided cpumaksk will be populated with the corresponding cpus that share the same (empty) table opp in DT. All good so far. The current opp core is not expecting an empty table and therefore some errors are thrown when this happens. Since we are now allowing this corner-case, I am presenting below where I think some minor corrections may be needed: --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table, struct device_node *required_np, *np; int count, i; /* Traversing the first OPP node is all we need */ np = of_get_next_available_child(opp_np, NULL); if (!np) { - dev_err(dev, "Empty OPP table\n"); + dev_warn(dev, "Empty OPP table\n"); + + /* + * With empty table we remove shared_opp. This is to leave the + * responsibility to decide which opp are shared to the opp users + */ + opp_table->shared_opp = OPP_TABLE_ACCESS_EXCLUSIVE; + return; } @@ int dev_pm_opp_of_find_icc_paths(struct device *dev, int ret, i, count, num_paths; struct icc_path **paths; ret = _bandwidth_supported(dev, opp_table); - if (ret <= 0) + if (ret == -EINVAL) + return 0; /* Empty OPP table is a valid corner-case, let's not fail */ + else if (ret <= 0) return ret; The above are not 'strictly' necessary to achieve the intended goal, but they make clearer that an empty table is now allowed and not an error anymore. What it is your point of view on this? In addition, I think it would also be appropriate to update the documentation (Documentation/devicetree/bindings/opp/opp.txt) to reflect this new case (required properties etc). Any different thoughts? 2) Once the driver gets the 'performance dependencies' by dev_pm_opp_of_get_sharing_cpus(), this information will have to be shared with EAS, thermal, etc.. The natural way to do so would be to add a new cpumask like I proposed in this RFC. I see this as an improvement for the whole subsystem and a scalable choice since we can unambiguously provide the correct information to whoever needs it, given that we don't enforce "hw dependencies" for related_cpus. The changes would be trivial (it's in the original RFC). On the other hand, we can't unload this h/w detail into related_cpus IMO as we are dealing with per-cpu systems in this context. Hope it makes sense? Once we are pretty much aligned with our ideas, I can run some other tests and make a V3. Thank you very much, Best regards Nicola On 10/14/20 5:25 AM, Viresh Kumar wrote: > On 12-10-20, 18:18, Lukasz Luba wrote: >> On 10/12/20 5:52 PM, Ionela Voinescu wrote: >>> On Monday 12 Oct 2020 at 16:49:30 (+0100), Sudeep Holla wrote: >>>> On Fri, Oct 09, 2020 at 11:09:21AM +0530, Viresh Kumar wrote: >>>>> - I wonder if we can keep using that instead of creating new bindings >>>>> for exact same stuff ? Though the difference here would be that the >>>>> OPP may not have any other entries. >>>> >>>> Well summarised, sorry for chiming in late. I could have not summarised >>>> any better. Just saw the big thread and was thinking of summarising. >>>> If the last point on OPP is possible(i.e. no OPP entries but just use >>>> it for fetch the information) for $subject patch is trying to achieve, >>>> then it would be good. > > Under normal circumstances, I wouldn't have suggested empty opp-tables > for sure but it doesn't seem worth adding another binding to get this > information out :) > >>> >>> Just to put in my two pennies worth: using opp-shared (in possibly empty >>> OPP table) as alternative to cpu-perf-dependencies sounds good enough >>> to me as well. >> >> +1 > > Now that (almost) everyone agrees, I don't think we need to make any > change anywhere, in code or bindings. This should work right now as > well. The code should never try to create OPP tables and the core > will not create one. Your driver (which want to get this information > out of empty OPP tables) shall call dev_pm_opp_of_get_sharing_cpus(), > which just parses the DT to get this information out. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel