| Message ID | 1583428023-19559-1-git-send-email-mkshah@codeaurora.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Invoke rpmh_flush for non OSI targets | expand |
Quoting Maulik Shah (2020-03-05 09:06:59) > Changes in v12: > - Kconfig change to remove COMPILE_TEST was dropped in v11, reinclude it. What is the difficulty in maintaining COMPILE_TEST support? If it's purely making a stub function for OSI support then I'm lost why we can't have another patch at the start of the series that makes the function return false when PSCI is disabled.
Hi, On Thu, Mar 5, 2020 at 9:07 AM Maulik Shah <mkshah@codeaurora.org> wrote: > > Changes in v12: > - Kconfig change to remove COMPILE_TEST was dropped in v11, reinclude it. > > Changes in v11: > - Address Doug's comments on change 2 and 3 > - Include change to invalidate TCSes before flush from [4] > > Changes in v10: > - Address Evan's comments to update commit message on change 2 > - Add Evan's Reviewed by on change 2 > - Remove comment from rpmh_flush() related to last CPU invoking it > - Rebase all changes on top of next-20200302 > > Changes in v9: > - Keep rpmh_flush() to invoke from within cache_lock > - Remove comments related to only last cpu invoking rpmh_flush() > > Changes in v8: > - Address Stephen's comments on changes 2 and 3 > - Add Reviewed by from Stephen on change 1 > > Changes in v7: > - Address Srinivas's comments to update commit text > - Add Reviewed by from Srinivas > > Changes in v6: > - Drop 1 & 2 changes from v5 as they already landed in maintainer tree > - Drop 3 & 4 changes from v5 as no user at present for power domain in rsc > - Rename subject to appropriate since power domain changes are dropped > - Rebase other changes on top of next-20200221 > > Changes in v5: > - Add Rob's Acked by on dt-bindings change > - Drop firmware psci change > - Update cpuidle stats in dtsi to follow PC mode > - Include change to update dirty flag when data is updated from [4] > - Add change to invoke rpmh_flush when caches are dirty > > Changes in v4: > - Add change to allow hierarchical topology in PC mode > - Drop hierarchical domain idle states converter from v3 > - Address Merge sc7180 dtsi change to add low power modes > > Changes in v3: > - Address Rob's comment on dt property value > - Address Stephen's comments on rpmh-rsc driver change > - Include sc7180 cpuidle low power mode changes from [1] > - Include hierarchical domain idle states converter change from [2] > > Changes in v2: > - Add Stephen's Reviewed-By to the first three patches > - Addressed Stephen's comments on fourth patch > - Include changes to connect rpmh domain to cpuidle and genpds > > Resource State Coordinator (RSC) is responsible for powering off/lowering > the requirements from CPU subsystem for the associated hardware like buses, > clocks, and regulators when all CPUs and cluster is powered down. > > RSC power domain uses last-man activities provided by genpd framework based > on Ulf Hansoon's patch series[3], when the cluster of CPUs enter deepest > idle states. As a part of domain poweroff, RSC can lower resource state > requirements by flushing the cached sleep and wake state votes for various > resources. > > [1] https://patchwork.kernel.org/patch/11218965 > [2] https://patchwork.kernel.org/patch/10941671 > [3] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=222355 > [4] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=236503 > > Maulik Shah (3): > arm64: dts: qcom: sc7180: Add cpuidle low power states > soc: qcom: rpmh: Update dirty flag only when data changes > soc: qcom: rpmh: Invoke rpmh_flush for dirty caches > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 78 ++++++++++++++++++++++++++++++++++++ > drivers/soc/qcom/rpmh.c | 27 ++++++++++--- > 2 files changed, 100 insertions(+), 5 deletions(-) One overall optimization idea? Should we add two API calls: rpmh_start_operations() rpmh_end_operations() These optional API calls would be an optimization a client could use. When rpmh_start_operations() is called then RPMH code will inhibit flushing (but will still update the "dirty" flag). When rpmh_end_operations() is called then the RPMH will flush if the dirty flag is set. This is a pretty simple concept but should have a huge impact in the number of times we program hardware in non-OSI mode. Specifically, if we don't do that and we look at what happens in the interconnect code: 1. We "invalidate" the batch. We have to flush the non-batch commands back into the hardware. 2. We program the "wake only" commands. We have to flush the batch wake-only commands and also the non-batch commands back into the hardware. 3. We program the "sleep only" commands. We have to flush yet again with everything. -Doug
On 3/6/2020 3:52 AM, Doug Anderson wrote: > Hi, > > On Thu, Mar 5, 2020 at 9:07 AM Maulik Shah <mkshah@codeaurora.org> wrote: >> Changes in v12: >> - Kconfig change to remove COMPILE_TEST was dropped in v11, reinclude it. >> >> Changes in v11: >> - Address Doug's comments on change 2 and 3 >> - Include change to invalidate TCSes before flush from [4] >> >> Changes in v10: >> - Address Evan's comments to update commit message on change 2 >> - Add Evan's Reviewed by on change 2 >> - Remove comment from rpmh_flush() related to last CPU invoking it >> - Rebase all changes on top of next-20200302 >> >> Changes in v9: >> - Keep rpmh_flush() to invoke from within cache_lock >> - Remove comments related to only last cpu invoking rpmh_flush() >> >> Changes in v8: >> - Address Stephen's comments on changes 2 and 3 >> - Add Reviewed by from Stephen on change 1 >> >> Changes in v7: >> - Address Srinivas's comments to update commit text >> - Add Reviewed by from Srinivas >> >> Changes in v6: >> - Drop 1 & 2 changes from v5 as they already landed in maintainer tree >> - Drop 3 & 4 changes from v5 as no user at present for power domain in rsc >> - Rename subject to appropriate since power domain changes are dropped >> - Rebase other changes on top of next-20200221 >> >> Changes in v5: >> - Add Rob's Acked by on dt-bindings change >> - Drop firmware psci change >> - Update cpuidle stats in dtsi to follow PC mode >> - Include change to update dirty flag when data is updated from [4] >> - Add change to invoke rpmh_flush when caches are dirty >> >> Changes in v4: >> - Add change to allow hierarchical topology in PC mode >> - Drop hierarchical domain idle states converter from v3 >> - Address Merge sc7180 dtsi change to add low power modes >> >> Changes in v3: >> - Address Rob's comment on dt property value >> - Address Stephen's comments on rpmh-rsc driver change >> - Include sc7180 cpuidle low power mode changes from [1] >> - Include hierarchical domain idle states converter change from [2] >> >> Changes in v2: >> - Add Stephen's Reviewed-By to the first three patches >> - Addressed Stephen's comments on fourth patch >> - Include changes to connect rpmh domain to cpuidle and genpds >> >> Resource State Coordinator (RSC) is responsible for powering off/lowering >> the requirements from CPU subsystem for the associated hardware like buses, >> clocks, and regulators when all CPUs and cluster is powered down. >> >> RSC power domain uses last-man activities provided by genpd framework based >> on Ulf Hansoon's patch series[3], when the cluster of CPUs enter deepest >> idle states. As a part of domain poweroff, RSC can lower resource state >> requirements by flushing the cached sleep and wake state votes for various >> resources. >> >> [1] https://patchwork.kernel.org/patch/11218965 >> [2] https://patchwork.kernel.org/patch/10941671 >> [3] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=222355 >> [4] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=236503 >> >> Maulik Shah (3): >> arm64: dts: qcom: sc7180: Add cpuidle low power states >> soc: qcom: rpmh: Update dirty flag only when data changes >> soc: qcom: rpmh: Invoke rpmh_flush for dirty caches >> >> arch/arm64/boot/dts/qcom/sc7180.dtsi | 78 ++++++++++++++++++++++++++++++++++++ >> drivers/soc/qcom/rpmh.c | 27 ++++++++++--- >> 2 files changed, 100 insertions(+), 5 deletions(-) > One overall optimization idea? > > Should we add two API calls: > > rpmh_start_operations() > rpmh_end_operations() > > These optional API calls would be an optimization a client could use. > When rpmh_start_operations() is called then RPMH code will inhibit > flushing (but will still update the "dirty" flag). When > rpmh_end_operations() is called then the RPMH will flush if the dirty > flag is set. > > This is a pretty simple concept but should have a huge impact in the > number of times we program hardware in non-OSI mode. Specifically, if > we don't do that and we look at what happens in the interconnect code: > > 1. We "invalidate" the batch. We have to flush the non-batch commands > back into the hardware. > > 2. We program the "wake only" commands. We have to flush the batch > wake-only commands and also the non-batch commands back into the > hardware. > > 3. We program the "sleep only" commands. We have to flush yet again > with everything. > > > > -Doug Implemented in v13. Thanks, Maulik
On 3/5/2020 11:20 PM, Stephen Boyd wrote: > Quoting Maulik Shah (2020-03-05 09:06:59) >> Changes in v12: >> - Kconfig change to remove COMPILE_TEST was dropped in v11, reinclude it. > What is the difficulty in maintaining COMPILE_TEST support? If it's > purely making a stub function for OSI support then I'm lost why we can't > have another patch at the start of the series that makes the function > return false when PSCI is disabled. Addressed in v13 to maintain COMPILE_TEST for rpmh driver. Althought its done from rpmh-rsc driver itself rather thatn by not making a new stub function in PSCI. Thanks, Maulik