Regression with .set_bias_level in pcm512x?

Regression with .set_bias_level in pcm512x?

[Peter Rosin]

Hi!

I'm working on adding support for DAIFMT_CBM_CFM and DAIFMT_CBM_CFS
to pcm512x, and it works nicely in the 3.10 kernel + Atmel-patches were I have
developed the code. Now I'm trying to forward-port it and test it on
3.18 + other-Atmel-patches in preparation for submission. However, there
seems to be a regression in that the (pre-existing) pcm512x_set_bias_level
callback is never called in the 3.18 version.

I see that there has been some core work on set_bias_level, and suspect
it has something to do with dapm and components, but I think someone else
will find the problem much quicker than I am able to.

I have feebly tried to revert 0bd2ac3dae74ee25c5ea171cb572731c7a89c248
and 68f831c2724ab72c0088471b2ed1dc99e81948ef, but that didn't help. That
also seems a bit backwards. So, what needs to be done in the pcm512x driver
to have set_bias_level called?

Please help.

Cheers,
Peter

Re: Regression with .set_bias_level in pcm512x?

[Lars-Peter Clausen]

On 01/13/2015 11:49 PM, Peter Rosin wrote:
> Hi!
>
> I'm working on adding support for DAIFMT_CBM_CFM and DAIFMT_CBM_CFS
> to pcm512x, and it works nicely in the 3.10 kernel + Atmel-patches were I have
> developed the code. Now I'm trying to forward-port it and test it on
> 3.18 + other-Atmel-patches in preparation for submission. However, there
> seems to be a regression in that the (pre-existing) pcm512x_set_bias_level
> callback is never called in the 3.18 version.
>
> I see that there has been some core work on set_bias_level, and suspect
> it has something to do with dapm and components, but I think someone else
> will find the problem much quicker than I am able to.
>
> I have feebly tried to revert 0bd2ac3dae74ee25c5ea171cb572731c7a89c248
> and 68f831c2724ab72c0088471b2ed1dc99e81948ef, but that didn't help. That
> also seems a bit backwards. So, what needs to be done in the pcm512x driver
> to have set_bias_level called?

It's unlikely that these commits have anything to do with it.

I think it is more likely that the DAPM state (for whatever reason) never 
actually changes. Check the state of the DAPM context and all the widgets in 
/sys/kernel/debug/asoc/your_card/... and compare their state when the system 
is idle to when it should not be idle.

- Lars

Re: Regression with .set_bias_level in pcm512x?

[Peter Rosin]

[-- Attachment #1: Type: text/plain, Size: 3142 bytes --]

Hi!

Lars-Peter Clausen wrote:
> On 01/13/2015 11:49 PM, Peter Rosin wrote:
> > Hi!
> >
> > I'm working on adding support for DAIFMT_CBM_CFM and
> DAIFMT_CBM_CFS to
> > pcm512x, and it works nicely in the 3.10 kernel + Atmel-patches were I
> > have developed the code. Now I'm trying to forward-port it and test it
> > on
> > 3.18 + other-Atmel-patches in preparation for submission. However,
> > there seems to be a regression in that the (pre-existing)
> > pcm512x_set_bias_level callback is never called in the 3.18 version.
> >
> > I see that there has been some core work on set_bias_level, and
> > suspect it has something to do with dapm and components, but I think
> > someone else will find the problem much quicker than I am able to.
> >
> > I have feebly tried to revert 0bd2ac3dae74ee25c5ea171cb572731c7a89c248
> > and 68f831c2724ab72c0088471b2ed1dc99e81948ef, but that didn't help.
> > That also seems a bit backwards. So, what needs to be done in the
> > pcm512x driver to have set_bias_level called?
> 
> It's unlikely that these commits have anything to do with it.

Agreed, I just wanted to show that I at least did try some things...

> I think it is more likely that the DAPM state (for whatever reason) never
> actually changes. Check the state of the DAPM context and all the widgets in
> /sys/kernel/debug/asoc/your_card/... and compare their state when the
> system is idle to when it should not be idle.

There is indeed something strange going on with dapm. When things should be
active, I have this (for the non-working 3.18 variant):

$ cat codec\:pcm512x.0-004c/dapm/Playback \
	codec\:pcm512x.0-004c/dapm/DACL \
	codec\:pcm512x.0-004c/dapm/OUTL \
	dapm/DAC1 \
	dapm/MIX1 \
	dapm/MUX1 \
	dapm/OUT1 \
	dapm/AIFINL
Playback: Off  in 1 out 0
 stream Playback active
 out "static" "DACR"
 out "static" "DACL"
DACL: Off  in 1 out 0
 in  "static" "Playback"
 out "static" "OUTL"
OUTL: Off  in 1 out 0
 in  "static" "DACL"
 out "static" "DAC1"
DAC1: Off  in 1 out 0
 in  "static" "OUTL"
 out "static" "MIX1"
MIX1: Off  in 1 out 0
 in  "static" "DAC1"
 out "Mixer" "MUX1"
MUX1: Off  in 1 out 0
 in  "Mixer" "MIX1"
 out "static" "OUT1"
OUT1: Off  in 1 out 0
 in  "static" "MUX1"
AIFINL: Off  in 0 out 0
 stream Playback inactive

When things are inactive, all "in 1" change to "in 0", and the top "stream Playback"
change to "inactive".

bias_level of both the card and the codec are "Off", regardless.

For reference, I have attached my current card driver, which is given this:

	sound {
		compatible = "axentia,asoc-linea-tse850";

		axentia,model = "TSE850 @ Linea";
		axentia,audio-routing =
			"DAC1", "OUTL",
			"DAC2", "OUTR";

		axentia,ssc-controller = <&ssc0>;
		axentia,audio-codec = <&pcm5142>;

		gpios = <&pioA 9 GPIO_ACTIVE_LOW>,
			<&pioA 8 GPIO_ACTIVE_LOW>,
			<&pioA 10 GPIO_ACTIVE_LOW>,
			<&pioA 11 GPIO_ACTIVE_LOW>;

		status = "okay";
	};

I suspect that it has something to do with the snd_soc_dapm_add_routes call in
linea_pcm512x_init? (I "stole" that piece from omap-abe-twl6040)

Cheers,
Peter


[-- Attachment #2: linea-pcm512x.c --]
[-- Type: text/plain, Size: 12792 bytes --]

/*
 * linea-pcm512x - Axentia ASoC driver for the Linea with a PCM512x.
 *
 * Copyright (C) 2014 Axentia Technologies AB
 *
 * Author: Peter Rosin <peda@axentia.se>
 *
 * Based on the Atmel ASoC driver by Bo Shen <voice.shen@atmel.com>
 * Copyright (C) 2012 Atmel
 *
 * GPLv2 or later
 */

#include <linux/clk.h>
#include <linux/module.h>
#include <linux/gpio.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/gcd.h>

#include <sound/soc.h>
#include <sound/pcm_params.h>

#include "../codecs/pcm512x.h"
#include "../atmel/atmel_ssc_dai.h"

struct pcm512x_priv {
	int ssc_id;
	int add1;
	int add1_inv;
	int add2;
	int add2_inv;
	int loop1;
	int loop1_inv;
	int loop2;
	int loop2_inv;
};

static int linea_pcm512x_get_mux1(struct snd_kcontrol *kctrl,
				  struct snd_ctl_elem_value *ucontrol)
{
	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
	struct snd_soc_card *card = dapm->card;
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);
	unsigned int val;

	val = !gpio_get_value(pcm512x->loop1) ^ !pcm512x->loop1_inv;
	ucontrol->value.enumerated.item[0] = val;

	return 0;
}

static int linea_pcm512x_put_mux1(struct snd_kcontrol *kctrl,
				  struct snd_ctl_elem_value *ucontrol)
{
	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
	struct snd_soc_card *card = dapm->card;
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);
	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
	unsigned int val = ucontrol->value.enumerated.item[0];

	if (val >= e->items)
		return -EINVAL;

	gpio_set_value(pcm512x->loop1, !val ^ !pcm512x->loop1_inv);

	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
}

static int linea_pcm512x_get_mux2(struct snd_kcontrol *kctrl,
				  struct snd_ctl_elem_value *ucontrol)
{
	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
	struct snd_soc_card *card = dapm->card;
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);
	unsigned int val;

	val = !gpio_get_value(pcm512x->loop2) ^ !pcm512x->loop2_inv;
	ucontrol->value.enumerated.item[0] = val;

	return 0;
}

static int linea_pcm512x_put_mux2(struct snd_kcontrol *kctrl,
				  struct snd_ctl_elem_value *ucontrol)
{
	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
	struct snd_soc_card *card = dapm->card;
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);
	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
	unsigned int val = ucontrol->value.enumerated.item[0];

	if (val >= e->items)
		return -EINVAL;

	gpio_set_value(pcm512x->loop2, !val ^ !pcm512x->loop2_inv);

	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
}

int linea_pcm512x_get_mix1(struct snd_kcontrol *kctrl,
			   struct snd_ctl_elem_value *ucontrol)
{
	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
	struct snd_soc_card *card = dapm->card;
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);
	unsigned int val;

	val = !gpio_get_value(pcm512x->add1) ^ !pcm512x->add1_inv;
	ucontrol->value.integer.value[0] = val;

	return 0;
}

int linea_pcm512x_put_mix1(struct snd_kcontrol *kctrl,
			   struct snd_ctl_elem_value *ucontrol)
{
	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
	struct snd_soc_card *card = dapm->card;
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);
	int old;
	int connect;

	connect = !!ucontrol->value.integer.value[0];
	old = !gpio_get_value(pcm512x->add1) ^ !pcm512x->add1_inv;

	if (old == connect)
		return 0;

	/*
	 * BUG the gpio_set_value call should happen inside
	 * snd_soc_dapm_mixer_update_power in the loop.
	 */
	gpio_set_value(pcm512x->add1, !connect ^ !pcm512x->add1_inv);

	snd_soc_dapm_mixer_update_power(dapm, kctrl, connect, NULL);
	return 1;
}

int linea_pcm512x_get_mix2(struct snd_kcontrol *kctrl,
			   struct snd_ctl_elem_value *ucontrol)
{
	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
	struct snd_soc_card *card = dapm->card;
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);
	unsigned int val;

	val = !gpio_get_value(pcm512x->add2) ^ !pcm512x->add2_inv;
	ucontrol->value.integer.value[0] = val;

	return 0;
}

int linea_pcm512x_put_mix2(struct snd_kcontrol *kctrl,
			   struct snd_ctl_elem_value *ucontrol)
{
	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
	struct snd_soc_card *card = dapm->card;
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);
	int old;
	int connect;

	connect = !!ucontrol->value.integer.value[0];
	old = !gpio_get_value(pcm512x->add2) ^ !pcm512x->add2_inv;

	if (old == connect)
		return 0;

	/*
	 * BUG the gpio_set_value call should happen inside
	 * snd_soc_dapm_mixer_update_power in the loop.
	 */
	gpio_set_value(pcm512x->add2, !connect ^ !pcm512x->add2_inv);

	snd_soc_dapm_mixer_update_power(dapm, kctrl, connect, NULL);
	return 1;
}

static const char *mux_text[] = { "Mixer", "Loop" };

static const struct soc_enum mux_enum =
	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, 2, mux_text);

static const struct snd_kcontrol_new mux1 =
	SOC_DAPM_ENUM_EXT("MUX1", mux_enum,
			  linea_pcm512x_get_mux1, linea_pcm512x_put_mux1);

static const struct snd_kcontrol_new mux2 =
	SOC_DAPM_ENUM_EXT("MUX2", mux_enum,
			  linea_pcm512x_get_mux2, linea_pcm512x_put_mux2);

