[PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Nenghua Cao]

From: nhcao <nhcao@marvell.com>

    DPCM can dynamically alter the FE to BE PCM links at runtime based
on mixer setting updates. DAPM has provided common get/put function for
mixer/mux. But these function doesn't call soc_dpcm_runtime_update()
func to update pcm links. This patch defines a DPCM DPCM kcontrol. For
this control, the common get/put function will update FE to BE links
dynamically. This patch has no impact to the current code.

Change-Id: I45e291f467e4fa17e21aa8923c2b712d52067aca
Signed-off-by: nhcao <nhcao@marvell.com>
---
 include/sound/control.h  |    4 ++++
 include/sound/soc-dapm.h |   32 ++++++++++++++++++++++++++++++++
 sound/core/control.c     |    1 +
 sound/soc/soc-dapm.c     |   28 ++++++++++++++++++++++++----
 4 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/include/sound/control.h b/include/sound/control.h
index 5358892..3788284 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -44,6 +44,8 @@ struct snd_kcontrol_new {
 	unsigned int index;		/* index of item */
 	unsigned int access;		/* access rights */
 	unsigned int count;		/* count of same elements */
+	/* this kcontrol impact fe<->be relationship at runtime */
+	unsigned char dpcm_checked;
 	snd_kcontrol_info_t *info;
 	snd_kcontrol_get_t *get;
 	snd_kcontrol_put_t *put;
@@ -63,6 +65,8 @@ struct snd_kcontrol {
 	struct list_head list;		/* list of controls */
 	struct snd_ctl_elem_id id;
 	unsigned int count;		/* count of same elements */
+	/* this kcontrol impact fe<->be relationship at runtime */
+	unsigned char dpcm_checked;
 	snd_kcontrol_info_t *info;
 	snd_kcontrol_get_t *get;
 	snd_kcontrol_put_t *put;
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 56ebdfc..8962884 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -336,6 +336,38 @@ struct device;
 	.put = snd_soc_dapm_put_pin_switch, \
 	.private_value = (unsigned long)xname }
 
+/* dapm & dpcm kcontrol types */
+#define SOC_DAPM_DPCM_SINGLE(xname, reg, shift, max, invert) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.dpcm_checked = 1, .info = snd_soc_info_volsw, \
+	.get = snd_soc_dapm_get_volsw, .put = snd_soc_dapm_put_volsw, \
+	.private_value =  SOC_SINGLE_VALUE(reg, shift, max, invert) }
+#define SOC_DAPM_DPCM_SINGLE_TLV(xname, reg, shift, max, invert, tlv_array) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.dpcm_checked = 1, .info = snd_soc_info_volsw, \
+	.access = SNDRV_CTL_ELEM_ACCESS_TLV_READ | SNDRV_CTL_ELEM_ACCESS_READWRITE,\
+	.tlv.p = (tlv_array), \
+	.get = snd_soc_dapm_get_volsw, .put = snd_soc_dapm_put_volsw, \
+	.private_value =  SOC_SINGLE_VALUE(reg, shift, max, invert) }
+#define SOC_DAPM_DPCM_ENUM(xname, xenum) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.dpcm_checked = 1, .info = snd_soc_info_enum_double, \
+	.get = snd_soc_dapm_get_enum_double, \
+	.put = snd_soc_dapm_put_enum_double, \
+	.private_value = (unsigned long)&xenum }
+#define SOC_DAPM_DPCM_ENUM_VIRT(xname, xenum)		    \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.dpcm_checked = 1, .info = snd_soc_info_enum_double, \
+	.get = snd_soc_dapm_get_enum_virt, \
+	.put = snd_soc_dapm_put_enum_virt, \
+	.private_value = (unsigned long)&xenum }
+#define SOC_DAPM_DPCM_VALUE_ENUM(xname, xenum) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.dpcm_checked = 1, .info = snd_soc_info_enum_double, \
+	.get = snd_soc_dapm_get_value_enum_double, \
+	.put = snd_soc_dapm_put_value_enum_double, \
+	.private_value = (unsigned long)&xenum }
+
 /* dapm stream operations */
 #define SND_SOC_DAPM_STREAM_NOP			0x0
 #define SND_SOC_DAPM_STREAM_START		0x1
diff --git a/sound/core/control.c b/sound/core/control.c
index d8aa206..34df2c4 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -254,6 +254,7 @@ struct snd_kcontrol *snd_ctl_new1(const struct snd_kcontrol_new *ncontrol,
 				      SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE|
 				      SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND|
 				      SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK));
+	kctl.dpcm_checked = ncontrol->dpcm_checked;
 	kctl.info = ncontrol->info;
 	kctl.get = ncontrol->get;
 	kctl.put = ncontrol->put;
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index dcade13..b568183 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -2868,6 +2868,7 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 	unsigned int val;
 	int connect, change;
 	struct snd_soc_dapm_update update;
+	int ret = 0;
 
 	if (snd_soc_volsw_is_stereo(mc))
 		dev_warn(codec->dapm.dev,
@@ -2901,12 +2902,16 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 			card->update = &update;
 		}
 
-		soc_dapm_mixer_update_power(card, kcontrol, connect);
+		ret = soc_dapm_mixer_update_power(card, kcontrol, connect);
 
 		card->update = NULL;
 	}
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (kcontrol->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_volsw);
@@ -2955,6 +2960,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 	unsigned int val, mux, change;
 	unsigned int mask;
 	struct snd_soc_dapm_update update;
+	int ret = 0;
 
 	if (ucontrol->value.enumerated.item[0] > e->max - 1)
 		return -EINVAL;
@@ -2978,12 +2984,16 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 		update.val = val;
 		card->update = &update;
 
-		soc_dapm_mux_update_power(card, kcontrol, mux, e);
+		ret = soc_dapm_mux_update_power(card, kcontrol, mux, e);
 
 		card->update = NULL;
 	}
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (kcontrol->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_double);
@@ -3019,6 +3029,7 @@ int snd_soc_dapm_put_enum_virt(struct snd_kcontrol *kcontrol,
 	struct soc_enum *e =
 		(struct soc_enum *)kcontrol->private_value;
 	int change;
+	int ret = 0;
 
 	if (ucontrol->value.enumerated.item[0] >= e->max)
 		return -EINVAL;
@@ -3028,9 +3039,13 @@ int snd_soc_dapm_put_enum_virt(struct snd_kcontrol *kcontrol,
 	value = ucontrol->value.enumerated.item[0];
 	change = dapm_kcontrol_set_value(kcontrol, value);
 	if (change)
-		soc_dapm_mux_update_power(card, kcontrol, value, e);
+		ret = soc_dapm_mux_update_power(card, kcontrol, value, e);
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (kcontrol->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_virt);
@@ -3097,6 +3112,7 @@ int snd_soc_dapm_put_value_enum_double(struct snd_kcontrol *kcontrol,
 	unsigned int val, mux, change;
 	unsigned int mask;
 	struct snd_soc_dapm_update update;
+	int ret = 0;
 
 	if (ucontrol->value.enumerated.item[0] > e->max - 1)
 		return -EINVAL;
@@ -3120,12 +3136,16 @@ int snd_soc_dapm_put_value_enum_double(struct snd_kcontrol *kcontrol,
 		update.val = val;
 		card->update = &update;
 
-		soc_dapm_mux_update_power(card, kcontrol, mux, e);
+		ret = soc_dapm_mux_update_power(card, kcontrol, mux, e);
 
 		card->update = NULL;
 	}
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (kcontrol->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_value_enum_double);
-- 
1.7.0.4

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 657 bytes --]

On Thu, Dec 12, 2013 at 04:06:59PM +0800, Nenghua Cao wrote:
> From: nhcao <nhcao@marvell.com>
> 
>     DPCM can dynamically alter the FE to BE PCM links at runtime based
> on mixer setting updates. DAPM has provided common get/put function for
> mixer/mux. But these function doesn't call soc_dpcm_runtime_update()
> func to update pcm links. This patch defines a DPCM DPCM kcontrol. For
> this control, the common get/put function will update FE to BE links
> dynamically. This patch has no impact to the current code.

Why shouldn't we just add the DPCM update to the standard controls -
what's the benefit in defining a new set of controls?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 565 bytes --]

On Thu, Dec 12, 2013 at 02:56:08AM -0800, Nenghua Cao wrote:

Don't top post.  Please also fix your mailer to word wrap within
paragaphs, it makes your mails much more legible.

>    For common mix/mux which aren't related with DPCM, it doesn't need
>    to do DPCM update. And it still can use the old controls. But for
>    the mix/mux which impacts fe<->be link, they need to do this works.
>    For them, they call use the new set of controls. I think it can
>    make our code has better backward-compatibility.

How does this improve backwards compatibility?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Takashi Iwai]

At Thu, 12 Dec 2013 16:06:59 +0800,
Nenghua Cao wrote:
> 
> From: nhcao <nhcao@marvell.com>
> 
>     DPCM can dynamically alter the FE to BE PCM links at runtime based
> on mixer setting updates. DAPM has provided common get/put function for
> mixer/mux. But these function doesn't call soc_dpcm_runtime_update()
> func to update pcm links. This patch defines a DPCM DPCM kcontrol. For
> this control, the common get/put function will update FE to BE links
> dynamically. This patch has no impact to the current code.
> 
> Change-Id: I45e291f467e4fa17e21aa8923c2b712d52067aca

Don't include such an internal tag in the public patch.

> Signed-off-by: nhcao <nhcao@marvell.com>

You have to provide your real name in from and signed-off-by tags.

> ---
>  include/sound/control.h  |    4 ++++
>  include/sound/soc-dapm.h |   32 ++++++++++++++++++++++++++++++++
>  sound/core/control.c     |    1 +
>  sound/soc/soc-dapm.c     |   28 ++++++++++++++++++++++++----
>  4 files changed, 61 insertions(+), 4 deletions(-)
> 
> diff --git a/include/sound/control.h b/include/sound/control.h
> index 5358892..3788284 100644
> --- a/include/sound/control.h
> +++ b/include/sound/control.h
> @@ -44,6 +44,8 @@ struct snd_kcontrol_new {
>  	unsigned int index;		/* index of item */
>  	unsigned int access;		/* access rights */
>  	unsigned int count;		/* count of same elements */
> +	/* this kcontrol impact fe<->be relationship at runtime */
> +	unsigned char dpcm_checked;
>  	snd_kcontrol_info_t *info;
>  	snd_kcontrol_get_t *get;
>  	snd_kcontrol_put_t *put;
> @@ -63,6 +65,8 @@ struct snd_kcontrol {
>  	struct list_head list;		/* list of controls */
>  	struct snd_ctl_elem_id id;
>  	unsigned int count;		/* count of same elements */
> +	/* this kcontrol impact fe<->be relationship at runtime */
> +	unsigned char dpcm_checked;
>  	snd_kcontrol_info_t *info;
>  	snd_kcontrol_get_t *get;
>  	snd_kcontrol_put_t *put;

You don't need to extend the ALSA core API for such a purpose.  It's
ASoC-specific, so rather extend struct soc_mixer_control instead, if
such a new flag is really needed.


Takashi

[PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Nenghua Cao]

From: Nenghua Cao <nhcao@marvell.com>

    DPCM can dynamically alter the FE to BE PCM links at runtime based
on mixer setting updates. DAPM has provided common get/put function for
mixer/mux. But these function doesn't call soc_dpcm_runtime_update()
func to update pcm links. This patch extends these common functions.
There is a flag to indicate if runtime_update is needed. And this flag
can be set in user's definition.

Signed-off-by: Nenghua Cao <nhcao@marvell.com>
---
 include/sound/soc-dapm.h |   14 ++++++++++++++
 include/sound/soc.h      |   19 +++++++++++++++++++
 sound/soc/soc-dapm.c     |   28 ++++++++++++++++++++++++----
 3 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 56ebdfc..1af6caa 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -336,6 +336,20 @@ struct device;
 	.put = snd_soc_dapm_put_pin_switch, \
 	.private_value = (unsigned long)xname }
 
+/* dapm & dpcm kcontrol types */
+#define SOC_DAPM_DPCM_SINGLE(xname, reg, shift, max, invert) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_volsw, \
+	.get = snd_soc_dapm_get_volsw, .put = snd_soc_dapm_put_volsw, \
+	.private_value =  SOC_SINGLE_VALUE_DPCM(reg, shift, max, invert, 1) }
+#define SOC_DAPM_DPCM_SINGLE_TLV(xname, reg, shift, max, invert, tlv_array) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_volsw, \
+	.access = SNDRV_CTL_ELEM_ACCESS_TLV_READ | SNDRV_CTL_ELEM_ACCESS_READWRITE,\
+	.tlv.p = (tlv_array), \
+	.get = snd_soc_dapm_get_volsw, .put = snd_soc_dapm_put_volsw, \
+	.private_value =  SOC_SINGLE_VALUE_DPCM(reg, shift, max, invert, 1) }
+
 /* dapm stream operations */
 #define SND_SOC_DAPM_STREAM_NOP			0x0
 #define SND_SOC_DAPM_STREAM_START		0x1
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 1f741cb..6766cc7 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -49,6 +49,13 @@
 	((unsigned long)&(struct soc_mixer_control) \
 	{.reg = xlreg, .rreg = xrreg, .shift = xshift, .rshift = xshift, \
 	.min = xmin, .max = xmax, .platform_max = xmax, .invert = xinvert})
