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 EF2F7C433FE for ; Wed, 9 Dec 2020 09:20:16 +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 8D94423B45 for ; Wed, 9 Dec 2020 09:20:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8D94423B45 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=+yYigsBUsfjVrknA1ekrOom8sZOiKNxYVCdQd+mjv+s=; b=0JjvJb/Cj9LZypKY8hneGjuSW XEAeVuSHtvUTwbFoDINeYVmu3HvJVCUiqEzpO+xrS0V7ri1Ans5RrSAbypOhatNzfPoGY9Ejxw3Zr ts3lcMX8pQtBP5iv8JPZYMGIfTfDU/5sMFT0O2MdoHEyOSPqc0IxvIUd0nQdsQWFx1mLcBPgWK+vI 0h90lwf2qQKKYk2G80RsBHDgaNeKxTZCGb77FfDMd7hwKn3XfGodpc2Zk+CNWjpGds31hS/zAS46/ Vm+NfkpzuRutLSmK/VLcvZWxmHHwES78sJM/CznSNkJGTZTDkoQ0zejgw8Bm/zwTzeitNO3Fik0lR IBSZKAqKg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kmvca-0001Rf-3W; Wed, 09 Dec 2020 09:18:52 +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 1kmvcW-0001Oe-Uy for linux-arm-kernel@lists.infradead.org; Wed, 09 Dec 2020 09:18:50 +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 EE5AF1FB; Wed, 9 Dec 2020 01:18:31 -0800 (PST) Received: from [10.57.54.135] (unknown [10.57.54.135]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CF69D3F718; Wed, 9 Dec 2020 01:18:29 -0800 (PST) Subject: Re: [PATCH v4 3/4] scmi-cpufreq: get opp_shared_cpus from opp-v2 for EM To: Viresh Kumar , Sudeep Holla References: <20201202172356.10508-1-nicola.mazzucato@arm.com> <20201202172356.10508-4-nicola.mazzucato@arm.com> <20201208055053.kggxw26kxtnpneua@vireshk-i7> <0e4d3134-f9b2-31fa-b454-fb30265a80b5@arm.com> <20201208072611.ptsqupv4y2wybs6p@vireshk-i7> <20201208112008.niesjrunxq2jz3kt@bogus> <20201209054502.ajomw6glcxx5hue2@vireshk-i7> From: Nicola Mazzucato Message-ID: Date: Wed, 9 Dec 2020 09:20:33 +0000 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: <20201209054502.ajomw6glcxx5hue2@vireshk-i7> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201209_041849_166683_ABD38CA2 X-CRM114-Status: GOOD ( 26.78 ) 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: nm@ti.com, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, sboyd@kernel.org, vireshk@kernel.org, daniel.lezcano@linaro.org, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, robh+dt@kernel.org, chris.redpath@arm.com, morten.rasmussen@arm.com, 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 both, thanks for looking into this. On 12/9/20 5:45 AM, Viresh Kumar wrote: > On 08-12-20, 11:20, Sudeep Holla wrote: >> It is because of per-CPU vs per domain drama here. Imagine a system with >> 4 CPUs which the firmware puts in individual domains while they all are >> in the same perf domain and hence OPP is marked shared in DT. >> >> Since this probe gets called for all the cpus, we need to skip adding >> OPPs for the last 3(add only for 1st one and mark others as shared). > > Okay and this wasn't happening before this series because the firmware > was only returning the current CPU from scmi_get_sharing_cpus() ? yes > > Is this driver also used for the cases where we have multiple CPUs in > a policy ? Otherwise we won't be required to call > dev_pm_opp_set_sharing_cpus(). > > So I assume that we want to support both the cases here ? yes, we want to support existing platforms (n cpus in a policy) + the per-cpu case. > >> If we attempt to add OPPs on second cpu probe, it *will* shout as duplicate >> OPP as we would have already marked it as shared table with the first cpu. >> Am I missing anything ? I suggested this as Nicola saw OPP duplicate >> warnings when he was hacking up this patch. > > The common stuff (for all the CPUs) is better moved to probe() in this > case, instead of the ->init() callback. Otherwise it will always be > messy. You can initialize the OPP and cpufreq tables in probe() > itself, save the pointer somewhere and then just use it here in > ->init(). > > Also do EM registration from there. > ok, will rework >>>> otherwise no need as they would be duplicated. >>>>> And we don't check the return value of >>>>> the below call anymore, moreover we have to call it twice now. >> >> Yes, that looks wrong, we need to add the check for non zero values, but .... >> >>>> >>>> This second get_opp_count is required such that we register em with the correct >>>> opp number after having added them. Without this the opp_count would not be correct. >>> >> >> ... I have a question here. Why do you need to call >> >> em_dev_register_perf_domain(cpu_dev, nr_opp, &em_cb, opp_shared_cpus..) >> >> on each CPU ? Why can't that be done once for unique opp_shared_cpus ? >> >> The whole drama of per-CPU vs perf domain is to have energy model and >> if feeding it opp_shared_cpus once is not sufficient, then something is >> wrong or simply duplicated or just not necessary IMO. >> >>> What if the count is still 0 ? What about deferred probe we were doing earlier ? >> >> OK, you made me think with that question. I think the check was original >> added for deferred probe but then scmi core was changed to add the cpufreq >> device only after everything needed is ready. So the condition must never >> occur now. > > The deferred probe shall be handled in a different patch in that case. > > Nicola, please break the patch into multiple patches, with one patch > dealing only with one task. Sure, I had the doubt and thanks for confirming. will do, thanks > Cheers, Nicola _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel