linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter
@ 2025-01-28 14:11 David Arcari
  2025-02-04 12:23 ` Artem Bityutskiy
                   ` (4 more replies)
  0 siblings, 5 replies; 32+ messages in thread
From: David Arcari @ 2025-01-28 14:11 UTC (permalink / raw)
  To: linux-pm
  Cc: David Arcari, Jonathan Corbet, Jacob Pan, Len Brown,
	Prarit Bhargava, linux-doc, linux-kernel

Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
without C-state tables")the intel_idle driver has had the ability to use
the ACPI _CST to populate C-states when the processor model is not
recognized. However, even when the processor model is recognized there are
cases where it is useful to make the driver ignore the per cpu idle states
in lieu of ACPI C-states (such as specific application performance). Add
the 'use_acpi_cst' module parameter to provide this functionality (not to
be confused with the 'use_acpi' module parameter which has a different
function).

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: David Arcari <darcari@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: David Arcari <darcari@redhat.com>
---
 Documentation/admin-guide/pm/intel_idle.rst |  5 +++++
 drivers/idle/intel_idle.c                   | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
index 39bd6ecce7de..a87238bcf33d 100644
--- a/Documentation/admin-guide/pm/intel_idle.rst
+++ b/Documentation/admin-guide/pm/intel_idle.rst
@@ -198,6 +198,11 @@ driver ignore the system's ACPI tables entirely or use them for all of the
 recognized processor models, respectively (they both are unset by default and
 ``use_acpi`` has no effect if ``no_acpi`` is set).
 
+The ``use_acpi_cst`` module parameter (recognized by ``intel_idle`` if the
+kernel has been configured with ACPI support) can be set to make the driver
+ignore the per cpu idle states in lieu of ACPI idle states. ``use_acpi_cst``
+has no effect if ``no_acpi`` is set).
+
 The value of the ``states_off`` module parameter (0 by default) represents a
 list of idle states to be disabled by default in the form of a bitmask.
 
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 118fe1d37c22..b8a536b930e7 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
 module_param_named(use_acpi, force_use_acpi, bool, 0444);
 MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
 
