On 18.07.2023 14:07, Stephan Gerhold wrote: > On Mon, Jul 17, 2023 at 09:02:29PM -0700, Bjorn Andersson wrote: >> On Mon, Jul 17, 2023 at 06:26:35PM +0200, Stephan Gerhold wrote: >>> On Mon, Jul 17, 2023 at 05:19:11PM +0200, Konrad Dybcio wrote: >>>> The GCC block on SM6375 is powered by the VDD_CX rail. We need to ensure >>>> that it's enabled to prevent unwanted power collapse. >>>> >>>> Enable runtime PM to keep the power flowing only when necessary. >>>> >>> >>> Are you sure this is necessary? If VDD_CX was really possible to fully >>> "power collapse" then I would expect that you lose all register >>> settings. This is not something we want or can even handle for GCC. >>> You would need to restore all frequency settings, branch bits etc etc. >>> >> >> This differ between platforms, some allow us to completely power down CX >> while keeping registers state using MX, others require that CX stays in >> retention at least. >> >> So, CX isn't the only rail powering GCC. For the most part though, we >> have a relationship between frequencies votes for by clients and the >> corner of CX, and hence I think the current description is ok... >> > > This patch is just about sending enable/disable votes for the power > domains though, based on runtime PM which triggers when all the clocks > are disabled. > > It's unrelated to voting for CX corners required by certain clock > frequencies (we handle those in the OPP tables of the consumers). > And it's also unrelated to ensuring rentention of register contents > since we actually release all votes when the clocks are idle. > > So while adding runtime PM to all the clock drivers sounds nice, I'm > a bit confused what problem we're actually solving with this patch. :) In a very specific and unfortunate situation, there could be no other CX votes, and trying to access (perhaps at least parts of) GCC would result in a failure. Konrad