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=-4.8 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, 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 06F64C5519F for ; Thu, 12 Nov 2020 20:44:12 +0000 (UTC) Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 7DC20216C4 for ; Thu, 12 Nov 2020 20:44:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="HxwjxsQm" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7DC20216C4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=driverdev-devel-bounces@linuxdriverproject.org Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id E8ED887096; Thu, 12 Nov 2020 20:44:10 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FRr-bhmCLzzA; Thu, 12 Nov 2020 20:44:09 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by whitealder.osuosl.org (Postfix) with ESMTP id 8764C870AD; Thu, 12 Nov 2020 20:44:09 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by ash.osuosl.org (Postfix) with ESMTP id BA8181BF2CF for ; Thu, 12 Nov 2020 20:44:08 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id AF95920113 for ; Thu, 12 Nov 2020 20:44:08 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6vyk498Ssq+u for ; Thu, 12 Nov 2020 20:44:03 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) by silver.osuosl.org (Postfix) with ESMTPS id 1981620116 for ; Thu, 12 Nov 2020 20:44:03 +0000 (UTC) Received: by mail-wr1-f52.google.com with SMTP id d12so7396181wrr.13 for ; Thu, 12 Nov 2020 12:44:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=q+20iQAaEm4q/hEUaeWqM8BE9cNbPFG1XCmKyaVmb68=; b=HxwjxsQm/Aehke3Fv5Qn1I/gx7cdcRVWHLoh7zMN5d6eja8wHhSwqI6sNuOlWiDgNH 0Yk86Dq0DkpNIvkBJ+FwawM++mZMAehPqdID+EWV27o8WwtUqxDUvInTFbiypDsJgwPO bpmLU7bpYlrsuR+o9ZncT+A0IlgSbNVB7rOK86hVP8U4ezJN17KnYlu/slvBwZGjh9fD OL9JGA3ZycVyXsWPE2zA+B7OjU+hXZxmZzWQOIcJLqf2hJ/js89Z7Qpa4wcNaVgbYqdj 1ME8LenkMJPlcTOvmGNzF4CxAdmiZJcOq12brSvwJezr+w4EP3VwLwyA2o6RZM1xn67d Jb3A== 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=q+20iQAaEm4q/hEUaeWqM8BE9cNbPFG1XCmKyaVmb68=; b=cvISKBYwj+QwuOL8WC49E8SBDtNlYGb00LuzmUXUgNR0qPxvTPcm6RgDDuO/NzNKWL eaF6HGKJ1reeje2KU4+0F5ukTN3ldlkkBG4iaZ1eKHkRlocgirj7bA9R91vbSwwamuWn NeQrHI84DUzVUmxwlssgdnOwIQXpGWudWcIfvaHwq7lkIaX3fNTUhrn2jSRvJS248XI+ tzaVb+Ok9n5weS9tFSJgDigtdLFE5FHI8Bim66R63Ppeo1b3P8kAAbHjcd7YK1Lx1qyq qIUlm5C1LWxm2VvmEvzJF/DBJwBtuGVrqvIJPTUX5PPVmF/6W49nVla2OmOAiRUHd6GE pRLw== X-Gm-Message-State: AOAM5317C4vbyOldlKt7KLvNqmZCcc4z7ldYR2/LzLvgIJLrBmYNMEdZ Bc/52tUZpmJ7rLn/6i+cUfY= X-Google-Smtp-Source: ABdhPJxE9+HQyG91KkMLLlz6E63XshyQv4iVzGZmBh44qaFqvMY2XiAJIXziTAWHEenvefZ/TDIvrg== X-Received: by 2002:adf:b109:: with SMTP id l9mr1479729wra.279.1605213841288; Thu, 12 Nov 2020 12:44:01 -0800 (PST) Received: from localhost ([217.111.27.204]) by smtp.gmail.com with ESMTPSA id t11sm1498448wrm.8.2020.11.12.12.43.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Nov 2020 12:43:59 -0800 (PST) Date: Thu, 12 Nov 2020 21:43:58 +0100 From: Thierry Reding To: Dmitry Osipenko Subject: Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs Message-ID: <20201112204358.GA1027187@ulmo> References: <20201104234427.26477-1-digetx@gmail.com> <2716c195-083a-112f-f1e5-2f6b7152a4b5@gmail.com> <1f7e90c4-6134-2e2b-4869-5afbda18ead3@gmail.com> MIME-Version: 1.0 In-Reply-To: <1f7e90c4-6134-2e2b-4869-5afbda18ead3@gmail.com> User-Agent: Mutt/1.14.7 (2020-08-29) X-BeenThere: driverdev-devel@linuxdriverproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Chen , Ulf Hansson , DTML , Viresh Kumar , dri-devel , Adrian Hunter , Lee Jones , Marek Szyprowski , driverdevel , linux-samsung-soc , Nicolas Chauvet , Krzysztof Kozlowski , Jonathan Hunter , Alan Stern , Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= , Linux Media Mailing List , linux-pwm@vger.kernel.org, Rob Herring , linux-tegra , Mauro Carvalho Chehab , Greg Kroah-Hartman , Linux USB List , "linux-mmc@vger.kernel.org" , Liam Girdwood , Linux Kernel Mailing List , Mark Brown , Peter Geis Content-Type: multipart/mixed; boundary="===============7908733464558060205==" Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" --===============7908733464558060205== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="BXVAT5kNtrzKuDFl" Content-Disposition: inline --BXVAT5kNtrzKuDFl Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Nov 12, 2020 at 10:57:27PM +0300, Dmitry Osipenko wrote: > 11.11.2020 14:38, Ulf Hansson =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > > On Sun, 8 Nov 2020 at 13:19, Dmitry Osipenko wrote: > >> > >> 05.11.2020 18:22, Dmitry Osipenko =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > >>> 05.11.2020 12:45, Ulf Hansson =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > >>> ... > >>>> I need some more time to review this, but just a quick check found a > >>>> few potential issues... > >>> > >>> Thank you for starting the review! I'm pretty sure it will take a cou= ple > >>> revisions until all the questions will be resolved :) > >>> > >>>> The "core-supply", that you specify as a regulator for each > >>>> controller's device node, is not the way we describe power domains. > >>>> Instead, it seems like you should register a power-domain provider > >>>> (with the help of genpd) and implement the ->set_performance_state() > >>>> callback for it. Each device node should then be hooked up to this > >>>> power-domain, rather than to a "core-supply". For DT bindings, please > >>>> have a look at Documentation/devicetree/bindings/power/power-domain.= yaml > >>>> and Documentation/devicetree/bindings/power/power_domain.txt. > >>>> > >>>> In regards to the "sync state" problem (preventing to change > >>>> performance states until all consumers have been attached), this can > >>>> then be managed by the genpd provider driver instead. > >>> > >>> I'll need to take a closer look at GENPD, thank you for the suggestio= n. > >>> > >>> Sounds like a software GENPD driver which manages clocks and voltages > >>> could be a good idea, but it also could be an unnecessary > >>> over-engineering. Let's see.. > >>> > >> > >> Hello Ulf and all, > >> > >> I took a detailed look at the GENPD and tried to implement it. Here is > >> what was found: > >> > >> 1. GENPD framework doesn't aggregate performance requests from the > >> attached devices. This means that if deviceA requests performance state > >> 10 and then deviceB requests state 3, then framework will set domain's > >> state to 3 instead of 10. > >> > >> https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/base/power/d= omain.c#L376 > >=20 > > As Viresh also stated, genpd does aggregate the votes. It even > > performs aggregation hierarchy (a genpd is allowed to have parent(s) > > to model a topology). >=20 > Yes, I already found and fixed the bug which confused me previously and > it's working well now. >=20 > >> 2. GENPD framework has a sync() callback in the genpd.domain structure, > >> but this callback isn't allowed to be used by the GENPD implementation. > >> The GENPD framework always overrides that callback for its own needs. > >> Hence GENPD doesn't allow to solve the bootstrapping > >> state-synchronization problem in a nice way. > >> > >> https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/base/power/d= omain.c#L2606 > >=20 > > That ->sync() callback isn't the callback you are looking for, it's a > > PM domain specific callback - and has other purposes. > >=20 > > To solve the problem you refer to, your genpd provider driver (a > > platform driver) should assign its ->sync_state() callback. The > > ->sync_state() callback will be invoked, when all consumer devices > > have been attached (and probed) to their corresponding provider. > >=20 > > You may have a look at drivers/cpuidle/cpuidle-psci-domain.c, to see > > an example of how this works. If there is anything unclear, just tell > > me and I will try to help. >=20 > Indeed, thank you for the clarification. This variant works well. >=20 > >> 3. Tegra doesn't have a dedicated hardware power-controller for the co= re > >> domain, instead there is only an external voltage regulator. Hence we > >> will need to create a phony device-tree node for the virtual power > >> domain, which is probably a wrong thing to do. > >=20 > > No, this is absolutely the correct thing to do. > >=20 > > This isn't a virtual power domain, it's a real power domain. You only > > happen to model the control of it as a regulator, as it fits nicely > > with that for *this* SoC. Don't get me wrong, that's fine as long as > > the supply is specified only in the power-domain provider node. > >=20 > > On another SoC, you might have a different FW interface for the power > > domain provider that doesn't fit well with the regulator. When that > > happens, all you need to do is to implement a new power domain > > provider and potentially re-define the power domain topology. More > > importantly, you don't need to re-invent yet another slew of device > > specific bindings - for each SoC. > >=20 > >> > >> =3D=3D=3D > >> > >> Perhaps it should be possible to create some hacks to work around > >> bullets 2 and 3 in order to achieve what we need for DVFS on Tegra, but > >> bullet 1 isn't solvable without changing how the GENPD core works. > >> > >> Altogether, the GENPD in its current form is a wrong abstraction for a > >> system-wide DVFS in a case where multiple devices share power domain a= nd > >> this domain is a voltage regulator. The regulator framework is the > >> correct abstraction in this case for today. > >=20 > > Well, I admit it's a bit complex. But it solves the problem in a > > nicely abstracted way that should work for everybody, at least in my > > opinion. >=20 > The OPP framework supports both voltage regulator and power domain, > hiding the implementation details from drivers. This means that OPP API > usage will be the same regardless of what approach (regulator or power > domain) is used for a particular SoC. >=20 > > Although, let's not exclude that there are pieces missing in genpd or > > the opp layer, as this DVFS feature is rather new - but then we should > > just extend/fix it. >=20 > Will be nice to have a per-device GENPD performance stats. >=20 > Thierry, could you please let me know what do you think about replacing > regulator with the power domain? Do you think it's a worthwhile change? >=20 > The difference in comparison to using voltage regulator directly is > minimal, basically the core-supply phandle is replaced is replaced with > a power-domain phandle in a device tree. These new power-domain handles would have to be added to devices that potentially already have a power-domain handle, right? Isn't that going to cause issues? I vaguely recall that we already have multiple power domains for the XUSB controller and we have to jump through extra hoops to make that work. > The only thing which makes me feel a bit uncomfortable is that there is > no real hardware node for the power domain node in a device-tree. Could we anchor the new power domain at the PMC for example? That would allow us to avoid the "virtual" node. On the other hand, if we were to use a regulator, we'd be adding a node for that, right? So isn't this effectively going to be the same node if we use a power domain? Both software constructs are using the same voltage regulator, so they should be able to be described by the same device tree node, shouldn't they? Thierry --BXVAT5kNtrzKuDFl Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAl+tnowACgkQ3SOs138+ s6GpRw//bBN1XxoinOezBRqhMIwuf0sQcMTik6JsV5TIwuTEmBN0jBur2rTXJ40P jKUu2leRszn9pkj693yplujBHu0Wreo6quD/n/AupD1JmaO8NfUlUzxpw/thwbCZ saonMKedQEVVGg4pYK90NenOJVMOb1cNuf907K4e+CjP1GZkxph9+ogJqFbUBoFh 0TNxTZu/eQm5jU+8KYYAXxMkbTvZIj/7G9KagAh0dpsxG9KkfQoU1ATU1z+10w2U uedMFgUOVlH8k2Y5AHgAYbGusCotyOlMjvhYqyaGIpjaBYJkFBwlpie+pkhRvQ/8 1QnLipuSmJlmjAzyReR+4zoyqES/e6WxIcIDhAVxhL+ptJLfd14m/B+Nk5PFbH2J wm+8pbGMYLK7n0YF2AaoR6Yzy5HkxlHyZcSDL4f62Va/XA56CZi7cons6UwU3Wn4 aXVDHbq+d4xSXMT1eFgnYVINdR43ebiFclmSci4XnAYsKnmoYMppu9Z4k3YsdPdI hgOLYLPhtVcw2eJaIYDlWkS7HaKy5JFWfUdbqeB2Roasy4WHsjsUma8anAUZ5LWu 9b5I47tMigGN37KAcFuTqQ1Erkag4sL++dt99j7m9a48JkTRRiE4QOXJbWR5tB7P p3pxybKsCYJuj+oX6QJYZPHYd4lzzz2z2skae4k/fSPXo9bsAS0= =8L5E -----END PGP SIGNATURE----- --BXVAT5kNtrzKuDFl-- --===============7908733464558060205== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel --===============7908733464558060205==--