Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b6c4b0e7-a6c2-61e8-0f02-cbc3b973a93f@quicinc.com>
Date:   Mon, 26 Jun 2023 12:14:59 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH 0/2] Add provision to keep idle state disabled
To:     Ulf Hansson <ulf.hansson@linaro.org>
CC:     "Rafael J . Wysocki" <rafael@kernel.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        <linux-pm@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <quic_lsrao@quicinc.com>,
        <quic_mkshah@quicinc.com>
References: <20230608085544.16211-1-quic_tnimkar@quicinc.com>
 <CAPDyKFqhVkMH42Vz0+a62j5kFh+R_CvGrcSU7hxoW__tjOhfLw@mail.gmail.com>
 <443b00b1-76b8-c31d-53d3-42e3592d26e8@quicinc.com>
 <CAPDyKFp5L454WmTPo2eYnBuZ=ZMKEtinLgYU09n=J=3DA1FSJQ@mail.gmail.com>
 <29ccc60e-9ef8-883f-9936-95e6ef842746@quicinc.com>
 <CAPDyKFqKw-6Qu7YoZw9LQDGvj1PFi3nCDkXcmCJTffxtX75naQ@mail.gmail.com>
Content-Language: en-US
From:   Tushar Nimkar <quic_tnimkar@quicinc.com>
In-Reply-To: <CAPDyKFqKw-6Qu7YoZw9LQDGvj1PFi3nCDkXcmCJTffxtX75naQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@quicinc.com> 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@quicinc.com> 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@quicinc.com/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