CPUidle already has CPUIDLE_FLAG_OFF to keep idle state disabled,
Lets extend the support to set this flag using device tree.
This allows to keep an idle state disabled and they can be enabled back using
sysfs after certain point using below command.
echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
This helps in cases where vendors want to keep cpuidle off until home
screen comes up. In the past attempt was done at [1] but it was not considered
safe option to export cpu_idle_poll_ctrl().
[1] https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/
Thanks,
Tushar Nimkar.
Tushar Nimkar (2):
dt-bindings: cpu: idle-states: Add idle-state-disabled property
cpuidle: dt: Add support to keep idle state disabled
Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
drivers/cpuidle/dt_idle_states.c | 3 +++
2 files changed, 11 insertions(+)
--
2.17.1
This change adds idle-state-disabled property using which certain or all
idle-states can be kept disabled during boot-up. Once boot-up is completed
same can be enabled using below command.
echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
Cc: [email protected]
Signed-off-by: Tushar Nimkar <[email protected]>
---
Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/cpu/idle-states.yaml b/Documentation/devicetree/bindings/cpu/idle-states.yaml
index b8cc826c9501..f999bc666bbd 100644
--- a/Documentation/devicetree/bindings/cpu/idle-states.yaml
+++ b/Documentation/devicetree/bindings/cpu/idle-states.yaml
@@ -358,6 +358,13 @@ patternProperties:
systems entry-latency-us + exit-latency-us will exceed
wakeup-latency-us by this duration.
+ idle-state-disabled:
+ description: |
+ If present the idle state stays disabled. It can be enabled back from
+ shell using below command.
+ echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
+ type: boolean
+
idle-state-name:
$ref: /schemas/types.yaml#/definitions/string
description:
@@ -548,6 +555,7 @@ examples:
CPU_SLEEP_0_0: cpu-sleep-0-0 {
compatible = "arm,idle-state";
local-timer-stop;
+ idle-state-disabled;
arm,psci-suspend-param = <0x0010000>;
entry-latency-us = <250>;
exit-latency-us = <500>;
--
2.17.1
This change adds idle-state-disabled property using which certain or all
idle-states can be kept disabled during boot-up. Once boot-up is completed
same can be enabled using below command.
echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
Cc: [email protected]
Signed-off-by: Tushar Nimkar <[email protected]>
---
Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/cpu/idle-states.yaml b/Documentation/devicetree/bindings/cpu/idle-states.yaml
index b8cc826c9501..f999bc666bbd 100644
--- a/Documentation/devicetree/bindings/cpu/idle-states.yaml
+++ b/Documentation/devicetree/bindings/cpu/idle-states.yaml
@@ -358,6 +358,13 @@ patternProperties:
systems entry-latency-us + exit-latency-us will exceed
wakeup-latency-us by this duration.
+ idle-state-disabled:
+ description: |
+ If present the idle state stays disabled. It can be enabled back from
+ shell using below command.
+ echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
+ type: boolean
+
idle-state-name:
$ref: /schemas/types.yaml#/definitions/string
description:
@@ -548,6 +555,7 @@ examples:
CPU_SLEEP_0_0: cpu-sleep-0-0 {
compatible = "arm,idle-state";
local-timer-stop;
+ idle-state-disabled;
arm,psci-suspend-param = <0x0010000>;
entry-latency-us = <250>;
exit-latency-us = <500>;
--
2.17.1
Hey Tushar,
On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote:
> This change adds idle-state-disabled property using which certain or all
> idle-states can be kept disabled during boot-up. Once boot-up is completed
> same can be enabled using below command.
>
> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
>
> Cc: [email protected]
Firstly, you should CC the dt-bindings maintainers like
get_maintainer.pl would tell you.
Secondly, there are two 1/2 patches in this series.
> Signed-off-by: Tushar Nimkar <[email protected]>
> ---
> Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/cpu/idle-states.yaml b/Documentation/devicetree/bindings/cpu/idle-states.yaml
> index b8cc826c9501..f999bc666bbd 100644
> --- a/Documentation/devicetree/bindings/cpu/idle-states.yaml
> +++ b/Documentation/devicetree/bindings/cpu/idle-states.yaml
> @@ -358,6 +358,13 @@ patternProperties:
> systems entry-latency-us + exit-latency-us will exceed
> wakeup-latency-us by this duration.
>
> + idle-state-disabled:
> + description: |
> + If present the idle state stays disabled.
> It can be enabled back from
> + shell using below command.
> + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
Thirdly, this is operating system specific behaviour, tied to Linux, and
has no place in a binding.
Cheers,
Conor.
> + type: boolean
> +
> idle-state-name:
> $ref: /schemas/types.yaml#/definitions/string
> description:
> @@ -548,6 +555,7 @@ examples:
> CPU_SLEEP_0_0: cpu-sleep-0-0 {
> compatible = "arm,idle-state";
> local-timer-stop;
> + idle-state-disabled;
> arm,psci-suspend-param = <0x0010000>;
> entry-latency-us = <250>;
> exit-latency-us = <500>;
> --
> 2.17.1
>
Mark the idle state as disabled using CPUIDLE_FLAG_OFF when
idle-state-disabled property is present.
Such idle states stays disabled and can be enabled using below command.
echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
Signed-off-by: Tushar Nimkar <[email protected]>
---
drivers/cpuidle/dt_idle_states.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/cpuidle/dt_idle_states.c b/drivers/cpuidle/dt_idle_states.c
index 12fec92a85fd..3d50181512a3 100644
--- a/drivers/cpuidle/dt_idle_states.c
+++ b/drivers/cpuidle/dt_idle_states.c
@@ -79,6 +79,9 @@ static int init_state_node(struct cpuidle_state *idle_state,
idle_state->flags = CPUIDLE_FLAG_RCU_IDLE;
if (of_property_read_bool(state_node, "local-timer-stop"))
idle_state->flags |= CPUIDLE_FLAG_TIMER_STOP;
+
+ if (of_property_read_bool(state_node, "idle-state-disabled"))
+ idle_state->flags |= CPUIDLE_FLAG_OFF;
/*
* TODO:
* replace with kstrdup and pointer assignment when name
--
2.17.1
Thanks Conor for reviewing.
On 6/8/2023 2:49 PM, Conor Dooley wrote:
> Hey Tushar,
>
> On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote:
>
> Firstly, you should CC the dt-bindings maintainers like
> get_maintainer.pl would tell you.
> Secondly, there are two 1/2 patches in this series.
>
1. Sure, I will include dt maintainer in next version.
2. Yes, one of the Patch 1/2 sent by mistake.
I will remove in next version.
>
> Thirdly, this is operating system specific behaviour, tied to Linux, and
> has no place in a binding.
>
3. I will remove [echo N >
/sys/devices/system/cpu/cpuX/cpuidle/stateX/disable] command from
bindings document.
> Cheers,
> Conor.
>
Thanks,
Tushar
On 08/06/2023 10:55, Tushar Nimkar wrote:
> This change adds idle-state-disabled property using which certain or all
Please do not use "This commit/patch", but imperative mood. See longer
explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> idle-states can be kept disabled during boot-up. Once boot-up is completed
> same can be enabled using below command.
>
I don't understand and you did not explain here, why this is useful and
why this is needed.
> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
>
> Cc: [email protected]
> Signed-off-by: Tushar Nimkar <[email protected]>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.
> ---
> Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/cpu/idle-states.yaml b/Documentation/devicetree/bindings/cpu/idle-states.yaml
> index b8cc826c9501..f999bc666bbd 100644
> --- a/Documentation/devicetree/bindings/cpu/idle-states.yaml
> +++ b/Documentation/devicetree/bindings/cpu/idle-states.yaml
> @@ -358,6 +358,13 @@ patternProperties:
> systems entry-latency-us + exit-latency-us will exceed
> wakeup-latency-us by this duration.
>
> + idle-state-disabled:
> + description: |
> + If present the idle state stays disabled. It can be enabled back from
> + shell using below command.
> + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
This is Linux specific command, so does not fit the bindings.
Best regards,
Krzysztof
On 08/06/2023 10:55, Tushar Nimkar wrote:
> This change adds idle-state-disabled property using which certain or all
> idle-states can be kept disabled during boot-up. Once boot-up is completed
> same can be enabled using below command.
>
> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disab
This is a duplicated patch. I already commented on other so to make it
clear: NAK for this one :)
Best regards,
Krzysztof
On Thu, 8 Jun 2023 at 10:56, Tushar Nimkar <[email protected]> wrote:
>
> CPUidle already has CPUIDLE_FLAG_OFF to keep idle state disabled,
> Lets extend the support to set this flag using device tree.
>
> This allows to keep an idle state disabled and they can be enabled back using
> sysfs after certain point using below command.
>
> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
>
> This helps in cases where vendors want to keep cpuidle off until home
> screen comes up. In the past attempt was done at [1] but it was not considered
> safe option to export cpu_idle_poll_ctrl().
>
> [1] https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/
Before considering this, I don't recall if I ever received a reply to
my earlier suggestion to the above thread. So, let me repeat my
question.
I am wondering if a similar improvement can be achieved by
modularizing the cpuidle-psci driver. If insmodding it after the
homescreen, we allow only ARM WFI during boot. This should achieve the
similar results as we get with $subject series, right?
[...]
>
> Thanks,
> Tushar Nimkar.
>
> Tushar Nimkar (2):
> dt-bindings: cpu: idle-states: Add idle-state-disabled property
> cpuidle: dt: Add support to keep idle state disabled
>
> Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
> drivers/cpuidle/dt_idle_states.c | 3 +++
> 2 files changed, 11 insertions(+)
>
Kind regards
Uffe
Thanks Krzysztof for reviewing,
On 6/9/2023 6:44 PM, Krzysztof Kozlowski wrote:
> On 08/06/2023 10:55, Tushar Nimkar wrote:
>> This change adds idle-state-disabled property using which certain or all
>
> Please do not use "This commit/patch", but imperative mood. See longer
> explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>
Sure, will update in next version.
>> idle-states can be kept disabled during boot-up. Once boot-up is completed
>> same can be enabled using below command.
>>
>
> I don't understand and you did not explain here, why this is useful and
> why this is needed.
>
I will update commit text to why this is useful in new version.
>> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
>
>
>>
>> Cc: [email protected]
>> Signed-off-by: Tushar Nimkar <[email protected]>
>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
Yes, In new version will take care.
>> ---
>> + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
>
> This is Linux specific command, so does not fit the bindings.
Will remove in new version.
>
> Best regards,
> Krzysztof
>
Thanks,
Tushar
Thanks for response Ulf.
On 6/12/2023 3:56 PM, Ulf Hansson wrote:
> On Thu, 8 Jun 2023 at 10:56, Tushar Nimkar <[email protected]> wrote:
>>
>> CPUidle already has CPUIDLE_FLAG_OFF to keep idle state disabled,
>> Lets extend the support to set this flag using device tree.
>>
>> This allows to keep an idle state disabled and they can be enabled back using
>> sysfs after certain point using below command.
>>
>> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
>>
>> This helps in cases where vendors want to keep cpuidle off until home
>> screen comes up. In the past attempt was done at [1] but it was not considered
>> safe option to export cpu_idle_poll_ctrl().
>>
>> [1] https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/
>
> Before considering this, I don't recall if I ever received a reply to
> my earlier suggestion to the above thread. So, let me repeat my
> question.
>
> I am wondering if a similar improvement can be achieved by
> modularizing the cpuidle-psci driver. If insmodding it after the
> homescreen, we allow only ARM WFI during boot. This should achieve the
> similar results as we get with $subject series, right?
>
> [...]
>
This will not work for targets which does not have DLKM support.
>>
>> Thanks,
>> Tushar Nimkar.
>>
>> Tushar Nimkar (2):
>> dt-bindings: cpu: idle-states: Add idle-state-disabled property
>> cpuidle: dt: Add support to keep idle state disabled
>>
>> Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
>> drivers/cpuidle/dt_idle_states.c | 3 +++
>> 2 files changed, 11 insertions(+)
>>
>
> Kind regards
> Uffe
Thanks,
Tushar
On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote:
> This change adds idle-state-disabled property using which certain or all
> idle-states can be kept disabled during boot-up. Once boot-up is completed
> same can be enabled using below command.
>
> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
>
> Cc: [email protected]
> Signed-off-by: Tushar Nimkar <[email protected]>
> ---
> Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/cpu/idle-states.yaml b/Documentation/devicetree/bindings/cpu/idle-states.yaml
> index b8cc826c9501..f999bc666bbd 100644
> --- a/Documentation/devicetree/bindings/cpu/idle-states.yaml
> +++ b/Documentation/devicetree/bindings/cpu/idle-states.yaml
> @@ -358,6 +358,13 @@ patternProperties:
> systems entry-latency-us + exit-latency-us will exceed
> wakeup-latency-us by this duration.
>
> + idle-state-disabled:
> + description: |
> + If present the idle state stays disabled. It can be enabled back from
> + shell using below command.
> + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
> + type: boolean
> +
This is clearly a policy and not a hardware or firmware feature to expose
in the device tree. So NACK, why can't you load it modules if you don't want
idle states in the boot.
It is same as choosing any default governor or performance states, will you
add those next ? It is simply policy not a feature/property to be exposed
in the device tree.
--
Regards,
Sudeep
Thanks for review Sundeep,
On 6/15/2023 2:26 PM, Sudeep Holla wrote:
> On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote:
>> + idle-state-disabled:
>> + description: |
>> + If present the idle state stays disabled. It can be enabled back from
>> + shell using below command.
>> + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
>> + type: boolean
>> +
>
> This is clearly a policy and not a hardware or firmware feature to expose
> in the device tree. So NACK, why can't you load it modules if you don't want
> idle states in the boot.
>
Attempt of making cpuidle governors to modular was rejected in past [2]
[2]
https://lore.kernel.org/lkml/[email protected]/#t
> It is same as choosing any default governor or performance states, will you
> add those next ? It is simply policy not a feature/property to be exposed
> in the device tree.
>
> --
> Regards,
> Sudeep
Thanks,
Tushar
On Wed, 14 Jun 2023 at 08:43, Tushar Nimkar <[email protected]> wrote:
>
> Thanks for response Ulf.
>
> On 6/12/2023 3:56 PM, Ulf Hansson wrote:
> > On Thu, 8 Jun 2023 at 10:56, Tushar Nimkar <[email protected]> wrote:
> >>
> >> CPUidle already has CPUIDLE_FLAG_OFF to keep idle state disabled,
> >> Lets extend the support to set this flag using device tree.
> >>
> >> This allows to keep an idle state disabled and they can be enabled back using
> >> sysfs after certain point using below command.
> >>
> >> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
> >>
> >> This helps in cases where vendors want to keep cpuidle off until home
> >> screen comes up. In the past attempt was done at [1] but it was not considered
> >> safe option to export cpu_idle_poll_ctrl().
> >>
> >> [1] https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/
> >
> > Before considering this, I don't recall if I ever received a reply to
> > my earlier suggestion to the above thread. So, let me repeat my
> > question.
> >
> > I am wondering if a similar improvement can be achieved by
> > modularizing the cpuidle-psci driver. If insmodding it after the
> > homescreen, we allow only ARM WFI during boot. This should achieve the
> > similar results as we get with $subject series, right?
> >
> > [...]
> >
> This will not work for targets which does not have DLKM support.
Right. I am not saying it's the perfect solution, but it seems like it
could potentially solve the problem for many cases.
If you want some help to turn the cpuidle-psci driver into a loadable
module, just reach out, I am happy to help.
[...]
Kind regards
Uffe
On Fri, Jun 16, 2023 at 11:26:18AM +0530, Tushar Nimkar wrote:
>
> Thanks for review Sundeep,
>
> On 6/15/2023 2:26 PM, Sudeep Holla wrote:
> > On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote:
> > > + idle-state-disabled:
> > > + description: |
> > > + If present the idle state stays disabled. It can be enabled back from
> > > + shell using below command.
> > > + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
> > > + type: boolean
> > > +
> >
> > This is clearly a policy and not a hardware or firmware feature to expose
> > in the device tree. So NACK, why can't you load it modules if you don't want
> > idle states in the boot.
> >
> Attempt of making cpuidle governors to modular was rejected in past [2]
>
OK try command line approach to disable all states(you can't get partial
on/off in that case). I don't think the build config is of any use as we
end up enabling it which will affect other platforms.
> [2] https://lore.kernel.org/lkml/[email protected]/#t
>
> > It is same as choosing any default governor or performance states, will you
> > add those next ? It is simply policy not a feature/property to be exposed
> > in the device tree.
> >
The above still holds, so still NACK. It is a policy and not a
hardware/firmware property or feature.
--
Regards,
Sudeep
Thanks again Sudeep,
On 6/16/2023 9:09 PM, Sudeep Holla wrote:
> On Fri, Jun 16, 2023 at 11:26:18AM +0530, Tushar Nimkar wrote:
>
> OK try command line approach to disable all states(you can't get partial
> on/off in that case). I don't think the build config is of any use as we
> end up enabling it which will affect other platforms.
>
Do you mean cpuidle.off=1 ?
It will disable idle states but this will not allow cpuidle_init() and
governors register to happen which mean no way to re-enable idle states.
Do you mean any other command line approach?
>
> The above still holds, so still NACK. It is a policy and not a
> hardware/firmware property or feature.
>
Yes, understood!
Thanks,
Tushar
Many thanks again,
On 6/16/2023 4:25 PM, Ulf Hansson wrote:
> On Wed, 14 Jun 2023 at 08:43, Tushar Nimkar <[email protected]> wrote:
>
> Right. I am not saying it's the perfect solution, but it seems like it
> could potentially solve the problem for many cases.
>
> If you want some help to turn the cpuidle-psci driver into a loadable
> module, just reach out, I am happy to help.
>
Thanks :)
Making cpuidle-psci as loadable does not hold good for target does not
support DLKM, in addition to it rpmh driver has dependency on
cpuidle-psci for pm-domain and rpmh probe will get defer, their are
driver which depends on rpmh probe like interconnect, clk etc. And
eventually dependent driver probe defers which are essential for Linux
boot-up.
Hope you got scenario for getting probe defer if we make cpuidle-psci as
loadable.
I have below options as well
[A]: Can we think of making "governor/param_governor"
module_param_string, string named governor only to load. In that way
need to remove check [3]. Let's say string passed as "teo" then it will
not load "menu" and loads "teo" once comes-up.
[B]: Can we think of making cpuidle.off as writable, let governors to
register (i.e remove check [4]) and allow cpuidle_init() to happen (i.e
remove check [5])
So in this way cpuidle.off=1, your idle state can not be selected
because [6] and later we can write off=0 to let same check [6] to fail.
[C]: Coming to this series approach...What is best way to utilize
already present Flag-CPUIDLE_FLAG_OFF ?
Since we can not add new DT property to take decision in driver as it's
not HW feature to be expose in device tree [7]. Can we introduce new
module_param() for making idle-state disable default and utilize
CPUIDLE_FLAG_OFF? maybe similar to [8]
happy to hear your thoughts!
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpuidle/governor.c?h=next-20230620#n93
[4]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpuidle/governor.c?h=next-20230620#n86
[5]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpuidle/cpuidle.c?h=next-20230620#n808
[6]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/kernel/sched/idle.c?h=next-20230620#n167
[7]
https://lore.kernel.org/lkml/[email protected]/T/#m5d6012b0dfcff700f48c0efbba629382f18ee33b
[8]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/idle/intel_idle.c?h=next-20230620#n2160
> [...]
>
> Kind regards
> Uffe
Thanks,
Tushar
On Wed, 21 Jun 2023 at 08:21, Tushar Nimkar <[email protected]> wrote:
>
> Many thanks again,
>
> On 6/16/2023 4:25 PM, Ulf Hansson wrote:
> > On Wed, 14 Jun 2023 at 08:43, Tushar Nimkar <[email protected]> wrote:
>
> >
> > Right. I am not saying it's the perfect solution, but it seems like it
> > could potentially solve the problem for many cases.
> >
> > If you want some help to turn the cpuidle-psci driver into a loadable
> > module, just reach out, I am happy to help.
> >
> Thanks :)
Np!
> Making cpuidle-psci as loadable does not hold good for target does not
> support DLKM, in addition to it rpmh driver has dependency on
> cpuidle-psci for pm-domain and rpmh probe will get defer, their are
> driver which depends on rpmh probe like interconnect, clk etc. And
> eventually dependent driver probe defers which are essential for Linux
> boot-up.
> Hope you got scenario for getting probe defer if we make cpuidle-psci as
> loadable.
I understand your concern, but you have got my idea wrong.
I was suggesting turning the cpuidle-psci driver into a loadable
module - not the cpuidle-psci-domain driver. The latter is the genpd
provider, which consumers like rpmh need to probe.
>
> I have below options as well
> [A]: Can we think of making "governor/param_governor"
> module_param_string, string named governor only to load. In that way
> need to remove check [3]. Let's say string passed as "teo" then it will
> not load "menu" and loads "teo" once comes-up.
>
> [B]: Can we think of making cpuidle.off as writable, let governors to
> register (i.e remove check [4]) and allow cpuidle_init() to happen (i.e
> remove check [5])
> So in this way cpuidle.off=1, your idle state can not be selected
> because [6] and later we can write off=0 to let same check [6] to fail.
>
> [C]: Coming to this series approach...What is best way to utilize
> already present Flag-CPUIDLE_FLAG_OFF ?
> Since we can not add new DT property to take decision in driver as it's
> not HW feature to be expose in device tree [7]. Can we introduce new
> module_param() for making idle-state disable default and utilize
> CPUIDLE_FLAG_OFF? maybe similar to [8]
>
> happy to hear your thoughts!
In general I am not in favor of module parameters, but maybe it's the
best option to solve this problem. We need Rafael's and Daniel's
opinion to conclude.
However, to me, I still think the easiest approach would be to turn
the cpuidle-psci driver into a loadable module. Let me hack on that
and post a few patches that you can test for this.
>
>
> [3]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpuidle/governor.c?h=next-20230620#n93
>
> [4]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpuidle/governor.c?h=next-20230620#n86
>
> [5]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpuidle/cpuidle.c?h=next-20230620#n808
>
> [6]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/kernel/sched/idle.c?h=next-20230620#n167
>
> [7]
> https://lore.kernel.org/lkml/[email protected]/T/#m5d6012b0dfcff700f48c0efbba629382f18ee33b
>
> [8]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/idle/intel_idle.c?h=next-20230620#n2160
> > [...]
> >
Kind regards
Uffe
Thank you for checking this Ulf,
On 6/22/2023 6:31 PM, Ulf Hansson wrote:
> On Wed, 21 Jun 2023 at 08:21, Tushar Nimkar <[email protected]> wrote:
>>
>> Many thanks again,
>>
>> On 6/16/2023 4:25 PM, Ulf Hansson wrote:
>>> On Wed, 14 Jun 2023 at 08:43, Tushar Nimkar <[email protected]> wrote:
>>
>>>
>>> Right. I am not saying it's the perfect solution, but it seems like it
>>> could potentially solve the problem for many cases.
>>>
>>> If you want some help to turn the cpuidle-psci driver into a loadable
>>> module, just reach out, I am happy to help.
>>>
>> Thanks :)
>
> Np!
>
>> Making cpuidle-psci as loadable does not hold good for target does not
>> support DLKM, in addition to it rpmh driver has dependency on
>> cpuidle-psci for pm-domain and rpmh probe will get defer, their are
>> driver which depends on rpmh probe like interconnect, clk etc. And
>> eventually dependent driver probe defers which are essential for Linux
>> boot-up.
>> Hope you got scenario for getting probe defer if we make cpuidle-psci as
>> loadable.
>
> I understand your concern, but you have got my idea wrong.
>
> I was suggesting turning the cpuidle-psci driver into a loadable
> module - not the cpuidle-psci-domain driver. The latter is the genpd
> provider, which consumers like rpmh need to probe.
>
hmm, I need to revisit.
>>
>> I have below options as well
>> [A]: Can we think of making "governor/param_governor"
>> module_param_string, string named governor only to load. In that way
>> need to remove check [3]. Let's say string passed as "teo" then it will
>> not load "menu" and loads "teo" once comes-up.
>>
>> [B]: Can we think of making cpuidle.off as writable, let governors to
>> register (i.e remove check [4]) and allow cpuidle_init() to happen (i.e
>> remove check [5])
>> So in this way cpuidle.off=1, your idle state can not be selected
>> because [6] and later we can write off=0 to let same check [6] to fail.
>>
>> [C]: Coming to this series approach...What is best way to utilize
>> already present Flag-CPUIDLE_FLAG_OFF ?
>> Since we can not add new DT property to take decision in driver as it's
>> not HW feature to be expose in device tree [7]. Can we introduce new
>> module_param() for making idle-state disable default and utilize
>> CPUIDLE_FLAG_OFF? maybe similar to [8]
>>
>> happy to hear your thoughts!
>
> In general I am not in favor of module parameters, but maybe it's the
> best option to solve this problem. We need Rafael's and Daniel's
> opinion to conclude.
>
Yea, I double thought before proposing module parameters options. But on
other hand to justify CPUIDLE_FLAG_OFF looks good.
Will wait for Rafael's and Daniel's opinion.
> However, to me, I still think the easiest approach would be to turn
> the cpuidle-psci driver into a loadable module. Let me hack on that
> and post a few patches that you can test for this.
>
Sure,
>>
>>
>> [3]
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpuidle/governor.c?h=next-20230620#n93
>>
>> [4]
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpuidle/governor.c?h=next-20230620#n86
>>
>> [5]
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/cpuidle/cpuidle.c?h=next-20230620#n808
>>
>> [6]
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/kernel/sched/idle.c?h=next-20230620#n167
>>
>> [7]
>> https://lore.kernel.org/lkml/[email protected]/T/#m5d6012b0dfcff700f48c0efbba629382f18ee33b
>>
>> [8]
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/idle/intel_idle.c?h=next-20230620#n2160
>>> [...]
>>>
>
> Kind regards
> Uffe
Thanks,
Tushar