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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EB09AC433F5 for ; Tue, 5 Oct 2021 13:11:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CCA0261354 for ; Tue, 5 Oct 2021 13:11:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234917AbhJENNR (ORCPT ); Tue, 5 Oct 2021 09:13:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54596 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234681AbhJENNE (ORCPT ); Tue, 5 Oct 2021 09:13:04 -0400 Received: from mail-lf1-x129.google.com (mail-lf1-x129.google.com [IPv6:2a00:1450:4864:20::129]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1C8A5C061753 for ; Tue, 5 Oct 2021 06:11:14 -0700 (PDT) Received: by mail-lf1-x129.google.com with SMTP id r19so3447990lfe.10 for ; Tue, 05 Oct 2021 06:11:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=jjr/5gs80cWEm3vhyk3aM1mKvl135EXltar2PLrRZ/k=; b=P2GJcTPd6Kk45HZ8Ezt3mOStexgciMfR/ERjqn7quus9WPj7c0pgJ31TVKDyKIyFGw Y+F7WGFBzXS/vp5jDxxMyzTz/hmBB5xPYGuEv6qEQ23KVdyCACYcfsWUkcNmRV/ofYHS vE4LfuNllox/XGETXWlqstK2Q0L/vr7oXCynROhlgtpi4vvXnVvj6wEyBllh0eoIaYn8 f40td9gch4cO2Jp1h51EkJPAQ6/xwpwREWWIr5YIzLYLVQSBM9my6mb9NpueANAdQfBZ CLGXLy2HZcaqGOasKeUtPNXPs47D7ubFHOmVm/lx2TuvCwySuEvv9iGf+AtaMpafWnSk y69w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=jjr/5gs80cWEm3vhyk3aM1mKvl135EXltar2PLrRZ/k=; b=VFes8N4WUNUbmJhDFy99W5++Qt3H/xtsLTwIcvEWM65ln0kBTkA3BgFFTn6e+OP1xN aq+voxX/HJeJMSroRyHtLIg+e4O7MuLkgaoMpatalJ6qiNrryRUySh7jwDIS7y2GV2dz G7bBbLxMVXEaZJnUzlnhGD/z6QaVkEpSMXBg+nHhlXAAxr+s/psis1KoFdv5v+P9Ca6s vaDUzjaiuoPvfiOfcC/4XJwljJpaHy4pEzwOrBs7Ia7ZdAUMqB85fTyPN7IkzOI8z3Fd 7UoHx9dEAJ/AvJYxSwRIEcTka6BRrdikU7si7TDEszvWOyrjc47nQ6G0+utJv9dUNLdM bKKA== X-Gm-Message-State: AOAM532eTIVLCS1DOT7Tvhz9G9ExzV3cCu+ZOjCb/7/h2fWMWgsQrCL8 131hHcI1wblHV4wbRHa9YFKiRS6Vw/FC8aOf1IBGfA== X-Google-Smtp-Source: ABdhPJzhN75cYX0QdxPZZrVkTo8LNm3eo8IzoivQ7mSAH2J9FquztKwBCIMLaRlaWOOdBUUox7ZGcX1wvMYGNrjgk1w= X-Received: by 2002:a05:651c:20b:: with SMTP id y11mr22911109ljn.463.1633439466354; Tue, 05 Oct 2021 06:11:06 -0700 (PDT) MIME-Version: 1.0 References: <20210926224058.1252-1-digetx@gmail.com> <20210926224058.1252-7-digetx@gmail.com> <24101cd6-d3f5-1e74-db39-145ecd30418b@gmail.com> In-Reply-To: <24101cd6-d3f5-1e74-db39-145ecd30418b@gmail.com> From: Ulf Hansson Date: Tue, 5 Oct 2021 15:10:30 +0200 Message-ID: Subject: Re: [PATCH v13 06/35] clk: tegra: Support runtime PM and power domain To: Dmitry Osipenko Cc: Thierry Reding , Jonathan Hunter , Viresh Kumar , Stephen Boyd , Peter De Schrijver , Mikko Perttunen , Peter Chen , Lee Jones , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , Nishanth Menon , Adrian Hunter , Michael Turquette , Linux Kernel Mailing List , linux-tegra , Linux PM , Linux USB List , linux-staging@lists.linux.dev, linux-pwm@vger.kernel.org, linux-mmc , dri-devel , DTML , linux-clk , Mark Brown , Vignesh Raghavendra , Richard Weinberger , Miquel Raynal , Lucas Stach , Stefan Agner , Mauro Carvalho Chehab , David Heidelberg Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 2 Oct 2021 at 22:44, Dmitry Osipenko wrote: > > 01.10.2021 15:32, Ulf Hansson =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > >> +static __maybe_unused int tegra_clock_pm_suspend(struct device *dev) > >> +{ > >> + struct tegra_clk_device *clk_dev =3D dev_get_drvdata(dev); > >> + > >> + /* > >> + * Power management of the clock is entangled with the Tegra P= MC > >> + * GENPD because PMC driver enables/disables clocks for toggli= ng > >> + * of the PD's on/off state. > >> + * > >> + * The PMC GENPD is resumed in NOIRQ phase, before RPM of the = clocks > >> + * becomes available, hence PMC can't use clocks at the early = resume > >> + * phase if RPM is involved. For example when 3d clock is enab= led, > >> + * it may enable the parent PLL clock that needs to be RPM-res= umed. > >> + * > >> + * Secondly, the PLL clocks may be enabled by the low level su= spend > >> + * code, so we need to assume that PLL is in enabled state dur= ing > >> + * suspend. > >> + * > >> + * We will keep PLLs and system clock resumed during suspend t= ime. > >> + * All PLLs on all SoCs are low power and system clock is alwa= ys-on, > >> + * so practically not much is changed here. > >> + */ > >> + > >> + return clk_prepare(clk_dev->hw->clk); > > I am trying to understand, more exactly, what you intend to achieve > > with the clk_prepare() here. It looks a bit weird, to me. Can you try > > to elaborate a bit more on the use case? > > The Tegra GENPD driver enable/disable clocks when domain is turned on. Okay. I noticed that in tegra_genpd_power_on(). And the same clocks are enabled/disabled also in tegra_genpd_power_off(), when powering off the PM domain. So I guess the problem kind of exists for tegra_genpd_power_off() too? > This can't be done during early system resume, when domains are getting > turned on by the drivers core, because when clock is enabled, it's > getting prepared (RPM-resumed) and this preparation fails because > performance state of the clock goes up and it doesn't work during the > early resume time since I2C, which applies the state to hardware, is > suspended and can't work at that early time. This sounds complicated and I still don't quite follow all of it, sorry. So, tegra_genpd_power_on() gets called from genpd_resume_noirq(), when the first device of the attached devices to genpd gets resumed. And vice versa for tegra_genpd_power_off() and genpd_suspend_noirq(). Are you saying that trying to enable/disable clocks from tegra_genpd_power_on|off() in these paths doesn't work, because it would also require the performance state to be changed, which would fail because the I2C bus/driver is suspended? > > Secondly, Tegra has arch-specific low level assembly which touches > clocks during last phase of system suspend and in the beginning of > resume. Hence, clocks should stay prepared during suspend just because > technically clock should be prepared before it can be enabled. So the low level code is gating and ungating the clock behind the back of the clock driver then? Why is that done like that, more exactly? > > > Is this rather about making sure that the clock's corresponding PM > > domain stays powered on during system suspend? In that case, I think > > there may be an alternative option.... > > > > This is not about domain staying powered on, this is about keeping the > performance state of the domain high during suspend. Right, so the PM domain managed in tegra_genpd_power_on|off() can still be powered on/off, as long as the clock remains ungated? Kind regards Uffe