#define PCM512x_DAPM_SINGLE_EXT(xname, reg, shift, max, invert, xget, xput) \
{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
	.info = snd_soc_info_volsw, \
	.get = xget, \
	.put = xput, \
	.private_value = SOC_SINGLE_VALUE(reg, shift, max, invert, 0) }

static const struct snd_kcontrol_new mix1[] = {
	PCM512x_DAPM_SINGLE_EXT("IN1 Switch", SND_SOC_NOPM, 0, 1, 0,
				linea_pcm512x_get_mix1, linea_pcm512x_put_mix1),
};

static const struct snd_kcontrol_new mix2[] = {
	PCM512x_DAPM_SINGLE_EXT("IN2 Switch", SND_SOC_NOPM, 0, 1, 0,
				linea_pcm512x_get_mix2, linea_pcm512x_put_mix2),
};

static const struct snd_soc_dapm_widget linea_pcm512x_dapm_widgets[] = {
	SND_SOC_DAPM_OUTPUT("OUT1"),
	SND_SOC_DAPM_OUTPUT("OUT2"),
	SND_SOC_DAPM_INPUT("IN1"),
	SND_SOC_DAPM_INPUT("IN2"),
	SND_SOC_DAPM_INPUT("DAC1"),
	SND_SOC_DAPM_INPUT("DAC2"),
	SND_SOC_DAPM_AIF_IN("AIFINL", "Playback", 0, SND_SOC_NOPM, 0, 0),
	SND_SOC_DAPM_AIF_IN("AIFINR", "Playback", 1, SND_SOC_NOPM, 0, 0),
	SOC_MIXER_ARRAY("MIX1", SND_SOC_NOPM, 0, 0, mix1),
	SOC_MIXER_ARRAY("MIX2", SND_SOC_NOPM, 0, 0, mix2),
	SND_SOC_DAPM_MUX("MUX1", SND_SOC_NOPM, 0, 0, &mux1),
	SND_SOC_DAPM_MUX("MUX2", SND_SOC_NOPM, 0, 0, &mux2),
};

static const struct snd_soc_dapm_route linea_pcm512x_intercon[] = {
	{ "OUT1", NULL, "MUX1" },
	{ "OUT2", NULL, "MUX2" },

	{ "MUX1", "Loop",  "IN1" },
	{ "MUX1", "Mixer", "MIX1" },

	{ "MUX2", "Loop",  "IN2" },
	{ "MUX2", "Mixer", "MIX2" },

	{ "MIX1", NULL, "DAC1" },
	{ "MIX1", "IN1 Switch", "IN1" },
	{ "MIX2", NULL, "DAC2" },
	{ "MIX2", "IN2 Switch", "IN2" },
};

static int linea_pcm512x_hw_params(struct snd_pcm_substream *substream,
	struct snd_pcm_hw_params *params)
{
	struct snd_soc_pcm_runtime *rtd = substream->private_data;
	struct device *dev = rtd->dev;
	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
	int dir;
	int period;
	int ret;

	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
		dir = 0;
	else
		dir = 1;

	period = snd_pcm_format_physical_width(params_format(params));
	ret = snd_soc_dai_set_clkdiv(cpu_dai,
		dir ? ATMEL_SSC_RCMR_PERIOD : ATMEL_SSC_TCMR_PERIOD,
		period - 1);
	if (ret < 0) {
		dev_err(dev,
			"%s - Failed to set cpu dai lrclk %d divider\n",
			__func__, dir);
		return ret;
	}

	return 0;
}

static struct snd_soc_ops linea_pcm512x_ops = {
	.hw_params = linea_pcm512x_hw_params,
};

static int linea_pcm512x_init(struct snd_soc_pcm_runtime *rtd)
{
	struct snd_soc_dapm_context *dapm = &rtd->card->dapm;

	return snd_soc_dapm_add_routes(dapm, linea_pcm512x_intercon,
				       ARRAY_SIZE(linea_pcm512x_intercon));
}