+#define SOC_DOUBLE_VALUE_DPCM(xreg, shift_left, shift_right, xmax, xinvert, dpcm) \
+	((unsigned long)&(struct soc_mixer_control) \
+	{.reg = xreg, .rreg = xreg, .shift = shift_left, \
+	.rshift = shift_right, .max = xmax, .platform_max = xmax, \
+	.invert = xinvert, .dpcm_checked = dpcm})
+#define SOC_SINGLE_VALUE_DPCM(xreg, xshift, xmax, xinvert, dpcm) \
+	SOC_DOUBLE_VALUE_DPCM(xreg, xshift, xshift, xmax, xinvert, dpcm)
 #define SOC_SINGLE(xname, reg, shift, max, invert) \
 {	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
 	.info = snd_soc_info_volsw, .get = snd_soc_get_volsw,\
@@ -175,6 +182,14 @@
 	.mask = xmask, .max = xmax, .texts = xtexts, .values = xvalues}
 #define SOC_VALUE_ENUM_SINGLE(xreg, xshift, xmask, xmax, xtexts, xvalues) \
 	SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xmax, xtexts, xvalues)
+#define SOC_ENUM_SINGLE_DPCM(xreg, xshift, xmax, dpcm, xtexts) \
+{	.reg = xreg, .shift_l = xshift, .shift_r = xshift, \
+	.max = xmax, .dpcm_checked = dpcm, .texts = xtexts, \
+	.mask = xmax ? roundup_pow_of_two(xmax) - 1 : 0}
+#define SOC_ENUM_DOUBLE_DPCM(xreg, xshift_l, xshift_r, xmax, dpcm,  xtexts) \
+{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
+	.max = xmax, .dpcm_checked = dpcm, .texts = xtexts, \
+	.mask = xmax ? roundup_pow_of_two(xmax) - 1 : 0}
 #define SOC_ENUM(xname, xenum) \
 {	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname,\
 	.info = snd_soc_info_enum_double, \
@@ -1055,6 +1070,8 @@ struct soc_mixer_control {
 	unsigned int shift, rshift;
 	unsigned int invert:1;
 	unsigned int autodisable:1;
+	/* this kcontrol impact fe<->be relationship at runtime */
+	unsigned int dpcm_checked:1;
 };
 
 struct soc_bytes {
@@ -1079,6 +1096,8 @@ struct soc_enum {
 	unsigned int mask;
 	const char * const *texts;
 	const unsigned int *values;
+	/* this kcontrol impact fe<->be relationship at runtime */
+	unsigned int dpcm_checked:1;
 };
 
 /* codec IO */
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index dcade13..b763d1d 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -2868,6 +2868,7 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 	unsigned int val;
 	int connect, change;
 	struct snd_soc_dapm_update update;
+	int ret = 0;
 
 	if (snd_soc_volsw_is_stereo(mc))
 		dev_warn(codec->dapm.dev,
@@ -2901,12 +2902,16 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 			card->update = &update;
 		}
 
-		soc_dapm_mixer_update_power(card, kcontrol, connect);
+		ret = soc_dapm_mixer_update_power(card, kcontrol, connect);
 
 		card->update = NULL;
 	}
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (mc->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_volsw);
@@ -2955,6 +2960,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 	unsigned int val, mux, change;
 	unsigned int mask;
 	struct snd_soc_dapm_update update;
+	int ret = 0;
 
 	if (ucontrol->value.enumerated.item[0] > e->max - 1)
 		return -EINVAL;
@@ -2978,12 +2984,16 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 		update.val = val;
 		card->update = &update;
 
-		soc_dapm_mux_update_power(card, kcontrol, mux, e);
+		ret = soc_dapm_mux_update_power(card, kcontrol, mux, e);
 
 		card->update = NULL;
 	}
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (e->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_double);
@@ -3019,6 +3029,7 @@ int snd_soc_dapm_put_enum_virt(struct snd_kcontrol *kcontrol,
 	struct soc_enum *e =
 		(struct soc_enum *)kcontrol->private_value;
 	int change;
+	int ret = 0;
 
 	if (ucontrol->value.enumerated.item[0] >= e->max)
 		return -EINVAL;
@@ -3028,9 +3039,13 @@ int snd_soc_dapm_put_enum_virt(struct snd_kcontrol *kcontrol,
 	value = ucontrol->value.enumerated.item[0];
 	change = dapm_kcontrol_set_value(kcontrol, value);
 	if (change)
-		soc_dapm_mux_update_power(card, kcontrol, value, e);
+		ret = soc_dapm_mux_update_power(card, kcontrol, value, e);
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (e->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_virt);
@@ -3097,6 +3112,7 @@ int snd_soc_dapm_put_value_enum_double(struct snd_kcontrol *kcontrol,
 	unsigned int val, mux, change;
 	unsigned int mask;
 	struct snd_soc_dapm_update update;
+	int ret = 0;
 
 	if (ucontrol->value.enumerated.item[0] > e->max - 1)
 		return -EINVAL;
@@ -3120,12 +3136,16 @@ int snd_soc_dapm_put_value_enum_double(struct snd_kcontrol *kcontrol,
 		update.val = val;
 		card->update = &update;
 
-		soc_dapm_mux_update_power(card, kcontrol, mux, e);
+		ret = soc_dapm_mux_update_power(card, kcontrol, mux, e);
 
 		card->update = NULL;
 	}
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (e->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_value_enum_double);
-- 
1.7.0.4

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Nenghua Cao]

On 12/13/2013 12:52 AM, Takashi Iwai wrote:
> At Thu, 12 Dec 2013 16:06:59 +0800,
> Nenghua Cao wrote:
>> > 
>> > From: nhcao <nhcao@marvell.com>
>> > 
>> >     DPCM can dynamically alter the FE to BE PCM links at runtime based
>> > on mixer setting updates. DAPM has provided common get/put function for
>> > mixer/mux. But these function doesn't call soc_dpcm_runtime_update()
>> > func to update pcm links. This patch defines a DPCM DPCM kcontrol. For
>> > this control, the common get/put function will update FE to BE links
>> > dynamically. This patch has no impact to the current code.
>> > 
>> > Change-Id: I45e291f467e4fa17e21aa8923c2b712d52067aca
> Don't include such an internal tag in the public patch.
> 
>> > Signed-off-by: nhcao <nhcao@marvell.com>
> You have to provide your real name in from and signed-off-by tags.
> 
Thanks for your reminder. Fixed them.
>> > ---
>> >  include/sound/control.h  |    4 ++++
>> >  include/sound/soc-dapm.h |   32 ++++++++++++++++++++++++++++++++
>> >  sound/core/control.c     |    1 +
>> >  sound/soc/soc-dapm.c     |   28 ++++++++++++++++++++++++----
>> >  4 files changed, 61 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/include/sound/control.h b/include/sound/control.h
>> > index 5358892..3788284 100644
>> > --- a/include/sound/control.h
>> > +++ b/include/sound/control.h
>> > @@ -44,6 +44,8 @@ struct snd_kcontrol_new {
>> >  	unsigned int index;		/* index of item */
>> >  	unsigned int access;		/* access rights */
>> >  	unsigned int count;		/* count of same elements */
>> > +	/* this kcontrol impact fe<->be relationship at runtime */
>> > +	unsigned char dpcm_checked;
>> >  	snd_kcontrol_info_t *info;
>> >  	snd_kcontrol_get_t *get;
>> >  	snd_kcontrol_put_t *put;
>> > @@ -63,6 +65,8 @@ struct snd_kcontrol {
>> >  	struct list_head list;		/* list of controls */
>> >  	struct snd_ctl_elem_id id;
>> >  	unsigned int count;		/* count of same elements */
>> > +	/* this kcontrol impact fe<->be relationship at runtime */
>> > +	unsigned char dpcm_checked;
>> >  	snd_kcontrol_info_t *info;
>> >  	snd_kcontrol_get_t *get;
>> >  	snd_kcontrol_put_t *put;
> You don't need to extend the ALSA core API for such a purpose.  It's
> ASoC-specific, so rather extend struct soc_mixer_control instead, if
> such a new flag is really needed.
> 
It is a good suggestion. We should avoid touching ALSA core. I also want
to introduce little change. I made another patch per your suggestion.
Please review it again. Thanks!
> 
> Takashi

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Nenghua Cao]

Hi, Mark:

On 12/12/2013 07:07 PM, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 02:56:08AM -0800, Nenghua Cao wrote:
> 
> Don't top post.  Please also fix your mailer to word wrap within
> paragaphs, it makes your mails much more legible.
> 
It is my fault and Thanks for your kindly reminder. I just made my
mailer work as community's request.
>> >    For common mix/mux which aren't related with DPCM, it doesn't need
>> >    to do DPCM update. And it still can use the old controls. But for
>> >    the mix/mux which impacts fe<->be link, they need to do this works.
>> >    For them, they call use the new set of controls. I think it can
>> >    make our code has better backward-compatibility.
> How does this improve backwards compatibility?
My thought is that we distinguish different requirement through flag
(you can find dpcm_checked in my patch).

Thanks

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Takashi Iwai]

At Fri, 13 Dec 2013 19:33:46 +0800,
Nenghua Cao wrote:
> 
> Hi, Mark:
> 
> On 12/12/2013 07:07 PM, Mark Brown wrote:
> > On Thu, Dec 12, 2013 at 02:56:08AM -0800, Nenghua Cao wrote:
> > 
> > Don't top post.  Please also fix your mailer to word wrap within
> > paragaphs, it makes your mails much more legible.
> > 
> It is my fault and Thanks for your kindly reminder. I just made my
> mailer work as community's request.
> >> >    For common mix/mux which aren't related with DPCM, it doesn't need
> >> >    to do DPCM update. And it still can use the old controls. But for
> >> >    the mix/mux which impacts fe<->be link, they need to do this works.
> >> >    For them, they call use the new set of controls. I think it can
> >> >    make our code has better backward-compatibility.
> > How does this improve backwards compatibility?
> My thought is that we distinguish different requirement through flag
> (you can find dpcm_checked in my patch).

The question is rather what do you mean as "backward compatibility".

Usually backward compatibility is concerned when something new breaks
the existing ones.  In your case, always updating DPCM would work,
too, even without an extra flag; it's just suboptimal.


Takashi

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 426 bytes --]

On Fri, Dec 13, 2013 at 12:37:20PM +0100, Takashi Iwai wrote:

> Usually backward compatibility is concerned when something new breaks
> the existing ones.  In your case, always updating DPCM would work,
> too, even without an extra flag; it's just suboptimal.

Right, and if we have separate versions then at some point we'll end up
having to define DPCM versions of everything which is going to get
tedious and error prone.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Nenghua Cao]

