| Message ID | 20230424110933.3908-1-quic_mkshah@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Use PSCI OS initiated mode for sc7280 | expand |
On Mon, 24 Apr 2023 at 13:09, Maulik Shah <quic_mkshah@quicinc.com> wrote: > > Changes in v4: > - Add missing s-o-b line and reviewed by in patch 1 > - Address ulf's comments for error handling in patch 2 > > Changes in v3: > - Add new change to provide helper function dt_idle_pd_remove_topology() > - Address ulf's comments for error handling > - Add reviewed by ulf for devicetree change > > Changes in v2: > - Add new change to Move enabling OSI mode after power domains creation > - Fix compatible string to domains-idle-states for cluster idle state. > - Update cover letter with some more details on OSI and PC mode > comparision > > The dependency [2] is now merged in trustedfirmware project. > > Stats comparision between OSI and PC mode are captured at [3] with > usecase > details, where during multiple CPUs online the residency in cluster idle > state is better with OSI and also inline with single CPU mode. In PC > mode > with multiple CPUs cluster idle state residency is dropping compare to > single CPU mode. > > Recording of this meeting is also available at [4]. > > This change adds power-domains for cpuidle states to use PSCI OS > initiated mode for sc7280. > > This change depends on external project changes [1] & [2] which are > under review/discussion to add PSCI os-initiated support in Arm Trusted > Firmware. > > I can update here once the dependency are in and change is ready to > merge. > > [1] https://review.trustedfirmware.org/q/topic:psci-osi > [2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/19487 > [3] https://www.trustedfirmware.org/docs/PSCI-OS-initiated.pdf > [4] https://www.trustedfirmware.org/meetings/tf-a-technical-forum > > Maulik Shah (3): > cpuidle: dt_idle_genpd: Add helper function to remove genpd topology > cpuidle: psci: Move enabling OSI mode after power domains creation > arm64: dts: qcom: sc7280: Add power-domains for cpuidle states > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 ++++++++++++++++++++------- > drivers/cpuidle/cpuidle-psci-domain.c | 39 ++++------- > drivers/cpuidle/dt_idle_genpd.c | 24 +++++++ > drivers/cpuidle/dt_idle_genpd.h | 7 ++ > 4 files changed, 117 insertions(+), 51 deletions(-) > Looks like this series has not been queued up yet. Note that patch1 and patch2 are needed for stable kernels too. Moreover, patch3 (Qcom DTS change) is dependent on patch 1 and patch2. Therefore I suggest Bjorn to pick this up via the Qcom SoC tree. Bjorn, is that okay for you? Kind regards Uffe
On Wed, May 24, 2023 at 11:56:28AM +0200, Ulf Hansson wrote: > On Mon, 24 Apr 2023 at 13:09, Maulik Shah <quic_mkshah@quicinc.com> wrote: > > > > Changes in v4: > > - Add missing s-o-b line and reviewed by in patch 1 > > - Address ulf's comments for error handling in patch 2 > > > > Changes in v3: > > - Add new change to provide helper function dt_idle_pd_remove_topology() > > - Address ulf's comments for error handling > > - Add reviewed by ulf for devicetree change > > > > Changes in v2: > > - Add new change to Move enabling OSI mode after power domains creation > > - Fix compatible string to domains-idle-states for cluster idle state. > > - Update cover letter with some more details on OSI and PC mode > > comparision > > > > The dependency [2] is now merged in trustedfirmware project. > > > > Stats comparision between OSI and PC mode are captured at [3] with > > usecase > > details, where during multiple CPUs online the residency in cluster idle > > state is better with OSI and also inline with single CPU mode. In PC > > mode > > with multiple CPUs cluster idle state residency is dropping compare to > > single CPU mode. > > > > Recording of this meeting is also available at [4]. > > > > This change adds power-domains for cpuidle states to use PSCI OS > > initiated mode for sc7280. > > > > This change depends on external project changes [1] & [2] which are > > under review/discussion to add PSCI os-initiated support in Arm Trusted > > Firmware. > > > > I can update here once the dependency are in and change is ready to > > merge. > > > > [1] https://review.trustedfirmware.org/q/topic:psci-osi > > [2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/19487 > > [3] https://www.trustedfirmware.org/docs/PSCI-OS-initiated.pdf > > [4] https://www.trustedfirmware.org/meetings/tf-a-technical-forum > > > > Maulik Shah (3): > > cpuidle: dt_idle_genpd: Add helper function to remove genpd topology > > cpuidle: psci: Move enabling OSI mode after power domains creation > > arm64: dts: qcom: sc7280: Add power-domains for cpuidle states > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 ++++++++++++++++++++------- > > drivers/cpuidle/cpuidle-psci-domain.c | 39 ++++------- > > drivers/cpuidle/dt_idle_genpd.c | 24 +++++++ > > drivers/cpuidle/dt_idle_genpd.h | 7 ++ > > 4 files changed, 117 insertions(+), 51 deletions(-) > > > > Looks like this series has not been queued up yet. Note that patch1 > and patch2 are needed for stable kernels too. Moreover, patch3 (Qcom > DTS change) is dependent on patch 1 and patch2. > > Therefore I suggest Bjorn to pick this up via the Qcom SoC tree. > Bjorn, is that okay for you? > Sorry, this fell between the chairs after you pointed me to it... I can certainly pick the 3 patches through my tree, but are they fixing any current regressions, or is it just that we need the first two patches to land before the 3rd patch? I also presume the 3rd patch is only needed when paired with the new ATF? Regards, Bjorn
On Thu, 25 May 2023 at 04:41, Bjorn Andersson <andersson@kernel.org> wrote: > > On Wed, May 24, 2023 at 11:56:28AM +0200, Ulf Hansson wrote: > > On Mon, 24 Apr 2023 at 13:09, Maulik Shah <quic_mkshah@quicinc.com> wrote: > > > > > > Changes in v4: > > > - Add missing s-o-b line and reviewed by in patch 1 > > > - Address ulf's comments for error handling in patch 2 > > > > > > Changes in v3: > > > - Add new change to provide helper function dt_idle_pd_remove_topology() > > > - Address ulf's comments for error handling > > > - Add reviewed by ulf for devicetree change > > > > > > Changes in v2: > > > - Add new change to Move enabling OSI mode after power domains creation > > > - Fix compatible string to domains-idle-states for cluster idle state. > > > - Update cover letter with some more details on OSI and PC mode > > > comparision > > > > > > The dependency [2] is now merged in trustedfirmware project. > > > > > > Stats comparision between OSI and PC mode are captured at [3] with > > > usecase > > > details, where during multiple CPUs online the residency in cluster idle > > > state is better with OSI and also inline with single CPU mode. In PC > > > mode > > > with multiple CPUs cluster idle state residency is dropping compare to > > > single CPU mode. > > > > > > Recording of this meeting is also available at [4]. > > > > > > This change adds power-domains for cpuidle states to use PSCI OS > > > initiated mode for sc7280. > > > > > > This change depends on external project changes [1] & [2] which are > > > under review/discussion to add PSCI os-initiated support in Arm Trusted > > > Firmware. > > > > > > I can update here once the dependency are in and change is ready to > > > merge. > > > > > > [1] https://review.trustedfirmware.org/q/topic:psci-osi > > > [2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/19487 > > > [3] https://www.trustedfirmware.org/docs/PSCI-OS-initiated.pdf > > > [4] https://www.trustedfirmware.org/meetings/tf-a-technical-forum > > > > > > Maulik Shah (3): > > > cpuidle: dt_idle_genpd: Add helper function to remove genpd topology > > > cpuidle: psci: Move enabling OSI mode after power domains creation > > > arm64: dts: qcom: sc7280: Add power-domains for cpuidle states > > > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 ++++++++++++++++++++------- > > > drivers/cpuidle/cpuidle-psci-domain.c | 39 ++++------- > > > drivers/cpuidle/dt_idle_genpd.c | 24 +++++++ > > > drivers/cpuidle/dt_idle_genpd.h | 7 ++ > > > 4 files changed, 117 insertions(+), 51 deletions(-) > > > > > > > Looks like this series has not been queued up yet. Note that patch1 > > and patch2 are needed for stable kernels too. Moreover, patch3 (Qcom > > DTS change) is dependent on patch 1 and patch2. > > > > Therefore I suggest Bjorn to pick this up via the Qcom SoC tree. > > Bjorn, is that okay for you? > > > > Sorry, this fell between the chairs after you pointed me to it... > > I can certainly pick the 3 patches through my tree, but are they fixing > any current regressions, or is it just that we need the first two > patches to land before the 3rd patch? I am not aware of any current regressions. > > I also presume the 3rd patch is only needed when paired with the new > ATF? Patch3 is beneficial to use with a new TF-A, but works with an old TF-A too. Anyway, forget what I said about patch3 earlier, as that was just not the complete information. The problem is that we can't be using a new TF-A (supporting both PSCI OSI and PC mode) without patch1 and patch2, unless we are using patch3. Thus, I strongly suggest we tag patch1 and patch2 for stable kernels, to avoid any potential conflicts of TF-A versions that may be used. > > Regards, > Bjorn Did that make more sense? Br Uffe
On Mon, May 29, 2023 at 10:58:23AM +0200, Ulf Hansson wrote: > On Thu, 25 May 2023 at 04:41, Bjorn Andersson <andersson@kernel.org> wrote: > > > > On Wed, May 24, 2023 at 11:56:28AM +0200, Ulf Hansson wrote: > > > On Mon, 24 Apr 2023 at 13:09, Maulik Shah <quic_mkshah@quicinc.com> wrote: > > > > > > > > Changes in v4: > > > > - Add missing s-o-b line and reviewed by in patch 1 > > > > - Address ulf's comments for error handling in patch 2 > > > > > > > > Changes in v3: > > > > - Add new change to provide helper function dt_idle_pd_remove_topology() > > > > - Address ulf's comments for error handling > > > > - Add reviewed by ulf for devicetree change > > > > > > > > Changes in v2: > > > > - Add new change to Move enabling OSI mode after power domains creation > > > > - Fix compatible string to domains-idle-states for cluster idle state. > > > > - Update cover letter with some more details on OSI and PC mode > > > > comparision > > > > > > > > The dependency [2] is now merged in trustedfirmware project. > > > > > > > > Stats comparision between OSI and PC mode are captured at [3] with > > > > usecase > > > > details, where during multiple CPUs online the residency in cluster idle > > > > state is better with OSI and also inline with single CPU mode. In PC > > > > mode > > > > with multiple CPUs cluster idle state residency is dropping compare to > > > > single CPU mode. > > > > > > > > Recording of this meeting is also available at [4]. > > > > > > > > This change adds power-domains for cpuidle states to use PSCI OS > > > > initiated mode for sc7280. > > > > > > > > This change depends on external project changes [1] & [2] which are > > > > under review/discussion to add PSCI os-initiated support in Arm Trusted > > > > Firmware. > > > > > > > > I can update here once the dependency are in and change is ready to > > > > merge. > > > > > > > > [1] https://review.trustedfirmware.org/q/topic:psci-osi > > > > [2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/19487 > > > > [3] https://www.trustedfirmware.org/docs/PSCI-OS-initiated.pdf > > > > [4] https://www.trustedfirmware.org/meetings/tf-a-technical-forum > > > > > > > > Maulik Shah (3): > > > > cpuidle: dt_idle_genpd: Add helper function to remove genpd topology > > > > cpuidle: psci: Move enabling OSI mode after power domains creation > > > > arm64: dts: qcom: sc7280: Add power-domains for cpuidle states > > > > > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 ++++++++++++++++++++------- > > > > drivers/cpuidle/cpuidle-psci-domain.c | 39 ++++------- > > > > drivers/cpuidle/dt_idle_genpd.c | 24 +++++++ > > > > drivers/cpuidle/dt_idle_genpd.h | 7 ++ > > > > 4 files changed, 117 insertions(+), 51 deletions(-) > > > > > > > > > > Looks like this series has not been queued up yet. Note that patch1 > > > and patch2 are needed for stable kernels too. Moreover, patch3 (Qcom > > > DTS change) is dependent on patch 1 and patch2. > > > > > > Therefore I suggest Bjorn to pick this up via the Qcom SoC tree. > > > Bjorn, is that okay for you? > > > > > > > Sorry, this fell between the chairs after you pointed me to it... > > > > I can certainly pick the 3 patches through my tree, but are they fixing > > any current regressions, or is it just that we need the first two > > patches to land before the 3rd patch? > > I am not aware of any current regressions. > Okay, that confirms my understanding. So not -rc material. > > > > I also presume the 3rd patch is only needed when paired with the new > > ATF? > > Patch3 is beneficial to use with a new TF-A, but works with an old > TF-A too. Anyway, forget what I said about patch3 earlier, as that was > just not the complete information. > > The problem is that we can't be using a new TF-A (supporting both PSCI > OSI and PC mode) without patch1 and patch2, unless we are using > patch3. > > Thus, I strongly suggest we tag patch1 and patch2 for stable kernels, > to avoid any potential conflicts of TF-A versions that may be used. > So you're suggesting that I pick them for v6.5 and add a Cc: stable? An alternative would be that you take the cpuidle patches for v6.4-rc and I pick the dt for v6.5 - given that the cpuidle patches actually resolves a problem, while the dts just introduces "new functionality". > > > > Regards, > > Bjorn > > Did that make more sense? > Yes, it's getting there :) Thanks, Bjorn > Br > Uffe
+ Rafael On Mon, 29 May 2023 at 18:05, Bjorn Andersson <andersson@kernel.org> wrote: > > On Mon, May 29, 2023 at 10:58:23AM +0200, Ulf Hansson wrote: > > On Thu, 25 May 2023 at 04:41, Bjorn Andersson <andersson@kernel.org> wrote: > > > > > > On Wed, May 24, 2023 at 11:56:28AM +0200, Ulf Hansson wrote: > > > > On Mon, 24 Apr 2023 at 13:09, Maulik Shah <quic_mkshah@quicinc.com> wrote: > > > > > > > > > > Changes in v4: > > > > > - Add missing s-o-b line and reviewed by in patch 1 > > > > > - Address ulf's comments for error handling in patch 2 > > > > > > > > > > Changes in v3: > > > > > - Add new change to provide helper function dt_idle_pd_remove_topology() > > > > > - Address ulf's comments for error handling > > > > > - Add reviewed by ulf for devicetree change > > > > > > > > > > Changes in v2: > > > > > - Add new change to Move enabling OSI mode after power domains creation > > > > > - Fix compatible string to domains-idle-states for cluster idle state. > > > > > - Update cover letter with some more details on OSI and PC mode > > > > > comparision > > > > > > > > > > The dependency [2] is now merged in trustedfirmware project. > > > > > > > > > > Stats comparision between OSI and PC mode are captured at [3] with > > > > > usecase > > > > > details, where during multiple CPUs online the residency in cluster idle > > > > > state is better with OSI and also inline with single CPU mode. In PC > > > > > mode > > > > > with multiple CPUs cluster idle state residency is dropping compare to > > > > > single CPU mode. > > > > > > > > > > Recording of this meeting is also available at [4]. > > > > > > > > > > This change adds power-domains for cpuidle states to use PSCI OS > > > > > initiated mode for sc7280. > > > > > > > > > > This change depends on external project changes [1] & [2] which are > > > > > under review/discussion to add PSCI os-initiated support in Arm Trusted > > > > > Firmware. > > > > > > > > > > I can update here once the dependency are in and change is ready to > > > > > merge. > > > > > > > > > > [1] https://review.trustedfirmware.org/q/topic:psci-osi > > > > > [2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/19487 > > > > > [3] https://www.trustedfirmware.org/docs/PSCI-OS-initiated.pdf > > > > > [4] https://www.trustedfirmware.org/meetings/tf-a-technical-forum > > > > > > > > > > Maulik Shah (3): > > > > > cpuidle: dt_idle_genpd: Add helper function to remove genpd topology > > > > > cpuidle: psci: Move enabling OSI mode after power domains creation > > > > > arm64: dts: qcom: sc7280: Add power-domains for cpuidle states > > > > > > > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 ++++++++++++++++++++------- > > > > > drivers/cpuidle/cpuidle-psci-domain.c | 39 ++++------- > > > > > drivers/cpuidle/dt_idle_genpd.c | 24 +++++++ > > > > > drivers/cpuidle/dt_idle_genpd.h | 7 ++ > > > > > 4 files changed, 117 insertions(+), 51 deletions(-) > > > > > > > > > > > > > Looks like this series has not been queued up yet. Note that patch1 > > > > and patch2 are needed for stable kernels too. Moreover, patch3 (Qcom > > > > DTS change) is dependent on patch 1 and patch2. > > > > > > > > Therefore I suggest Bjorn to pick this up via the Qcom SoC tree. > > > > Bjorn, is that okay for you? > > > > > > > > > > Sorry, this fell between the chairs after you pointed me to it... > > > > > > I can certainly pick the 3 patches through my tree, but are they fixing > > > any current regressions, or is it just that we need the first two > > > patches to land before the 3rd patch? > > > > I am not aware of any current regressions. > > > > Okay, that confirms my understanding. So not -rc material. > > > > > > > I also presume the 3rd patch is only needed when paired with the new > > > ATF? > > > > Patch3 is beneficial to use with a new TF-A, but works with an old > > TF-A too. Anyway, forget what I said about patch3 earlier, as that was > > just not the complete information. > > > > The problem is that we can't be using a new TF-A (supporting both PSCI > > OSI and PC mode) without patch1 and patch2, unless we are using > > patch3. > > > > Thus, I strongly suggest we tag patch1 and patch2 for stable kernels, > > to avoid any potential conflicts of TF-A versions that may be used. > > > > So you're suggesting that I pick them for v6.5 and add a Cc: stable? > > An alternative would be that you take the cpuidle patches for v6.4-rc > and I pick the dt for v6.5 - given that the cpuidle patches actually > resolves a problem, while the dts just introduces "new functionality". Right, that's probably the best option. Although I don't have a tree to take these patches through, let's ask Rafael if he can help with this. Rafael, can you pick patch 1 and patch 2 from $subject series for v6.4-rc and tag them for stable? Then Bjorn can pick patch3 for v6.5. Kind regards Uffe
[...] > > > > > > > > > > Looks like this series has not been queued up yet. Note that patch1 > > > > > and patch2 are needed for stable kernels too. Moreover, patch3 (Qcom > > > > > DTS change) is dependent on patch 1 and patch2. > > > > > > > > > > Therefore I suggest Bjorn to pick this up via the Qcom SoC tree. > > > > > Bjorn, is that okay for you? > > > > > > > > > > > > > Sorry, this fell between the chairs after you pointed me to it... > > > > > > > > I can certainly pick the 3 patches through my tree, but are they fixing > > > > any current regressions, or is it just that we need the first two > > > > patches to land before the 3rd patch? > > > > > > I am not aware of any current regressions. > > > > > > > Okay, that confirms my understanding. So not -rc material. > > > > > > > > > > I also presume the 3rd patch is only needed when paired with the new > > > > ATF? > > > > > > Patch3 is beneficial to use with a new TF-A, but works with an old > > > TF-A too. Anyway, forget what I said about patch3 earlier, as that was > > > just not the complete information. > > > > > > The problem is that we can't be using a new TF-A (supporting both PSCI > > > OSI and PC mode) without patch1 and patch2, unless we are using > > > patch3. > > > > > > Thus, I strongly suggest we tag patch1 and patch2 for stable kernels, > > > to avoid any potential conflicts of TF-A versions that may be used. > > > > > > > So you're suggesting that I pick them for v6.5 and add a Cc: stable? > > > > An alternative would be that you take the cpuidle patches for v6.4-rc > > and I pick the dt for v6.5 - given that the cpuidle patches actually > > resolves a problem, while the dts just introduces "new functionality". > > Right, that's probably the best option. Although I don't have a tree > to take these patches through, let's ask Rafael if he can help with > this. > > Rafael, can you pick patch 1 and patch 2 from $subject series for > v6.4-rc and tag them for stable? Then Bjorn can pick patch3 for v6.5. Rafael, Bjorn, sorry for nagging about this series. Can you please help to pick it up? Kind regards Uffe