Message-ID: <oysexostrdzcyapwpf2ele22lje4limgdknz3xcjgbs5tpvr46@cxzefx6ep477>
Date: Thu, 8 Jan 2026 11:28:27 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>
Cc: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>,
        Bjorn Andersson <andersson@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, Taniya Das <quic_tdas@...cinc.com>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Bartosz Golaszewski <brgl@...nel.org>,
        Shazad Hussain <quic_shazhuss@...cinc.com>,
        Sibi Sankar <sibi.sankar@....qualcomm.com>,
        Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Melody Olvera <quic_molvera@...cinc.com>,
        Dmitry Baryshkov <lumag@...nel.org>,
        Taniya Das <taniya.das@....qualcomm.com>,
        Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
        Imran Shaik <quic_imrashai@...cinc.com>,
        Abel Vesa <abelvesa@...nel.org>, linux-arm-msm@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        Rajendra Nayak <quic_rjendra@...cinc.com>, stable@...r.kernel.org
Subject: Re: [PATCH 0/7] clk: qcom: gcc: Do not turn off PCIe GDSCs during
 gdsc_disable()

On Mon, Jan 05, 2026 at 10:47:29AM +0100, Stephan Gerhold wrote:
> On Mon, Jan 05, 2026 at 10:44:39AM +0530, Manivannan Sadhasivam wrote:
> > On Fri, Jan 02, 2026 at 02:57:56PM +0100, Konrad Dybcio wrote:
> > > On 1/2/26 2:19 PM, Krishna Chaitanya Chundru wrote:
> > > > On 1/2/2026 5:09 PM, Konrad Dybcio wrote:
> > > >> On 1/2/26 12:36 PM, Krishna Chaitanya Chundru wrote:
> > > >>> On 1/2/2026 5:04 PM, Konrad Dybcio wrote:
> > > >>>> On 1/2/26 10:43 AM, Krishna Chaitanya Chundru wrote:
> > > >>>>> With PWRSTS_OFF_ON, PCIe GDSCs are turned off during gdsc_disable(). This
> > > >>>>> can happen during scenarios such as system suspend and breaks the resume
> > > >>>>> of PCIe controllers from suspend.
> > > >>>> Isn't turning the GDSCs off what we want though? At least during system
> > > >>>> suspend?
> > > >>> If we are keeping link in D3cold it makes sense, but currently we are not keeping in D3cold
> > > >>> so we don't expect them to get off.
> > > >> Since we seem to be tackling that in parallel, it seems to make sense
> > > >> that adding a mechanism to let the PCIe driver select "on" vs "ret" vs
> > > >> "off" could be useful for us
> > > > At least I am not aware of such API where we can tell genpd not to turn off gdsc
> > > > at runtime if we are keeping the device in D3cold state.
> > > > But anyway the PCIe gdsc supports Retention, in that case adding this flag here makes
> > > > more sense as it represents HW.
> > > > sm8450,sm8650 also had similar problem which are fixed by mani[1].
> > > 
> > > Perhaps I should ask for a clarification - is retention superior to
> > > powering the GDSC off? Does it have any power costs?
> > > 
> > 
> > In terms of power saving it is not superior, but that's not the only factor we
> > should consider here. If we keep GDSCs PWRSTS_OFF_ON, then the devices (PCIe)
> > need to be be in D3Cold. Sure we can change that using the new genpd API
> > dev_pm_genpd_rpm_always_on() dynamically, but I would prefer to avoid doing
> > that.
> > 
> > In my POV, GDSCs default state should be retention, so that the GDSCs will stay
> > ON if the rentention is not entered in hw and enter retention otherwise. This
> > requires no extra modification in the genpd client drivers. One more benefit is,
> > the hw can enter low power state even when the device is not in D3Cold state
> > i.e., during s2idle (provided we unvote other resources).
> > 
> 
> What about PCIe instances that are completely unused? The boot firmware
> on X1E for example is notorious for powering on completely unused PCIe
> links and powering them down in some half-baked off state (the &pcie3
> instance, in particular). I'm not sure if the GDSC remains on, but if it
> does then the unused PD cleanup would also only put them in retention
> state. I can't think of a good reason to keep those on at all.
>

This is a good point. I didn't think of it.

> The implementation of PWRSTS_RET_ON essentially makes the PD power_off()
> callback a no-op. Everything in Linux (sysfs, debugfs, ...) will tell
> you that the power domain has been shut down, but at the end it will
> remain fully powered until you manage to reach a retention state for the
> parent power domain. Due to other consumers, that will likely happen
> only if you reach VDDmin or some equivalent SoC-wide low-power state,
> something barely any (or none?) of the platforms supported upstream is
> capable of today.
> 

Unfortunately, that's the current state of retention today. It is only a
firmware visible state. Ofc, the OS could query SMEM and figure out after
resume, but there is no way currently to translate that to individual power
domains.

> PWRSTS_RET_ON is actually pretty close to setting GENPD_FLAG_ALWAYS_ON,
> the only advantage of PWRSTS_RET_ON I can think of is that unused GDSCs
> remain off iff you are lucky enough that the boot firmware has not
> already turned them on.
> 
> IMHO, for GDSCs that support OFF state in the hardware, PWRSTS_RET_ON is
> a hack to workaround limitations in the consumer drivers. They should
> either save/restore registers and handle the power collapse or they
> should vote for the power domain to stay on. That way, sysfs/debugfs
> will show the real votes held by Linux and you won't be mislead when
> looking at those while trying to optimize power consumption.
>

Maybe we should just use dev_pm_genpd_rpm_always_on() in the client drivers if
they know for sure that the device context should be preserved and keep
PWRSTS_OFF_ON flag.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists