* [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-11 0:17 [PATCH 0/5] x86 CPPC usage Srinivas Pandruvada
@ 2016-08-11 0:17 ` Srinivas Pandruvada
2016-08-12 9:13 ` Alexey Klimov
2016-08-12 16:35 ` Hoan Tran
2016-08-11 0:17 ` [PATCH 2/5] acpi: cpcc: Add integer read support Srinivas Pandruvada
` (4 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-11 0:17 UTC (permalink / raw)
To: rjw, viresh.kumar
Cc: linux-acpi, linux-pm, ashwin.chaugule, Srinivas Pandruvada
Some newer x86 platforms have support for both _CPC and _PSS object. So
kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
defined.
Also for legacy systems with only _PSS, we shouldn't bail out if
acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/acpi/Kconfig | 1 -
drivers/acpi/processor_driver.c | 5 ++++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 445ce28..c6bb6aa 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -227,7 +227,6 @@ config ACPI_MCFG
config ACPI_CPPC_LIB
bool
depends on ACPI_PROCESSOR
- depends on !ACPI_CPU_FREQ_PSS
select MAILBOX
select PCC
help
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 0553aee..0e0b629 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -245,8 +245,11 @@ static int __acpi_processor_start(struct acpi_device *device)
return 0;
result = acpi_cppc_processor_probe(pr);
- if (result)
+ if (result) {
+#ifndef CONFIG_ACPI_CPU_FREQ_PSS
return -ENODEV;
+#endif
+ }
if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
acpi_processor_power_init(pr);
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-11 0:17 ` [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config Srinivas Pandruvada
@ 2016-08-12 9:13 ` Alexey Klimov
2016-08-12 12:34 ` Srinivas Pandruvada
2016-08-12 16:04 ` Prakash, Prashanth
2016-08-12 16:35 ` Hoan Tran
1 sibling, 2 replies; 18+ messages in thread
From: Alexey Klimov @ 2016-08-12 9:13 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: rjw, viresh.kumar, linux-acpi, linux-pm, ashwin.chaugule,
pprakash, sudeep.holla
(adding Sudeep and Prashanth in c/c)
On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada wrote:
> Some newer x86 platforms have support for both _CPC and _PSS object. So
> kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
> restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
> defined.
> Also for legacy systems with only _PSS, we shouldn't bail out if
> acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/acpi/Kconfig | 1 -
> drivers/acpi/processor_driver.c | 5 ++++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 445ce28..c6bb6aa 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -227,7 +227,6 @@ config ACPI_MCFG
> config ACPI_CPPC_LIB
> bool
> depends on ACPI_PROCESSOR
> - depends on !ACPI_CPU_FREQ_PSS
> select MAILBOX
> select PCC
> help
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 0553aee..0e0b629 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct acpi_device *device)
> return 0;
>
> result = acpi_cppc_processor_probe(pr);
> - if (result)
> + if (result) {
> +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> return -ENODEV;
> +#endif
> + }
>
> if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
> acpi_processor_power_init(pr);
If PSS is not defined and kernel fails to probe CPPC then why we should not
execute acpi_processor_power_init()?
Best regards,
Alexey
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-12 9:13 ` Alexey Klimov
@ 2016-08-12 12:34 ` Srinivas Pandruvada
2016-08-12 16:04 ` Prakash, Prashanth
1 sibling, 0 replies; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-12 12:34 UTC (permalink / raw)
To: Alexey Klimov
Cc: rjw, viresh.kumar, linux-acpi, linux-pm, ashwin.chaugule,
pprakash, sudeep.holla
On Fri, 2016-08-12 at 10:13 +0100, Alexey Klimov wrote:
> >
> (adding Sudeep and Prashanth in c/c)
>
> On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada wrote:
[...]
> > result = acpi_cppc_processor_probe(pr);
> > - if (result)
> > + if (result) {
> > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> > return -ENODEV;
> > +#endif
> > + }
> >
> > if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> > &acpi_idle_driver)
> > acpi_processor_power_init(pr);
> If PSS is not defined and kernel fails to probe CPPC then why we
> should not
> execute acpi_processor_power_init()?
Did I change the current behavior? Currently when
acpi_cppc_processor_probe() fails, then -ENODEV is returned.
Thanks,
Srinivas
>
> Best regards,
> Alexey
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-12 9:13 ` Alexey Klimov
2016-08-12 12:34 ` Srinivas Pandruvada
@ 2016-08-12 16:04 ` Prakash, Prashanth
2016-08-12 16:32 ` Hoan Tran
1 sibling, 1 reply; 18+ messages in thread
From: Prakash, Prashanth @ 2016-08-12 16:04 UTC (permalink / raw)
To: Alexey Klimov, Srinivas Pandruvada
Cc: rjw, viresh.kumar, linux-acpi, linux-pm, ashwin.chaugule, sudeep.holla
Hi Alexey,
On 8/12/2016 3:13 AM, Alexey Klimov wrote:
> (adding Sudeep and Prashanth in c/c)
>
> On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada wrote:
>> Some newer x86 platforms have support for both _CPC and _PSS object. So
>> kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
>> restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
>> defined.
>> Also for legacy systems with only _PSS, we shouldn't bail out if
>> acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.
>>
>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>> ---
>> drivers/acpi/Kconfig | 1 -
>> drivers/acpi/processor_driver.c | 5 ++++-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 445ce28..c6bb6aa 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -227,7 +227,6 @@ config ACPI_MCFG
>> config ACPI_CPPC_LIB
>> bool
>> depends on ACPI_PROCESSOR
>> - depends on !ACPI_CPU_FREQ_PSS
>> select MAILBOX
>> select PCC
>> help
>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>> index 0553aee..0e0b629 100644
>> --- a/drivers/acpi/processor_driver.c
>> +++ b/drivers/acpi/processor_driver.c
>> @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct acpi_device *device)
>> return 0;
>>
>> result = acpi_cppc_processor_probe(pr);
>> - if (result)
>> + if (result) {
>> +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
>> return -ENODEV;
>> +#endif
>> + }
>>
>> if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>> acpi_processor_power_init(pr);
> If PSS is not defined and kernel fails to probe CPPC then why we should not
> execute acpi_processor_power_init()?
Returning on cppc probe failure looks like a bug. We can just print
a warning and continue to acpi_processor_power_init().
Thanks,
Prashanth
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-12 16:04 ` Prakash, Prashanth
@ 2016-08-12 16:32 ` Hoan Tran
2016-08-12 16:53 ` Srinivas Pandruvada
0 siblings, 1 reply; 18+ messages in thread
From: Hoan Tran @ 2016-08-12 16:32 UTC (permalink / raw)
To: Prakash, Prashanth
Cc: Alexey Klimov, Srinivas Pandruvada, Rafael J. Wysocki,
viresh.kumar, linux acpi, linux-pm, Ashwin Chaugule,
Sudeep Holla
Hi,
On Fri, Aug 12, 2016 at 9:04 AM, Prakash, Prashanth
<pprakash@codeaurora.org> wrote:
> Hi Alexey,
>
> On 8/12/2016 3:13 AM, Alexey Klimov wrote:
>> (adding Sudeep and Prashanth in c/c)
>>
>> On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada wrote:
>>> Some newer x86 platforms have support for both _CPC and _PSS object. So
>>> kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
>>> restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
>>> defined.
>>> Also for legacy systems with only _PSS, we shouldn't bail out if
>>> acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.
>>>
>>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>>> ---
>>> drivers/acpi/Kconfig | 1 -
>>> drivers/acpi/processor_driver.c | 5 ++++-
>>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index 445ce28..c6bb6aa 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -227,7 +227,6 @@ config ACPI_MCFG
>>> config ACPI_CPPC_LIB
>>> bool
>>> depends on ACPI_PROCESSOR
>>> - depends on !ACPI_CPU_FREQ_PSS
>>> select MAILBOX
>>> select PCC
>>> help
>>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>>> index 0553aee..0e0b629 100644
>>> --- a/drivers/acpi/processor_driver.c
>>> +++ b/drivers/acpi/processor_driver.c
>>> @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct acpi_device *device)
>>> return 0;
>>>
>>> result = acpi_cppc_processor_probe(pr);
>>> - if (result)
>>> + if (result) {
>>> +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
>>> return -ENODEV;
>>> +#endif
>>> + }
>>>
>>> if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>>> acpi_processor_power_init(pr);
>> If PSS is not defined and kernel fails to probe CPPC then why we should not
>> execute acpi_processor_power_init()?
> Returning on cppc probe failure looks like a bug. We can just print
> a warning and continue to acpi_processor_power_init().
Yes, it is. We should continue. I saw an issue about that. If the CPPC
probe fails, CPUidle can NOT be registered.
Thanks
Hoan
>
> Thanks,
> Prashanth
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-12 16:32 ` Hoan Tran
@ 2016-08-12 16:53 ` Srinivas Pandruvada
0 siblings, 0 replies; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-12 16:53 UTC (permalink / raw)
To: Hoan Tran, Prakash, Prashanth
Cc: Alexey Klimov, Rafael J. Wysocki, viresh.kumar, linux acpi,
linux-pm, Ashwin Chaugule, Sudeep Holla
On Fri, 2016-08-12 at 09:32 -0700, Hoan Tran wrote:
> Hi,
>
> On Fri, Aug 12, 2016 at 9:04 AM, Prakash, Prashanth
> <pprakash@codeaurora.org> wrote:
> >
> > Hi Alexey,
> >
> > On 8/12/2016 3:13 AM, Alexey Klimov wrote:
> > >
> > > (adding Sudeep and Prashanth in c/c)
> > >
> > > On Wed, Aug 10, 2016 at 05:17:22PM -0700, Srinivas Pandruvada
> > > wrote:
> > > >
> > > > Some newer x86 platforms have support for both _CPC and _PSS
> > > > object. So
> > > > kernel config can have both ACPI_CPU_FREQ_PSS and
> > > > ACPI_CPPC_LIB. So remove
> > > > restriction for ACPI_CPPC_LIB to build only when
> > > > ACPI_CPU_FREQ_PSS is not
> > > > defined.
> > > > Also for legacy systems with only _PSS, we shouldn't bail out
> > > > if
> > > > acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also
> > > > defined.
> > > >
> > > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.i
> > > > ntel.com>
> > > > ---
> > > > drivers/acpi/Kconfig | 1 -
> > > > drivers/acpi/processor_driver.c | 5 ++++-
> > > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > > > index 445ce28..c6bb6aa 100644
> > > > --- a/drivers/acpi/Kconfig
> > > > +++ b/drivers/acpi/Kconfig
> > > > @@ -227,7 +227,6 @@ config ACPI_MCFG
> > > > config ACPI_CPPC_LIB
> > > > bool
> > > > depends on ACPI_PROCESSOR
> > > > - depends on !ACPI_CPU_FREQ_PSS
> > > > select MAILBOX
> > > > select PCC
> > > > help
> > > > diff --git a/drivers/acpi/processor_driver.c
> > > > b/drivers/acpi/processor_driver.c
> > > > index 0553aee..0e0b629 100644
> > > > --- a/drivers/acpi/processor_driver.c
> > > > +++ b/drivers/acpi/processor_driver.c
> > > > @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct
> > > > acpi_device *device)
> > > > return 0;
> > > >
> > > > result = acpi_cppc_processor_probe(pr);
> > > > - if (result)
> > > > + if (result) {
> > > > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> > > > return -ENODEV;
> > > > +#endif
> > > > + }
> > > >
> > > > if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> > > > &acpi_idle_driver)
> > > > acpi_processor_power_init(pr);
> > > If PSS is not defined and kernel fails to probe CPPC then why we
> > > should not
> > > execute acpi_processor_power_init()?
> > Returning on cppc probe failure looks like a bug. We can just print
> > a warning and continue to acpi_processor_power_init().
> Yes, it is. We should continue. I saw an issue about that. If the
> CPPC
> probe fails, CPUidle can NOT be registered.
I wanted to keep the existing functionality as is. But I can submit
another patch on top of it to ignore cppc probe failure.
Thanks,
Srinivas
> Thanks
> Hoan
>
> >
> >
> > Thanks,
> > Prashanth
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-11 0:17 ` [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config Srinivas Pandruvada
2016-08-12 9:13 ` Alexey Klimov
@ 2016-08-12 16:35 ` Hoan Tran
2016-08-12 16:52 ` Srinivas Pandruvada
1 sibling, 1 reply; 18+ messages in thread
From: Hoan Tran @ 2016-08-12 16:35 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: Rafael J. Wysocki, viresh.kumar, linux acpi, linux-pm, Ashwin Chaugule
Hi Srinivas,
On Wed, Aug 10, 2016 at 5:17 PM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> Some newer x86 platforms have support for both _CPC and _PSS object. So
> kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So remove
> restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS is not
> defined.
> Also for legacy systems with only _PSS, we shouldn't bail out if
> acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also defined.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/acpi/Kconfig | 1 -
> drivers/acpi/processor_driver.c | 5 ++++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 445ce28..c6bb6aa 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -227,7 +227,6 @@ config ACPI_MCFG
> config ACPI_CPPC_LIB
> bool
> depends on ACPI_PROCESSOR
> - depends on !ACPI_CPU_FREQ_PSS
>From ACPI 6.1 spec, if _CPC is present, its use supersedes the use of
PSS. So I think, config ACPI_CPU_FREQ_PSS should depend on
!ACPI_CPPC_LIB.
Thanks
Hoan
> select MAILBOX
> select PCC
> help
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 0553aee..0e0b629 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct acpi_device *device)
> return 0;
>
> result = acpi_cppc_processor_probe(pr);
> - if (result)
> + if (result) {
> +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> return -ENODEV;
> +#endif
> + }
>
> if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
> acpi_processor_power_init(pr);
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-12 16:35 ` Hoan Tran
@ 2016-08-12 16:52 ` Srinivas Pandruvada
2016-08-12 16:58 ` Hoan Tran
0 siblings, 1 reply; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-12 16:52 UTC (permalink / raw)
To: Hoan Tran
Cc: Rafael J. Wysocki, viresh.kumar, linux acpi, linux-pm, Ashwin Chaugule
On Fri, 2016-08-12 at 09:35 -0700, Hoan Tran wrote:
> Hi Srinivas,
>
> On Wed, Aug 10, 2016 at 5:17 PM, Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> >
> > Some newer x86 platforms have support for both _CPC and _PSS
> > object. So
> > kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So
> > remove
> > restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS
> > is not
> > defined.
> > Also for legacy systems with only _PSS, we shouldn't bail out if
> > acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also
> > defined.
> >
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel
> > .com>
> > ---
> > drivers/acpi/Kconfig | 1 -
> > drivers/acpi/processor_driver.c | 5 ++++-
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 445ce28..c6bb6aa 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -227,7 +227,6 @@ config ACPI_MCFG
> > config ACPI_CPPC_LIB
> > bool
> > depends on ACPI_PROCESSOR
> > - depends on !ACPI_CPU_FREQ_PSS
> From ACPI 6.1 spec, if _CPC is present, its use supersedes the use of
> PSS. So I think, config ACPI_CPU_FREQ_PSS should depend on
> !ACPI_CPPC_LIB.
>
Distro want to have a single binary kernel image, so they will turn on
all configs. So this is not a compile time decision.
On runtime if the ACPI contains both tables than _CPC should be used
(but that also if the kernel is capable of handling _CPC, as legacy
kernel will not).
Thanks,
Srinivas
> Thanks
> Hoan
>
> >
> > select MAILBOX
> > select PCC
> > help
> > diff --git a/drivers/acpi/processor_driver.c
> > b/drivers/acpi/processor_driver.c
> > index 0553aee..0e0b629 100644
> > --- a/drivers/acpi/processor_driver.c
> > +++ b/drivers/acpi/processor_driver.c
> > @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct
> > acpi_device *device)
> > return 0;
> >
> > result = acpi_cppc_processor_probe(pr);
> > - if (result)
> > + if (result) {
> > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> > return -ENODEV;
> > +#endif
> > + }
> >
> > if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> > &acpi_idle_driver)
> > acpi_processor_power_init(pr);
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-12 16:52 ` Srinivas Pandruvada
@ 2016-08-12 16:58 ` Hoan Tran
2016-08-12 17:16 ` Srinivas Pandruvada
0 siblings, 1 reply; 18+ messages in thread
From: Hoan Tran @ 2016-08-12 16:58 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: Rafael J. Wysocki, viresh.kumar, linux acpi, linux-pm, Ashwin Chaugule
Hi Srinivas,
On Fri, Aug 12, 2016 at 9:52 AM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> On Fri, 2016-08-12 at 09:35 -0700, Hoan Tran wrote:
>> Hi Srinivas,
>>
>> On Wed, Aug 10, 2016 at 5:17 PM, Srinivas Pandruvada
>> <srinivas.pandruvada@linux.intel.com> wrote:
>> >
>> > Some newer x86 platforms have support for both _CPC and _PSS
>> > object. So
>> > kernel config can have both ACPI_CPU_FREQ_PSS and ACPI_CPPC_LIB. So
>> > remove
>> > restriction for ACPI_CPPC_LIB to build only when ACPI_CPU_FREQ_PSS
>> > is not
>> > defined.
>> > Also for legacy systems with only _PSS, we shouldn't bail out if
>> > acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also
>> > defined.
>> >
>> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel
>> > .com>
>> > ---
>> > drivers/acpi/Kconfig | 1 -
>> > drivers/acpi/processor_driver.c | 5 ++++-
>> > 2 files changed, 4 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> > index 445ce28..c6bb6aa 100644
>> > --- a/drivers/acpi/Kconfig
>> > +++ b/drivers/acpi/Kconfig
>> > @@ -227,7 +227,6 @@ config ACPI_MCFG
>> > config ACPI_CPPC_LIB
>> > bool
>> > depends on ACPI_PROCESSOR
>> > - depends on !ACPI_CPU_FREQ_PSS
>> From ACPI 6.1 spec, if _CPC is present, its use supersedes the use of
>> PSS. So I think, config ACPI_CPU_FREQ_PSS should depend on
>> !ACPI_CPPC_LIB.
>>
> Distro want to have a single binary kernel image, so they will turn on
> all configs. So this is not a compile time decision.
> On runtime if the ACPI contains both tables than _CPC should be used
> (but that also if the kernel is capable of handling _CPC, as legacy
> kernel will not).
Agreed. If so, this "depends on" is not need, right ? And a runtime
code will be added to decide which is used instead ?
Thanks
Hoan
>
> Thanks,
> Srinivas
>
>> Thanks
>> Hoan
>>
>> >
>> > select MAILBOX
>> > select PCC
>> > help
>> > diff --git a/drivers/acpi/processor_driver.c
>> > b/drivers/acpi/processor_driver.c
>> > index 0553aee..0e0b629 100644
>> > --- a/drivers/acpi/processor_driver.c
>> > +++ b/drivers/acpi/processor_driver.c
>> > @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct
>> > acpi_device *device)
>> > return 0;
>> >
>> > result = acpi_cppc_processor_probe(pr);
>> > - if (result)
>> > + if (result) {
>> > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
>> > return -ENODEV;
>> > +#endif
>> > + }
>> >
>> > if (!cpuidle_get_driver() || cpuidle_get_driver() ==
>> > &acpi_idle_driver)
>> > acpi_processor_power_init(pr);
>> > --
>> > 2.7.4
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-
>> > acpi" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm"
>> in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
2016-08-12 16:58 ` Hoan Tran
@ 2016-08-12 17:16 ` Srinivas Pandruvada
0 siblings, 0 replies; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-12 17:16 UTC (permalink / raw)
To: Hoan Tran
Cc: Rafael J. Wysocki, viresh.kumar, linux acpi, linux-pm, Ashwin Chaugule
On Fri, 2016-08-12 at 09:58 -0700, Hoan Tran wrote:
> Hi Srinivas,
>
> On Fri, Aug 12, 2016 at 9:52 AM, Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> >
> > On Fri, 2016-08-12 at 09:35 -0700, Hoan Tran wrote:
> > >
> > > Hi Srinivas,
> > >
> > > On Wed, Aug 10, 2016 at 5:17 PM, Srinivas Pandruvada
> > > <srinivas.pandruvada@linux.intel.com> wrote:
> > > >
> > > >
> > > > Some newer x86 platforms have support for both _CPC and _PSS
> > > > object. So
> > > > kernel config can have both ACPI_CPU_FREQ_PSS and
> > > > ACPI_CPPC_LIB. So
> > > > remove
> > > > restriction for ACPI_CPPC_LIB to build only when
> > > > ACPI_CPU_FREQ_PSS
> > > > is not
> > > > defined.
> > > > Also for legacy systems with only _PSS, we shouldn't bail out
> > > > if
> > > > acpi_cppc_processor_probe() fails, if ACPI_CPU_FREQ_PSS is also
> > > > defined.
> > > >
> > > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.i
> > > > ntel
> > > > .com>
> > > > ---
> > > > drivers/acpi/Kconfig | 1 -
> > > > drivers/acpi/processor_driver.c | 5 ++++-
> > > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > > > index 445ce28..c6bb6aa 100644
> > > > --- a/drivers/acpi/Kconfig
> > > > +++ b/drivers/acpi/Kconfig
> > > > @@ -227,7 +227,6 @@ config ACPI_MCFG
> > > > config ACPI_CPPC_LIB
> > > > bool
> > > > depends on ACPI_PROCESSOR
> > > > - depends on !ACPI_CPU_FREQ_PSS
> > > From ACPI 6.1 spec, if _CPC is present, its use supersedes the
> > > use of
> > > PSS. So I think, config ACPI_CPU_FREQ_PSS should depend on
> > > !ACPI_CPPC_LIB.
> > >
> > Distro want to have a single binary kernel image, so they will turn
> > on
> > all configs. So this is not a compile time decision.
> > On runtime if the ACPI contains both tables than _CPC should be
> > used
> > (but that also if the kernel is capable of handling _CPC, as legacy
> > kernel will not).
> Agreed. If so, this "depends on" is not need, right ? And a runtime
> code will be added to decide which is used instead ?
Yes, _CPC will be used if present.
Thanks,
Srinivas
>
> Thanks
> Hoan
>
> >
> >
> > Thanks,
> > Srinivas
> >
> > >
> > > Thanks
> > > Hoan
> > >
> > > >
> > > >
> > > > select MAILBOX
> > > > select PCC
> > > > help
> > > > diff --git a/drivers/acpi/processor_driver.c
> > > > b/drivers/acpi/processor_driver.c
> > > > index 0553aee..0e0b629 100644
> > > > --- a/drivers/acpi/processor_driver.c
> > > > +++ b/drivers/acpi/processor_driver.c
> > > > @@ -245,8 +245,11 @@ static int __acpi_processor_start(struct
> > > > acpi_device *device)
> > > > return 0;
> > > >
> > > > result = acpi_cppc_processor_probe(pr);
> > > > - if (result)
> > > > + if (result) {
> > > > +#ifndef CONFIG_ACPI_CPU_FREQ_PSS
> > > > return -ENODEV;
> > > > +#endif
> > > > + }
> > > >
> > > > if (!cpuidle_get_driver() || cpuidle_get_driver() ==
> > > > &acpi_idle_driver)
> > > > acpi_processor_power_init(pr);
> > > > --
> > > > 2.7.4
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe
> > > > linux-
> > > > acpi" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.h
> > > > tml
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-
> > > pm"
> > > in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.htm
> > > l
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/5] acpi: cpcc: Add integer read support
2016-08-11 0:17 [PATCH 0/5] x86 CPPC usage Srinivas Pandruvada
2016-08-11 0:17 ` [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config Srinivas Pandruvada
@ 2016-08-11 0:17 ` Srinivas Pandruvada
2016-08-11 0:17 ` [PATCH 3/5] acpi: cppc: Add support for function fixed hardware address Srinivas Pandruvada
` (3 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-11 0:17 UTC (permalink / raw)
To: rjw, viresh.kumar
Cc: linux-acpi, linux-pm, ashwin.chaugule, Srinivas Pandruvada
The _CPC performance limits can also be simple integer not just buffer
with register information. Add support for integer read via cpc_read.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/acpi/cppc_acpi.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 2e98173..34209f5 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -651,11 +651,18 @@ EXPORT_SYMBOL_GPL(acpi_cppc_processor_exit);
* we can directly write to it.
*/
-static int cpc_read(struct cpc_reg *reg, u64 *val)
+static int cpc_read(struct cpc_register_resource *res, u64 *val)
{
+ struct cpc_reg *reg = &res->cpc_entry.reg;
int ret_val = 0;
*val = 0;
+
+ if (res->type == ACPI_TYPE_INTEGER) {
+ *val = res->cpc_entry.int_value;
+ return 0;
+ }
+
if (reg->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
void __iomem *vaddr = GET_PCC_VADDR(reg->address);
@@ -754,16 +761,16 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
}
}
- cpc_read(&highest_reg->cpc_entry.reg, &high);
+ cpc_read(highest_reg, &high);
perf_caps->highest_perf = high;
- cpc_read(&lowest_reg->cpc_entry.reg, &low);
+ cpc_read(lowest_reg, &low);
perf_caps->lowest_perf = low;
- cpc_read(&ref_perf->cpc_entry.reg, &ref);
+ cpc_read(ref_perf, &ref);
perf_caps->reference_perf = ref;
- cpc_read(&nom_perf->cpc_entry.reg, &nom);
+ cpc_read(nom_perf, &nom);
perf_caps->nominal_perf = nom;
if (!ref)
@@ -812,8 +819,8 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs)
}
}
- cpc_read(&delivered_reg->cpc_entry.reg, &delivered);
- cpc_read(&reference_reg->cpc_entry.reg, &reference);
+ cpc_read(delivered_reg, &delivered);
+ cpc_read(reference_reg, &reference);
if (!delivered || !reference) {
ret = -EFAULT;
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/5] acpi: cppc: Add support for function fixed hardware address
2016-08-11 0:17 [PATCH 0/5] x86 CPPC usage Srinivas Pandruvada
2016-08-11 0:17 ` [PATCH 1/5] acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config Srinivas Pandruvada
2016-08-11 0:17 ` [PATCH 2/5] acpi: cpcc: Add integer read support Srinivas Pandruvada
@ 2016-08-11 0:17 ` Srinivas Pandruvada
2016-08-11 0:17 ` [PATCH 4/5] acpi: cppc: Add prefix cppc to cpudata structure name Srinivas Pandruvada
` (2 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-11 0:17 UTC (permalink / raw)
To: rjw, viresh.kumar
Cc: linux-acpi, linux-pm, ashwin.chaugule, Srinivas Pandruvada
The CPPC registers can also be accessed via function fixed hardware
addresses in X86. Add support by modifying cpc_read and cpc_write
to be able to read/write MSRs on x86 platform. Also with this change,
acpi_cppc_processor_probe doesn't bail out if space id is not equal to
PCC or memory address space.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/acpi/cppc_acpi.c | 77 +++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 66 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 34209f5..939fb5c 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -42,6 +42,10 @@
#include <linux/ktime.h>
#include <acpi/cppc_acpi.h>
+#ifdef CONFIG_X86
+#include <asm/msr.h>
+#endif
+
/*
* Lock to provide mutually exclusive access to the PCC
* channel. e.g. When the remote updates the shared region
@@ -585,8 +589,9 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
pr_debug("Mismatched PCC ids.\n");
goto out_free;
}
- } else if (gas_t->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY) {
- /* Support only PCC and SYS MEM type regs */
+ } else if (gas_t->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+ gas_t->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE) {
+ /* Support only PCC, FFH and SYS MEM type regs */
pr_debug("Unsupported register type: %d\n", gas_t->space_id);
goto out_free;
}
@@ -645,13 +650,59 @@ void acpi_cppc_processor_exit(struct acpi_processor *pr)
}
EXPORT_SYMBOL_GPL(acpi_cppc_processor_exit);
+#ifdef CONFIG_X86
+static int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val)
+{
+ int err;
+
+ err = rdmsrl_on_cpu(cpunum, reg->address, val);
+ if (!err) {
+ u64 mask = GENMASK_ULL(reg->bit_offset + reg->bit_width - 1,
+ reg->bit_offset);
+
+ *val &= mask;
+ *val >>= reg->bit_offset;
+ }
+ return err;
+}
+
+static int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
+{
+ u64 rd_val;
+ int err;
+
+ err = rdmsrl_on_cpu(cpunum, reg->address, &rd_val);
+ if (!err) {
+ u64 mask = GENMASK_ULL(reg->bit_offset + reg->bit_width - 1,
+ reg->bit_offset);
+
+ val <<= reg->bit_offset;
+ val &= mask;
+ rd_val &= ~mask;
+ rd_val |= val;
+ err = wrmsrl_on_cpu(cpunum, reg->address, rd_val);
+ }
+ return err;
+}
+#else
+static int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val)
+{
+ return -EINVAL;
+}
+static int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
+{
+ return -EINVAL;
+
+}
+#endif
+
/*
* Since cpc_read and cpc_write are called while holding pcc_lock, it should be
* as fast as possible. We have already mapped the PCC subspace during init, so
* we can directly write to it.
*/
-static int cpc_read(struct cpc_register_resource *res, u64 *val)
+static int cpc_read(int cpunum, struct cpc_register_resource *res, u64 *val)
{
struct cpc_reg *reg = &res->cpc_entry.reg;
int ret_val = 0;
@@ -684,13 +735,15 @@ static int cpc_read(struct cpc_register_resource *res, u64 *val)
reg->bit_width);
ret_val = -EFAULT;
}
+ } else if (reg->space_id == ACPI_ADR_SPACE_FIXED_HARDWARE) {
+ ret_val = cpc_read_ffh(cpunum, reg, val);
} else
ret_val = acpi_os_read_memory((acpi_physical_address)reg->address,
val, reg->bit_width);
return ret_val;
}
-static int cpc_write(struct cpc_reg *reg, u64 val)
+static int cpc_write(int cpunum, struct cpc_reg *reg, u64 val)
{
int ret_val = 0;
@@ -716,6 +769,8 @@ static int cpc_write(struct cpc_reg *reg, u64 val)
ret_val = -EFAULT;
break;
}
+ } else if (reg->space_id == ACPI_ADR_SPACE_FIXED_HARDWARE) {
+ ret_val = cpc_write_ffh(cpunum, reg, val);
} else
ret_val = acpi_os_write_memory((acpi_physical_address)reg->address,
val, reg->bit_width);
@@ -761,16 +816,16 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
}
}
- cpc_read(highest_reg, &high);
+ cpc_read(cpunum, highest_reg, &high);
perf_caps->highest_perf = high;
- cpc_read(lowest_reg, &low);
+ cpc_read(cpunum, lowest_reg, &low);
perf_caps->lowest_perf = low;
- cpc_read(ref_perf, &ref);
+ cpc_read(cpunum, ref_perf, &ref);
perf_caps->reference_perf = ref;
- cpc_read(nom_perf, &nom);
+ cpc_read(cpunum, nom_perf, &nom);
perf_caps->nominal_perf = nom;
if (!ref)
@@ -819,8 +874,8 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs)
}
}
- cpc_read(delivered_reg, &delivered);
- cpc_read(reference_reg, &reference);
+ cpc_read(cpunum, delivered_reg, &delivered);
+ cpc_read(cpunum, reference_reg, &reference);
if (!delivered || !reference) {
ret = -EFAULT;
@@ -875,7 +930,7 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
* Skip writing MIN/MAX until Linux knows how to come up with
* useful values.
*/
- cpc_write(&desired_reg->cpc_entry.reg, perf_ctrls->desired_perf);
+ cpc_write(cpu, &desired_reg->cpc_entry.reg, perf_ctrls->desired_perf);
/* Is this a PCC reg ?*/
if (desired_reg->cpc_entry.reg.space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/5] acpi: cppc: Add prefix cppc to cpudata structure name
2016-08-11 0:17 [PATCH 0/5] x86 CPPC usage Srinivas Pandruvada
` (2 preceding siblings ...)
2016-08-11 0:17 ` [PATCH 3/5] acpi: cppc: Add support for function fixed hardware address Srinivas Pandruvada
@ 2016-08-11 0:17 ` Srinivas Pandruvada
2016-08-11 0:17 ` [PATCH 5/5] acpi: bus: Enable HWP CPPC objects Srinivas Pandruvada
2016-08-18 21:14 ` [PATCH 0/5] x86 CPPC usage Srinivas Pandruvada
5 siblings, 0 replies; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-11 0:17 UTC (permalink / raw)
To: rjw, viresh.kumar
Cc: linux-acpi, linux-pm, ashwin.chaugule, Srinivas Pandruvada
Since struct cpudata is defined in a header file, add prefix cppc_ to
make it not a generic name. Otherwise it causes compile issue in locally
define structure with the same name.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/acpi/cppc_acpi.c | 4 ++--
drivers/cpufreq/cppc_cpufreq.c | 14 +++++++-------
include/acpi/cppc_acpi.h | 4 ++--
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 939fb5c..5ca517d 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -276,13 +276,13 @@ end:
*
* Return: 0 for success or negative value for err.
*/
-int acpi_get_psd_map(struct cpudata **all_cpu_data)
+int acpi_get_psd_map(struct cppc_cpudata **all_cpu_data)
{
int count_target;
int retval = 0;
unsigned int i, j;
cpumask_var_t covered_cpus;
- struct cpudata *pr, *match_pr;
+ struct cppc_cpudata *pr, *match_pr;
struct acpi_psd_package *pdomain;
struct acpi_psd_package *match_pdomain;
struct cpc_desc *cpc_ptr, *match_cpc_ptr;
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 8882b8e..b1b3549 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -30,13 +30,13 @@
* performance capabilities, desired performance level
* requested etc.
*/
-static struct cpudata **all_cpu_data;
+static struct cppc_cpudata **all_cpu_data;
static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
unsigned int target_freq,
unsigned int relation)
{
- struct cpudata *cpu;
+ struct cppc_cpudata *cpu;
struct cpufreq_freqs freqs;
int ret = 0;
@@ -66,7 +66,7 @@ static int cppc_verify_policy(struct cpufreq_policy *policy)
static void cppc_cpufreq_stop_cpu(struct cpufreq_policy *policy)
{
int cpu_num = policy->cpu;
- struct cpudata *cpu = all_cpu_data[cpu_num];
+ struct cppc_cpudata *cpu = all_cpu_data[cpu_num];
int ret;
cpu->perf_ctrls.desired_perf = cpu->perf_caps.lowest_perf;
@@ -79,7 +79,7 @@ static void cppc_cpufreq_stop_cpu(struct cpufreq_policy *policy)
static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
{
- struct cpudata *cpu;
+ struct cppc_cpudata *cpu;
unsigned int cpu_num = policy->cpu;
int ret = 0;
@@ -134,7 +134,7 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
static int __init cppc_cpufreq_init(void)
{
int i, ret = 0;
- struct cpudata *cpu;
+ struct cppc_cpudata *cpu;
if (acpi_disabled)
return -ENODEV;
@@ -144,7 +144,7 @@ static int __init cppc_cpufreq_init(void)
return -ENOMEM;
for_each_possible_cpu(i) {
- all_cpu_data[i] = kzalloc(sizeof(struct cpudata), GFP_KERNEL);
+ all_cpu_data[i] = kzalloc(sizeof(struct cppc_cpudata), GFP_KERNEL);
if (!all_cpu_data[i])
goto out;
@@ -175,7 +175,7 @@ out:
static void __exit cppc_cpufreq_exit(void)
{
- struct cpudata *cpu;
+ struct cppc_cpudata *cpu;
int i;
cpufreq_unregister_driver(&cppc_cpufreq_driver);
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 284965c..b7816c2 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -114,7 +114,7 @@ struct cppc_perf_fb_ctrs {
};
/* Per CPU container for runtime CPPC management. */
-struct cpudata {
+struct cppc_cpudata {
int cpu;
struct cppc_perf_caps perf_caps;
struct cppc_perf_ctrls perf_ctrls;
@@ -127,6 +127,6 @@ struct cpudata {
extern int cppc_get_perf_ctrs(int cpu, struct cppc_perf_fb_ctrs *perf_fb_ctrs);
extern int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls);
extern int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps);
-extern int acpi_get_psd_map(struct cpudata **);
+extern int acpi_get_psd_map(struct cppc_cpudata **);
#endif /* _CPPC_ACPI_H*/
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/5] acpi: bus: Enable HWP CPPC objects
2016-08-11 0:17 [PATCH 0/5] x86 CPPC usage Srinivas Pandruvada
` (3 preceding siblings ...)
2016-08-11 0:17 ` [PATCH 4/5] acpi: cppc: Add prefix cppc to cpudata structure name Srinivas Pandruvada
@ 2016-08-11 0:17 ` Srinivas Pandruvada
2016-08-16 14:32 ` Alexey Klimov
2016-08-18 21:14 ` [PATCH 0/5] x86 CPPC usage Srinivas Pandruvada
5 siblings, 1 reply; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-11 0:17 UTC (permalink / raw)
To: rjw, viresh.kumar
Cc: linux-acpi, linux-pm, ashwin.chaugule, Srinivas Pandruvada
Need to set platform wide _OSC bits CPC version 1 and version 2 bits
so that BIOS presents CPPC objects.
Even though the cppc_acpi supports only version 2, need to set both
_OSC bits for version 1 and version 2, otherwise BIOS ignore _OSC
settings.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/acpi/bus.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 85b7d07..61643a5 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -330,6 +330,13 @@ static void acpi_bus_osc_support(void)
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT;
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT;
+#ifdef CONFIG_X86
+ if (boot_cpu_has(X86_FEATURE_HWP)) {
+ capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
+ capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
+ }
+#endif
+
if (!ghes_disable)
capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] acpi: bus: Enable HWP CPPC objects
2016-08-11 0:17 ` [PATCH 5/5] acpi: bus: Enable HWP CPPC objects Srinivas Pandruvada
@ 2016-08-16 14:32 ` Alexey Klimov
2016-08-16 16:50 ` Srinivas Pandruvada
0 siblings, 1 reply; 18+ messages in thread
From: Alexey Klimov @ 2016-08-16 14:32 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: rjw, viresh.kumar, linux-acpi, linux-pm, ashwin.chaugule
Hi Srinivas,
On Wed, Aug 10, 2016 at 05:17:26PM -0700, Srinivas Pandruvada wrote:
> Need to set platform wide _OSC bits CPC version 1 and version 2 bits
> so that BIOS presents CPPC objects.
> Even though the cppc_acpi supports only version 2, need to set both
> _OSC bits for version 1 and version 2, otherwise BIOS ignore _OSC
> settings.
Does such behaviour go against ACPI specs?
If yes, it's better to add comment in the code.
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/acpi/bus.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 85b7d07..61643a5 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -330,6 +330,13 @@ static void acpi_bus_osc_support(void)
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT;
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT;
>
> +#ifdef CONFIG_X86
> + if (boot_cpu_has(X86_FEATURE_HWP)) {
> + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
> + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
> + }
> +#endif
Just to check if I understand things correctly.
I thought that OSC method is kind of 'handshake'. OS describes features
that it supports and then platform indicates back what features it can
provide/support.
Here in this patch I see only first part and I don't see where you check
confirmation from platform that it supports/enabled CPPC.
You don't need that, do you?
It will be nice to see explanation.
Best regards,
Alexey.
> if (!ghes_disable)
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
> if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] acpi: bus: Enable HWP CPPC objects
2016-08-16 14:32 ` Alexey Klimov
@ 2016-08-16 16:50 ` Srinivas Pandruvada
0 siblings, 0 replies; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-16 16:50 UTC (permalink / raw)
To: Alexey Klimov; +Cc: rjw, viresh.kumar, linux-acpi, linux-pm, ashwin.chaugule
Hi Alexey,
On Tue, 2016-08-16 at 15:32 +0100, Alexey Klimov wrote:
> Hi Srinivas,
>
> On Wed, Aug 10, 2016 at 05:17:26PM -0700, Srinivas Pandruvada wrote:
> >
> > Need to set platform wide _OSC bits CPC version 1 and version 2
> > bits
> > so that BIOS presents CPPC objects.
> > Even though the cppc_acpi supports only version 2, need to set both
> > _OSC bits for version 1 and version 2, otherwise BIOS ignore _OSC
> > settings.
> Does such behaviour go against ACPI specs?
> If yes, it's better to add comment in the code.
I need to fix the commit description here. The bit 5 is generic CPPC
support and bit 6 is CPPC_V2 support capability. So if we set only bit
5 then we can support only CPPC v1. If we set bit 6, we also support
CPPC v2. So we have to set both bits for CPPC v2 (Table 6-176 Platform-
Wide _OSC Capabilities DWORD 2 ACPI 6.0 spec).
> >
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel
> > .com>
> > ---
> > drivers/acpi/bus.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 85b7d07..61643a5 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -330,6 +330,13 @@ static void acpi_bus_osc_support(void)
> > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT;
> > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT;
> >
> > +#ifdef CONFIG_X86
> > + if (boot_cpu_has(X86_FEATURE_HWP)) {
> > + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
> > + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
> > + }
> > +#endif
> Just to check if I understand things correctly.
> I thought that OSC method is kind of 'handshake'. OS describes
> features
> that it supports and then platform indicates back what features it
> can
> provide/support.
> Here in this patch I see only first part and I don't see where you
> check
> confirmation from platform that it supports/enabled CPPC.
> You don't need that, do you?
Once we set these bits the BIOS will expose the CPPC tables. This will
result in acpi_cppc_processor_probe will be successful. Also _OSC
response will set bit 5 and 6 here to confirm the acceptance, but I
can't do any meaningful processing with that confirmation till
acpi_cppc_processor_probe() returns successfully, which is the
confirmation.
How the CPPC data is used in x86 as I indicated in the cover page will
be a separate series. I submitted CPPC ACPI changes before so that they
can be reviewed first.
Thanks,
Srinivas
> It will be nice to see explanation.
>
> Best regards,
> Alexey.
>
> >
> > if (!ghes_disable)
> > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
> > if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] x86 CPPC usage
2016-08-11 0:17 [PATCH 0/5] x86 CPPC usage Srinivas Pandruvada
` (4 preceding siblings ...)
2016-08-11 0:17 ` [PATCH 5/5] acpi: bus: Enable HWP CPPC objects Srinivas Pandruvada
@ 2016-08-18 21:14 ` Srinivas Pandruvada
5 siblings, 0 replies; 18+ messages in thread
From: Srinivas Pandruvada @ 2016-08-18 21:14 UTC (permalink / raw)
To: rjw, viresh.kumar; +Cc: linux-acpi, linux-pm, ashwin.chaugule
On Wed, 2016-08-10 at 17:17 -0700, Srinivas Pandruvada wrote:
> The x86 HWP (Hardware Controlled Performance States) is an
> implementation
> of the ACPI-defined Collaborative Processor Performance Control
> (CPPC).
> The current implementation of HWP doesn't use CPPC, but CPPC brings
> in
> some advanced features. So we will be submitting changes to use CPPC.
>
> The first series only contains ACPI CPPC changes required for x86.
>
Since I didn't see any major objection to this series, I will resubmit
these changes along with the use case of CPPC on x86 to support turbo
feature.
Thanks,
Srinivas
> Srinivas Pandruvada (5):
> acpi: cppc: Allow build with ACPI_CPU_FREQ_PSS config
> acpi: cpcc: Add integer read support
> acpi: cppc: Add support for function fixed hardware address
> acpi: cppc: Add prefix cppc to cpudata structure name
> acpi: bus: Enable HWP CPPC objects
>
> drivers/acpi/Kconfig | 1 -
> drivers/acpi/bus.c | 7 ++++
> drivers/acpi/cppc_acpi.c | 88
> +++++++++++++++++++++++++++++++++++------
> drivers/acpi/processor_driver.c | 5 ++-
> drivers/cpufreq/cppc_cpufreq.c | 14 +++----
> include/acpi/cppc_acpi.h | 4 +-
> 6 files changed, 95 insertions(+), 24 deletions(-)
>
^ permalink raw reply [flat|nested] 18+ messages in thread