static struct snd_soc_dai_link linea_pcm512x_dailink = {
	.name = "PCM512x",
	.stream_name = "PCM512x PCM",
	.codec_dai_name = "pcm512x-hifi",
	.dai_fmt = SND_SOC_DAIFMT_I2S
		| SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFS,
	.init = linea_pcm512x_init,
	.ops = &linea_pcm512x_ops,
};

static struct snd_soc_card linea_pcm512x_card = {
	.name = "linea_pcm512x",
	.owner = THIS_MODULE,
	.dai_link = &linea_pcm512x_dailink,
	.num_links = 1,
	.dapm_widgets = linea_pcm512x_dapm_widgets,
	.num_dapm_widgets = ARRAY_SIZE(linea_pcm512x_dapm_widgets),
	.fully_routed = true,
};

static int linea_pcm512x_dt_init(struct platform_device *pdev)
{
	struct device_node *np = pdev->dev.of_node;
	struct device_node *codec_np, *cpu_np;
	struct snd_soc_card *card = &linea_pcm512x_card;
	struct snd_soc_dai_link *dailink = &linea_pcm512x_dailink;
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);
	enum of_gpio_flags flags;
	int ret;

	if (!np) {
		dev_err(&pdev->dev, "only device tree supported\n");
		return -EINVAL;
	}

	ret = snd_soc_of_parse_card_name(card, "axentia,model");
	if (ret) {
		dev_err(&pdev->dev, "failed to parse card name\n");
		return ret;
	}

	ret = snd_soc_of_parse_audio_routing(card, "axentia,audio-routing");
	if (ret) {
		dev_err(&pdev->dev, "failed to parse audio routing\n");
		return ret;
	}

	cpu_np = of_parse_phandle(np, "axentia,ssc-controller", 0);
	if (!cpu_np) {
		dev_err(&pdev->dev, "failed to get dai and pcm info\n");
		ret = -EINVAL;
		return ret;
	}
	dailink->cpu_of_node = cpu_np;
	dailink->platform_of_node = cpu_np;
	pcm512x->ssc_id = of_alias_get_id(cpu_np, "ssc");
	of_node_put(cpu_np);

	codec_np = of_parse_phandle(np, "axentia,audio-codec", 0);
	if (!codec_np) {
		dev_err(&pdev->dev, "failed to get codec info\n");
		ret = -EINVAL;
		return ret;
	}
	dailink->codec_of_node = codec_np;
	of_node_put(codec_np);

	pcm512x->add1 = of_get_gpio_flags(np, 0, &flags);
	pcm512x->add1_inv = flags == OF_GPIO_ACTIVE_LOW;
	pcm512x->add2 = of_get_gpio_flags(np, 1, &flags);
	pcm512x->add2_inv = flags == OF_GPIO_ACTIVE_LOW;
	pcm512x->loop1 = of_get_gpio_flags(np, 2, &flags);
	pcm512x->loop1_inv = flags == OF_GPIO_ACTIVE_LOW;
	pcm512x->loop2 = of_get_gpio_flags(np, 3, &flags);
	pcm512x->loop2_inv = flags == OF_GPIO_ACTIVE_LOW;

	return 0;
}

static int linea_pcm512x_gpio_output(struct device *dev, const char *name,
				     int gpio, int value)
{
	int res;

	if (!gpio_is_valid(gpio))
		return 0;

	res = devm_gpio_request(dev, gpio, name);
	if (res < 0) {
		dev_err(dev, "can't request %s gpio %d\n", name, gpio);
		return res;
	}

	res = gpio_direction_output(gpio, value);
	if (res < 0) {
		dev_err(dev, "can't request %s output %d\n", name, gpio);
		return res;
	}

	return 0;
}

