[PATCH 0/5] ACPI / cpuidle: More driver cleanups

[PATCH 0/5] ACPI / cpuidle: More driver cleanups

[Rafael J. Wysocki]

Hi,

More ACPI cpuidle driver cleanups on top of the one I sent yesterday:

https://patchwork.kernel.org/patch/5734101/

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 1/5] ACPI / cpuidle: Drop unnecessary calls from ->enter callback routines

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

acpi_idle_enter_simple() and acpi_idle_enter_bm() don't need to
call sched_clock_idle_sleep/wakeup_event(), because that's taken
care of by the core already.  Namely, sched_clock_idle_sleep_event()
is called by tick_nohz_start_idle() called by __tick_nohz_idle_enter()
which in turn is called by tick_nohz_idle_enter() and that is called
by cpu_idle_loop().  Analogously for sched_clock_idle_wakeup_event().

For this reason, drop those calls from the ACPI cpuidle driver's
->enter callback routines.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/processor_idle.c |    8 --------
 1 file changed, 8 deletions(-)

Index: linux-pm/drivers/acpi/processor_idle.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_idle.c
+++ linux-pm/drivers/acpi/processor_idle.c
@@ -791,12 +791,8 @@ static int acpi_idle_enter_simple(struct
 	if (cx->type == ACPI_STATE_C3)
 		ACPI_FLUSH_CPU_CACHE();
 
-	/* Tell the scheduler that we are going deep-idle: */
-	sched_clock_idle_sleep_event();
 	acpi_idle_do_entry(cx);
 
-	sched_clock_idle_wakeup_event(0);
-
 	lapic_timer_state_broadcast(pr, cx, 0);
 	return index;
 }
