On Wed, 27 Jul 2022 at 14:14, Sudeep Holla <sudeep.holla@xxxxxxx> wrote: > > On Wed, Jul 27, 2022 at 12:09:27PM +0300, Dmitry Baryshkov wrote: > > Hi, > > > > Lately I have been working on improving the msm8996 platform support. > > Vendor kernel seems to support domain-like idle (see [1], [2]). > > However when I tried changing upstream msm8996.dtsi to use PSCI > > domains, I faced the firmware reporting NOT_SUPPORTED to an attempt to > > enable OSI (thus rendering PSCI domains useless, as they are now > > marked with ALWAYS_ON). > > > > That's not good to hear 🙁. > > > I noticed that vendor kernel makes a call to cpu_suspend() with > > power_state following the original format (described in PSCI spec > > 5.4.2.1). What would be the best way to support this? > > And why is this not possible with the existing code ? Not sure if I > understood it right, I am assuming you are mentioning that it is not > possible. It's not possible with the cpuidle-psci-domains.c. The driver marks all genpds as ALWAYS_ON, thus making sure that they are never suspended. > > - Allow DTS forcing the PSCI power domains even if OSI enablement fails? > > Meaning DTS flag for this ? If OSI enable fails, why would you want to > still proceed. It is non-compliant and must be fixed if the firmware > supports OSI and expects OSPM to use the same. I'm not sure at this moment. PSCI firmware reports that OSI mode is supported, but then when psci_pd_try_set_osi_mode() tries to switch into OSI mode, it gets NOT_SUPPORTED. Just for the sake of completeness, I added a print to the psci.c to dump the result of the psci_set_osi_mode(false). It also returns NOT_SUPPORTED! My logical assumption would be that the firmware reports support for OS_INITIATED, but then just fails to properly support SET_SUSPEND_MODE. I should probably try ignoring the error psci-domain.c and continue with binding power domains. What would be the best way to check that the domains setup works as expected? > > > - Add a separate cpuidle driver? > > I would avoid that. > > > - Just forget about it and use plain PSCI as we currently do? > > > > Worst case yes. My main worry is how many of the old SDM SoC has such a > behaviour and how much they wary from each other. The OSI mode was pushed > after lengthy discussions to support all these platforms and now we have > platforms needing separate idle driver ? I'm not sure. 32-bit SoCs use non-PSCI idle driver. MSM8916, sdm845 and later SoCs are using proper domains support. I tested the sdm660, it looks like it can work with the power domains. So this leaves only MSM8992/4/8 in question (at least from the platforms that are supported by the mainline). > > > Additional topic: for one of idle states the vendor kernel uses a > > proprietary call into the hypervisor ([3]). > > Again I would say it is not spec compliant. Yes, of course. Otherwise there would not be such a question. Judging from the vendor kernels, this call is limited to 8996 only. > > > Up to now we have ignored this, as 8996 seems to be the only platform using > > it. I suppose that adding it to cpuidle-psci.c would be frowned upon. > > Indeed. > > > Is this assumption correct? Would it add another point for adding a separate > > cpuidle driver? > > > > I am getting a sense that this would be cleaner approach but I would like > to understand how much of these non-compliance is carried to the other > relatively newer SoCs. I understand this is atleast 5-6+ years old. I don't > want this to set example to deviate from standard driver by adding new > drivers though they all are supposedly using PSCI(and are not fully compliant) At this moment I fear that it might be limited to msm8996 only. Maybe including 8992/4/8. -- With best wishes Dmitry