On 12/13/2013 07:37 PM, Takashi Iwai wrote:
> At Fri, 13 Dec 2013 19:33:46 +0800,
> Nenghua Cao wrote:
>> > 
>> > Hi, Mark:
>> > 
>> > On 12/12/2013 07:07 PM, Mark Brown wrote:
>>> > > On Thu, Dec 12, 2013 at 02:56:08AM -0800, Nenghua Cao wrote:
>>> > > 
>>> > > Don't top post.  Please also fix your mailer to word wrap within
>>> > > paragaphs, it makes your mails much more legible.
>>> > > 
>> > It is my fault and Thanks for your kindly reminder. I just made my
>> > mailer work as community's request.
>>>>> > >> >    For common mix/mux which aren't related with DPCM, it doesn't need
>>>>> > >> >    to do DPCM update. And it still can use the old controls. But for
>>>>> > >> >    the mix/mux which impacts fe<->be link, they need to do this works.
>>>>> > >> >    For them, they call use the new set of controls. I think it can
>>>>> > >> >    make our code has better backward-compatibility.
>>> > > How does this improve backwards compatibility?
>> > My thought is that we distinguish different requirement through flag
>> > (you can find dpcm_checked in my patch).
> The question is rather what do you mean as "backward compatibility".
> 
> Usually backward compatibility is concerned when something new breaks
> the existing ones.  In your case, always updating DPCM would work,
> too, even without an extra flag; it's just suboptimal.
> 
Yes, you are right. I add this flag only for avoiding the traversing
dai_link in soc_dpcm_runtime_update(). So you think this work is
unnecessary. I will make another patch which will do DPCM runtime update
always. Is it Ok?
> 
> Takashi

