Re: [PATCH 0/2] Add provision to keep idle state disabled

[Tushar Nimkar]

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 <quic_tnimkar@xxxxxxxxxxx> wrote:
> > Many thanks again,
> >
> > On 6/16/2023 4:25 PM, Ulf Hansson wrote:
> > > On Wed, 14 Jun 2023 at 08:43, Tushar Nimkar <quic_tnimkar@xxxxxxxxxxx> 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/20230608085544.16211-1-quic_tnimkar@xxxxxxxxxxx/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