@@ -842,8 +838,6 @@ static int acpi_idle_enter_bm(struct cpu
 
 	acpi_unlazy_tlb(smp_processor_id());
 
-	/* Tell the scheduler that we are going deep-idle: */
-	sched_clock_idle_sleep_event();
 	/*
 	 * Must be done before busmaster disable as we might need to
 	 * access HPET !
@@ -881,8 +875,6 @@ static int acpi_idle_enter_bm(struct cpu
 		raw_spin_unlock(&c3_lock);
 	}
 
-	sched_clock_idle_wakeup_event(0);
-
 	lapic_timer_state_broadcast(pr, cx, 0);
 	return index;
 }

[PATCH 2/5] ACPI / cpuidle: Clean up fallback to C1 checks

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

acpi_idle_enter_simple() and acpi_idle_enter_bm() both check
if C2/C3 type entry is supported on MP in the same way, so move
those checks to a separate function and call it from both
places (and it doesn't need to check if the state type is not
C1, because the functions in question won't be called otherwise).

While at it, use IS_ENABLED() for the CONFIG_HOTPLUG_CPU check.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/processor_idle.c |   19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/acpi/processor_idle.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_idle.c
+++ linux-pm/drivers/acpi/processor_idle.c
@@ -758,6 +758,13 @@ static int acpi_idle_play_dead(struct cp
 	return 0;
 }
 
+static bool acpi_idle_fallback_to_c1(struct acpi_processor *pr)
+{
+	return IS_ENABLED(CONFIG_HOTPLUG_CPU) && num_online_cpus() > 1 &&
+		!(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED) &&
+		!pr->flags.has_cst;
+}
+
 /**
  * acpi_idle_enter_simple - enters an ACPI state without BM handling
  * @dev: the target CPU
@@ -775,12 +782,8 @@ static int acpi_idle_enter_simple(struct
 	if (unlikely(!pr))
 		return -EINVAL;
 
-#ifdef CONFIG_HOTPLUG_CPU
-	if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
-	    !pr->flags.has_cst &&
-	    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
+	if (acpi_idle_fallback_to_c1(pr))
 		return acpi_idle_enter_c1(dev, drv, CPUIDLE_DRIVER_STATE_START);
-#endif
 
 	/*
 	 * Must be done before busmaster disable as we might need to
@@ -819,12 +822,8 @@ static int acpi_idle_enter_bm(struct cpu
 	if (unlikely(!pr))
 		return -EINVAL;
 
-#ifdef CONFIG_HOTPLUG_CPU
-	if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
-	    !pr->flags.has_cst &&
-	    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
+	if (acpi_idle_fallback_to_c1(pr))
 		return acpi_idle_enter_c1(dev, drv, CPUIDLE_DRIVER_STATE_START);
-#endif
 
 	if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
 		if (drv->safe_state_index >= 0) {

[PATCH 3/5] ACPI / cpuidle: Drop irrelevant comment from acpi_idle_enter_simple()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The comment about bus master disable in acpi_idle_enter_simple() is
irrelevant, because the function doesn't disable bus mastering, so
drop it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/processor_idle.c |    4 ----
 1 file changed, 4 deletions(-)

Index: linux-pm/drivers/acpi/processor_idle.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_idle.c
+++ linux-pm/drivers/acpi/processor_idle.c
@@ -785,10 +785,6 @@ static int acpi_idle_enter_simple(struct
 	if (acpi_idle_fallback_to_c1(pr))
 		return acpi_idle_enter_c1(dev, drv, CPUIDLE_DRIVER_STATE_START);
 
-	/*
-	 * Must be done before busmaster disable as we might need to
-	 * access HPET !
-	 */
 	lapic_timer_state_broadcast(pr, cx, 1);
 
 	if (cx->type == ACPI_STATE_C3)

[PATCH 4/5] ACPI / cpuidle: Clean up white space in a switch statement

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

White space in the switch statement in acpi_processor_setup_cpuidle_states()
does not adhere to the kernel coding style and that makes the code
difficult to read.  Clean that up.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/processor_idle.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/acpi/processor_idle.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_idle.c
+++ linux-pm/drivers/acpi/processor_idle.c
@@ -968,20 +968,20 @@ static int acpi_processor_setup_cpuidle_
 
 		state->flags = 0;
 		switch (cx->type) {
-			case ACPI_STATE_C1:
 
+		case ACPI_STATE_C1:
 			state->enter = acpi_idle_enter_c1;
 			state->enter_dead = acpi_idle_play_dead;
 			drv->safe_state_index = count;
 			break;
 
-			case ACPI_STATE_C2:
+		case ACPI_STATE_C2:
 			state->enter = acpi_idle_enter_simple;
 			state->enter_dead = acpi_idle_play_dead;
 			drv->safe_state_index = count;
 			break;
 
-			case ACPI_STATE_C3:
+		case ACPI_STATE_C3:
 			state->enter = pr->flags.bm_check ?
 					acpi_idle_enter_bm :
 					acpi_idle_enter_simple;

[PATCH 5/5] ACPI / cpuidle: Drop flags.bm_check tests from acpi_idle_enter_bm()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since acpi_idle_enter_bm() is only used if flags.bm_check is set for
the given acpi_processor object, it doesn't make sense to check that
flag in there.

For this reason, drop flags.bm_check tests (and some code depending
on them) from acpi_idle_enter_bm().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/processor_idle.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/acpi/processor_idle.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_idle.c
+++ linux-pm/drivers/acpi/processor_idle.c
@@ -842,28 +842,25 @@ static int acpi_idle_enter_bm(struct cpu
 	/*
 	 * disable bus master
 	 * bm_check implies we need ARB_DIS
-	 * !bm_check implies we need cache flush
 	 * bm_control implies whether we can do ARB_DIS
 	 *
 	 * That leaves a case where bm_check is set and bm_control is
 	 * not set. In that case we cannot do much, we enter C3
 	 * without doing anything.
 	 */
-	if (pr->flags.bm_check && pr->flags.bm_control) {
+	if (pr->flags.bm_control) {
 		raw_spin_lock(&c3_lock);
 		c3_cpu_count++;
 		/* Disable bus master arbitration when all CPUs are in C3 */
 		if (c3_cpu_count == num_online_cpus())
 			acpi_write_bit_register(ACPI_BITREG_ARB_DISABLE, 1);
 		raw_spin_unlock(&c3_lock);
-	} else if (!pr->flags.bm_check) {
-		ACPI_FLUSH_CPU_CACHE();
 	}
 
 	acpi_idle_do_entry(cx);
 
 	/* Re-enable bus master arbitration */
-	if (pr->flags.bm_check && pr->flags.bm_control) {
+	if (pr->flags.bm_control) {
 		raw_spin_lock(&c3_lock);
 		acpi_write_bit_register(ACPI_BITREG_ARB_DISABLE, 0);
 		c3_cpu_count--;