Thanks

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 349 bytes --]

On Fri, Dec 13, 2013 at 07:53:21PM +0800, Nenghua Cao wrote:

> Yes, you are right. I add this flag only for avoiding the traversing
> dai_link in soc_dpcm_runtime_update(). So you think this work is
> unnecessary. I will make another patch which will do DPCM runtime update
> always. Is it Ok?

It seems safer, we can optimise later if we need to.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH V2] ASOC:DAPM: update DPCM runtime when mixer/mux changes

[Nenghua Cao]

From: Nenghua Cao <nhcao@marvell.com>

V2 -> V1:

   Removes the flag and update DPCM always when mixer/mux changes, according
to Takashi's and Mark's suggestion.

>> Usually backward compatibility is concerned when something new breaks
>> the existing ones.  In your case, always updating DPCM would work,
>> too, even without an extra flag; it's just suboptimal.

>Right, and if we have separate versions then at some point we'll end up
>having to define DPCM versions of everything which is going to get
>tedious and error prone.
 

V1 -> V0:

     refined patch, and add flag into soc_mixer_control and soc_enum 
instead of the alsa core, according to Takashi's suggestion.

> You don't need to extend the ALSA core API for such a purpose.  It's
> ASoC-specific, so rather extend struct soc_mixer_control instead, if
> such a new flag is really needed.


Nenghua Cao (1):
  ASOC:DAPM: update DPCM runtime when mixer/mux changes

 sound/soc/soc-dapm.c |   28 ++++++++++++++++++++++++----
 1 files changed, 24 insertions(+), 4 deletions(-)

[PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Nenghua Cao]

From: nhcao <nhcao@marvell.com>

    DPCM can dynamically alter the FE to BE PCM links at runtime based
on mixer setting updates. DAPM has provided common get/put function for
mixer/mux. But these function doesn't call soc_dpcm_runtime_update()
func to update pcm links. This patch extends these common functions.
There is a flag to indicate if runtime_update is needed. And this flag
can be set in user's definition.

Change-Id: I6d9f165a9e20c438e3d550906531251d3199fe46
Signed-off-by: Nenghua Cao <nhcao@marvell.com>
---
 include/sound/soc-dapm.h |   14 ++++++++++++++
 include/sound/soc.h      |   19 +++++++++++++++++++
 sound/soc/soc-dapm.c     |   28 ++++++++++++++++++++++++----
 3 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 56ebdfc..1af6caa 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -336,6 +336,20 @@ struct device;
 	.put = snd_soc_dapm_put_pin_switch, \
 	.private_value = (unsigned long)xname }
 
+/* dapm & dpcm kcontrol types */
+#define SOC_DAPM_DPCM_SINGLE(xname, reg, shift, max, invert) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_volsw, \
+	.get = snd_soc_dapm_get_volsw, .put = snd_soc_dapm_put_volsw, \
+	.private_value =  SOC_SINGLE_VALUE_DPCM(reg, shift, max, invert, 1) }
+#define SOC_DAPM_DPCM_SINGLE_TLV(xname, reg, shift, max, invert, tlv_array) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_volsw, \
+	.access = SNDRV_CTL_ELEM_ACCESS_TLV_READ | SNDRV_CTL_ELEM_ACCESS_READWRITE,\
+	.tlv.p = (tlv_array), \
+	.get = snd_soc_dapm_get_volsw, .put = snd_soc_dapm_put_volsw, \
+	.private_value =  SOC_SINGLE_VALUE_DPCM(reg, shift, max, invert, 1) }
+
 /* dapm stream operations */
 #define SND_SOC_DAPM_STREAM_NOP			0x0
 #define SND_SOC_DAPM_STREAM_START		0x1
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 1f741cb..6766cc7 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -49,6 +49,13 @@
 	((unsigned long)&(struct soc_mixer_control) \
 	{.reg = xlreg, .rreg = xrreg, .shift = xshift, .rshift = xshift, \
 	.min = xmin, .max = xmax, .platform_max = xmax, .invert = xinvert})
