[03/12] ASoC: dapm: Only mark paths dirty when the connection status changed

Commit Message

Lars-Peter Clausen Oct. 20, 2014, 5:36 p.m. UTC

Rework soc_dapm_{mixer,mux}_update_power() to only mark a path dirty if the
connect state if the path has actually changed. This avoids unnecessary
power state checks for the widgets involved.

Also factor out the common code that is involved in this into a helper
function.

Signed-off-by: Lars-Peter Clausen <[e-mail]>
---
 sound/soc/soc-dapm.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

Comments

Mark Brown Oct. 22, 2014, 11 a.m. UTC | #1

On Mon, Oct 20, 2014 at 07:36:35PM +0200, Lars-Peter Clausen wrote:

> +static bool soc_dapm_connect_path(struct snd_soc_dapm_path *path,
> +	bool connect, const char *reason)

Why is this returning a value which is then ignored?

Lars-Peter Clausen Oct. 22, 2014, 12:10 p.m. UTC | #2

On 10/22/2014 01:00 PM, Mark Brown wrote:
> On Mon, Oct 20, 2014 at 07:36:35PM +0200, Lars-Peter Clausen wrote:
>
>> +static bool soc_dapm_connect_path(struct snd_soc_dapm_path *path,
>> +	bool connect, const char *reason)
>
> Why is this returning a value which is then ignored?
>

I was initially using it to not schedule the dapm_power_widgets(), but there 
is potential for problems when the DAPM graph and the hardware state are 
out-of-sync, in which case we still want to apply the register update. There 
are some corner cases where this can happen and which need to be fixed first.

I can drop the return type for now.

- Lars

Mark Brown Oct. 22, 2014, 3:29 p.m. UTC | #3

On Wed, Oct 22, 2014 at 02:10:29PM +0200, Lars-Peter Clausen wrote:
> On 10/22/2014 01:00 PM, Mark Brown wrote:

> >Why is this returning a value which is then ignored?

> I was initially using it to not schedule the dapm_power_widgets(), but there
> is potential for problems when the DAPM graph and the hardware state are
> out-of-sync, in which case we still want to apply the register update. There
> are some corner cases where this can happen and which need to be fixed
> first.

> I can drop the return type for now.

Probably best, I was looking for a user and didn't find one.  Seems like
a bit of a microoptimisation either way.

If it was just returning an error code and we were ignoring that I'd not
have worried!

Patch

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 12f9f5f..518c532 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -2002,12 +2002,35 @@  static inline void dapm_debugfs_cleanup(struct snd_soc_dapm_context *dapm)
 
 #endif
 
+/**
+ * soc_dapm_connect_path() - Connects or disconnects a path
+ * @path: The path to update
+ * @connect: The new connect state of the path. True if the path is connected,
+ *  false if it is disconneted.
+ * @reason: The reason why the path changed (for debugging only)
+ *
+ * Returns true if the path connect state changed, false otherwise.
+ */
+static bool soc_dapm_connect_path(struct snd_soc_dapm_path *path,
+	bool connect, const char *reason)
+{
+	if (path->connect == connect)
+		return false;
+
+	path->connect = connect;
+	dapm_mark_dirty(path->source, reason);
+	dapm_mark_dirty(path->sink, reason);
+
+	return true;
+}
+
 /* test and update the power status of a mux widget */
 static int soc_dapm_mux_update_power(struct snd_soc_card *card,
 				 struct snd_kcontrol *kcontrol, int mux, struct soc_enum *e)
 {
 	struct snd_soc_dapm_path *path;
 	int found = 0;
+	bool connect;
 
 	lockdep_assert_held(&card->dapm_mutex);
 
@@ -2018,16 +2041,12 @@  static int soc_dapm_mux_update_power(struct snd_soc_card *card,
 
 		found = 1;
 		/* we now need to match the string in the enum to the path */
-		if (!(strcmp(path->name, e->texts[mux]))) {
-			path->connect = 1; /* new connection */
-			dapm_mark_dirty(path->source, "mux connection");
-		} else {
-			if (path->connect)
-				dapm_mark_dirty(path->source,
-						"mux disconnection");
-			path->connect = 0; /* old connection must be powered down */
-		}
-		dapm_mark_dirty(path->sink, "mux change");
+		if (!(strcmp(path->name, e->texts[mux])))
+			connect = true;
+		else
+			connect = false;
+
+		soc_dapm_connect_path(path, connect, "mux update");
 	}
 
 	if (found)
@@ -2066,9 +2085,7 @@  static int soc_dapm_mixer_update_power(struct snd_soc_card *card,
 	/* find dapm widget path assoc with kcontrol */
 	dapm_kcontrol_for_each_path(path, kcontrol) {
 		found = 1;
-		path->connect = connect;
-		dapm_mark_dirty(path->source, "mixer connection");
-		dapm_mark_dirty(path->sink, "mixer update");
+		soc_dapm_connect_path(path, connect, "mixer update");
 	}
 
 	if (found)