+static bool use_acpi_cst __read_mostly; /* No effect if no_acpi is set. */
+module_param_named(use_acpi_cst, use_acpi_cst, bool, 0444);
+MODULE_PARM_DESC(use_acpi_cst, "Ignore cpu specific idle states in lieu of ACPI idle states");
+
 static struct acpi_processor_power acpi_state_table __initdata;
 
 /**
@@ -1836,6 +1840,7 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
 }
 #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
 #define force_use_acpi	(false)
+#define use_acpi_cst	(false)
 
 static inline bool intel_idle_acpi_cst_extract(void) { return false; }
 static inline void intel_idle_init_cstates_acpi(struct cpuidle_driver *drv) { }
@@ -2328,6 +2333,12 @@ static int __init intel_idle_init(void)
 	pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
 
 	icpu = (const struct idle_cpu *)id->driver_data;
+	if (use_acpi_cst && !no_acpi) {
+		if (icpu) {
+			pr_debug("ignoring cpu idle states\n");
+			icpu = NULL;
+		}
+	}
 	if (icpu) {
 		if (icpu->state_table)
 			cpuidle_state_table = icpu->state_table;
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter
  2025-01-28 14:11 [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter David Arcari
@ 2025-02-04 12:23 ` Artem Bityutskiy
  2025-02-04 12:52   ` David Arcari
  2025-02-06 16:40 ` [PATCH v2] intel_idle: introduce 'no_native' " David Arcari
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-04 12:23 UTC (permalink / raw)
  To: David Arcari, linux-pm
  Cc: Jonathan Corbet, Jacob Pan, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel

Hi David,

On Tue, 2025-01-28 at 09:11 -0500, David Arcari wrote:

> +The ``use_acpi_cst`` module parameter (recognized by ``intel_idle`` if the
> +kernel has been configured with ACPI support) can be set to make the driver
> +ignore the per cpu idle states in lieu of ACPI idle states. ``use_acpi_cst``
> +has no effect if ``no_acpi`` is set).

With this change, there will be three parameters:

* no_acpi
* use_acpi
* use_acpi_cst

I would like to make the naming as intuitive as possible. We do not rename the
first 2, but for the 3rd one, I think "force_acpi" would be a better name. Or
perhaps "no_native"?

* no_acpi - Do not use ACPI at all. Only native mode is available, no ACPI mode.
* use_acpi - No-op in ACPI mode, consult ACPI tables for C-states on/off
  status in native mode.
* force_acpi (or no_native?) - Work only in ACPI mode, no native mode available
  (ignore all custom tables).

Additionally, I think we should enhance the documentation for 'no_acpi' and
'use_acpi' while we're at it. Otherwise, it is hard to distinguish between these
three options. Would you consider another patch that improves the documentation
for 'no_acpi' and 'use_acpi', and then adds the third parameter?

Thanks, Artem!

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter
  2025-02-04 12:23 ` Artem Bityutskiy
@ 2025-02-04 12:52   ` David Arcari
  2025-02-04 13:04     ` Prarit Bhargava
                       ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: David Arcari @ 2025-02-04 12:52 UTC (permalink / raw)
  To: dedekind1, linux-pm
  Cc: Jonathan Corbet, Jacob Pan, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel


Hi Artem,

On 2/4/25 7:23 AM, Artem Bityutskiy wrote:
> Hi David,
> 
> On Tue, 2025-01-28 at 09:11 -0500, David Arcari wrote:
> 
>> +The ``use_acpi_cst`` module parameter (recognized by ``intel_idle`` if the
>> +kernel has been configured with ACPI support) can be set to make the driver
>> +ignore the per cpu idle states in lieu of ACPI idle states. ``use_acpi_cst``
>> +has no effect if ``no_acpi`` is set).
> 
> With this change, there will be three parameters:
> 
> * no_acpi
> * use_acpi
> * use_acpi_cst
> 
> I would like to make the naming as intuitive as possible. We do not rename the
> first 2, but for the 3rd one, I think "force_acpi" would be a better name. Or
> perhaps "no_native"?

The problem with force_acpi is it is very similar to force_use_acpi 
which is what intel_idle.c uses internally:

drivers/idle/intel_idle.c:module_param_named(use_acpi, force_use_acpi, 
bool, 0444);

That said, I am not attached to the 'use_acpi_cst' parameter name.

> 
> * no_acpi - Do not use ACPI at all. Only native mode is available, no ACPI mode.
> * use_acpi - No-op in ACPI mode, consult ACPI tables for C-states on/off
>    status in native mode.
> * force_acpi (or no_native?) - Work only in ACPI mode, no native mode available
>    (ignore all custom tables).
> 
> Additionally, I think we should enhance the documentation for 'no_acpi' and
> 'use_acpi' while we're at it. Otherwise, it is hard to distinguish between these
> three options. Would you consider another patch that improves the documentation
> for 'no_acpi' and 'use_acpi', and then adds the third parameter?

I'm happy to resubmit. I guess I could use 'no_native' for the new 
parameter and then update the documentation as you suggest above.

Does that work?

> 
> Thanks, Artem!
> 

Best,
-DA


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter
  2025-02-04 12:52   ` David Arcari
@ 2025-02-04 13:04     ` Prarit Bhargava
  2025-02-04 15:21     ` Rafael J. Wysocki
  2025-02-04 16:26     ` Artem Bityutskiy
  2 siblings, 0 replies; 32+ messages in thread
From: Prarit Bhargava @ 2025-02-04 13:04 UTC (permalink / raw)
  To: David Arcari, dedekind1, linux-pm
  Cc: Jonathan Corbet, Jacob Pan, Len Brown, linux-doc, linux-kernel

On 2/4/25 7:52 AM, David Arcari wrote:
> 
> Hi Artem,
> 
> On 2/4/25 7:23 AM, Artem Bityutskiy wrote:
>> Hi David,
>>
>> On Tue, 2025-01-28 at 09:11 -0500, David Arcari wrote:
>>
>>> +The ``use_acpi_cst`` module parameter (recognized by ``intel_idle`` 
>>> if the
>>> +kernel has been configured with ACPI support) can be set to make the 
>>> driver
>>> +ignore the per cpu idle states in lieu of ACPI idle states. 
>>> ``use_acpi_cst``
>>> +has no effect if ``no_acpi`` is set).
>>
>> With this change, there will be three parameters:
>>
>> * no_acpi
>> * use_acpi
>> * use_acpi_cst
>>
>> I would like to make the naming as intuitive as possible. We do not 
>> rename the
>> first 2, but for the 3rd one, I think "force_acpi" would be a better 
>> name. Or
>> perhaps "no_native"?
> 
> The problem with force_acpi is it is very similar to force_use_acpi 
> which is what intel_idle.c uses internally:
> 

Given that @Artem is suggesting we change the names of things -- perhaps 
we should fix "force_use_acpi" as well?

P.

> drivers/idle/intel_idle.c:module_param_named(use_acpi, force_use_acpi, 
> bool, 0444);
> 
> That said, I am not attached to the 'use_acpi_cst' parameter name.
> 
>>
>> * no_acpi - Do not use ACPI at all. Only native mode is available, no 
>> ACPI mode.
>> * use_acpi - No-op in ACPI mode, consult ACPI tables for C-states on/off
>>    status in native mode.
>> * force_acpi (or no_native?) - Work only in ACPI mode, no native mode 
>> available
>>    (ignore all custom tables).
>>
>> Additionally, I think we should enhance the documentation for 
>> 'no_acpi' and
>> 'use_acpi' while we're at it. Otherwise, it is hard to distinguish 
>> between these
>> three options. Would you consider another patch that improves the 
>> documentation
>> for 'no_acpi' and 'use_acpi', and then adds the third parameter?
> 
> I'm happy to resubmit. I guess I could use 'no_native' for the new 
> parameter and then update the documentation as you suggest above.
> 
> Does that work?
> 
>>
>> Thanks, Artem!
>>
> 
> Best,
> -DA
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter
  2025-02-04 12:52   ` David Arcari
  2025-02-04 13:04     ` Prarit Bhargava
@ 2025-02-04 15:21     ` Rafael J. Wysocki
  2025-02-04 16:30       ` Artem Bityutskiy
  2025-02-04 16:26     ` Artem Bityutskiy
  2 siblings, 1 reply; 32+ messages in thread
From: Rafael J. Wysocki @ 2025-02-04 15:21 UTC (permalink / raw)
  To: David Arcari
  Cc: dedekind1, linux-pm, Jonathan Corbet, Jacob Pan, Len Brown,
	Prarit Bhargava, linux-doc, linux-kernel

On Tue, Feb 4, 2025 at 2:07 PM David Arcari <darcari@redhat.com> wrote:
>
>
> Hi Artem,
>
> On 2/4/25 7:23 AM, Artem Bityutskiy wrote:
> > Hi David,
> >
> > On Tue, 2025-01-28 at 09:11 -0500, David Arcari wrote:
> >
> >> +The ``use_acpi_cst`` module parameter (recognized by ``intel_idle`` if the
> >> +kernel has been configured with ACPI support) can be set to make the driver
> >> +ignore the per cpu idle states in lieu of ACPI idle states. ``use_acpi_cst``
> >> +has no effect if ``no_acpi`` is set).
> >
> > With this change, there will be three parameters:
> >
> > * no_acpi
> > * use_acpi
> > * use_acpi_cst
> >
> > I would like to make the naming as intuitive as possible. We do not rename the
> > first 2, but for the 3rd one, I think "force_acpi" would be a better name. Or
> > perhaps "no_native"?
>
> The problem with force_acpi is it is very similar to force_use_acpi
> which is what intel_idle.c uses internally:
>
> drivers/idle/intel_idle.c:module_param_named(use_acpi, force_use_acpi,
> bool, 0444);
>
> That said, I am not attached to the 'use_acpi_cst' parameter name.

IMV this is rather about ignoring the built-in states table
altogether, IOW something like "pretend that you don't recognize the
processor".

But it could be something like "prefer_acpi" as far as I'm concerned.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter
  2025-02-04 12:52   ` David Arcari
  2025-02-04 13:04     ` Prarit Bhargava
  2025-02-04 15:21     ` Rafael J. Wysocki
@ 2025-02-04 16:26     ` Artem Bityutskiy
  2 siblings, 0 replies; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-04 16:26 UTC (permalink / raw)
  To: David Arcari, linux-pm
  Cc: Jonathan Corbet, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel

On Tue, 2025-02-04 at 07:52 -0500, David Arcari wrote:
> I'm happy to resubmit. I guess I could use 'no_native' for the new 
> parameter and then update the documentation as you suggest above.
> 
> Does that work?

I'd suggest to wait for Rafael's response first. Thanks!

Artem.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter
  2025-02-04 15:21     ` Rafael J. Wysocki
@ 2025-02-04 16:30       ` Artem Bityutskiy
  2025-02-04 17:33         ` Rafael J. Wysocki
  0 siblings, 1 reply; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-04 16:30 UTC (permalink / raw)
  To: Rafael J. Wysocki, David Arcari
  Cc: linux-pm, Jonathan Corbet, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel

On Tue, 2025-02-04 at 16:21 +0100, Rafael J. Wysocki wrote:
> But it could be something like "prefer_acpi" as far as I'm concerned.

When I see "prefer_acpi", my intuition tells that it is just a preference:
"prefer ACPI, but may be native too". But I understood that the patch is about
"only ACPI and never native".

The reasons I suggested "no_native":
* Sort of consistent with "no_acpi"
* Suggests that native won't work.

Thanks.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter
  2025-02-04 16:30       ` Artem Bityutskiy
@ 2025-02-04 17:33         ` Rafael J. Wysocki
  2025-02-05 12:09           ` David Arcari
  0 siblings, 1 reply; 32+ messages in thread
From: Rafael J. Wysocki @ 2025-02-04 17:33 UTC (permalink / raw)
  To: dedekind1
  Cc: Rafael J. Wysocki, David Arcari, linux-pm, Jonathan Corbet,
	Len Brown, Prarit Bhargava, linux-doc, linux-kernel

On Tue, Feb 4, 2025 at 5:30 PM Artem Bityutskiy <dedekind1@gmail.com> wrote:
>
> On Tue, 2025-02-04 at 16:21 +0100, Rafael J. Wysocki wrote:
> > But it could be something like "prefer_acpi" as far as I'm concerned.
>
> When I see "prefer_acpi", my intuition tells that it is just a preference:
> "prefer ACPI, but may be native too". But I understood that the patch is about
> "only ACPI and never native".
>
> The reasons I suggested "no_native":
> * Sort of consistent with "no_acpi"
> * Suggests that native won't work.

"no_native" would be fine by me too.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter
  2025-02-04 17:33         ` Rafael J. Wysocki
@ 2025-02-05 12:09           ` David Arcari
  0 siblings, 0 replies; 32+ messages in thread
From: David Arcari @ 2025-02-05 12:09 UTC (permalink / raw)
  To: Rafael J. Wysocki, dedekind1
  Cc: linux-pm, Jonathan Corbet, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel



On 2/4/25 12:33 PM, Rafael J. Wysocki wrote:
> On Tue, Feb 4, 2025 at 5:30 PM Artem Bityutskiy <dedekind1@gmail.com> wrote:
>>
>> On Tue, 2025-02-04 at 16:21 +0100, Rafael J. Wysocki wrote:
>>> But it could be something like "prefer_acpi" as far as I'm concerned.
>>
>> When I see "prefer_acpi", my intuition tells that it is just a preference:
>> "prefer ACPI, but may be native too". But I understood that the patch is about
>> "only ACPI and never native".
>>
>> The reasons I suggested "no_native":
>> * Sort of consistent with "no_acpi"
>> * Suggests that native won't work.
> 
> "no_native" would be fine by me too.
> 

I will send out a v2 using 'no_native' for the new parameter.

-DA


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2] intel_idle: introduce 'no_native' module parameter
  2025-01-28 14:11 [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter David Arcari
  2025-02-04 12:23 ` Artem Bityutskiy
@ 2025-02-06 16:40 ` David Arcari
  2025-02-07 15:55   ` Artem Bityutskiy
  2025-02-11 13:27 ` [PATCH v3] " David Arcari
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: David Arcari @ 2025-02-06 16:40 UTC (permalink / raw)
  To: linux-pm
  Cc: David Arcari, Jonathan Corbet, Jacob Pan, Len Brown,
	Prarit Bhargava, linux-doc, linux-kernel

Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
without C-state tables") the intel_idle driver has had the ability to use
the ACPI _CST to populate C-states when the processor model is not
recognized. However, even when the processor model is recognized (native
mode) there are cases where it is useful to make the driver ignore the per
cpu idle states in lieu of ACPI C-states (such as specific application
performance). Add the 'no_native' module parameter to provide this
functionality.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: David Arcari <darcari@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: David Arcari <darcari@redhat.com>
---
v2: renamed parameter, cleaned up documentation

 Documentation/admin-guide/pm/intel_idle.rst | 17 ++++++++++++-----
 drivers/idle/intel_idle.c                   | 11 +++++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
index 39bd6ecce7de..1f02a880836c 100644
--- a/Documentation/admin-guide/pm/intel_idle.rst
+++ b/Documentation/admin-guide/pm/intel_idle.rst
@@ -192,11 +192,18 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in
 Documentation/admin-guide/pm/cpuidle.rst).
 Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.
 
-The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle``
-if the kernel has been configured with ACPI support) can be set to make the
-driver ignore the system's ACPI tables entirely or use them for all of the
-recognized processor models, respectively (they both are unset by default and
-``use_acpi`` has no effect if ``no_acpi`` is set).
+The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
+recognized by ``intel_idle`` if the kernel has been configured with ACPI
+support).
+
+``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
+ACPI mode.
+
+``use_acpi`` - No-op in ACPI mode; however, the driver will consult ACPI
+tables for C-states on/off status in native mode.
+
+``no_native`` - Work only in ACPI mode, no native mode available (ignore
+all custom tables).
 
 The value of the ``states_off`` module parameter (0 by default) represents a
 list of idle states to be disabled by default in the form of a bitmask.
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 118fe1d37c22..5e5bd3fd3064 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
 module_param_named(use_acpi, force_use_acpi, bool, 0444);
 MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
 
+static bool no_native __read_mostly; /* No effect if no_acpi is set. */
+module_param_named(no_native, no_native, bool, 0444);
+MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states");
+
 static struct acpi_processor_power acpi_state_table __initdata;
 
 /**
@@ -1836,6 +1840,7 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
 }
 #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
 #define force_use_acpi	(false)
+#define no_native	(false)
 
 static inline bool intel_idle_acpi_cst_extract(void) { return false; }
 static inline void intel_idle_init_cstates_acpi(struct cpuidle_driver *drv) { }
@@ -2328,6 +2333,12 @@ static int __init intel_idle_init(void)
 	pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
 
 	icpu = (const struct idle_cpu *)id->driver_data;
+	if (no_native && !no_acpi) {
+		if (icpu) {
+			pr_debug("ignoring native cpu idle states\n");
+			icpu = NULL;
+		}
+	}
 	if (icpu) {
 		if (icpu->state_table)
 			cpuidle_state_table = icpu->state_table;
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] intel_idle: introduce 'no_native' module parameter
  2025-02-06 16:40 ` [PATCH v2] intel_idle: introduce 'no_native' " David Arcari
@ 2025-02-07 15:55   ` Artem Bityutskiy
  2025-02-07 17:13     ` David Arcari
  0 siblings, 1 reply; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-07 15:55 UTC (permalink / raw)
  To: David Arcari, linux-pm
  Cc: Jonathan Corbet, Jacob Pan, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel

Hi David,

On Thu, 2025-02-06 at 11:40 -0500, David Arcari wrote:
> +The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
> +recognized by ``intel_idle`` if the kernel has been configured with ACPI
> +support).

And if kernel was not configured with ACPI support, are these not recognized? Or
they are just no-op basically?

Looks like there is a stray ")" at the end.

> +
> +``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
> +ACPI mode.
> +
> +``use_acpi`` - No-op in ACPI mode; however, the driver will consult ACPI
> +tables for C-states on/off status in native mode.

I think "however" part is a bit confusing. Would you consider re-phrasing
without "however" ?

> +
> +``no_native`` - Work only in ACPI mode, no native mode available (ignore
> +all custom tables).

Other than these small nitpicks,

Reviewed-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Tested-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

(I tested it on an Intel Broadwell platform).

Thanks!

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] intel_idle: introduce 'no_native' module parameter
  2025-02-07 15:55   ` Artem Bityutskiy
@ 2025-02-07 17:13     ` David Arcari
  2025-02-08 10:37       ` Artem Bityutskiy
  0 siblings, 1 reply; 32+ messages in thread
From: David Arcari @ 2025-02-07 17:13 UTC (permalink / raw)
  To: dedekind1, linux-pm
  Cc: Jonathan Corbet, Jacob Pan, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel


Hi Artem,

On 2/7/25 10:55 AM, Artem Bityutskiy wrote:
> Hi David,
> 
> On Thu, 2025-02-06 at 11:40 -0500, David Arcari wrote:
>> +The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
>> +recognized by ``intel_idle`` if the kernel has been configured with ACPI
>> +support).
> 
> And if kernel was not configured with ACPI support, are these not recognized? Or
> they are just no-op basically?

They are a no-op - the flags are all set to false so ACPI C-state tables 
are ignored.

> 
> Looks like there is a stray ")" at the end.
> 

Yes I will fix that.

>> +
>> +``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
>> +ACPI mode.
>> +
>> +``use_acpi`` - No-op in ACPI mode; however, the driver will consult ACPI
>> +tables for C-states on/off status in native mode.
> 
> I think "however" part is a bit confusing. Would you consider re-phrasing
> without "however" ?

Sure - so is this better:

``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tabees 
for C-states on/off status in native mode.

Thanks,
-DA
> 
>> +
>> +``no_native`` - Work only in ACPI mode, no native mode available (ignore
>> +all custom tables).
> 
> Other than these small nitpicks,
> 
> Reviewed-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Tested-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> 
> (I tested it on an Intel Broadwell platform).
> 
> Thanks!
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] intel_idle: introduce 'no_native' module parameter
  2025-02-07 17:13     ` David Arcari
@ 2025-02-08 10:37       ` Artem Bityutskiy
  2025-02-08 19:56         ` David Arcari
  0 siblings, 1 reply; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-08 10:37 UTC (permalink / raw)
  To: David Arcari, linux-pm
  Cc: Jonathan Corbet, Jacob Pan, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel

On Fri, 2025-02-07 at 12:13 -0500, David Arcari wrote:
> > And if kernel was not configured with ACPI support, are these not
> > recognized? Or
> > they are just no-op basically?
> 
> They are a no-op - the flags are all set to false so ACPI C-state tables 
> are ignored.

It would be nice to mention this too. Otherwise it sounds a bit incomplete. Like
this:

	If A then B. (nothing about "else").

Better way:

	If A then B, else C.

:-)

> > 
> Sure - so is this better:
> 
> ``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tabees 
> for C-states on/off status in native mode.

Yes, thanks!


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] intel_idle: introduce 'no_native' module parameter
  2025-02-08 10:37       ` Artem Bityutskiy
@ 2025-02-08 19:56         ` David Arcari
  2025-02-09  9:08           ` Artem Bityutskiy
  0 siblings, 1 reply; 32+ messages in thread
From: David Arcari @ 2025-02-08 19:56 UTC (permalink / raw)
  To: dedekind1, linux-pm
  Cc: Jonathan Corbet, Jacob Pan, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel



On 2/8/25 5:37 AM, Artem Bityutskiy wrote:
> On Fri, 2025-02-07 at 12:13 -0500, David Arcari wrote:
>>> And if kernel was not configured with ACPI support, are these not
>>> recognized? Or
>>> they are just no-op basically?
>>
>> They are a no-op - the flags are all set to false so ACPI C-state tables
>> are ignored.
> 
> It would be nice to mention this too. Otherwise it sounds a bit incomplete. Like
> this:
> 
> 	If A then B. (nothing about "else").
> 
> Better way:
> 
> 	If A then B, else C.
> 
> :-)

I actually took that from what was already there.

So I can add "In the case that ACPI is not configured these flags have 
no impact on functionality."

Does that work?

-DA

> 
>>>
>> Sure - so is this better:
>>
>> ``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tabees
>> for C-states on/off status in native mode.
> 
> Yes, thanks!
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] intel_idle: introduce 'no_native' module parameter
  2025-02-08 19:56         ` David Arcari
@ 2025-02-09  9:08           ` Artem Bityutskiy
  0 siblings, 0 replies; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-09  9:08 UTC (permalink / raw)
  To: David Arcari, linux-pm
  Cc: Jonathan Corbet, Jacob Pan, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel

On Sat, 2025-02-08 at 14:56 -0500, David Arcari wrote:
> I actually took that from what was already there.
> 
> So I can add "In the case that ACPI is not configured these flags have 
> no impact on functionality."
> 
> Does that work?

Works for me!

Thanks,
Artem.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v3] intel_idle: introduce 'no_native' module parameter
  2025-01-28 14:11 [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter David Arcari
  2025-02-04 12:23 ` Artem Bityutskiy
  2025-02-06 16:40 ` [PATCH v2] intel_idle: introduce 'no_native' " David Arcari
@ 2025-02-11 13:27 ` David Arcari
  2025-02-12  7:04   ` Artem Bityutskiy
                     ` (2 more replies)
  2025-02-13 16:07 ` [PATCH v4] " David Arcari
  2025-02-20 15:11 ` [PATCH v5] " David Arcari
  4 siblings, 3 replies; 32+ messages in thread
From: David Arcari @ 2025-02-11 13:27 UTC (permalink / raw)
  To: linux-pm
  Cc: David Arcari, Jonathan Corbet, Jacob Pan, Len Brown,
	Artem Bityutskiy, Prarit Bhargava, linux-doc, linux-kernel

Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
without C-state tables") the intel_idle driver has had the ability to use
the ACPI _CST to populate C-states when the processor model is not
recognized. However, even when the processor model is recognized (native
mode) there are cases where it is useful to make the driver ignore the per
cpu idle states in lieu of ACPI C-states (such as specific application
performance). Add the 'no_native' module parameter to provide this
functionality.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: David Arcari <darcari@redhat.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: David Arcari <darcari@redhat.com>
---
v3: more documentation cleanup
v2: renamed parameter, cleaned up documentation

 Documentation/admin-guide/pm/intel_idle.rst | 18 +++++++++++++-----
 drivers/idle/intel_idle.c                   | 11 +++++++++++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
index 39bd6ecce7de..5940528146eb 100644
--- a/Documentation/admin-guide/pm/intel_idle.rst
+++ b/Documentation/admin-guide/pm/intel_idle.rst
@@ -192,11 +192,19 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in
 Documentation/admin-guide/pm/cpuidle.rst).
 Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.
 
-The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle``
-if the kernel has been configured with ACPI support) can be set to make the
-driver ignore the system's ACPI tables entirely or use them for all of the
-recognized processor models, respectively (they both are unset by default and
-``use_acpi`` has no effect if ``no_acpi`` is set).
+The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
+recognized by ``intel_idle`` if the kernel has been configured with ACPI
+support.  In the case that ACPI is not configured these flags have no impact
+on functionality.
+
+``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
+ACPI mode.
+
+``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tables for
+C-states on/off status in native mode.
+
+``no_native`` - Work only in ACPI mode, no native mode available (ignore
+all custom tables).
 
 The value of the ``states_off`` module parameter (0 by default) represents a
 list of idle states to be disabled by default in the form of a bitmask.
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 118fe1d37c22..5e5bd3fd3064 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
 module_param_named(use_acpi, force_use_acpi, bool, 0444);
 MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
 
+static bool no_native __read_mostly; /* No effect if no_acpi is set. */
+module_param_named(no_native, no_native, bool, 0444);
+MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states");
+
 static struct acpi_processor_power acpi_state_table __initdata;
 
 /**
@@ -1836,6 +1840,7 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
 }
 #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
 #define force_use_acpi	(false)
+#define no_native	(false)
 
 static inline bool intel_idle_acpi_cst_extract(void) { return false; }
 static inline void intel_idle_init_cstates_acpi(struct cpuidle_driver *drv) { }
@@ -2328,6 +2333,12 @@ static int __init intel_idle_init(void)
 	pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
 
 	icpu = (const struct idle_cpu *)id->driver_data;
+	if (no_native && !no_acpi) {
+		if (icpu) {
+			pr_debug("ignoring native cpu idle states\n");
+			icpu = NULL;
+		}
+	}
 	if (icpu) {
 		if (icpu->state_table)
 			cpuidle_state_table = icpu->state_table;
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] intel_idle: introduce 'no_native' module parameter
  2025-02-11 13:27 ` [PATCH v3] " David Arcari
@ 2025-02-12  7:04   ` Artem Bityutskiy
  2025-02-12 10:09   ` kernel test robot
  2025-02-13 11:49   ` kernel test robot
  2 siblings, 0 replies; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-12  7:04 UTC (permalink / raw)
  To: David Arcari, linux-pm
  Cc: Jonathan Corbet, Len Brown, Prarit Bhargava, linux-doc,
	linux-kernel

On Tue, 2025-02-11 at 08:27 -0500, David Arcari wrote:
> Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
> without C-state tables") the intel_idle driver has had the ability to use
> the ACPI _CST to populate C-states when the processor model is not
> recognized. However, even when the processor model is recognized (native
> mode) there are cases where it is useful to make the driver ignore the per
> cpu idle states in lieu of ACPI C-states (such as specific application
> performance). Add the 'no_native' module parameter to provide this
> functionality.
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: David Arcari <darcari@redhat.com>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: David Arcari <darcari@redhat.com>
> ---
> v3: more documentation cleanup
> v2: renamed parameter, cleaned up documentation

Reviewed-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Tested-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] intel_idle: introduce 'no_native' module parameter
  2025-02-11 13:27 ` [PATCH v3] " David Arcari
  2025-02-12  7:04   ` Artem Bityutskiy
@ 2025-02-12 10:09   ` kernel test robot
  2025-02-12 11:32     ` Artem Bityutskiy
  2025-02-13 11:49   ` kernel test robot
  2 siblings, 1 reply; 32+ messages in thread
From: kernel test robot @ 2025-02-12 10:09 UTC (permalink / raw)
  To: David Arcari, linux-pm
  Cc: oe-kbuild-all, David Arcari, Jonathan Corbet, Jacob Pan,
	Len Brown, Artem Bityutskiy, Prarit Bhargava, linux-doc,
	linux-kernel

Hi David,

kernel test robot noticed the following build errors:

[auto build test ERROR on acpi/next]
[also build test ERROR on amd-pstate/linux-next amd-pstate/bleeding-edge linus/master v6.14-rc2 next-20250212]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Arcari/intel_idle-introduce-no_native-module-parameter/20250211-213031
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git next
patch link:    https://lore.kernel.org/r/20250211132741.99944-1-darcari%40redhat.com
patch subject: [PATCH v3] intel_idle: introduce 'no_native' module parameter
config: i386-buildonly-randconfig-006-20250212 (https://download.01.org/0day-ci/archive/20250212/202502121732.P7lZkbhm-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250212/202502121732.P7lZkbhm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502121732.P7lZkbhm-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/idle/intel_idle.c: In function 'intel_idle_init':
>> drivers/idle/intel_idle.c:2289:27: error: 'no_acpi' undeclared (first use in this function); did you mean 'no_action'?
    2289 |         if (no_native && !no_acpi) {
         |                           ^~~~~~~
         |                           no_action
   drivers/idle/intel_idle.c:2289:27: note: each undeclared identifier is reported only once for each function it appears in


vim +2289 drivers/idle/intel_idle.c

  2248	
  2249	static int __init intel_idle_init(void)
  2250	{
  2251		const struct x86_cpu_id *id;
  2252		unsigned int eax, ebx, ecx;
  2253		int retval;
  2254	
  2255		/* Do not load intel_idle at all for now if idle= is passed */
  2256		if (boot_option_idle_override != IDLE_NO_OVERRIDE)
  2257			return -ENODEV;
  2258	
  2259		if (max_cstate == 0) {
  2260			pr_debug("disabled\n");
  2261			return -EPERM;
  2262		}
  2263	
  2264		id = x86_match_cpu(intel_idle_ids);
  2265		if (id) {
  2266			if (!boot_cpu_has(X86_FEATURE_MWAIT)) {
  2267				pr_debug("Please enable MWAIT in BIOS SETUP\n");
  2268				return -ENODEV;
  2269			}
  2270		} else {
  2271			id = x86_match_cpu(intel_mwait_ids);
  2272			if (!id)
  2273				return -ENODEV;
  2274		}
  2275	
  2276		if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
  2277			return -ENODEV;
  2278	
  2279		cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates);
  2280	
  2281		if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) ||
  2282		    !(ecx & CPUID5_ECX_INTERRUPT_BREAK) ||
  2283		    !mwait_substates)
  2284				return -ENODEV;
  2285	
  2286		pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
  2287	
  2288		icpu = (const struct idle_cpu *)id->driver_data;
