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.5 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,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 25BCCC4338F for ; Wed, 28 Jul 2021 14:02:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F182360C3F for ; Wed, 28 Jul 2021 14:01:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236444AbhG1OCA (ORCPT ); Wed, 28 Jul 2021 10:02:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60880 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235420AbhG1OB6 (ORCPT ); Wed, 28 Jul 2021 10:01:58 -0400 Received: from mail-lf1-x131.google.com (mail-lf1-x131.google.com [IPv6:2a00:1450:4864:20::131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5E2DBC0613C1 for ; Wed, 28 Jul 2021 07:01:55 -0700 (PDT) Received: by mail-lf1-x131.google.com with SMTP id z2so4214280lft.1 for ; Wed, 28 Jul 2021 07:01:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=HkzKATWQiJzwj9+f4qauTnrP7HOjchHXfR7sVxWkLd4=; b=Z2XfG/9ykSGrj/xt0ovI/QuVJGiIPo8ma/mlpZecQ5KGkrdwUl0o4wNcgsrB5M6CPy k+LRIVKdGv37hjQyssh7sOp4ReHsKazfJU8T2WDcfrvC/knSTILmluDY0QDU/Ev5vgAr o9pGwdEvesM0RkO4A5bCBuH1Zuq64s3JJJf+OUpGokUqIjL7s/mZw4+Td14Tb+Qes4ge AvfPzyB90XtBS5DYsTZHW+8RWQTx7tX1AYN3lldi1CDuFzT7cD5Ez6DMLV1WIfHu89iF N4+wbNvwWcIAOwcD7tR6wMUsNunVzqXRYy0Dwm3t12W9ILVQPFKbvd0V3qDg8bScDJW4 3cnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=HkzKATWQiJzwj9+f4qauTnrP7HOjchHXfR7sVxWkLd4=; b=PsEEkxGuO0Qa08+GF9a+PAP51K9oE9eJOH/V9CC/wWlHTAqyTLie3TmXDNFXsfa+km SzDT4mNYsAZoJcKzGWEJhypHbIVTuF7ueSldcPqCJ/Y+FVZb8LgLh+Y/MXyad895fQvJ EQs1FNiYdpHAXzFSDQ03ANTUV+RknvyZlPow65UUUAq2qyEaIqzRk4Qd16/BfXFJExhL 8rLvzvf4WRbaRdJyLzjYaiA9WYxWUmk+KJUZFFFo4vkynhQKV2zSse+by4NUWrtVGnbg y2fBV/8HOi8dXqrhX/5OU032ZJV2ts5ovSSfF1Lyq3sB4Gi7tEhV4yEeOJMAzMj7o/H2 PPEw== X-Gm-Message-State: AOAM531rypuqn9OU4csNWO5MteBk7OgJD5AXKsdG+uzewsfkbD26rT1+ ZhInU6WEzwkqTdZE00di6s6d4A== X-Google-Smtp-Source: ABdhPJyX6gIG42z6TXISGYD1AbaGbevxFM65kRY0gZHdDQyTHym/lcLS3jklXa5hLa0Zm/Hprzll9g== X-Received: by 2002:a05:6512:400c:: with SMTP id br12mr21749856lfb.268.1627480913485; Wed, 28 Jul 2021 07:01:53 -0700 (PDT) Received: from [192.168.1.211] ([37.153.55.125]) by smtp.gmail.com with ESMTPSA id l13sm514250ljj.43.2021.07.28.07.01.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 28 Jul 2021 07:01:53 -0700 (PDT) Subject: Re: [PATCH v4 2/2] arm64: dts: sc7180: Add required-opps for i2c To: Bjorn Andersson , Rajendra Nayak Cc: Stephen Boyd , ulf.hansson@linaro.org, viresh.kumar@linaro.org, linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, rojay@codeaurora.org, stephan@gerhold.net References: <12711a61-e16c-d2bc-6e04-ab94c7551abe@codeaurora.org> From: Dmitry Baryshkov Message-ID: Date: Wed, 28 Jul 2021 17:01:52 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 28/07/2021 06:46, Bjorn Andersson wrote: > On Tue 27 Jul 02:35 CDT 2021, Rajendra Nayak wrote: > >> >> On 7/25/2021 10:31 PM, Bjorn Andersson wrote: >>> On Mon 19 Jul 23:29 CDT 2021, Rajendra Nayak wrote: >>> >>>> >>>> >>>> On 7/20/2021 12:49 AM, Bjorn Andersson wrote: >>>>> On Mon 19 Jul 04:37 CDT 2021, Rajendra Nayak wrote: >>>>> >>>>>> >>>>>> >>>>>> On 7/17/2021 3:29 AM, Bjorn Andersson wrote: >>>>>>> On Fri 16 Jul 16:49 CDT 2021, Stephen Boyd wrote: >>>>>>> >>>>>>>> Quoting Bjorn Andersson (2021-07-16 13:52:12) >>>>>>>>> On Fri 16 Jul 15:21 CDT 2021, Stephen Boyd wrote: >>>>>>>>> >>>>>>>>>> Quoting Bjorn Andersson (2021-07-16 13:18:56) >>>>>>>>>>> On Fri 16 Jul 05:00 CDT 2021, Rajendra Nayak wrote: >>>>>>>>>>> >>>>>>>>>>>> qup-i2c devices on sc7180 are clocked with a fixed clock (19.2 MHz) >>>>>>>>>>>> Though qup-i2c does not support DVFS, it still needs to vote for a >>>>>>>>>>>> performance state on 'CX' to satisfy the 19.2 Mhz clock frequency >>>>>>>>>>>> requirement. >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Sounds good, but... >>>>>>>>>>> >>>>>>>>>>>> Use 'required-opps' to pass this information from >>>>>>>>>>>> device tree, and also add the power-domains property to specify >>>>>>>>>>>> the CX power-domain. >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> ..is the required-opps really needed with my rpmhpd patch in place? >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Yes? Because rpmhpd_opp_low_svs is not the lowest performance state for >>>>>>>>>> CX. >>>>>>>>> >>>>>>>>> On e.g. sm8250 the first available non-zero corner presented in cmd-db >>>>>>>>> is low_svs. >>>>>> >>>>>> what rail is this? the mmcx? Perhaps it does not support RET. >>>>>> cx usually supports both collapse state and RET. >>>>>> >>>>> >>>>> That was the one I was specifically looking at for the MDSS_GDSC->MMCX >>>>> issue, so it's likely I didn't look elsewhere. >>>>> >>>>>>>> >>>>>>>> Indeed. On sc7180 it's not the first non-zero corner. I suppose >>>>>>>> retention for CX isn't actually used when the SoC is awake so your >>>>>>>> rpmhpd patch is putting in a vote for something that doesn't do anything >>>>>>>> at runtime for CX? I imagine that rpmh only sets the aggregate corner to >>>>>>>> retention when the whole SoC is suspended/sleeping, otherwise things >>>>>>>> wouldn't go very well. Similarly, min_svs may be VDD minimization? If >>>>>>>> so, those first two states are basically states that shouldn't be used >>>>>>>> at runtime, almost like sleep states. >>>>>>>> >>>>>>> >>>>>>> But if that's the case, I don't think it's appropriate for the "enabled >>>>>>> state" of the domain to use any of those corners. >>>>>> >>>>>> I rechecked the downstream kernels where all this voting happens from within >>>>>> the clock drivers, and I do see votes to min_svs for some clocks, but Stephen is >>>>>> right that RET is not something that's voted on while in active state. >>>>>> >>>>>> But always going with something just above the ret level while active will also >>>>>> not work for all devices, for instance for i2c on 7180, it needs a cx vote of >>>>>> low svs while the rail (cx) does support something lower than that which is min svs. >>>>>> (why can't it just work with min svs?, I don't know, these values and recommendations >>>>>> come in from the voltage plans published by HW teams for every SoC and we just end up >>>>>> using them in SW, perhaps something to dig further and understand which I will try and >>>>>> do but these are the values in voltage plans and downstream kernels which work for now) >>>>>> >>>>> >>>>> So to some degree this invalidates my argumentation about the >>>>> enabled_corner in rpmhpd, given that "enabled" means a different corner >>>>> for each rail - not just the one with lowest non-zero value. >>>> >>>> Right, it might work in some cases but might not work for all. >>>> >>> >>> Which makes it way less desirable. >>> >>> The enable state for rpmhpd power domains doesn't meet my expectations >>> for how a power domain should behave, >> >> Right and that's perhaps because these are not the usual power-domains, >> which have one "on/active" state and one or more "off/inactive" states (off/ret/clock-stop) >> Rpmhpd has multiple "on/active" states, and whats "on/active" for one consumer >> might not be "on/active" for another, so this information is hard to be managed >> at a generic level and these requests in some way or the other need to come >> in explicitly from the resp. consumers. >> > > I think it's fine if we just acknowledge that this is how the rpmhpd > domains works. > > But I am worried about how we're going to handle the case where the > consumer is indirectly referencing one of these power-domains using a > subdomain (gdsc). With the proper subdomain relationship in place and with Ulf's patches, this seems to be handled correctly. gdsc sets proper level for the parent power domain, which gets voted and unvoted by the core pm code when gdsc's power domain is powered on or off. > And the open question is if a solution to that problem will solve this > problem as well, or if we need to have this and some mechanism to > describe the "on state" for the parent of a subdomain. -- With best wishes Dmitry