static int linea_pcm512x_probe(struct platform_device *pdev)
{
	struct snd_soc_card *card = &linea_pcm512x_card;
	struct pcm512x_priv *pcm512x;
	int ret;

	card->dev = &pdev->dev;

	pcm512x = devm_kzalloc(card->dev, sizeof(*pcm512x), GFP_KERNEL);
	if (!pcm512x)
		return -ENOMEM;

	snd_soc_card_set_drvdata(card, pcm512x);

	ret = linea_pcm512x_dt_init(pdev);
	if (ret) {
		dev_err(&pdev->dev, "failed to init dt info\n");
		return ret;
	}

	ret = linea_pcm512x_gpio_output(card->dev, "ADD1",
					pcm512x->add1, !pcm512x->add1_inv);
	if (ret < 0)
		return ret;
	ret = linea_pcm512x_gpio_output(card->dev, "ADD2",
					pcm512x->add2, !pcm512x->add2_inv);
	if (ret < 0)
		return ret;
	ret = linea_pcm512x_gpio_output(card->dev, "LOOP1",
				       pcm512x->loop1, !pcm512x->loop1_inv);
	if (ret < 0)
		return ret;
	ret = linea_pcm512x_gpio_output(card->dev, "LOOP2",
					pcm512x->loop2, !pcm512x->loop2_inv);
	if (ret < 0)
		return ret;

	ret = atmel_ssc_set_audio(pcm512x->ssc_id);
	if (ret != 0) {
		dev_err(&pdev->dev,
			"failed to set SSC %d for audio\n", pcm512x->ssc_id);
		return ret;
	}

	ret = snd_soc_register_card(card);
	if (ret) {
		dev_err(&pdev->dev, "snd_soc_register_card failed\n");
		goto err_set_audio;
	}

	return 0;

err_set_audio:
	atmel_ssc_put_audio(pcm512x->ssc_id);
	return ret;
}

static int linea_pcm512x_remove(struct platform_device *pdev)
{
	struct snd_soc_card *card = platform_get_drvdata(pdev);
	struct pcm512x_priv *pcm512x = snd_soc_card_get_drvdata(card);

	snd_soc_unregister_card(card);
	atmel_ssc_put_audio(pcm512x->ssc_id);

	return 0;
}

#ifdef CONFIG_OF
static const struct of_device_id linea_pcm512x_dt_ids[] = {
	{ .compatible = "axentia,asoc-linea-tse850", },
	{ }
};
#endif

static struct platform_driver linea_pcm512x_driver = {
	.driver = {
		.name = "axentia-pcm512x-audio",
		.of_match_table = of_match_ptr(linea_pcm512x_dt_ids),
	},
	.probe = linea_pcm512x_probe,
	.remove = linea_pcm512x_remove,
};

module_platform_driver(linea_pcm512x_driver);

/* Module information */
MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
MODULE_DESCRIPTION("ALSA SoC machine driver for Linea/PCM512x");
MODULE_LICENSE("GPL");

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

Re: Regression with .set_bias_level in pcm512x?

[Lars-Peter Clausen]