+#define SOC_DOUBLE_VALUE_DPCM(xreg, shift_left, shift_right, xmax, xinvert, dpcm) \
+	((unsigned long)&(struct soc_mixer_control) \
+	{.reg = xreg, .rreg = xreg, .shift = shift_left, \
+	.rshift = shift_right, .max = xmax, .platform_max = xmax, \
+	.invert = xinvert, .dpcm_checked = dpcm})
+#define SOC_SINGLE_VALUE_DPCM(xreg, xshift, xmax, xinvert, dpcm) \
+	SOC_DOUBLE_VALUE_DPCM(xreg, xshift, xshift, xmax, xinvert, dpcm)
 #define SOC_SINGLE(xname, reg, shift, max, invert) \
 {	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
 	.info = snd_soc_info_volsw, .get = snd_soc_get_volsw,\
@@ -175,6 +182,14 @@
 	.mask = xmask, .max = xmax, .texts = xtexts, .values = xvalues}
 #define SOC_VALUE_ENUM_SINGLE(xreg, xshift, xmask, xmax, xtexts, xvalues) \
 	SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xmax, xtexts, xvalues)
+#define SOC_ENUM_SINGLE_DPCM(xreg, xshift, xmax, dpcm, xtexts) \
+{	.reg = xreg, .shift_l = xshift, .shift_r = xshift, \
+	.max = xmax, .dpcm_checked = dpcm, .texts = xtexts, \
+	.mask = xmax ? roundup_pow_of_two(xmax) - 1 : 0}
+#define SOC_ENUM_DOUBLE_DPCM(xreg, xshift_l, xshift_r, xmax, dpcm,  xtexts) \
+{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
+	.max = xmax, .dpcm_checked = dpcm, .texts = xtexts, \
+	.mask = xmax ? roundup_pow_of_two(xmax) - 1 : 0}
 #define SOC_ENUM(xname, xenum) \
 {	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname,\
 	.info = snd_soc_info_enum_double, \
@@ -1055,6 +1070,8 @@ struct soc_mixer_control {
 	unsigned int shift, rshift;
 	unsigned int invert:1;
 	unsigned int autodisable:1;
+	/* this kcontrol impact fe<->be relationship at runtime */
+	unsigned int dpcm_checked:1;
 };
 
 struct soc_bytes {
@@ -1079,6 +1096,8 @@ struct soc_enum {
 	unsigned int mask;
 	const char * const *texts;
 	const unsigned int *values;
+	/* this kcontrol impact fe<->be relationship at runtime */
+	unsigned int dpcm_checked:1;
 };
 
 /* codec IO */
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index dcade13..b763d1d 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -2868,6 +2868,7 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 	unsigned int val;
 	int connect, change;
 	struct snd_soc_dapm_update update;
+	int ret = 0;
 
 	if (snd_soc_volsw_is_stereo(mc))
 		dev_warn(codec->dapm.dev,
@@ -2901,12 +2902,16 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 			card->update = &update;
 		}
 
-		soc_dapm_mixer_update_power(card, kcontrol, connect);
+		ret = soc_dapm_mixer_update_power(card, kcontrol, connect);
 
 		card->update = NULL;
 	}
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (mc->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_volsw);
@@ -2955,6 +2960,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 	unsigned int val, mux, change;
 	unsigned int mask;
 	struct snd_soc_dapm_update update;
+	int ret = 0;
 
 	if (ucontrol->value.enumerated.item[0] > e->max - 1)
 		return -EINVAL;
@@ -2978,12 +2984,16 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 		update.val = val;
 		card->update = &update;
 
-		soc_dapm_mux_update_power(card, kcontrol, mux, e);
+		ret = soc_dapm_mux_update_power(card, kcontrol, mux, e);
 
 		card->update = NULL;
 	}
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (e->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_double);
@@ -3019,6 +3029,7 @@ int snd_soc_dapm_put_enum_virt(struct snd_kcontrol *kcontrol,
 	struct soc_enum *e =
 		(struct soc_enum *)kcontrol->private_value;
 	int change;
+	int ret = 0;
 
 	if (ucontrol->value.enumerated.item[0] >= e->max)
 		return -EINVAL;
@@ -3028,9 +3039,13 @@ int snd_soc_dapm_put_enum_virt(struct snd_kcontrol *kcontrol,
 	value = ucontrol->value.enumerated.item[0];
 	change = dapm_kcontrol_set_value(kcontrol, value);
 	if (change)
-		soc_dapm_mux_update_power(card, kcontrol, value, e);
+		ret = soc_dapm_mux_update_power(card, kcontrol, value, e);
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (e->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_virt);
@@ -3097,6 +3112,7 @@ int snd_soc_dapm_put_value_enum_double(struct snd_kcontrol *kcontrol,
 	unsigned int val, mux, change;
 	unsigned int mask;
 	struct snd_soc_dapm_update update;
+	int ret = 0;
 
 	if (ucontrol->value.enumerated.item[0] > e->max - 1)
 		return -EINVAL;
@@ -3120,12 +3136,16 @@ int snd_soc_dapm_put_value_enum_double(struct snd_kcontrol *kcontrol,
 		update.val = val;
 		card->update = &update;
 
-		soc_dapm_mux_update_power(card, kcontrol, mux, e);
+		ret = soc_dapm_mux_update_power(card, kcontrol, mux, e);
 
 		card->update = NULL;
 	}
 
 	mutex_unlock(&card->dapm_mutex);
+
+	if ((ret > 0) && (e->dpcm_checked == 1))
+		soc_dpcm_runtime_update(card);
+
 	return change;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_value_enum_double);
-- 
1.7.0.4

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Nenghua Cao]

Hi, Takashi and Mark:

On 12/13/2013 07:42 PM, Mark Brown wrote:
> On Fri, Dec 13, 2013 at 12:37:20PM +0100, Takashi Iwai wrote:
> 
>> > Usually backward compatibility is concerned when something new breaks
>> > the existing ones.  In your case, always updating DPCM would work,
>> > too, even without an extra flag; it's just suboptimal.
> Right, and if we have separate versions then at some point we'll end up
> having to define DPCM versions of everything which is going to get
> tedious and error prone.

According to your suggestion, i removed the flag, and updated the patch.
Please review it again.

Thanks

Re: [PATCH] ASOC:DAPM: extend dapm kcontrol to support runtime route update

[Vinod Koul]

[-- Attachment #1.1: Type: text/plain, Size: 825 bytes --]

On Thu, Dec 12, 2013 at 10:43:47AM +0000, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 04:06:59PM +0800, Nenghua Cao wrote:
> > From: nhcao <nhcao@marvell.com>
> > 
> >     DPCM can dynamically alter the FE to BE PCM links at runtime based
> > on mixer setting updates. DAPM has provided common get/put function for
> > mixer/mux. But these function doesn't call soc_dpcm_runtime_update()
> > func to update pcm links. This patch defines a DPCM DPCM kcontrol. For
> > this control, the common get/put function will update FE to BE links
> > dynamically. This patch has no impact to the current code.
> 
> Why shouldn't we just add the DPCM update to the standard controls -
> what's the benefit in defining a new set of controls?
Precisely my thoughts too, I think that would be a better approach!

--
~Vinod

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]