> 2289		if (no_native && !no_acpi) {
  2290			if (icpu) {
  2291				pr_debug("ignoring native cpu idle states\n");
  2292				icpu = NULL;
  2293			}
  2294		}
  2295		if (icpu) {
  2296			if (icpu->state_table)
  2297				cpuidle_state_table = icpu->state_table;
  2298			else if (!intel_idle_acpi_cst_extract())
  2299				return -ENODEV;
  2300	
  2301			auto_demotion_disable_flags = icpu->auto_demotion_disable_flags;
  2302			if (icpu->disable_promotion_to_c1e)
  2303				c1e_promotion = C1E_PROMOTION_DISABLE;
  2304			if (icpu->use_acpi || force_use_acpi)
  2305				intel_idle_acpi_cst_extract();
  2306		} else if (!intel_idle_acpi_cst_extract()) {
  2307			return -ENODEV;
  2308		}
  2309	
  2310		pr_debug("v" INTEL_IDLE_VERSION " model 0x%X\n",
  2311			 boot_cpu_data.x86_model);
  2312	
  2313		intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
  2314		if (!intel_idle_cpuidle_devices)
  2315			return -ENOMEM;
  2316	
  2317		intel_idle_cpuidle_driver_init(&intel_idle_driver);
  2318	
  2319		retval = cpuidle_register_driver(&intel_idle_driver);
  2320		if (retval) {
  2321			struct cpuidle_driver *drv = cpuidle_get_driver();
  2322			printk(KERN_DEBUG pr_fmt("intel_idle yielding to %s\n"),
  2323			       drv ? drv->name : "none");
  2324			goto init_driver_fail;
  2325		}
  2326	
  2327		retval = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "idle/intel:online",
  2328					   intel_idle_cpu_online, NULL);
  2329		if (retval < 0)
  2330			goto hp_setup_fail;
  2331	
  2332		pr_debug("Local APIC timer is reliable in %s\n",
  2333			 boot_cpu_has(X86_FEATURE_ARAT) ? "all C-states" : "C1");
  2334	
  2335		return 0;
  2336	
  2337	hp_setup_fail:
  2338		intel_idle_cpuidle_devices_uninit();
  2339		cpuidle_unregister_driver(&intel_idle_driver);
  2340	init_driver_fail:
  2341		free_percpu(intel_idle_cpuidle_devices);
  2342		return retval;
  2343	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] intel_idle: introduce 'no_native' module parameter
  2025-02-12 10:09   ` kernel test robot
@ 2025-02-12 11:32     ` Artem Bityutskiy
  2025-02-12 12:41       ` David Arcari
  0 siblings, 1 reply; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-12 11:32 UTC (permalink / raw)
  To: kernel test robot, David Arcari, linux-pm
  Cc: oe-kbuild-all, Jonathan Corbet, Jacob Pan, Len Brown,
	Prarit Bhargava, linux-doc, linux-kernel

On Wed, 2025-02-12 at 18:09 +0800, kernel test robot wrote:
>    drivers/idle/intel_idle.c: In function 'intel_idle_init':
> > > drivers/idle/intel_idle.c:2289:27: error: 'no_acpi' undeclared (first use
> > > in this function); did you mean 'no_action'?
>     2289 |         if (no_native && !no_acpi) {
>          |                           ^~~~~~~
>          |                           no_action
>    drivers/idle/intel_idle.c:2289:27: note: each undeclared identifier is
> reported only once for each function it appears in

David, this must be the !CONFIG_ACPI_PROCESSOR_CSTATE case.

Thanks!

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] intel_idle: introduce 'no_native' module parameter
  2025-02-12 11:32     ` Artem Bityutskiy