On 01/14/2015 10:31 AM, Peter Rosin wrote:
> Hi!
>
> Lars-Peter Clausen wrote:
>> On 01/13/2015 11:49 PM, Peter Rosin wrote:
>>> Hi!
>>>
>>> I'm working on adding support for DAIFMT_CBM_CFM and
>> DAIFMT_CBM_CFS to
>>> pcm512x, and it works nicely in the 3.10 kernel + Atmel-patches were I
>>> have developed the code. Now I'm trying to forward-port it and test it
>>> on
>>> 3.18 + other-Atmel-patches in preparation for submission. However,
>>> there seems to be a regression in that the (pre-existing)
>>> pcm512x_set_bias_level callback is never called in the 3.18 version.
>>>
>>> I see that there has been some core work on set_bias_level, and
>>> suspect it has something to do with dapm and components, but I think
>>> someone else will find the problem much quicker than I am able to.
>>>
>>> I have feebly tried to revert 0bd2ac3dae74ee25c5ea171cb572731c7a89c248
>>> and 68f831c2724ab72c0088471b2ed1dc99e81948ef, but that didn't help.
>>> That also seems a bit backwards. So, what needs to be done in the
>>> pcm512x driver to have set_bias_level called?
>>
>> It's unlikely that these commits have anything to do with it.
>
> Agreed, I just wanted to show that I at least did try some things...
>
>> I think it is more likely that the DAPM state (for whatever reason) never
>> actually changes. Check the state of the DAPM context and all the widgets in
>> /sys/kernel/debug/asoc/your_card/... and compare their state when the
>> system is idle to when it should not be idle.
>
> There is indeed something strange going on with dapm. When things should be
> active, I have this (for the non-working 3.18 variant):
>
> $ cat codec\:pcm512x.0-004c/dapm/Playback \
> 	codec\:pcm512x.0-004c/dapm/DACL \
> 	codec\:pcm512x.0-004c/dapm/OUTL \
> 	dapm/DAC1 \
> 	dapm/MIX1 \
> 	dapm/MUX1 \
> 	dapm/OUT1 \
> 	dapm/AIFINL
> Playback: Off  in 1 out 0
>   stream Playback active
>   out "static" "DACR"
>   out "static" "DACL"
> DACL: Off  in 1 out 0
>   in  "static" "Playback"
>   out "static" "OUTL"
> OUTL: Off  in 1 out 0
>   in  "static" "DACL"
>   out "static" "DAC1"
> DAC1: Off  in 1 out 0
>   in  "static" "OUTL"
>   out "static" "MIX1"
> MIX1: Off  in 1 out 0
>   in  "static" "DAC1"
>   out "Mixer" "MUX1"
> MUX1: Off  in 1 out 0
>   in  "Mixer" "MIX1"
>   out "static" "OUT1"
> OUT1: Off  in 1 out 0
>   in  "static" "MUX1"
> AIFINL: Off  in 0 out 0
>   stream Playback inactive
>
> When things are inactive, all "in 1" change to "in 0", and the top "stream Playback"
> change to "inactive".
>
> bias_level of both the card and the codec are "Off", regardless.
>
> For reference, I have attached my current card driver, which is given this:
>
> 	sound {
> 		compatible = "axentia,asoc-linea-tse850";
>
> 		axentia,model = "TSE850 @ Linea";
> 		axentia,audio-routing =
> 			"DAC1", "OUTL",
> 			"DAC2", "OUTR";
>
> 		axentia,ssc-controller = <&ssc0>;
> 		axentia,audio-codec = <&pcm5142>;
>
> 		gpios = <&pioA 9 GPIO_ACTIVE_LOW>,
> 			<&pioA 8 GPIO_ACTIVE_LOW>,
> 			<&pioA 10 GPIO_ACTIVE_LOW>,
> 			<&pioA 11 GPIO_ACTIVE_LOW>;
>
> 		status = "okay";
> 	};
>
> I suspect that it has something to do with the snd_soc_dapm_add_routes call in
> linea_pcm512x_init? (I "stole" that piece from omap-abe-twl6040)


There does not seem to be a audio sink in your DAPM graph. You have a source 
which is the Playback stream, hence a one the "1" for "in". But a widget 
will only power up if it is on a active path from a source to a sink. Try to 
add a speaker, lineout or similar widget to the end of your graph.

- Lars

Re: Regression with .set_bias_level in pcm512x?

[Peter Rosin]

