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=-10.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham 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 39A18C4363D for ; Wed, 7 Oct 2020 01:17:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C7F902080A for ; Wed, 7 Oct 2020 01:17:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="azsmjPD0" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726073AbgJGBRV (ORCPT ); Tue, 6 Oct 2020 21:17:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50798 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725996AbgJGBRU (ORCPT ); Tue, 6 Oct 2020 21:17:20 -0400 Received: from mail-vs1-xe42.google.com (mail-vs1-xe42.google.com [IPv6:2607:f8b0:4864:20::e42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B6EC8C061755 for ; Tue, 6 Oct 2020 18:17:20 -0700 (PDT) Received: by mail-vs1-xe42.google.com with SMTP id a16so272187vsp.12 for ; Tue, 06 Oct 2020 18:17:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=0yTeji9j9CJ8RmpTCa7gaMjTAdqxTBobL0tzN5/mK1k=; b=azsmjPD091LNc2UbH2401z0TMRKWug0UVY0ZxoU/33AhwYTmlS8RKUCXxDrGGYye8w LwW9iZXC9S8pAKH4F+/m3Ipa8qcea1bJ2Ha64dI+G2RBJCpAjb5Q/QzEKiiByexwaba/ 0Cnh0oAcRTF1vqMM8Jl+zCxCu4jU1vls8uyhQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0yTeji9j9CJ8RmpTCa7gaMjTAdqxTBobL0tzN5/mK1k=; b=FdWrqGPjAUwRak/iRs5AqcEZXkC54kKxSmd4WXhJwIYOxukpks3LJAf7cU+6ZNLmxe MdqQZ/fJAiOpPKEiJhI3xz38evQ7MGdCjEYa7dRepCAELbAwGoIXj7iApR93GMVBpPTd q6t2S9/knRUpENCZS3wUN0yOxidGGgucE7TikBoQ4XttA5JmDnC/ckuERyK8jLKONGUn 8XnsCB+kYGC4nGuQImRa9b1vhLmJaRbUvvPZySYiFVG1tZYH95Yv1SIDlhWnSTUPSFt2 JzHfWOHda2ytdOyPPqxFdUIQ5/Pm4q2i+qYulQcinAeYPXOekvlNAgRftOCxrHe5hjCl rMuw== X-Gm-Message-State: AOAM530QrRdrOuCXqad6VX+UugPItlVQggzXsuc36EIRLzU45xs1BVHU UGaNYP4PUrsrrYD3uHCBm5dR+NQFb4p/Xw== X-Google-Smtp-Source: ABdhPJz3ThVQ6T93lJscixyXcGSt/Z0sH3YqrtdZKcEPjNTV95iiCwu9Z9+NMNdnuk9JuCYnq7OxhQ== X-Received: by 2002:a67:d710:: with SMTP id p16mr386174vsj.28.1602033439507; Tue, 06 Oct 2020 18:17:19 -0700 (PDT) Received: from mail-ua1-f49.google.com (mail-ua1-f49.google.com. [209.85.222.49]) by smtp.gmail.com with ESMTPSA id q29sm100247vsl.0.2020.10.06.18.17.17 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 06 Oct 2020 18:17:19 -0700 (PDT) Received: by mail-ua1-f49.google.com with SMTP id d18so239818uae.0 for ; Tue, 06 Oct 2020 18:17:17 -0700 (PDT) X-Received: by 2002:ab0:6984:: with SMTP id t4mr361085uaq.0.1602033436993; Tue, 06 Oct 2020 18:17:16 -0700 (PDT) MIME-Version: 1.0 References: <20201002114426.31277-1-lukasz.luba@arm.com> <20201002114426.31277-4-lukasz.luba@arm.com> In-Reply-To: From: Doug Anderson Date: Tue, 6 Oct 2020 18:17:05 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 3/3] dt-bindings: thermal: update sustainable-power with abstract scale To: Rob Herring Cc: Lukasz Luba , LKML , Linux PM , Linux Doc Mailing List , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Amit Kucheria , Jonathan Corbet , Daniel Lezcano , Dietmar.Eggemann@arm.com, Quentin Perret , Matthias Kaehlcke , Rajendra Nayak , "Rafael J. Wysocki" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Hi, On Tue, Oct 6, 2020 at 3:24 PM Rob Herring wrote: > > On Fri, Oct 2, 2020 at 12:39 PM Doug Anderson wrote: > > > > Hi, > > > > On Fri, Oct 2, 2020 at 9:40 AM Lukasz Luba wrote: > > > > > > On 10/2/20 4:47 PM, Doug Anderson wrote: > > > > Hi, > > > > > > > > On Fri, Oct 2, 2020 at 8:13 AM Lukasz Luba wrote: > > > >> > > > >> Hi Doug, > > > >> > > > >> On 10/2/20 3:31 PM, Doug Anderson wrote: > > > >>> Hi, > > > >>> > > > >>> On Fri, Oct 2, 2020 at 4:45 AM Lukasz Luba wrote: > > > >>>> > > > >>>> Update the documentation for the binding 'sustainable-power' and allow > > > >>>> to provide values in an abstract scale. It is required when the cooling > > > >>>> devices use an abstract scale for their power values. > > > >>>> > > > >>>> Signed-off-by: Lukasz Luba > > > >>>> --- > > > >>>> .../devicetree/bindings/thermal/thermal-zones.yaml | 13 +++++++++---- > > > >>>> 1 file changed, 9 insertions(+), 4 deletions(-) > > > >>>> > > > >>>> diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml > > > >>>> index 3ec9cc87ec50..4d8f2e37d1e6 100644 > > > >>>> --- a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml > > > >>>> +++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml > > > >>>> @@ -99,10 +99,15 @@ patternProperties: > > > >>>> sustainable-power: > > > >>>> $ref: /schemas/types.yaml#/definitions/uint32 > > > >>>> description: > > > >>>> - An estimate of the sustainable power (in mW) that this thermal zone > > > >>>> - can dissipate at the desired control temperature. For reference, the > > > >>>> - sustainable power of a 4-inch phone is typically 2000mW, while on a > > > >>>> - 10-inch tablet is around 4500mW. > > > >>>> + An estimate of the sustainable power (in mW or in an abstract scale) > > > >>>> + that this thermal zone can dissipate at the desired control > > > >>>> + temperature. For reference, the sustainable power of a 4-inch phone > > > >>>> + is typically 2000mW, while on a 10-inch tablet is around 4500mW. > > > >>>> + > > > >>>> + It is possible to express the sustainable power in an abstract > > > >>>> + scale. This is the case when the related cooling devices use also > > > >>>> + abstract scale to express their power usage. The scale must be > > > >>>> + consistent. > > > >>> > > > >>> Two thoughts: > > > >>> > > > >>> 1. If we're going to allow "sustainable-power" to be in abstract > > > >>> scale, why not allow "dynamic-power-coefficient" to be in abstract > > > >>> scale too? I assume that the whole reason against that originally was > > > >>> the idea of device tree purity, but if we're allowing the abstract > > > >>> scale here then there seems no reason not to allow it for > > > >>> "dynamic-power-coefficient". > > > >> > > > >> With this binding it's a bit more tricky. > > > >> I also have to discuss a few things internally. This requirement of > > > >> uW/MHz/V^2 makes the code easier also for potential drivers > > > >> like GPU (which are going to register the devfreq cooling with EM). > > > >> > > > >> Let me think about it, but for now I would just update these bits. > > > >> These are required to proper IPA operation, the dyn.-pow.-coef. is a > > > >> nice to have and possible next step. > > > > > > > > I guess the problem is that Rajendra is currently planning to remove > > > > all the "dynamic-power-coefficient" values from device tree right now > > > > and move them to the source code because the numbers we currently have > > > > in the device tree _are_ in abstract scale and thus violate the > > > > bindings. Moving this to source code won't help us get to more real > > > > power numbers (since it'll still be abstract scale), it'll just be > > > > pure churn. If we're OK with the abstract scale in general then we > > > > should allow it everywhere and not add churn for no reason. > > > > > > IIUC he is still going to use the Energy Model, but with different > > > registration function. We have such a driver: scmi-cpufreq.c, which > > > uses em_dev_register_perf_domain(). He can still use EM, EAS, IPA > > > not violating anything. > > > > Right. He's going to take the exact same "abstract scale" numbers > > that he has today and take them out of device tree and put them in the > > cpufreq driver. Doing so magically makes it so that he's not > > violating anything since "abstract scale" is not currently allowed in > > device tree but is allowed in the cpufreq driver. I'm not saying that > > he's doing anything wrong, I'm just saying that it's pointless churn. > > If we're OK with "abstract scale" in one place in the device tree we > > should be OK with it everywhere in the device tree. Then Rajendra > > wouldn't need his patch at all and he could leave his numbers in the > > device tree. > > > > > > > The real problem that we want to address is with sustainable-power in > > > IPA. It is used in power budget calculation and if the devices operate > > > in abstract scale, then there is an issue. > > > There are two options to get that value: > > > 1. from DT, which can have optimized value, stored by OEM engineer > > > 2. from IPA estimation code, which just calculates it as a sum of > > > minimum OPP power for each cooling device. > > > > > > The 2nd option might not be the best for a platform, so vendor/OEM > > > engineer might want to provide a better value in DT -> 1st option. > > > This is currently against the binding description and I have to fix it. > > > > Right, things are already broken today because a SoC vendor could > > (without violating any rules) provide their SoC core > > "dynamic-power-coefficient" in "abstract scale" in code and there > > would be no way to for a board to (without violating DT bindings) > > specify a "sustainable-power". ...so, in that sense, your patch does > > provide a benefit even if we don't make any changes to the rules for > > "sustainable-power". All I'm saying is that if these new rules for > > allowing an abstract scale for "sustainable-power" in the device tree > > are OK that it should _also_ be OK to add new rules to allow an > > abstract scale for "dynamic-power-coefficient". > > Didn't we beat this one to death with "dynamic-power-coefficient"? We did? Where / when? I'm not sure I was involved, but right now both "sustainable-power" and "dynamic-power-coefficient" are still defined in the device tree to be in real units, not abstract scale. Are you saying that we beat it to death and decided that it needed to be in real units, or we beat it to death and decided that abstract scale was OK and we just didn't put it in the bindings? > That is the abstract scale because I don't think you can really ever > measure it That's debatable. it's not very hard to get reasonable measurements. Matthias provided a recipe earlier in the thread. See commit ac60c5e33df4 ("ARM: dts: rockchip: Add dynamic-power-coefficient for rk3288"). In that case he used a machine that could easily measure power on the CPU rail, but if you simply keep all other rails in the system constant (and/or run a long enough test), you can easily accomplish this by just querying the smart battery in systems. > and because vendors don't want to advertise their absolute > power. That is certainly true, though after a device has shipped it's not that hard to measure. > > > >>> 2. Is it worth adding some type of indication of what type of units > > > >>> "sustainable-power" is represented in? Maybe even a made up unit so > > > >>> that you could tell the difference between made up units in the same > > > >>> system? I'd envision something like: > > > >>> > > > >>> sustainable-power-units = "qualcomm,sc7180-bogoWatts" > > > >>> > > > >>> ...and on the dynamic-power-coefficient side, the same: > > > >>> > > > >>> dynamic-power-coefficient-units = "qualcomm,sc7180-bogoWatts" > > > >>> > > > >>> One could imagine someone even later (after devices are widely > > > >>> distributed) figuring out translations between these bogoWatts numbers > > > >>> and real Watts if someone could come up with a case where it matters. > > > >> > > > >> To figure this out we don't need a new binding. > > > >> I think a simple comment in the DT would be enough for this, even e.g.: > > > >> > > > >> sustainable-power = <100> /* bogoWatts */ > > > > > > > > There are some important differences: > > > > > > > > a) Your comment is gone when the device tree is compiled. If we > > > > actually add a string to the device tree then, in theory, we can add > > > > conversions in code (without touching the device tree) down the road. > > > > > > We don't need code and binding with a bogoscale. It is up to the > > > platform integrator to make sure the scale in consistent in all devices. > > > Comment in DT is good enough. > > > > One other nice thing about having the units is that the device tree is > > supposed to be more of a "pure" thing, less sullied about what's > > convenient and more about a real description of a thing. Presumably > > that's why "abstract scale" wasn't allowed originally? In any case, > > giving quantifiable units to the number somehow makes it feel less > > made up because it's possible to come up with a way to convert it back > > to real units. > > > > > > > > b) I believe there can be more than one abstract scale present in a > > > > single device tree, at least in theory. Adding a string allows you to > > > > know if you're comparing apples to apples or apples to organges. > > > > > > IMHO DT is not the place for such abstractions, but Rob might correct me > > > here. > > > > Yup, seems like we're blocked waiting for Rob to chime in unless > > someone else has the authority to make the call about how to deal with > > "abstract scale" numbers in the device tree. > > I don't really know nor completely follow the issues. I just get all > these PM related bindings piece by piece with everyone solving their > own single issue. It's death by 1000 cuts. So my default position is > NAK. All the missing pieces and deficiencies can build up until > there's a coherent picture (maybe?). I'm totally confused. NAK on what? NAK on Lukasz's patch? ...or Lukasz's patch is totally fine but NAK on also allowing abstract scale for 'dynamic-power-coefficient". Or NAK on adding units? NAK on something else? -Doug