@ 2025-02-12 12:41       ` David Arcari
  2025-02-12 12:46         ` Artem Bityutskiy
  2025-02-12 12:49         ` Artem Bityutskiy
  0 siblings, 2 replies; 32+ messages in thread
From: David Arcari @ 2025-02-12 12:41 UTC (permalink / raw)
  To: dedekind1, kernel test robot, linux-pm
  Cc: oe-kbuild-all, Jonathan Corbet, Jacob Pan, Len Brown,
	Prarit Bhargava, linux-doc, linux-kernel


Hi Artem,

On 2/12/25 6:32 AM, Artem Bityutskiy wrote:
> On Wed, 2025-02-12 at 18:09 +0800, kernel test robot wrote:
>>     drivers/idle/intel_idle.c: In function 'intel_idle_init':
>>>> drivers/idle/intel_idle.c:2289:27: error: 'no_acpi' undeclared (first use
>>>> in this function); did you mean 'no_action'?
>>      2289 |         if (no_native && !no_acpi) {
>>           |                           ^~~~~~~
>>           |                           no_action
>>     drivers/idle/intel_idle.c:2289:27: note: each undeclared identifier is
>> reported only once for each function it appears in
> 
> David, this must be the !CONFIG_ACPI_PROCESSOR_CSTATE case.
> 
> Thanks!

Oh - I see the problem.

After a quick look I see two options:

- #ifdef the code that doesn't compile
- default no_acpi=true in the !CONFIG_ACPI_PROCESSOR_CSTATE case

I sort of like the second option better, but I worry about the 
documentation.  Specifically:

"In the case that ACPI is not configured these flags have no impact
+on functionality."

I guess that is still true.

Perhaps there is a better option.  What do you think?

Thanks,
-DA


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] intel_idle: introduce 'no_native' module parameter
  2025-02-12 12:41       ` David Arcari
@ 2025-02-12 12:46         ` Artem Bityutskiy
  2025-02-12 12:53           ` David Arcari
  2025-02-12 12:49         ` Artem Bityutskiy
  1 sibling, 1 reply; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-12 12:46 UTC (permalink / raw)
  To: David Arcari, kernel test robot, linux-pm
  Cc: oe-kbuild-all, Jonathan Corbet, Jacob Pan, Len Brown,
	Prarit Bhargava, linux-doc, linux-kernel

On Wed, 2025-02-12 at 07:41 -0500, David Arcari wrote:
> - #ifdef the code that doesn't compile
> - default no_acpi=true in the !CONFIG_ACPI_PROCESSOR_CSTATE case
> 
> I sort of like the second option better, but I worry about the 
> documentation.  Specifically:
> 
> "In the case that ACPI is not configured these flags have no impact
> +on functionality."
> 
> I guess that is still true.
> 
> Perhaps there is a better option.  What do you think?

I've not been involved into kernel that much for long time. In old days
sprinkling #ifdefs around was an anti-pattern. Most probably nowadays too. So
the second option sounds better to me.

Artem.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] intel_idle: introduce 'no_native' module parameter
  2025-02-12 12:41       ` David Arcari
  2025-02-12 12:46         ` Artem Bityutskiy
@ 2025-02-12 12:49         ` Artem Bityutskiy
  1 sibling, 0 replies; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-12 12:49 UTC (permalink / raw)
  To: David Arcari, kernel test robot, linux-pm
  Cc: oe-kbuild-all, Jonathan Corbet, Jacob Pan, Len Brown,
	Prarit Bhargava, linux-doc, linux-kernel

On Wed, 2025-02-12 at 07:41 -0500, David Arcari wrote:
> - #ifdef the code that doesn't compile
> - default no_acpi=true in the !CONFIG_ACPI_PROCESSOR_CSTATE case
> 
> I sort of like the second option better, but I worry about the 
> documentation.  Specifically:

The most important part is build !CONFIG_ACPI_PROCESSOR_CSTATE, run and test
that it works.

Thanks!

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] intel_idle: introduce 'no_native' module parameter
  2025-02-12 12:46         ` Artem Bityutskiy
@ 2025-02-12 12:53           ` David Arcari
  0 siblings, 0 replies; 32+ messages in thread
From: David Arcari @ 2025-02-12 12:53 UTC (permalink / raw)
  To: dedekind1, kernel test robot, linux-pm
  Cc: oe-kbuild-all, Jonathan Corbet, Jacob Pan, Len Brown,
	Prarit Bhargava, linux-doc, linux-kernel