Lars-Peter Clausen wrote:
> On 01/14/2015 10:31 AM, Peter Rosin wrote:
> > Hi!
> >
> > Lars-Peter Clausen wrote:
> >> On 01/13/2015 11:49 PM, Peter Rosin wrote:
> >>> Hi!
> >>>
> >>> I'm working on adding support for DAIFMT_CBM_CFM and
> >> DAIFMT_CBM_CFS to
> >>> pcm512x, and it works nicely in the 3.10 kernel + Atmel-patches were
> >>> I have developed the code. Now I'm trying to forward-port it and
> >>> test it on
> >>> 3.18 + other-Atmel-patches in preparation for submission. However,
> >>> there seems to be a regression in that the (pre-existing)
> >>> pcm512x_set_bias_level callback is never called in the 3.18 version.
> >>>
> >>> I see that there has been some core work on set_bias_level, and
> >>> suspect it has something to do with dapm and components, but I think
> >>> someone else will find the problem much quicker than I am able to.
> >>>
> >>> I have feebly tried to revert
> >>> 0bd2ac3dae74ee25c5ea171cb572731c7a89c248
> >>> and 68f831c2724ab72c0088471b2ed1dc99e81948ef, but that didn't help.
> >>> That also seems a bit backwards. So, what needs to be done in the
> >>> pcm512x driver to have set_bias_level called?
> >>
> >> It's unlikely that these commits have anything to do with it.
> >
> > Agreed, I just wanted to show that I at least did try some things...
> >
> >> I think it is more likely that the DAPM state (for whatever reason)
> >> never actually changes. Check the state of the DAPM context and all
> >> the widgets in /sys/kernel/debug/asoc/your_card/... and compare their
> >> state when the system is idle to when it should not be idle.
> >
> > There is indeed something strange going on with dapm. When things
> > should be active, I have this (for the non-working 3.18 variant):
> >
> > $ cat codec\:pcm512x.0-004c/dapm/Playback \
> > 	codec\:pcm512x.0-004c/dapm/DACL \
> > 	codec\:pcm512x.0-004c/dapm/OUTL \
> > 	dapm/DAC1 \
> > 	dapm/MIX1 \
> > 	dapm/MUX1 \
> > 	dapm/OUT1 \
> > 	dapm/AIFINL
> > Playback: Off  in 1 out 0
> >   stream Playback active
> >   out "static" "DACR"
> >   out "static" "DACL"
> > DACL: Off  in 1 out 0
> >   in  "static" "Playback"
> >   out "static" "OUTL"
> > OUTL: Off  in 1 out 0
> >   in  "static" "DACL"
> >   out "static" "DAC1"
> > DAC1: Off  in 1 out 0
> >   in  "static" "OUTL"
> >   out "static" "MIX1"
> > MIX1: Off  in 1 out 0
> >   in  "static" "DAC1"
> >   out "Mixer" "MUX1"
> > MUX1: Off  in 1 out 0
> >   in  "Mixer" "MIX1"
> >   out "static" "OUT1"
> > OUT1: Off  in 1 out 0
> >   in  "static" "MUX1"
> > AIFINL: Off  in 0 out 0
> >   stream Playback inactive
> >
> > When things are inactive, all "in 1" change to "in 0", and the top "stream Playback"
> > change to "inactive".
> >
> > bias_level of both the card and the codec are "Off", regardless.
> >
> > For reference, I have attached my current card driver, which is given this:
> >
> > 	sound {
> > 		compatible = "axentia,asoc-linea-tse850";
> >
> > 		axentia,model = "TSE850 @ Linea";
> > 		axentia,audio-routing =
> > 			"DAC1", "OUTL",
> > 			"DAC2", "OUTR";
> >
> > 		axentia,ssc-controller = <&ssc0>;
> > 		axentia,audio-codec = <&pcm5142>;
> >
> > 		gpios = <&pioA 9 GPIO_ACTIVE_LOW>,
> > 			<&pioA 8 GPIO_ACTIVE_LOW>,
> > 			<&pioA 10 GPIO_ACTIVE_LOW>,
> > 			<&pioA 11 GPIO_ACTIVE_LOW>;
> >
> > 		status = "okay";
> > 	};
> >
> > I suspect that it has something to do with the snd_soc_dapm_add_routes
> > call in linea_pcm512x_init? (I "stole" that piece from
> > omap-abe-twl6040)
> 
> There does not seem to be a audio sink in your DAPM graph. You have a
> source which is the Playback stream, hence a one the "1" for "in". But a
> widget will only power up if it is on a active path from a source to a sink. Try
> to add a speaker, lineout or similar widget to the end of your graph.

Indeed. I changed "OUT1", "OUT2", "IN1" and "IN2" to be
SND_SOC_DAPM_LINEs instead and pcm512x_set_bias_level is now called.
Thanks!  (and it works too!)

Cheers,
Peter