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


Groups > linux.kernel > #1401029

[PATCH 4/5] cpufreq: Split cpufreq_governor() into simpler functions

From "Rafael J. Wysocki" <rjw@rjwysocki.net>
Newsgroups linux.kernel
Subject [PATCH 4/5] cpufreq: Split cpufreq_governor() into simpler functions
Date 2016-05-14 01:10 +0200
Message-ID <ryunv-4M0-9@gated-at.bofh.it> (permalink)
References <ryudP-4h8-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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

The cpufreq_governor() routine is used by the cpufreq core to invoke
the current governor's ->governor() callback with appropriate arguments
and do some housekeeping related to that.  Unfortunately, the way it
mixes different governor events in one code path makes it rather hard
to follow the code.

For this reason, split cpufreq_governor() into five simpler functions
that each will handle just one specific governor event and put all of
the code related to the given event into its own function.

This change is a prerequisite for a redesign of the cpufreq governor
API that will be done subsequently.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq.c |  115 ++++++++++++++++++++++++++++------------------
 1 file changed, 71 insertions(+), 44 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -74,19 +74,12 @@ static inline bool has_target(void)
 }
 
 /* internal prototypes */
-static int cpufreq_governor(struct cpufreq_policy *policy, unsigned int event);
 static unsigned int __cpufreq_get(struct cpufreq_policy *policy);
+static int cpufreq_init_governor(struct cpufreq_policy *policy);
+static void cpufreq_exit_governor(struct cpufreq_policy *policy);
 static int cpufreq_start_governor(struct cpufreq_policy *policy);
-
-static inline void cpufreq_exit_governor(struct cpufreq_policy *policy)
-{
-	(void)cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
-}
-
-static inline void cpufreq_stop_governor(struct cpufreq_policy *policy)
-{
-	(void)cpufreq_governor(policy, CPUFREQ_GOV_STOP);
-}
+static void cpufreq_stop_governor(struct cpufreq_policy *policy);
+static void cpufreq_governor_limits(struct cpufreq_policy *policy);
 
 /**
  * Two notifier lists: the "policy" list is involved in the
@@ -1997,7 +1990,7 @@ __weak struct cpufreq_governor *cpufreq_
 	return NULL;
 }
 
-static int cpufreq_governor(struct cpufreq_policy *policy, unsigned int event)
+static int cpufreq_init_governor(struct cpufreq_policy *policy)
 {
 	int ret;
 
@@ -2011,57 +2004,91 @@ static int cpufreq_governor(struct cpufr
 	if (!policy->governor)
 		return -EINVAL;
 
-	if (event == CPUFREQ_GOV_POLICY_INIT) {
-		if (policy->governor->max_transition_latency &&
-		    policy->cpuinfo.transition_latency >
-		    policy->governor->max_transition_latency) {
-			struct cpufreq_governor *gov = cpufreq_fallback_governor();
-
-			if (gov) {
-				pr_warn("%s governor failed, too long transition latency of HW, fallback to %s governor\n",
-					policy->governor->name, gov->name);
-				policy->governor = gov;
-			} else {
-				return -EINVAL;
-			}
-		}
-
-		if (!try_module_get(policy->governor->owner))
+	if (policy->governor->max_transition_latency &&
+	    policy->cpuinfo.transition_latency >
+	    policy->governor->max_transition_latency) {
+		struct cpufreq_governor *gov = cpufreq_fallback_governor();
+
+		if (gov) {
+			pr_warn("%s governor failed, too long transition latency of HW, fallback to %s governor\n",
+				policy->governor->name, gov->name);
+			policy->governor = gov;
+		} else {
 			return -EINVAL;
+		}
 	}
 
-	pr_debug("%s: for CPU %u, event %u\n", __func__, policy->cpu, event);
+	if (!try_module_get(policy->governor->owner))
+		return -EINVAL;
 
-	ret = policy->governor->governor(policy, event);
+	pr_debug("%s: for CPU %u\n", __func__, policy->cpu);
 
-	if (event == CPUFREQ_GOV_POLICY_INIT) {
-		if (ret)
-			module_put(policy->governor->owner);
-		else
-			policy->governor->initialized++;
-	} else if (event == CPUFREQ_GOV_POLICY_EXIT) {
-		policy->governor->initialized--;
+	ret = policy->governor->governor(policy, CPUFREQ_GOV_POLICY_INIT);
+	if (ret) {
 		module_put(policy->governor->owner);
+		return ret;
 	}
 
-	return ret;
+	policy->governor->initialized++;
+	return 0;
+}
+
+static void cpufreq_exit_governor(struct cpufreq_policy *policy)
+{
+	if (cpufreq_suspended || !policy->governor)
+		return;
+
+	pr_debug("%s: for CPU %u\n", __func__, policy->cpu);
+
+	policy->governor->governor(policy, CPUFREQ_GOV_POLICY_EXIT);
+
+	policy->governor->initialized--;
+	module_put(policy->governor->owner);
 }
 
 static int cpufreq_start_governor(struct cpufreq_policy *policy)
 {
 	int ret;
 
+	if (cpufreq_suspended)
+		return 0;
+
+	if (!policy->governor)
+		return -EINVAL;
+
+	pr_debug("%s: for CPU %u\n", __func__, policy->cpu);
+
 	if (cpufreq_driver->get && !cpufreq_driver->setpolicy)
 		cpufreq_update_current_freq(policy);
 
-	ret = cpufreq_governor(policy, CPUFREQ_GOV_START);
+	ret = policy->governor->governor(policy, CPUFREQ_GOV_START);
 	if (ret)
 		return ret;
 
-	cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
+	policy->governor->governor(policy, CPUFREQ_GOV_LIMITS);
 	return 0;
 }
 
+static void cpufreq_stop_governor(struct cpufreq_policy *policy)
+{
+	if (cpufreq_suspended || !policy->governor)
+		return;
+
+	pr_debug("%s: for CPU %u\n", __func__, policy->cpu);
+
+	policy->governor->governor(policy, CPUFREQ_GOV_STOP);
+}
+
+static void cpufreq_governor_limits(struct cpufreq_policy *policy)
+{
+	if (cpufreq_suspended || !policy->governor)
+		return;
+
+	pr_debug("%s: for CPU %u\n", __func__, policy->cpu);
+
+	policy->governor->governor(policy, CPUFREQ_GOV_LIMITS);
+}
+
 int cpufreq_register_governor(struct cpufreq_governor *governor)
 {
 	int err;
@@ -2200,7 +2227,7 @@ static int cpufreq_set_policy(struct cpu
 
 	if (new_policy->governor == policy->governor) {
 		pr_debug("cpufreq: governor limits update\n");
-		cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
+		cpufreq_governor_limits(policy);
 		return 0;
 	}
 
@@ -2216,7 +2243,7 @@ static int cpufreq_set_policy(struct cpu
 
 	/* start new governor */
 	policy->governor = new_policy->governor;
-	ret = cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT);
+	ret = cpufreq_init_governor(policy);
 	if (!ret) {
 		ret = cpufreq_start_governor(policy);
 		if (!ret) {
@@ -2230,7 +2257,7 @@ static int cpufreq_set_policy(struct cpu
 	pr_debug("starting governor %s failed\n", policy->governor->name);
 	if (old_gov) {
 		policy->governor = old_gov;
-		if (cpufreq_governor(policy, CPUFREQ_GOV_POLICY_INIT))
+		if (cpufreq_init_governor(policy))
 			policy->governor = NULL;
 		else
 			cpufreq_start_governor(policy);
@@ -2328,7 +2355,7 @@ static int cpufreq_boost_set_sw(int stat
 
 			down_write(&policy->rwsem);
 			policy->user_policy.max = policy->max;
-			cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
+			cpufreq_governor_limits(policy);
 			up_write(&policy->rwsem);
 		}
 	}

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/5] cpufreq: governor: Rework API to use callbacks instead of events "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-05-14 01:00 +0200
  [PATCH 2/5] cpufreq: governor: Check transition latecy at init time only "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-05-14 01:00 +0200
  [PATCH 5/5] cpufreq: governor: Get rid of governor events "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-05-14 01:10 +0200
    Re: [PATCH 5/5] cpufreq: governor: Get rid of governor events Viresh Kumar <viresh.kumar@linaro.org> - 2016-05-16 07:00 +0200
  [PATCH 4/5] cpufreq: Split cpufreq_governor() into simpler functions "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-05-14 01:10 +0200
  Re: [PATCH 0/5] cpufreq: governor: Rework API to use callbacks  instead of events Viresh Kumar <viresh.kumar@linaro.org> - 2016-05-16 07:00 +0200

csiph-web