On 2/12/25 7:46 AM, Artem Bityutskiy wrote:
> On Wed, 2025-02-12 at 07:41 -0500, David Arcari wrote:
>> - #ifdef the code that doesn't compile
>> - default no_acpi=true in the !CONFIG_ACPI_PROCESSOR_CSTATE case
>>
>> I sort of like the second option better, but I worry about the
>> documentation.  Specifically:
>>
>> "In the case that ACPI is not configured these flags have no impact
>> +on functionality."
>>
>> I guess that is still true.
>>
>> Perhaps there is a better option.  What do you think?
> 
> I've not been involved into kernel that much for long time. In old days
> sprinkling #ifdefs around was an anti-pattern. Most probably nowadays too. So
> the second option sounds better to me.

Another option would be to change the offending code to a function call:

if (ignore_native()) {

And have ignore_native() always return false when ACPI is not configured.

And yes I should have built and tested the kernel with ACPI disabled. 
My apologies.

I will do that for v4.

-DA

> 
> Artem.
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] intel_idle: introduce 'no_native' module parameter
  2025-02-11 13:27 ` [PATCH v3] " David Arcari
  2025-02-12  7:04   ` Artem Bityutskiy
  2025-02-12 10:09   ` kernel test robot
@ 2025-02-13 11:49   ` kernel test robot
  2 siblings, 0 replies; 32+ messages in thread
From: kernel test robot @ 2025-02-13 11:49 UTC (permalink / raw)
  To: David Arcari, linux-pm
  Cc: llvm, oe-kbuild-all, David Arcari, Jonathan Corbet, Jacob Pan,
	Len Brown, Artem Bityutskiy, Prarit Bhargava, linux-doc,
	linux-kernel

Hi David,

kernel test robot noticed the following build errors:

[auto build test ERROR on acpi/next]
[also build test ERROR on amd-pstate/linux-next amd-pstate/bleeding-edge linus/master v6.14-rc2 next-20250213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Arcari/intel_idle-introduce-no_native-module-parameter/20250211-213031
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git next
patch link:    https://lore.kernel.org/r/20250211132741.99944-1-darcari%40redhat.com
patch subject: [PATCH v3] intel_idle: introduce 'no_native' module parameter
config: i386-buildonly-randconfig-002-20250213 (https://download.01.org/0day-ci/archive/20250213/202502131953.d3fHaDCE-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250213/202502131953.d3fHaDCE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502131953.d3fHaDCE-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from drivers/idle/intel_idle.c:48:
   In file included from include/trace/events/power.h:12:
   In file included from include/linux/trace_events.h:6:
   In file included from include/linux/ring_buffer.h:5:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/idle/intel_idle.c:2289:20: error: use of undeclared identifier 'no_acpi'; did you mean 'no_action'?
    2289 |         if (no_native && !no_acpi) {
         |                           ^~~~~~~
         |                           no_action
   include/linux/interrupt.h:138:20: note: 'no_action' declared here
     138 | extern irqreturn_t no_action(int cpl, void *dev_id);
         |                    ^
>> drivers/idle/intel_idle.c:2289:20: warning: address of function 'no_action' will always evaluate to 'true' [-Wpointer-bool-conversion]
    2289 |         if (no_native && !no_acpi) {
         |                          ~^~~~~~~
   drivers/idle/intel_idle.c:2289:20: note: prefix with the address-of operator to silence this warning
    2289 |         if (no_native && !no_acpi) {
         |                           ^
         |                           &
   2 warnings and 1 error generated.


vim +2289 drivers/idle/intel_idle.c

  2248	
  2249	static int __init intel_idle_init(void)
  2250	{
  2251		const struct x86_cpu_id *id;
  2252		unsigned int eax, ebx, ecx;
  2253		int retval;
  2254	
  2255		/* Do not load intel_idle at all for now if idle= is passed */
  2256		if (boot_option_idle_override != IDLE_NO_OVERRIDE)
  2257			return -ENODEV;
  2258	
  2259		if (max_cstate == 0) {
  2260			pr_debug("disabled\n");
  2261			return -EPERM;
  2262		}
  2263	
  2264		id = x86_match_cpu(intel_idle_ids);
  2265		if (id) {
  2266			if (!boot_cpu_has(X86_FEATURE_MWAIT)) {
  2267				pr_debug("Please enable MWAIT in BIOS SETUP\n");
  2268				return -ENODEV;
  2269			}
  2270		} else {
  2271			id = x86_match_cpu(intel_mwait_ids);
  2272			if (!id)
  2273				return -ENODEV;
  2274		}
  2275	
  2276		if (boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
  2277			return -ENODEV;
  2278	
  2279		cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &mwait_substates);
  2280	
  2281		if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) ||
  2282		    !(ecx & CPUID5_ECX_INTERRUPT_BREAK) ||
  2283		    !mwait_substates)
  2284				return -ENODEV;
  2285	
  2286		pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
  2287	
  2288		icpu = (const struct idle_cpu *)id->driver_data;
> 2289		if (no_native && !no_acpi) {
  2290			if (icpu) {
  2291				pr_debug("ignoring native cpu idle states\n");
  2292				icpu = NULL;
  2293			}
  2294		}
  2295		if (icpu) {
  2296			if (icpu->state_table)
  2297				cpuidle_state_table = icpu->state_table;
  2298			else if (!intel_idle_acpi_cst_extract())
  2299				return -ENODEV;
  2300	
  2301			auto_demotion_disable_flags = icpu->auto_demotion_disable_flags;
  2302			if (icpu->disable_promotion_to_c1e)
  2303				c1e_promotion = C1E_PROMOTION_DISABLE;
  2304			if (icpu->use_acpi || force_use_acpi)
  2305				intel_idle_acpi_cst_extract();
  2306		} else if (!intel_idle_acpi_cst_extract()) {
  2307			return -ENODEV;
  2308		}
  2309	
  2310		pr_debug("v" INTEL_IDLE_VERSION " model 0x%X\n",
  2311			 boot_cpu_data.x86_model);
  2312	
  2313		intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
  2314		if (!intel_idle_cpuidle_devices)
  2315			return -ENOMEM;
  2316	
  2317		intel_idle_cpuidle_driver_init(&intel_idle_driver);
  2318	
  2319		retval = cpuidle_register_driver(&intel_idle_driver);
  2320		if (retval) {
  2321			struct cpuidle_driver *drv = cpuidle_get_driver();
  2322			printk(KERN_DEBUG pr_fmt("intel_idle yielding to %s\n"),
  2323			       drv ? drv->name : "none");
  2324			goto init_driver_fail;
  2325		}
  2326	
  2327		retval = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "idle/intel:online",
  2328					   intel_idle_cpu_online, NULL);
  2329		if (retval < 0)
  2330			goto hp_setup_fail;
  2331	
  2332		pr_debug("Local APIC timer is reliable in %s\n",
  2333			 boot_cpu_has(X86_FEATURE_ARAT) ? "all C-states" : "C1");
  2334	
  2335		return 0;
  2336	
  2337	hp_setup_fail:
  2338		intel_idle_cpuidle_devices_uninit();
  2339		cpuidle_unregister_driver(&intel_idle_driver);
  2340	init_driver_fail:
  2341		free_percpu(intel_idle_cpuidle_devices);
  2342		return retval;
  2343	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v4] intel_idle: introduce 'no_native' module parameter
  2025-01-28 14:11 [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter David Arcari
                   ` (2 preceding siblings ...)
  2025-02-11 13:27 ` [PATCH v3] " David Arcari
@ 2025-02-13 16:07 ` David Arcari
  2025-02-18 19:57   ` Rafael J. Wysocki
  2025-02-19 21:27   ` Rafael J. Wysocki
  2025-02-20 15:11 ` [PATCH v5] " David Arcari
  4 siblings, 2 replies; 32+ messages in thread
From: David Arcari @ 2025-02-13 16:07 UTC (permalink / raw)
  To: linux-pm
  Cc: David Arcari, Jonathan Corbet, Jacob Pan, Len Brown,
	Artem Bityutskiy, Prarit Bhargava, linux-doc, linux-kernel

Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
without C-state tables") the intel_idle driver has had the ability to use
the ACPI _CST to populate C-states when the processor model is not
recognized. However, even when the processor model is recognized (native
mode) there are cases where it is useful to make the driver ignore the per
cpu idle states in lieu of ACPI C-states (such as specific application
performance). Add the 'no_native' module parameter to provide this
functionality.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: David Arcari <darcari@redhat.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: David Arcari <darcari@redhat.com>
---
v4: fix !CONFIG_ACPI_PROCESSOR_CSTATE compilation issue
v3: more documentation cleanup
v2: renamed parameter, cleaned up documentation

Documentation/admin-guide/pm/intel_idle.rst | 18 +++++++++++++-----
 drivers/idle/intel_idle.c                   | 16 ++++++++++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
index 39bd6ecce7de..5940528146eb 100644
--- a/Documentation/admin-guide/pm/intel_idle.rst
+++ b/Documentation/admin-guide/pm/intel_idle.rst
@@ -192,11 +192,19 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in
 Documentation/admin-guide/pm/cpuidle.rst).
 Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.
 
-The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle``
-if the kernel has been configured with ACPI support) can be set to make the
-driver ignore the system's ACPI tables entirely or use them for all of the
-recognized processor models, respectively (they both are unset by default and
-``use_acpi`` has no effect if ``no_acpi`` is set).
+The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
+recognized by ``intel_idle`` if the kernel has been configured with ACPI
+support.  In the case that ACPI is not configured these flags have no impact
+on functionality.
+
+``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
+ACPI mode.
+
+``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tables for
+C-states on/off status in native mode.
+
+``no_native`` - Work only in ACPI mode, no native mode available (ignore
+all custom tables).
 
 The value of the ``states_off`` module parameter (0 by default) represents a
 list of idle states to be disabled by default in the form of a bitmask.
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 118fe1d37c22..b0be5ef43ffc 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
 module_param_named(use_acpi, force_use_acpi, bool, 0444);
 MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
 
+static bool no_native __read_mostly; /* No effect if no_acpi is set. */
+module_param_named(no_native, no_native, bool, 0444);
+MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states");
+
 static struct acpi_processor_power acpi_state_table __initdata;
 
 /**
@@ -1834,6 +1838,11 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
 	}
 	return true;
 }
+
+static inline bool ignore_native(void)
+{
+	return no_native & !no_acpi;
+}
 #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
 #define force_use_acpi	(false)
 
@@ -1843,6 +1852,7 @@ static inline bool intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
 {
 	return false;
 }
+static inline bool ignore_native(void) { return false; }
 #endif /* !CONFIG_ACPI_PROCESSOR_CSTATE */
 
 /**
@@ -2328,6 +2338,12 @@ static int __init intel_idle_init(void)
 	pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
 
 	icpu = (const struct idle_cpu *)id->driver_data;
+	if (ignore_native()) {
+		if (icpu) {
+			pr_debug("ignoring native cpu idle states\n");
+			icpu = NULL;
+		}
+	}
 	if (icpu) {
 		if (icpu->state_table)
 			cpuidle_state_table = icpu->state_table;
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v4] intel_idle: introduce 'no_native' module parameter
  2025-02-13 16:07 ` [PATCH v4] " David Arcari
@ 2025-02-18 19:57   ` Rafael J. Wysocki
  2025-02-20 12:50     ` Artem Bityutskiy
  2025-02-19 21:27   ` Rafael J. Wysocki
  1 sibling, 1 reply; 32+ messages in thread
From: Rafael J. Wysocki @ 2025-02-18 19:57 UTC (permalink / raw)
  To: David Arcari, Artem Bityutskiy
  Cc: linux-pm, Jonathan Corbet, Jacob Pan, Len Brown, Prarit Bhargava,
	linux-doc, linux-kernel

On Thu, Feb 13, 2025 at 5:07 PM David Arcari <darcari@redhat.com> wrote:
>
> Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
> without C-state tables") the intel_idle driver has had the ability to use
> the ACPI _CST to populate C-states when the processor model is not
> recognized. However, even when the processor model is recognized (native
> mode) there are cases where it is useful to make the driver ignore the per
> cpu idle states in lieu of ACPI C-states (such as specific application
> performance). Add the 'no_native' module parameter to provide this
> functionality.
>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: David Arcari <darcari@redhat.com>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: David Arcari <darcari@redhat.com>
> ---
> v4: fix !CONFIG_ACPI_PROCESSOR_CSTATE compilation issue

Artem, have all of your comments been addressed in this version?

> v3: more documentation cleanup
> v2: renamed parameter, cleaned up documentation
>
> Documentation/admin-guide/pm/intel_idle.rst | 18 +++++++++++++-----
>  drivers/idle/intel_idle.c                   | 16 ++++++++++++++++
>  2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
> index 39bd6ecce7de..5940528146eb 100644
> --- a/Documentation/admin-guide/pm/intel_idle.rst
> +++ b/Documentation/admin-guide/pm/intel_idle.rst
> @@ -192,11 +192,19 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in
>  Documentation/admin-guide/pm/cpuidle.rst).
>  Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.
>
> -The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle``
> -if the kernel has been configured with ACPI support) can be set to make the
> -driver ignore the system's ACPI tables entirely or use them for all of the
> -recognized processor models, respectively (they both are unset by default and
> -``use_acpi`` has no effect if ``no_acpi`` is set).
> +The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
> +recognized by ``intel_idle`` if the kernel has been configured with ACPI
> +support.  In the case that ACPI is not configured these flags have no impact
> +on functionality.
> +
> +``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
> +ACPI mode.
> +
> +``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tables for
> +C-states on/off status in native mode.
> +
> +``no_native`` - Work only in ACPI mode, no native mode available (ignore
> +all custom tables).
>
>  The value of the ``states_off`` module parameter (0 by default) represents a
>  list of idle states to be disabled by default in the form of a bitmask.
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 118fe1d37c22..b0be5ef43ffc 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
>  module_param_named(use_acpi, force_use_acpi, bool, 0444);
>  MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
>
> +static bool no_native __read_mostly; /* No effect if no_acpi is set. */
> +module_param_named(no_native, no_native, bool, 0444);
> +MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states");
> +
>  static struct acpi_processor_power acpi_state_table __initdata;
>
>  /**
> @@ -1834,6 +1838,11 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
>         }
>         return true;
>  }
> +
> +static inline bool ignore_native(void)
> +{
> +       return no_native & !no_acpi;
> +}
>  #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
>  #define force_use_acpi (false)
>
> @@ -1843,6 +1852,7 @@ static inline bool intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
>  {
>         return false;
>  }
> +static inline bool ignore_native(void) { return false; }
>  #endif /* !CONFIG_ACPI_PROCESSOR_CSTATE */
>
>  /**
> @@ -2328,6 +2338,12 @@ static int __init intel_idle_init(void)
>         pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
>
>         icpu = (const struct idle_cpu *)id->driver_data;
> +       if (ignore_native()) {
> +               if (icpu) {
> +                       pr_debug("ignoring native cpu idle states\n");
> +                       icpu = NULL;
> +               }
> +       }
>         if (icpu) {
>                 if (icpu->state_table)
>                         cpuidle_state_table = icpu->state_table;
> --
> 2.48.1
>
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4] intel_idle: introduce 'no_native' module parameter
  2025-02-13 16:07 ` [PATCH v4] " David Arcari
  2025-02-18 19:57   ` Rafael J. Wysocki
@ 2025-02-19 21:27   ` Rafael J. Wysocki
  2025-02-20 12:21     ` David Arcari
  1 sibling, 1 reply; 32+ messages in thread
From: Rafael J. Wysocki @ 2025-02-19 21:27 UTC (permalink / raw)
  To: David Arcari
  Cc: linux-pm, Jonathan Corbet, Len Brown, Artem Bityutskiy,
	Prarit Bhargava, linux-doc, linux-kernel

On Thu, Feb 13, 2025 at 5:07 PM David Arcari <darcari@redhat.com> wrote:
>
> Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
> without C-state tables") the intel_idle driver has had the ability to use
> the ACPI _CST to populate C-states when the processor model is not
> recognized. However, even when the processor model is recognized (native
> mode) there are cases where it is useful to make the driver ignore the per
> cpu idle states in lieu of ACPI C-states (such as specific application
> performance). Add the 'no_native' module parameter to provide this
> functionality.
>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: David Arcari <darcari@redhat.com>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: David Arcari <darcari@redhat.com>
> ---
> v4: fix !CONFIG_ACPI_PROCESSOR_CSTATE compilation issue
> v3: more documentation cleanup
> v2: renamed parameter, cleaned up documentation
>
> Documentation/admin-guide/pm/intel_idle.rst | 18 +++++++++++++-----
>  drivers/idle/intel_idle.c                   | 16 ++++++++++++++++
>  2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
> index 39bd6ecce7de..5940528146eb 100644
> --- a/Documentation/admin-guide/pm/intel_idle.rst
> +++ b/Documentation/admin-guide/pm/intel_idle.rst
> @@ -192,11 +192,19 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in
>  Documentation/admin-guide/pm/cpuidle.rst).
>  Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.
>
> -The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle``
> -if the kernel has been configured with ACPI support) can be set to make the
> -driver ignore the system's ACPI tables entirely or use them for all of the
> -recognized processor models, respectively (they both are unset by default and
> -``use_acpi`` has no effect if ``no_acpi`` is set).
> +The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
> +recognized by ``intel_idle`` if the kernel has been configured with ACPI
> +support.  In the case that ACPI is not configured these flags have no impact
> +on functionality.
> +
> +``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
> +ACPI mode.
> +
> +``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tables for
> +C-states on/off status in native mode.
> +
> +``no_native`` - Work only in ACPI mode, no native mode available (ignore
> +all custom tables).
>
>  The value of the ``states_off`` module parameter (0 by default) represents a
>  list of idle states to be disabled by default in the form of a bitmask.
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 118fe1d37c22..b0be5ef43ffc 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
>  module_param_named(use_acpi, force_use_acpi, bool, 0444);
>  MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
>
> +static bool no_native __read_mostly; /* No effect if no_acpi is set. */
> +module_param_named(no_native, no_native, bool, 0444);
> +MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states");
> +
>  static struct acpi_processor_power acpi_state_table __initdata;
>
>  /**
> @@ -1834,6 +1838,11 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
>         }
>         return true;
>  }
> +
> +static inline bool ignore_native(void)
> +{
> +       return no_native & !no_acpi;
> +}
>  #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
>  #define force_use_acpi (false)
>
> @@ -1843,6 +1852,7 @@ static inline bool intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
>  {
>         return false;
>  }
> +static inline bool ignore_native(void) { return false; }
>  #endif /* !CONFIG_ACPI_PROCESSOR_CSTATE */
>
>  /**
> @@ -2328,6 +2338,12 @@ static int __init intel_idle_init(void)
>         pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
>
>         icpu = (const struct idle_cpu *)id->driver_data;
> +       if (ignore_native()) {
> +               if (icpu) {
> +                       pr_debug("ignoring native cpu idle states\n");
> +                       icpu = NULL;
> +               }
> +       }

Why not

+       if (icpu && ignore_native()) {
+              pr_debug("disregarding built-in CPU idle states table\n");
+              icpu = NULL;
+       }

>         if (icpu) {
>                 if (icpu->state_table)
>                         cpuidle_state_table = icpu->state_table;
> --

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4] intel_idle: introduce 'no_native' module parameter
  2025-02-19 21:27   ` Rafael J. Wysocki
@ 2025-02-20 12:21     ` David Arcari
  2025-02-20 12:23       ` Rafael J. Wysocki
  0 siblings, 1 reply; 32+ messages in thread
From: David Arcari @ 2025-02-20 12:21 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-pm, Jonathan Corbet, Len Brown, Artem Bityutskiy,
	Prarit Bhargava, linux-doc, linux-kernel


Hi Rafael,

On 2/19/25 4:27 PM, Rafael J. Wysocki wrote:
> On Thu, Feb 13, 2025 at 5:07 PM David Arcari <darcari@redhat.com> wrote:
>>
>> Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
>> without C-state tables") the intel_idle driver has had the ability to use
>> the ACPI _CST to populate C-states when the processor model is not
>> recognized. However, even when the processor model is recognized (native
>> mode) there are cases where it is useful to make the driver ignore the per
>> cpu idle states in lieu of ACPI C-states (such as specific application
>> performance). Add the 'no_native' module parameter to provide this
>> functionality.
>>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: David Arcari <darcari@redhat.com>
>> Cc: Artem Bityutskiy <dedekind1@gmail.com>
>> Cc: Prarit Bhargava <prarit@redhat.com>
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: David Arcari <darcari@redhat.com>
>> ---
>> v4: fix !CONFIG_ACPI_PROCESSOR_CSTATE compilation issue
>> v3: more documentation cleanup
>> v2: renamed parameter, cleaned up documentation
>>
>> Documentation/admin-guide/pm/intel_idle.rst | 18 +++++++++++++-----
>>   drivers/idle/intel_idle.c                   | 16 ++++++++++++++++
>>   2 files changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
>> index 39bd6ecce7de..5940528146eb 100644
>> --- a/Documentation/admin-guide/pm/intel_idle.rst
>> +++ b/Documentation/admin-guide/pm/intel_idle.rst
>> @@ -192,11 +192,19 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in
>>   Documentation/admin-guide/pm/cpuidle.rst).
>>   Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.
>>
>> -The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle``
>> -if the kernel has been configured with ACPI support) can be set to make the
>> -driver ignore the system's ACPI tables entirely or use them for all of the
>> -recognized processor models, respectively (they both are unset by default and
>> -``use_acpi`` has no effect if ``no_acpi`` is set).
>> +The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
>> +recognized by ``intel_idle`` if the kernel has been configured with ACPI
>> +support.  In the case that ACPI is not configured these flags have no impact
>> +on functionality.
>> +
>> +``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
>> +ACPI mode.
>> +
>> +``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tables for
>> +C-states on/off status in native mode.
>> +
>> +``no_native`` - Work only in ACPI mode, no native mode available (ignore
>> +all custom tables).
>>
>>   The value of the ``states_off`` module parameter (0 by default) represents a
>>   list of idle states to be disabled by default in the form of a bitmask.
>> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
>> index 118fe1d37c22..b0be5ef43ffc 100644
>> --- a/drivers/idle/intel_idle.c
>> +++ b/drivers/idle/intel_idle.c
>> @@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
>>   module_param_named(use_acpi, force_use_acpi, bool, 0444);
>>   MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
>>
>> +static bool no_native __read_mostly; /* No effect if no_acpi is set. */
>> +module_param_named(no_native, no_native, bool, 0444);
>> +MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states");
>> +
>>   static struct acpi_processor_power acpi_state_table __initdata;
>>
>>   /**
>> @@ -1834,6 +1838,11 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
>>          }
>>          return true;
>>   }
>> +
>> +static inline bool ignore_native(void)
>> +{
>> +       return no_native & !no_acpi;
>> +}
>>   #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
>>   #define force_use_acpi (false)
>>
>> @@ -1843,6 +1852,7 @@ static inline bool intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
>>   {
>>          return false;
>>   }
>> +static inline bool ignore_native(void) { return false; }
>>   #endif /* !CONFIG_ACPI_PROCESSOR_CSTATE */
>>
>>   /**
>> @@ -2328,6 +2338,12 @@ static int __init intel_idle_init(void)
>>          pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
>>
>>          icpu = (const struct idle_cpu *)id->driver_data;
>> +       if (ignore_native()) {
>> +               if (icpu) {
>> +                       pr_debug("ignoring native cpu idle states\n");
>> +                       icpu = NULL;
>> +               }
>> +       }
> 
> Why not
> 
> +       if (icpu && ignore_native()) {
> +              pr_debug("disregarding built-in CPU idle states table\n");
> +              icpu = NULL;
> +       }

That's cleaner.  I'll submit a v5 shortly.

Should the pr_debug be a pr_info?  I've waffled on this, but think that 
pr_info is probably a good idea.  Or do you prefer pr_debug?

Thanks,
-DA

> 
>>          if (icpu) {
>>                  if (icpu->state_table)
>>                          cpuidle_state_table = icpu->state_table;
>> --
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4] intel_idle: introduce 'no_native' module parameter
  2025-02-20 12:21     ` David Arcari
@ 2025-02-20 12:23       ` Rafael J. Wysocki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafael J. Wysocki @ 2025-02-20 12:23 UTC (permalink / raw)
  To: David Arcari
  Cc: Rafael J. Wysocki, linux-pm, Jonathan Corbet, Len Brown,
	Artem Bityutskiy, Prarit Bhargava, linux-doc, linux-kernel

Hi David,

On Thu, Feb 20, 2025 at 1:21 PM David Arcari <darcari@redhat.com> wrote:
>
>
> Hi Rafael,
>
> On 2/19/25 4:27 PM, Rafael J. Wysocki wrote:
> > On Thu, Feb 13, 2025 at 5:07 PM David Arcari <darcari@redhat.com> wrote:
> >>
> >> Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
> >> without C-state tables") the intel_idle driver has had the ability to use
> >> the ACPI _CST to populate C-states when the processor model is not
> >> recognized. However, even when the processor model is recognized (native
> >> mode) there are cases where it is useful to make the driver ignore the per
> >> cpu idle states in lieu of ACPI C-states (such as specific application
> >> performance). Add the 'no_native' module parameter to provide this
> >> functionality.
> >>
> >> Cc: Jonathan Corbet <corbet@lwn.net>
> >> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> >> Cc: Len Brown <lenb@kernel.org>
> >> Cc: David Arcari <darcari@redhat.com>
> >> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> >> Cc: Prarit Bhargava <prarit@redhat.com>
> >> Cc: linux-doc@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: David Arcari <darcari@redhat.com>
> >> ---
> >> v4: fix !CONFIG_ACPI_PROCESSOR_CSTATE compilation issue
> >> v3: more documentation cleanup
> >> v2: renamed parameter, cleaned up documentation
> >>
> >> Documentation/admin-guide/pm/intel_idle.rst | 18 +++++++++++++-----
> >>   drivers/idle/intel_idle.c                   | 16 ++++++++++++++++
> >>   2 files changed, 29 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
> >> index 39bd6ecce7de..5940528146eb 100644
> >> --- a/Documentation/admin-guide/pm/intel_idle.rst
> >> +++ b/Documentation/admin-guide/pm/intel_idle.rst
> >> @@ -192,11 +192,19 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in
> >>   Documentation/admin-guide/pm/cpuidle.rst).
> >>   Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.
> >>
> >> -The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle``
> >> -if the kernel has been configured with ACPI support) can be set to make the
> >> -driver ignore the system's ACPI tables entirely or use them for all of the
> >> -recognized processor models, respectively (they both are unset by default and
> >> -``use_acpi`` has no effect if ``no_acpi`` is set).
> >> +The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
> >> +recognized by ``intel_idle`` if the kernel has been configured with ACPI
> >> +support.  In the case that ACPI is not configured these flags have no impact
> >> +on functionality.
> >> +
> >> +``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
> >> +ACPI mode.
> >> +
> >> +``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tables for
> >> +C-states on/off status in native mode.
> >> +
> >> +``no_native`` - Work only in ACPI mode, no native mode available (ignore
> >> +all custom tables).
> >>
> >>   The value of the ``states_off`` module parameter (0 by default) represents a
> >>   list of idle states to be disabled by default in the form of a bitmask.
> >> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> >> index 118fe1d37c22..b0be5ef43ffc 100644
> >> --- a/drivers/idle/intel_idle.c
> >> +++ b/drivers/idle/intel_idle.c
> >> @@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
> >>   module_param_named(use_acpi, force_use_acpi, bool, 0444);
> >>   MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
> >>
> >> +static bool no_native __read_mostly; /* No effect if no_acpi is set. */
> >> +module_param_named(no_native, no_native, bool, 0444);
> >> +MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states");
> >> +
> >>   static struct acpi_processor_power acpi_state_table __initdata;
> >>
> >>   /**
> >> @@ -1834,6 +1838,11 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
> >>          }
> >>          return true;
> >>   }
> >> +
> >> +static inline bool ignore_native(void)
> >> +{
> >> +       return no_native & !no_acpi;
> >> +}
> >>   #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
> >>   #define force_use_acpi (false)
> >>
> >> @@ -1843,6 +1852,7 @@ static inline bool intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
> >>   {
> >>          return false;
> >>   }
> >> +static inline bool ignore_native(void) { return false; }
> >>   #endif /* !CONFIG_ACPI_PROCESSOR_CSTATE */
> >>
> >>   /**
> >> @@ -2328,6 +2338,12 @@ static int __init intel_idle_init(void)
> >>          pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
> >>
> >>          icpu = (const struct idle_cpu *)id->driver_data;
> >> +       if (ignore_native()) {
> >> +               if (icpu) {
> >> +                       pr_debug("ignoring native cpu idle states\n");
> >> +                       icpu = NULL;
> >> +               }
> >> +       }
> >
> > Why not
> >
> > +       if (icpu && ignore_native()) {
> > +              pr_debug("disregarding built-in CPU idle states table\n");
> > +              icpu = NULL;
> > +       }
>
> That's cleaner.  I'll submit a v5 shortly.
>
> Should the pr_debug be a pr_info?  I've waffled on this, but think that
> pr_info is probably a good idea.  Or do you prefer pr_debug?

I think that this is debug mostly as it allows one to verify that
no_native is taken into account as expected.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4] intel_idle: introduce 'no_native' module parameter
  2025-02-18 19:57   ` Rafael J. Wysocki
@ 2025-02-20 12:50     ` Artem Bityutskiy
  0 siblings, 0 replies; 32+ messages in thread
From: Artem Bityutskiy @ 2025-02-20 12:50 UTC (permalink / raw)
  To: Rafael J. Wysocki, David Arcari
  Cc: linux-pm, Jonathan Corbet, Jacob Pan, Len Brown, Prarit Bhargava,
	linux-doc, linux-kernel

On Tue, 2025-02-18 at 20:57 +0100, Rafael J. Wysocki wrote:
> On Thu, Feb 13, 2025 at 5:07 PM David Arcari <darcari@redhat.com> wrote:
> > 
> > Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
> > without C-state tables") the intel_idle driver has had the ability to use
> > the ACPI _CST to populate C-states when the processor model is not
> > recognized. However, even when the processor model is recognized (native
> > mode) there are cases where it is useful to make the driver ignore the per
> > cpu idle states in lieu of ACPI C-states (such as specific application
> > performance). Add the 'no_native' module parameter to provide this
> > functionality.
> > 
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: David Arcari <darcari@redhat.com>
> > Cc: Artem Bityutskiy <dedekind1@gmail.com>
> > Cc: Prarit Bhargava <prarit@redhat.com>
> > Cc: linux-doc@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: David Arcari <darcari@redhat.com>
> > ---
> > v4: fix !CONFIG_ACPI_PROCESSOR_CSTATE compilation issue
> 
> Artem, have all of your comments been addressed in this version?

Hi, I was away for few days. Yes, this patch looks good to me. Works, and
useful. Granted your comment is also addressed:

Reviewed-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v5] intel_idle: introduce 'no_native' module parameter
  2025-01-28 14:11 [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter David Arcari
                   ` (3 preceding siblings ...)
  2025-02-13 16:07 ` [PATCH v4] " David Arcari
@ 2025-02-20 15:11 ` David Arcari
  2025-02-20 20:02   ` Rafael J. Wysocki
  4 siblings, 1 reply; 32+ messages in thread
From: David Arcari @ 2025-02-20 15:11 UTC (permalink / raw)
  To: linux-pm
  Cc: David Arcari, Jonathan Corbet, Jacob Pan, Len Brown,
	Artem Bityutskiy, Prarit Bhargava, linux-doc, linux-kernel

Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
without C-state tables") the intel_idle driver has had the ability to use
the ACPI _CST to populate C-states when the processor model is not
recognized. However, even when the processor model is recognized (native
mode) there are cases where it is useful to make the driver ignore the per
cpu idle states in lieu of ACPI C-states (such as specific application
performance). Add the 'no_native' module parameter to provide this
functionality.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: David Arcari <darcari@redhat.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: David Arcari <darcari@redhat.com>
---
v5: if statement simplication, also add missing '&' to ignore_native()
v4: fix !CONFIG_ACPI_PROCESSOR_CSTATE compilation issue
v3: more documentation cleanup
v2: renamed parameter, cleaned up documentation

 Documentation/admin-guide/pm/intel_idle.rst | 18 +++++++++++++-----
 drivers/idle/intel_idle.c                   | 14 ++++++++++++++
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
index 39bd6ecce7de..5940528146eb 100644
--- a/Documentation/admin-guide/pm/intel_idle.rst
+++ b/Documentation/admin-guide/pm/intel_idle.rst
@@ -192,11 +192,19 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in
 Documentation/admin-guide/pm/cpuidle.rst).
 Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.
 
-The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle``
-if the kernel has been configured with ACPI support) can be set to make the
-driver ignore the system's ACPI tables entirely or use them for all of the
-recognized processor models, respectively (they both are unset by default and
-``use_acpi`` has no effect if ``no_acpi`` is set).
+The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
+recognized by ``intel_idle`` if the kernel has been configured with ACPI
+support.  In the case that ACPI is not configured these flags have no impact
+on functionality.
+
+``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
+ACPI mode.
+
+``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tables for
+C-states on/off status in native mode.
+
+``no_native`` - Work only in ACPI mode, no native mode available (ignore
+all custom tables).
 
 The value of the ``states_off`` module parameter (0 by default) represents a
 list of idle states to be disabled by default in the form of a bitmask.
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 118fe1d37c22..d0b23ea03e6f 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
 module_param_named(use_acpi, force_use_acpi, bool, 0444);
 MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
 
+static bool no_native __read_mostly; /* No effect if no_acpi is set. */
+module_param_named(no_native, no_native, bool, 0444);
+MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states");
+
 static struct acpi_processor_power acpi_state_table __initdata;
 
 /**
@@ -1834,6 +1838,11 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
 	}
 	return true;
 }
+
+static inline bool ignore_native(void)
+{
+	return no_native && !no_acpi;
+}
 #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
 #define force_use_acpi	(false)
 
@@ -1843,6 +1852,7 @@ static inline bool intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
 {
 	return false;
 }
+static inline bool ignore_native(void) { return false; }
 #endif /* !CONFIG_ACPI_PROCESSOR_CSTATE */
 
 /**
@@ -2328,6 +2338,10 @@ static int __init intel_idle_init(void)
 	pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
 
 	icpu = (const struct idle_cpu *)id->driver_data;
+	if (icpu && ignore_native()) {
+		pr_debug("ignoring native cpu idle states\n");
+		icpu = NULL;
+	}
 	if (icpu) {
 		if (icpu->state_table)
 			cpuidle_state_table = icpu->state_table;
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v5] intel_idle: introduce 'no_native' module parameter
  2025-02-20 15:11 ` [PATCH v5] " David Arcari
@ 2025-02-20 20:02   ` Rafael J. Wysocki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafael J. Wysocki @ 2025-02-20 20:02 UTC (permalink / raw)
  To: David Arcari
  Cc: linux-pm, Jonathan Corbet, Jacob Pan, Len Brown, Artem Bityutskiy,
	Prarit Bhargava, linux-doc, linux-kernel

On Thu, Feb 20, 2025 at 4:11 PM David Arcari <darcari@redhat.com> wrote:
>
> Since commit 18734958e9bf ("intel_idle: Use ACPI _CST for processor models
> without C-state tables") the intel_idle driver has had the ability to use
> the ACPI _CST to populate C-states when the processor model is not
> recognized. However, even when the processor model is recognized (native
> mode) there are cases where it is useful to make the driver ignore the per
> cpu idle states in lieu of ACPI C-states (such as specific application
> performance). Add the 'no_native' module parameter to provide this
> functionality.
>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: David Arcari <darcari@redhat.com>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: David Arcari <darcari@redhat.com>
> ---
> v5: if statement simplication, also add missing '&' to ignore_native()
> v4: fix !CONFIG_ACPI_PROCESSOR_CSTATE compilation issue
> v3: more documentation cleanup
> v2: renamed parameter, cleaned up documentation
>
>  Documentation/admin-guide/pm/intel_idle.rst | 18 +++++++++++++-----
>  drivers/idle/intel_idle.c                   | 14 ++++++++++++++
>  2 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/pm/intel_idle.rst b/Documentation/admin-guide/pm/intel_idle.rst
> index 39bd6ecce7de..5940528146eb 100644
> --- a/Documentation/admin-guide/pm/intel_idle.rst
> +++ b/Documentation/admin-guide/pm/intel_idle.rst
> @@ -192,11 +192,19 @@ even if they have been enumerated (see :ref:`cpu-pm-qos` in
>  Documentation/admin-guide/pm/cpuidle.rst).
>  Setting ``max_cstate`` to 0 causes the ``intel_idle`` initialization to fail.
>
> -The ``no_acpi`` and ``use_acpi`` module parameters (recognized by ``intel_idle``
> -if the kernel has been configured with ACPI support) can be set to make the
> -driver ignore the system's ACPI tables entirely or use them for all of the
> -recognized processor models, respectively (they both are unset by default and
> -``use_acpi`` has no effect if ``no_acpi`` is set).
> +The ``no_acpi``, ``use_acpi`` and ``no_native`` module parameters are
> +recognized by ``intel_idle`` if the kernel has been configured with ACPI
> +support.  In the case that ACPI is not configured these flags have no impact
> +on functionality.
> +
> +``no_acpi`` - Do not use ACPI at all.  Only native mode is available, no
> +ACPI mode.
> +
> +``use_acpi`` - No-op in ACPI mode, the driver will consult ACPI tables for
> +C-states on/off status in native mode.
> +
> +``no_native`` - Work only in ACPI mode, no native mode available (ignore
> +all custom tables).
>
>  The value of the ``states_off`` module parameter (0 by default) represents a
>  list of idle states to be disabled by default in the form of a bitmask.
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 118fe1d37c22..d0b23ea03e6f 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -1695,6 +1695,10 @@ static bool force_use_acpi __read_mostly; /* No effect if no_acpi is set. */
>  module_param_named(use_acpi, force_use_acpi, bool, 0444);
>  MODULE_PARM_DESC(use_acpi, "Use ACPI _CST for building the idle states list");
>
> +static bool no_native __read_mostly; /* No effect if no_acpi is set. */
> +module_param_named(no_native, no_native, bool, 0444);
> +MODULE_PARM_DESC(no_native, "Ignore cpu specific (native) idle states in lieu of ACPI idle states");
> +
>  static struct acpi_processor_power acpi_state_table __initdata;
>
>  /**
> @@ -1834,6 +1838,11 @@ static bool __init intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
>         }
>         return true;
>  }
> +
> +static inline bool ignore_native(void)
> +{
> +       return no_native && !no_acpi;
> +}
>  #else /* !CONFIG_ACPI_PROCESSOR_CSTATE */
>  #define force_use_acpi (false)
>
> @@ -1843,6 +1852,7 @@ static inline bool intel_idle_off_by_default(unsigned int flags, u32 mwait_hint)
>  {
>         return false;
>  }
> +static inline bool ignore_native(void) { return false; }
>  #endif /* !CONFIG_ACPI_PROCESSOR_CSTATE */
>
>  /**
> @@ -2328,6 +2338,10 @@ static int __init intel_idle_init(void)
>         pr_debug("MWAIT substates: 0x%x\n", mwait_substates);
>
>         icpu = (const struct idle_cpu *)id->driver_data;
> +       if (icpu && ignore_native()) {
> +               pr_debug("ignoring native cpu idle states\n");
> +               icpu = NULL;
> +       }
>         if (icpu) {
>                 if (icpu->state_table)
>                         cpuidle_state_table = icpu->state_table;
> --

Applied as 6.15 material, thanks!

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2025-02-20 20:02 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-28 14:11 [PATCH] intel_idle: introduce 'use_acpi_cst' module parameter David Arcari
2025-02-04 12:23 ` Artem Bityutskiy
2025-02-04 12:52   ` David Arcari
2025-02-04 13:04     ` Prarit Bhargava
2025-02-04 15:21     ` Rafael J. Wysocki
2025-02-04 16:30       ` Artem Bityutskiy
2025-02-04 17:33         ` Rafael J. Wysocki
2025-02-05 12:09           ` David Arcari
2025-02-04 16:26     ` Artem Bityutskiy
2025-02-06 16:40 ` [PATCH v2] intel_idle: introduce 'no_native' " David Arcari
2025-02-07 15:55   ` Artem Bityutskiy
2025-02-07 17:13     ` David Arcari
2025-02-08 10:37       ` Artem Bityutskiy
2025-02-08 19:56         ` David Arcari
2025-02-09  9:08           ` Artem Bityutskiy
2025-02-11 13:27 ` [PATCH v3] " David Arcari
2025-02-12  7:04   ` Artem Bityutskiy
2025-02-12 10:09   ` kernel test robot
2025-02-12 11:32     ` Artem Bityutskiy
2025-02-12 12:41       ` David Arcari
2025-02-12 12:46         ` Artem Bityutskiy
2025-02-12 12:53           ` David Arcari
2025-02-12 12:49         ` Artem Bityutskiy
2025-02-13 11:49   ` kernel test robot
2025-02-13 16:07 ` [PATCH v4] " David Arcari
2025-02-18 19:57   ` Rafael J. Wysocki
2025-02-20 12:50     ` Artem Bityutskiy
2025-02-19 21:27   ` Rafael J. Wysocki
2025-02-20 12:21     ` David Arcari
2025-02-20 12:23       ` Rafael J. Wysocki
2025-02-20 15:11 ` [PATCH v5] " David Arcari
2025-02-20 20:02   ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).