lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 7 Aug 2014 19:46:05 +0530
From:	Vinod Koul <vinod.koul@...el.com>
To:	Nikesh Oswal <nikesh@...nsource.wolfsonmicro.com>
Cc:	broonie@...nel.org, lgirdwood@...il.com, tiwai@...e.de,
	alsa-devel@...a-project.org, patches@...nsource.wolfsonmicro.com,
	linux-kernel@...r.kernel.org
Subject: Re: [alsa-devel] [PATCH v3] ASOC: dapm: add code to configure dai
 link parameters

On Wed, Aug 06, 2014 at 05:16:26PM +0100, Nikesh Oswal wrote:
> dai-link params for codec-codec links were fixed. The fixed
> link between codec and another chip which may be another codec,
> baseband, bluetooth codec etc may require run time configuaration
> changes. This change provides an optional alsa control to select
> one of the params from a list of params.

Sorry haven't followed the previous versions, so excuse me if questions were
discussed already.

I was thinking about this as well a while back, wont it make sense to let
machine driver expose meaningful controls for this. Just like we have fixup
for back-ends, we can add fixups for codec-codec links and let machine tell
you the parameters to configure.

Thoughts...?

-- 
~Vinod

> 
> Signed-off-by: Nikesh Oswal <nikesh@...nsource.wolfsonmicro.com>
> ---
>  include/sound/soc-dapm.h |    3 +
>  include/sound/soc.h      |    1 +
>  sound/soc/soc-core.c     |    4 +-
>  sound/soc/soc-dapm.c     |  140 ++++++++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 141 insertions(+), 7 deletions(-)
> 
> diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
> index 6b59471..3ee031e 100644
> --- a/include/sound/soc-dapm.h
> +++ b/include/sound/soc-dapm.h
> @@ -378,6 +378,7 @@ int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card);
>  void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card);
>  int snd_soc_dapm_new_pcm(struct snd_soc_card *card,
>  			 const struct snd_soc_pcm_stream *params,
> +			 unsigned int num_params,
>  			 struct snd_soc_dapm_widget *source,
>  			 struct snd_soc_dapm_widget *sink);
>  
> @@ -531,6 +532,8 @@ struct snd_soc_dapm_widget {
>  	void *priv;				/* widget specific data */
>  	struct regulator *regulator;		/* attached regulator */
>  	const struct snd_soc_pcm_stream *params; /* params for dai links */
> +	unsigned int num_params; /* number of params for dai links */
> +	unsigned int params_select; /* currently selected param for dai link */
>  
>  	/* dapm control */
>  	int reg;				/* negative reg = no direct dapm */
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index ed9e2d7..51c6c4f 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -906,6 +906,7 @@ struct snd_soc_dai_link {
>  	int be_id;	/* optional ID for machine driver BE identification */
>  
>  	const struct snd_soc_pcm_stream *params;
> +	unsigned int num_params;
>  
>  	unsigned int dai_fmt;           /* format to set on init */
>  
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index b87d7d8..db1572a 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1461,7 +1461,7 @@ static int soc_link_dai_widgets(struct snd_soc_card *card,
>  	capture_w = cpu_dai->capture_widget;
>  	if (play_w && capture_w) {
>  		ret = snd_soc_dapm_new_pcm(card, dai_link->params,
> -					   capture_w, play_w);
> +					   dai_link->num_params, capture_w, play_w);
>  		if (ret != 0) {
>  			dev_err(card->dev, "ASoC: Can't link %s to %s: %d\n",
>  				play_w->name, capture_w->name, ret);
> @@ -1473,7 +1473,7 @@ static int soc_link_dai_widgets(struct snd_soc_card *card,
>  	capture_w = codec_dai->capture_widget;
>  	if (play_w && capture_w) {
>  		ret = snd_soc_dapm_new_pcm(card, dai_link->params,
> -					   capture_w, play_w);
> +					   dai_link->num_params, capture_w, play_w);
>  		if (ret != 0) {
>  			dev_err(card->dev, "ASoC: Can't link %s to %s: %d\n",
>  				play_w->name, capture_w->name, ret);
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index cdc837e..41f835b 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -729,6 +729,33 @@ static int dapm_new_pga(struct snd_soc_dapm_widget *w)
>  	return 0;
>  }
>  
> +/* create new dapm dai link control */
> +static int dapm_new_dai_link(struct snd_soc_dapm_widget *w)
> +{
> +	int i, ret;
> +	struct snd_kcontrol *kcontrol;
> +	struct snd_soc_dapm_context *dapm = w->dapm;
> +	struct snd_card *card = dapm->card->snd_card;
> +
> +
> +	/* add kcontrol */
> +	for (i = 0; i < w->num_kcontrols; i++) {
> +		kcontrol = snd_soc_cnew(&w->kcontrol_news[i], w,
> +		w->name, NULL);
> +		ret = snd_ctl_add(card, kcontrol);
> +		if (ret < 0) {
> +			dev_err(dapm->dev,
> +			"ASoC: failed to add widget %s dapm kcontrol %s: %d\n",
> +			w->name, w->kcontrol_news[i].name, ret);
> +			return ret;
> +		}
> +		kcontrol->private_data = w;
> +		w->kcontrols[i] = kcontrol;
> +	}
> +
> +	return 0;
> +}
> +
>  /* reset 'walked' bit for each dapm path */
>  static void dapm_clear_walk_output(struct snd_soc_dapm_context *dapm,
>  				   struct list_head *sink)
> @@ -2664,6 +2691,9 @@ int snd_soc_dapm_new_widgets(struct snd_soc_card *card)
>  		case snd_soc_dapm_out_drv:
>  			dapm_new_pga(w);
>  			break;
> +		case snd_soc_dapm_dai_link:
> +			dapm_new_dai_link(w);
> +			break;
>  		default:
>  			break;
>  		}
> @@ -3142,6 +3172,9 @@ static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w,
>  	source = source_p->source->priv;
>  	sink = sink_p->sink->priv;
>  
> +	/* Select the configuration set by alsa control */
> +	config += w->params_select;
> +
>  	/* Be a little careful as we don't want to overflow the mask array */
>  	if (config->formats) {
>  		fmt = ffs(config->formats) - 1;
> @@ -3222,8 +3255,35 @@ out:
>  	return ret;
>  }
>  
> +static int snd_soc_dapm_dai_link_get(struct snd_kcontrol *kcontrol,
> +			  struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_dapm_widget *w = snd_kcontrol_chip(kcontrol);
> +
> +	ucontrol->value.integer.value[0] = w->params_select;
> +
> +	return 0;
> +}
> +
> +static int snd_soc_dapm_dai_link_put(struct snd_kcontrol *kcontrol,
> +			  struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_dapm_widget *w = snd_kcontrol_chip(kcontrol);
> +
> +	if (ucontrol->value.integer.value[0] == w->params_select)
> +		return 0;
> +
> +	if (ucontrol->value.integer.value[0] >= w->num_params)
> +		return -EINVAL;
> +
> +	w->params_select = ucontrol->value.integer.value[0];
> +
> +	return 0;
> +}
> +
>  int snd_soc_dapm_new_pcm(struct snd_soc_card *card,
>  			 const struct snd_soc_pcm_stream *params,
> +			 unsigned int num_params,
>  			 struct snd_soc_dapm_widget *source,
>  			 struct snd_soc_dapm_widget *sink)
>  {
> @@ -3231,12 +3291,44 @@ int snd_soc_dapm_new_pcm(struct snd_soc_card *card,
>  	struct snd_soc_dapm_widget *w;
>  	size_t len;
>  	char *link_name;
> -	int ret;
> +	int ret, count;
> +	unsigned long private_value;
> +	char **w_param_text;
> +	struct soc_enum w_param_enum[] = {
> +		SOC_ENUM_SINGLE(0, 0, 0, NULL),
> +	};
> +	struct snd_kcontrol_new kcontrol_dai_link[] = {
> +		SOC_ENUM_EXT(NULL, w_param_enum[0],
> +			     snd_soc_dapm_dai_link_get, snd_soc_dapm_dai_link_put),
> +	};
> +	const struct snd_soc_pcm_stream *config = params;
> +
> +	w_param_text = kzalloc(sizeof(char *) * num_params, GFP_KERNEL);
> +	if (!w_param_text)
> +		return -ENOMEM;
> +
> +	for (count = 0 ; count < num_params; count++) {
> +		w_param_text[count] =  kzalloc(sizeof(char) * 100, GFP_KERNEL);
> +		if (!w_param_text) {
> +			ret = -ENOMEM;
> +			goto  outfree_w_param;
> +		}
> +
> +		sprintf(w_param_text[count], "SampleRate=%d:Channels=%d:BitsPerSample=%d",
> +			config->rate_min, config->channels_min,
> +			snd_pcm_format_width(ffs(config->formats) - 1));
> +		config++;
> +	}
> +	w_param_enum[0].items = num_params;
> +	w_param_enum[0].texts = (const char * const *) w_param_text;
>  
>  	len = strlen(source->name) + strlen(sink->name) + 2;
>  	link_name = devm_kzalloc(card->dev, len, GFP_KERNEL);
> -	if (!link_name)
> -		return -ENOMEM;
> +	if (!link_name) {
> +		ret = -ENOMEM;
> +		goto  outfree_w_param;
> +	}
> +
>  	snprintf(link_name, len, "%s-%s", source->name, sink->name);
>  
>  	memset(&template, 0, sizeof(template));
> @@ -3246,6 +3338,25 @@ int snd_soc_dapm_new_pcm(struct snd_soc_card *card,
>  	template.event = snd_soc_dai_link_event;
>  	template.event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
>  		SND_SOC_DAPM_PRE_PMD;
> +	template.num_kcontrols = 1;
> +	private_value =
> +	(unsigned long) kmemdup((void *)(kcontrol_dai_link[0].private_value),
> +	sizeof(struct soc_enum), GFP_KERNEL);
> +	if (!private_value) {
> +		dev_err(card->dev, "ASoC: Failed to create control for %s widget\n",
> +			link_name);
> +		ret = -ENOMEM;
> +		goto outfree_link_name;
> +	}
> +	kcontrol_dai_link[0].private_value = private_value;
> +	template.kcontrol_news = kmemdup(&kcontrol_dai_link[0],
> +			sizeof(struct snd_kcontrol_new), GFP_KERNEL);
> +	if (!template.kcontrol_news) {
> +		dev_err(card->dev, "ASoC: Failed to create control for %s widget\n",
> +			link_name);
> +		ret = -ENOMEM;
> +		goto outfree_private_value;
> +	}
>  
>  	dev_dbg(card->dev, "ASoC: adding %s widget\n", link_name);
>  
> @@ -3253,15 +3364,34 @@ int snd_soc_dapm_new_pcm(struct snd_soc_card *card,
>  	if (!w) {
>  		dev_err(card->dev, "ASoC: Failed to create %s widget\n",
>  			link_name);
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto outfree_kcontrol_news;
>  	}
>  
>  	w->params = params;
> +	w->num_params = num_params;
>  
>  	ret = snd_soc_dapm_add_path(&card->dapm, source, w, NULL, NULL);
>  	if (ret)
> -		return ret;
> +		goto outfree_w;
>  	return snd_soc_dapm_add_path(&card->dapm, w, sink, NULL, NULL);
> +
> +outfree_w:
> +	kfree(w);
> +outfree_kcontrol_news:
> +	kfree(template.kcontrol_news);
> +outfree_private_value:
> +	kfree((void *)private_value);
> +outfree_link_name:
> +	kfree(link_name);
> +outfree_w_param:
> +	for (count = 0 ; count < num_params; count++) {
> +		if (w_param_text[count])
> +			kfree(w_param_text[count]);
> +	}
> +	kfree(w_param_text);
> +
> +	return ret;
>  }
>  
>  int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@...a-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists