Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1432085 > unrolled thread

Re: [PATCH v6 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v6 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-06-27 16:40 +0200
    Re: [PATCH v6 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Sudeep Holla <sudeep.holla@arm.com> - 2016-06-27 17:10 +0200
      Re: [PATCH v6 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-06-27 17:10 +0200
        Re: [PATCH v6 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-06-27 17:20 +0200
        Re: [PATCH v6 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Sudeep Holla <sudeep.holla@arm.com> - 2016-06-27 17:20 +0200
    Re: [PATCH v6 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Sudeep Holla <sudeep.holla@arm.com> - 2016-06-27 17:10 +0200
      Re: [PATCH v6 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64 Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-06-27 17:10 +0200

#1432085 — Re: [PATCH v6 5/5] ACPI : enable ACPI_PROCESSOR_IDLE on ARM64

On 06/14/2016 04:48 PM, Sudeep Holla wrote:
> Now that ACPI processor idle driver supports LPI(Low Power Idle), lets
> enable ACPI_PROCESSOR_IDLE for ARM64 too.
>
> This patch just removes the IA64 and X86 dependency on ACPI_PROCESSOR_IDLE
>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---

Hi Sudeep,

now that ACPI processor supports ARM64 did you check the 
CPUIDLE_DRIVER_STATE_START trick in the code and its derivative ?

I deleted the patch 2/5 but there is a place where:

if (max_cstate=0)
	max_cstate=1;

Probably this is because the POLL state is inserted, so there is always 
an idle state. But for ARM, that is not the case.

Also, there are some places where the idle state index begins to 1. I 
think it should be 0 for ARM.

>   drivers/acpi/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 1358fb7d7a68..d74275c0f374 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -238,7 +238,7 @@ config ACPI_CPPC_LIB
>   config ACPI_PROCESSOR
>   	tristate "Processor"
>   	depends on X86 || IA64 || ARM64
> -	select ACPI_PROCESSOR_IDLE if X86 || IA64
> +	select ACPI_PROCESSOR_IDLE
>   	select ACPI_CPU_FREQ_PSS if X86 || IA64
>   	default y
>   	help
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[toc] | [next] | [standalone]

#1432106

On 27/06/16 16:05, Daniel Lezcano wrote:
> On 06/27/2016 05:03 PM, Sudeep Holla wrote:
>> Hi Daniel,
>>
>> On 27/06/16 15:33, Daniel Lezcano wrote:
>>> On 06/14/2016 04:48 PM, Sudeep Holla wrote:
>>>> Now that ACPI processor idle driver supports LPI(Low Power Idle), lets
>>>> enable ACPI_PROCESSOR_IDLE for ARM64 too.
>>>>
>>>> This patch just removes the IA64 and X86 dependency on
>>>> ACPI_PROCESSOR_IDLE
>>>>
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>> ---
>>>
>>> Hi Sudeep,
>>>
>>> now that ACPI processor supports ARM64 did you check the
>>> CPUIDLE_DRIVER_STATE_START trick in the code and its derivative ?
>>>
>>
>> No, that is used only for C-State and ARM64 doesn't support it.
>> Patch 1/5 puts all the C-State code under #ifdef so that it's not
>> compiled on ARM64.
>>
>>> I deleted the patch 2/5 but there is a place where:
>>>
>>
>> Sorry, I don't follow what you mean by that.
>
> I meant I just deleted from my mailbox the patch 2/5, so I can't do
> inline comment.
>

Ah ok, anyways LPI always starts from index 0. IIUC that was your main
concern.

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]

#1432107

On 06/27/2016 05:06 PM, Sudeep Holla wrote:
>
>
> On 27/06/16 16:05, Daniel Lezcano wrote:
>> On 06/27/2016 05:03 PM, Sudeep Holla wrote:
>>> Hi Daniel,
>>>
>>> On 27/06/16 15:33, Daniel Lezcano wrote:
>>>> On 06/14/2016 04:48 PM, Sudeep Holla wrote:
>>>>> Now that ACPI processor idle driver supports LPI(Low Power Idle), lets
>>>>> enable ACPI_PROCESSOR_IDLE for ARM64 too.
>>>>>
>>>>> This patch just removes the IA64 and X86 dependency on
>>>>> ACPI_PROCESSOR_IDLE
>>>>>
>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>>> ---
>>>>
>>>> Hi Sudeep,
>>>>
>>>> now that ACPI processor supports ARM64 did you check the
>>>> CPUIDLE_DRIVER_STATE_START trick in the code and its derivative ?
>>>>
>>>
>>> No, that is used only for C-State and ARM64 doesn't support it.
>>> Patch 1/5 puts all the C-State code under #ifdef so that it's not
>>> compiled on ARM64.
>>>
>>>> I deleted the patch 2/5 but there is a place where:
>>>>
>>>
>>> Sorry, I don't follow what you mean by that.
>>
>> I meant I just deleted from my mailbox the patch 2/5, so I can't do
>> inline comment.
>>
>
> Ah ok, anyways LPI always starts from index 0. IIUC that was your main
> concern.

Do you have a repo where I can see the code ?

[toc] | [prev] | [next] | [standalone]

#1432121

On 06/27/2016 05:11 PM, Sudeep Holla wrote:
>
>
> On 27/06/16 16:08, Daniel Lezcano wrote:
>> On 06/27/2016 05:06 PM, Sudeep Holla wrote:
>
> [...]
>
>>> Ah ok, anyways LPI always starts from index 0. IIUC that was your main
>>> concern.
>>
>> Do you have a repo where I can see the code ?
>>
>
> Yes, you can get it from [1]

Great. Thanks Sudeep !

> Regards,
> Sudeep
>
> [1] http://git.kernel.org/sudeep.holla/linux/h/for_review/arm64_lpi


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[toc] | [prev] | [next] | [standalone]

#1432122

On 27/06/16 16:08, Daniel Lezcano wrote:
> On 06/27/2016 05:06 PM, Sudeep Holla wrote:

[...]

>> Ah ok, anyways LPI always starts from index 0. IIUC that was your main
>> concern.
>
> Do you have a repo where I can see the code ?
>

Yes, you can get it from [1]

--
Regards,
Sudeep

[1] http://git.kernel.org/sudeep.holla/linux/h/for_review/arm64_lpi

[toc] | [prev] | [next] | [standalone]

#1432111

Hi Daniel,

On 27/06/16 15:33, Daniel Lezcano wrote:
> On 06/14/2016 04:48 PM, Sudeep Holla wrote:
>> Now that ACPI processor idle driver supports LPI(Low Power Idle), lets
>> enable ACPI_PROCESSOR_IDLE for ARM64 too.
>>
>> This patch just removes the IA64 and X86 dependency on
>> ACPI_PROCESSOR_IDLE
>>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>
> Hi Sudeep,
>
> now that ACPI processor supports ARM64 did you check the
> CPUIDLE_DRIVER_STATE_START trick in the code and its derivative ?
>

No, that is used only for C-State and ARM64 doesn't support it.
Patch 1/5 puts all the C-State code under #ifdef so that it's not
compiled on ARM64.

> I deleted the patch 2/5 but there is a place where:
>

Sorry, I don't follow what you mean by that.

> if (max_cstate=0)
>      max_cstate=1;
>
> Probably this is because the POLL state is inserted, so there is always
> an idle state. But for ARM, that is not the case.
>

Yes

> Also, there are some places where the idle state index begins to 1. I
> think it should be 0 for ARM.
>

Yes for LPI, it does start from 0.

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]

#1432112

On 06/27/2016 05:03 PM, Sudeep Holla wrote:
> Hi Daniel,
>
> On 27/06/16 15:33, Daniel Lezcano wrote:
>> On 06/14/2016 04:48 PM, Sudeep Holla wrote:
>>> Now that ACPI processor idle driver supports LPI(Low Power Idle), lets
>>> enable ACPI_PROCESSOR_IDLE for ARM64 too.
>>>
>>> This patch just removes the IA64 and X86 dependency on
>>> ACPI_PROCESSOR_IDLE
>>>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>
>> Hi Sudeep,
>>
>> now that ACPI processor supports ARM64 did you check the
>> CPUIDLE_DRIVER_STATE_START trick in the code and its derivative ?
>>
>
> No, that is used only for C-State and ARM64 doesn't support it.
> Patch 1/5 puts all the C-State code under #ifdef so that it's not
> compiled on ARM64.
>
>> I deleted the patch 2/5 but there is a place where:
>>
>
> Sorry, I don't follow what you mean by that.

I meant I just deleted from my mailbox the patch 2/5, so I can't do 
inline comment.

>> if (max_cstate=0)
>>      max_cstate=1;
>>
>> Probably this is because the POLL state is inserted, so there is always
>> an idle state. But for ARM, that is not the case.
>>
>
> Yes
>
>> Also, there are some places where the idle state index begins to 1. I
>> think it should be 0 for ARM.
>>
>
> Yes for LPI, it does start from 0.
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web