Received: by 2002:a05:6a10:f3d0:0:0:0:0 with SMTP id a16csp301327pxv; Wed, 30 Jun 2021 21:28:31 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxHgbTsnvFwnsoXIKU8da5YkMVeRnFwiNrJZN4Gxq2FiBaD3W5SmPphkeWJ4kbMCaK5oAJh X-Received: by 2002:a17:907:9809:: with SMTP id ji9mr4164910ejc.235.1625113711149; Wed, 30 Jun 2021 21:28:31 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1625113711; cv=none; d=google.com; s=arc-20160816; b=nLxHh0H3sUvhBtKs6Of1tc/djN1wVCc6eXVMJLapFcQNYFlqnnx6V6iW3DhquhneHH pH4JjBzLG92rk+S7GyKRIXYmEsVTdkh1gMApbXp6cxbmmzXJtqBzUOiW+iEyrMNg8E1H Wo9GhzWAUDHNzKUACfHEC7I2qFyW/Nvg+V84GlsxsfiluuF0sjgZsmI0JihJhI1iHFjd LRxn5w+fdS/zUZkolaEMa+aUs17MUDhdDwoMBXxf4yQTRl3az64iVF4WVP0NhQWk591s lVLHktf9WXUQZpbOrxpqsZC4GBp4ZkCcjDztH8pca09VAg6iOq5hfz6Jl7XgmPkaG3u0 fxOQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=SjIkznd62qf0N2/dK9GJzXrfd0rBEDC5ueAUYi6JXT8=; b=A22+EV5rRuyPfbNpdR5CLm99U5LMwelYDP2Bh/fvfDWrWEJsJuP17ibuzw4u6pIj3d AVVedRiFZyqohZisjUZxuIbbcIaIP9QwlNN/jIE7rv6Md4yLDvAThMXvgz61Fle841cZ WG4Y0JCq1EJg7jlol0XgejkenefuKbTLlTkuNdIMpQQMLHVh5Ksj+sXR884CVuKp2+ct tVsXwsliEv36LVM6xt8+T2vGJ4j1UdankjukMHkfJa6eMtSbnZPv1onokzvWscT2T0kn Y15YNg+0oVUNmO4X7VkV/Mhq0ky0T4zNvdEtMAN2rsubb7qgczRF2r9QBMcMHsjm7fgH DWDQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=Q7zdFbK5; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id hd32si2280871ejc.260.2021.06.30.21.27.35; Wed, 30 Jun 2021 21:28:31 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=Q7zdFbK5; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229577AbhGAEZe (ORCPT + 99 others); Thu, 1 Jul 2021 00:25:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40638 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229664AbhGAEZc (ORCPT ); Thu, 1 Jul 2021 00:25:32 -0400 Received: from mail-oo1-xc2b.google.com (mail-oo1-xc2b.google.com [IPv6:2607:f8b0:4864:20::c2b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1FCC9C0617A8 for ; Wed, 30 Jun 2021 21:23:03 -0700 (PDT) Received: by mail-oo1-xc2b.google.com with SMTP id g3-20020a4ae8830000b029024c9afa2547so1236442ooe.6 for ; Wed, 30 Jun 2021 21:23:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=SjIkznd62qf0N2/dK9GJzXrfd0rBEDC5ueAUYi6JXT8=; b=Q7zdFbK5igGTcfVQOu4L33G/7Tz6XEb5PGkaU7fcUbR4Mzm9M/0JUIBoycrc/sLBS5 8zRGzxme9L1Tvc66bmXrwHecEpBlL726C5pC7gd05Qn+1Qa+icC63XUzAVMKGtD5YrNt WpX+6NvFfNOSx52dykN3gNuSbQr7ui0flDFJhQ+fQjGdnn44XitejCpzINUa2ZXOM/r/ 5xoBzIGBw+WG8liSkXOZPnNghoZAoDmVDjfIBNunRafIYA88O6wknebK1okQlha8nHw/ IVvzn1mscofD9wkgvaExPVmvTKr9XQwTBLOqQGHSbLqGlvLiQ6Hdp1gUabnXrk5GLVEa ZUDQ== 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; bh=SjIkznd62qf0N2/dK9GJzXrfd0rBEDC5ueAUYi6JXT8=; b=eYrspDdIEITMpb/NlOj+VvpBQ1kchdU9MN1B9HEcKJHuSfKuRO8qq115sogVHupo8i mGlsC4odmEus3VdpqQswJffaNgWzipCJ1yevYvjWPjUBpwExbdiaYo2KYnS1ouUxYSYW LuFFftHNAS8BuAQMjmSmkmrB3UvwzpIkYYfykxfPTSzizVaOoEboD5Oj9Gk571g2VK4q O/twM8xBVIsBoJh6aDLSbRN8YKDRH5OBpdeMKeckIBZ+kAP0PdVSr17wCmImY+YwD2WK gyIfF0wdaWS76GFMFj7ruNtfwVHDeQUFXvCWrUC7bmLAYFD0+rtFCFslaKsQ4Yz8py50 dLeA== X-Gm-Message-State: AOAM5309aYwU8XExLg76frmMGkUmLGrsGCTkKkS0wGevJK1fISpH8Nzd 1B1o1iLeQHGanhHGkMfotKRuhg== X-Received: by 2002:a4a:8901:: with SMTP id f1mr11392225ooi.66.1625113382138; Wed, 30 Jun 2021 21:23:02 -0700 (PDT) Received: from yoga (104-57-184-186.lightspeed.austtx.sbcglobal.net. [104.57.184.186]) by smtp.gmail.com with ESMTPSA id o25sm4993775ood.20.2021.06.30.21.23.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Jun 2021 21:23:01 -0700 (PDT) Date: Wed, 30 Jun 2021 23:22:59 -0500 From: Bjorn Andersson To: Dmitry Baryshkov , Stephen Boyd , rnayak@codeaurora.org Cc: Andy Gross , Rob Herring , Jonathan Marek , Taniya Das , Michael Turquette , "open list:DRM DRIVER FOR MSM ADRENO GPU" , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , "open list:COMMON CLK FRAMEWORK" , Bryan O'Donoghue , Mark Brown , Ulf Hansson , open list Subject: Re: [PATCH 3/6] clk: qcom: gdsc: enable optional power domain support Message-ID: References: <20210630133149.3204290-1-dmitry.baryshkov@linaro.org> <20210630133149.3204290-4-dmitry.baryshkov@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 30 Jun 15:29 CDT 2021, Dmitry Baryshkov wrote: > On Wed, 30 Jun 2021 at 20:11, Bjorn Andersson > wrote: > > > > On Wed 30 Jun 10:47 CDT 2021, Dmitry Baryshkov wrote: > > > > > Hi, > > > > > > On Wed, 30 Jun 2021 at 18:00, Bjorn Andersson > > > wrote: > > > > > > > > On Wed 30 Jun 08:31 CDT 2021, Dmitry Baryshkov wrote: > > > > > > > > > On sm8250 dispcc and videocc registers are powered up by the MMCX power > > > > > domain. Currently we used a regulator to enable this domain on demand, > > > > > however this has some consequences, as genpd code is not reentrant. > > > > > > > > > > Teach Qualcomm clock controller code about setting up power domains and > > > > > using them for gdsc control. > > > > > > > > > > Signed-off-by: Dmitry Baryshkov > > > > > > > > There's a proposal to add a generic binding for statically assigning a > > > > performance states here: > > > > > > > > https://lore.kernel.org/linux-arm-msm/1622095949-2014-1-git-send-email-rnayak@codeaurora.org/ > > I checked this thread. It looks like Rajendra will also switch to the > "required-opps" property. So if that series goes in first, we can drop > the call to set_performance_state. If this one goes in first, we can > drop the set_performance_state call after getting Rajendra's work in. > > > > > > > > > > > > > But that said, do you really need this? > > > > > > > > The requirement for driving MMCX to LOW_SVS on SM8250 (and NOM on > > > > SM8150/SC8180x) seems to only come from the fact that you push MDP_CLK > > > > to 460MHz in &mdss. > > > > > > > > But then in &mdss_mdp you do the same using an opp-table based on the > > > > actual MDP_CLK, which per its power-domains will scale MMCX accordingly. > > > > > > MDSS and DSI would bump up MMCX performance state requirements on > > > their own, depending on the frequency being selected. > > > > > > > Right, but as I copied things from the sm8250.dtsi to come up with > > sm8150/sc8180x.dtsi I concluded that as soon as the assigned-clockrate > > in &mdss kicks in I need the performance state to be at NOM. > > > > So keeping the assigned-clockrate in &mdss means that MMCX will never go > > below NOM. > > No, because once MDP is fully running, it will lower the clock frequency: > > # grep mdp_clk /sys/kernel/debug/clk/clk_summary > disp_cc_mdss_mdp_clk_src 1 1 0 > 150000000 0 0 50000 ? > disp_cc_mdss_mdp_clk 2 2 0 > 150000000 0 0 50000 Y > But won't that just lower the performance state requested by the &mdss_mdp, while the &mdss still votes for NOM - with the outcome being that we maintain NOM even if the clock goes down? > > > > > > So wouldn't it be sufficient to ensure that MDSS_GDSC is parented by > > > > MMCX and then use opp-tables associated with the devices that scales the > > > > clock and thereby actually carries the "required-opps". > > > > > > Actually no. I set the performance state in the qcom_cc_map, so that > > > further register access is possible. Initially I was doing this in the > > > qcom_cc_really_probe() and it was already too late. > > > Just to remind: this patchset is not about MDSS_GDSC being parented by > > > MMCX, it is about dispcc/videocc registers being gated with MMCX. > > > > > > > So you're saying that just enabling MMCX isn't enough to touch the > > dispcc/videocc registers? If that's the case it seems like MMCX's > > definition of "on" needs to be adjusted - because just specifying MMCX > > as the power-domain for dispcc/videocc and enabling pm_runtime should > > ensure that MMCX is enabled when the clock registers are accessed (I > > don't see anything like that for the GDSC part though). > > No, it is not enough. If I comment out the set_performance_state call, > the board reboots. > > However I can set the opps as low as RET and register access will work. > I'll run more experiments and if everything works as expected, I can > use retention or min_svs level in the next iteration. > Just note that downstream specifies low_svs as minimum voltage level > for MMCX regulator. > It doesn't make sense to me that a lone power_on on the power-domain wouldn't give us enough juice to poke the registers. But digging into the rpmhpd implementation answers the question, simply invoking rpmhpd_power_on() is a nop, unless rpmhpd_set_performance_state() has previously been called, because pd->corner is 0. So this explains why enable isn't sufficient. Compare this with the rpmpd implementation that will send an enable request to the RPM in this case. > > I thought our problem you had was that you need to set a > > performance_state in order to clock up some of the clocks - e.g. > > MDP_CLK. > > No, even register access needs proper perf state. > Per above finding you're right, enabling a rpmhpd power-domain doesn't do anything. And I don't find this intuitive or even in line with the expectations of the api... A quick test booting rb3 and rb5 seems to indicate that it's possible to initialize pd->corner to 1 (to ensure that enable at least gives us the lowest level). set_performance_state(0) will however then result in voting for "off", rather than the lowest enabled level. Rajendra, Stephen, is this really how rpmhpd is supposed to work?! Regards, Bjorn