[PATCH 0/6] ASoC: ak4613: add TDM256 test support
Renesas is the only user of ak4613 driver on upstream for now. It has STEREO/TDM512/TDM256/TDM128 mode, but STEREO only is used, because of Renesas board connection.
I noticed that I can test first 2ch out of TDM256 mode 8ch Playback even in such a situation.
[PATCH 1/6] - [PATCH 5/6] are prepare for TDM mode support. [PATCH 6/6] uses "ifdef style" to enable TDM because I can't test full TDM256 and/or other TDM mode. "ifdef style" has no effect to current supported STEREO mode.
Kuninori Morimoto (6): 1) ASoC: ak4613: add missing mutex_lock() 2) ASoC: ak4613: tidyup ak4613_interface 3) ASoC: ak4613: return error if it was setup as clock provider 4) ASoC: ak4613: priv has ctrl1 instead of iface 5) ASoC: ak4613: rename constraint to constraint_rates 6) ASoC: ak4613: add TDM256 support
sound/soc/codecs/ak4613.c | 367 +++++++++++++++++++++++++++++++------- 1 file changed, 300 insertions(+), 67 deletions(-)
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
We need to use mutex_lock() for priv->cnt / priv->iface, but we are missing it at ak4613_dai_startup(). This patch adds missing mutex_lock() for it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/codecs/ak4613.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/soc/codecs/ak4613.c b/sound/soc/codecs/ak4613.c index 034195c83bd7..e0d9a8c58e10 100644 --- a/sound/soc/codecs/ak4613.c +++ b/sound/soc/codecs/ak4613.c @@ -304,7 +304,9 @@ static int ak4613_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_component *component = dai->component; struct ak4613_priv *priv = snd_soc_component_get_drvdata(component);
+ mutex_lock(&priv->lock); priv->cnt++; + mutex_unlock(&priv->lock);
ak4613_hw_constraints(priv, substream->runtime);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
ak4613 driver is assuming symmetric format. Thus, we don't need to have asymmetric table for judging the iface at .hw_param.
This patch cleanup ak4613_interface for it. This is prepare for TDM support.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/codecs/ak4613.c | 55 +++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 31 deletions(-)
diff --git a/sound/soc/codecs/ak4613.c b/sound/soc/codecs/ak4613.c index e0d9a8c58e10..a20bbf82e8df 100644 --- a/sound/soc/codecs/ak4613.c +++ b/sound/soc/codecs/ak4613.c @@ -78,14 +78,10 @@ /* OCTRL */ #define OCTRL_MASK (0x3F)
-struct ak4613_formats { +struct ak4613_interface { unsigned int width; unsigned int fmt; -}; - -struct ak4613_interface { - struct ak4613_formats capture; - struct ak4613_formats playback; + u8 dif; };
struct ak4613_priv { @@ -137,13 +133,22 @@ static const struct reg_default ak4613_reg[] = { { 0x14, 0x00 }, { 0x15, 0x00 }, { 0x16, 0x00 }, };
-#define AUDIO_IFACE_TO_VAL(fmts) ((fmts - ak4613_iface) << 3) -#define AUDIO_IFACE(b, fmt) { b, SND_SOC_DAIFMT_##fmt } +/* + * CTRL1 register + * see + * Table 11/12/13/14 + */ +#define AUDIO_IFACE(_val, _width, _fmt) \ + { \ + .dif = (_val << 3), \ + .width = _width, \ + .fmt = SND_SOC_DAIFMT_##_fmt,\ + } static const struct ak4613_interface ak4613_iface[] = { - /* capture */ /* playback */ - /* [0] - [2] are not supported */ - [3] = { AUDIO_IFACE(24, LEFT_J), AUDIO_IFACE(24, LEFT_J) }, - [4] = { AUDIO_IFACE(24, I2S), AUDIO_IFACE(24, I2S) }, + /* It doesn't support asymmetric format */ + + AUDIO_IFACE(0x03, 24, LEFT_J), + AUDIO_IFACE(0x04, 24, I2S), };
static const struct regmap_config ak4613_regmap_cfg = { @@ -344,20 +349,13 @@ static int ak4613_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) }
static bool ak4613_dai_fmt_matching(const struct ak4613_interface *iface, - int is_play, unsigned int fmt, unsigned int width) { - const struct ak4613_formats *fmts; - - fmts = (is_play) ? &iface->playback : &iface->capture; + if ((iface->fmt == fmt) && + (iface->width == width)) + return true;
- if (fmts->fmt != fmt) - return false; - - if (fmts->width != width) - return false; - - return true; + return false; }
static int ak4613_dai_hw_params(struct snd_pcm_substream *substream, @@ -371,9 +369,8 @@ static int ak4613_dai_hw_params(struct snd_pcm_substream *substream, unsigned int width = params_width(params); unsigned int fmt = priv->fmt; unsigned int rate; - int is_play = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; int i, ret; - u8 fmt_ctrl, ctrl2; + u8 ctrl2;
rate = params_rate(params); switch (rate) { @@ -401,18 +398,16 @@ static int ak4613_dai_hw_params(struct snd_pcm_substream *substream, * * It doesn't support TDM at this point */ - fmt_ctrl = NO_FMT; ret = -EINVAL; iface = NULL;
mutex_lock(&priv->lock); if (priv->iface) { - if (ak4613_dai_fmt_matching(priv->iface, is_play, fmt, width)) + if (ak4613_dai_fmt_matching(priv->iface, fmt, width)) iface = priv->iface; } else { for (i = ARRAY_SIZE(ak4613_iface) - 1; i >= 0; i--) { if (!ak4613_dai_fmt_matching(ak4613_iface + i, - is_play, fmt, width)) continue; iface = ak4613_iface + i; @@ -430,9 +425,7 @@ static int ak4613_dai_hw_params(struct snd_pcm_substream *substream, if (ret < 0) goto hw_params_end;
- fmt_ctrl = AUDIO_IFACE_TO_VAL(iface); - - snd_soc_component_update_bits(component, CTRL1, FMT_MASK, fmt_ctrl); + snd_soc_component_update_bits(component, CTRL1, FMT_MASK, iface->dif); snd_soc_component_update_bits(component, CTRL2, DFS_MASK, ctrl2);
snd_soc_component_update_bits(component, ICTRL, ICTRL_MASK, priv->ic);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Renesas is only user of ak4613 on upstream, and it is tested only under "clock consumer" because of board mounting situation.
Thus, "clock provider" is not supperted/tested. This patch return error if it was setup as clock provider.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/codecs/ak4613.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/sound/soc/codecs/ak4613.c b/sound/soc/codecs/ak4613.c index a20bbf82e8df..b19c7c4a1971 100644 --- a/sound/soc/codecs/ak4613.c +++ b/sound/soc/codecs/ak4613.c @@ -329,13 +329,13 @@ static int ak4613_dai_set_sysclk(struct snd_soc_dai *codec_dai, return 0; }
-static int ak4613_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) +static int ak4613_dai_set_fmt(struct snd_soc_dai *dai, unsigned int format) { struct snd_soc_component *component = dai->component; struct ak4613_priv *priv = snd_soc_component_get_drvdata(component); + unsigned int fmt;
- fmt &= SND_SOC_DAIFMT_FORMAT_MASK; - + fmt = format & SND_SOC_DAIFMT_FORMAT_MASK; switch (fmt) { case SND_SOC_DAIFMT_LEFT_J: case SND_SOC_DAIFMT_I2S: @@ -345,6 +345,19 @@ static int ak4613_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) return -EINVAL; }
+ fmt = format & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK; + switch (fmt) { + case SND_SOC_DAIFMT_CBC_CFC: + break; + default: + /* + * SUPPORTME + * + * "clock provider" is not yet supperted + */ + return -EINVAL; + } + return 0; }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current priv is using ->iface, but it is not good match to support TDM. This patch adds ->ctrl1 instead of it. This is prepare for TDM support.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/codecs/ak4613.c | 61 ++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 33 deletions(-)
diff --git a/sound/soc/codecs/ak4613.c b/sound/soc/codecs/ak4613.c index b19c7c4a1971..73fae6ffe92b 100644 --- a/sound/soc/codecs/ak4613.c +++ b/sound/soc/codecs/ak4613.c @@ -86,7 +86,6 @@ struct ak4613_interface {
struct ak4613_priv { struct mutex lock; - const struct ak4613_interface *iface; struct snd_pcm_hw_constraint_list constraint; struct work_struct dummy_write_work; struct snd_soc_component *component; @@ -94,9 +93,10 @@ struct ak4613_priv { unsigned int sysclk;
unsigned int fmt; + int cnt; + u8 ctrl1; u8 oc; u8 ic; - int cnt; };
/* @@ -138,9 +138,9 @@ static const struct reg_default ak4613_reg[] = { * see * Table 11/12/13/14 */ -#define AUDIO_IFACE(_val, _width, _fmt) \ +#define AUDIO_IFACE(_dif, _width, _fmt) \ { \ - .dif = (_val << 3), \ + .dif = _dif, \ .width = _width, \ .fmt = SND_SOC_DAIFMT_##_fmt,\ } @@ -255,7 +255,7 @@ static void ak4613_dai_shutdown(struct snd_pcm_substream *substream, priv->cnt = 0; } if (!priv->cnt) - priv->iface = NULL; + priv->ctrl1 = 0; mutex_unlock(&priv->lock); }
@@ -361,23 +361,12 @@ static int ak4613_dai_set_fmt(struct snd_soc_dai *dai, unsigned int format) return 0; }
-static bool ak4613_dai_fmt_matching(const struct ak4613_interface *iface, - unsigned int fmt, unsigned int width) -{ - if ((iface->fmt == fmt) && - (iface->width == width)) - return true; - - return false; -} - static int ak4613_dai_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) { struct snd_soc_component *component = dai->component; struct ak4613_priv *priv = snd_soc_component_get_drvdata(component); - const struct ak4613_interface *iface; struct device *dev = component->dev; unsigned int width = params_width(params); unsigned int fmt = priv->fmt; @@ -412,33 +401,39 @@ static int ak4613_dai_hw_params(struct snd_pcm_substream *substream, * It doesn't support TDM at this point */ ret = -EINVAL; - iface = NULL;
mutex_lock(&priv->lock); - if (priv->iface) { - if (ak4613_dai_fmt_matching(priv->iface, fmt, width)) - iface = priv->iface; + if (priv->cnt > 1) { + /* + * If it was already working, use current priv->ctrl1 + */ + ret = 0; } else { + /* + * It is not yet working, + */ for (i = ARRAY_SIZE(ak4613_iface) - 1; i >= 0; i--) { - if (!ak4613_dai_fmt_matching(ak4613_iface + i, - fmt, width)) - continue; - iface = ak4613_iface + i; - break; + const struct ak4613_interface *iface = ak4613_iface + i; + + if ((iface->fmt == fmt) && (iface->width == width)) { + /* + * Ctrl1 + * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 | + * |TDM1|TDM0|DIF2|DIF1|DIF0|ATS1|ATS0|SMUTE| + * < iface->dif > + */ + priv->ctrl1 = (iface->dif << 3); + ret = 0; + break; + } } } - - if ((priv->iface == NULL) || - (priv->iface == iface)) { - priv->iface = iface; - ret = 0; - } mutex_unlock(&priv->lock);
if (ret < 0) goto hw_params_end;
- snd_soc_component_update_bits(component, CTRL1, FMT_MASK, iface->dif); + snd_soc_component_update_bits(component, CTRL1, FMT_MASK, priv->ctrl1); snd_soc_component_update_bits(component, CTRL2, DFS_MASK, ctrl2);
snd_soc_component_update_bits(component, ICTRL, ICTRL_MASK, priv->ic); @@ -675,7 +670,7 @@ static int ak4613_i2c_probe(struct i2c_client *i2c,
ak4613_parse_of(priv, dev);
- priv->iface = NULL; + priv->ctrl1 = 0; priv->cnt = 0; priv->sysclk = 0; INIT_WORK(&priv->dummy_write_work, ak4613_dummy_write);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
TDM support needs to use constraint_channels. This patch renames current constraint to constraint_rates for it. This is prepare for TDM support.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/codecs/ak4613.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/ak4613.c b/sound/soc/codecs/ak4613.c index 73fae6ffe92b..2ec6313e823d 100644 --- a/sound/soc/codecs/ak4613.c +++ b/sound/soc/codecs/ak4613.c @@ -86,7 +86,7 @@ struct ak4613_interface {
struct ak4613_priv { struct mutex lock; - struct snd_pcm_hw_constraint_list constraint; + struct snd_pcm_hw_constraint_list constraint_rates; struct work_struct dummy_write_work; struct snd_soc_component *component; unsigned int rate; @@ -272,10 +272,11 @@ static void ak4613_hw_constraints(struct ak4613_priv *priv, 176400, 192000, }; - struct snd_pcm_hw_constraint_list *constraint = &priv->constraint; + struct snd_pcm_hw_constraint_list *constraint; unsigned int fs; int i;
+ constraint = &priv->constraint_rates; constraint->list = ak4613_rates; constraint->mask = 0; constraint->count = 0;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
AK4613 has STEREO/TDM512/TDM256/TDM128 mode. Renesas is the only user of ak4613 on upstream for now, and is using it as STEREO mode, because of board connection. Thus, current driver is supporting STEREO mode only, and other modes are not supported.
But I noticed that I can try first 2ch out of TDM256 mode 8ch Playback even in such a situation.
But because of board connection, I can't test full TDM256 mode, and/or other TDM mode. Thus I don't want to add new DT propaty for now. This patch enables TDM256 mode test by "ifdef style", but it has no effect to current supported STEREO mode. You can define AK4613_ENABLE_TDM_TEST to try TDM256 mode.
Please don't hesitate to break current code if you can add full TDM256 and/or other TDM mode. You don't need to care compatibility with Renesas.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/codecs/ak4613.c | 245 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 237 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/ak4613.c b/sound/soc/codecs/ak4613.c index 2ec6313e823d..03b829930769 100644 --- a/sound/soc/codecs/ak4613.c +++ b/sound/soc/codecs/ak4613.c @@ -10,11 +10,97 @@ // Based on ak4535.c by Richard Purdie // Based on wm8753.c by Liam Girdwood
+/* + * +-------+ + * |AK4613 | + * SDTO1 <-| | + * | | + * SDTI1 ->| | + * SDTI2 ->| | + * SDTI3 ->| | + * +-------+ + * + * +---+ + * clk | |___________________________________________... + * + * [TDM512] + * SDTO1 [L1][R1][L2][R2] + * SDTI1 [L1][R1][L2][R2][L3][R3][L4][R4][L5][R5][L6][R6] + * + * [TDM256] + * SDTO1 [L1][R1][L2][R2] + * SDTI1 [L1][R1][L2][R2][L3][R3][L4][R4] + * SDTI2 [L5][R5][L6][R6] + * + * [TDM128] + * SDTO1 [L1][R1][L2][R2] + * SDTI1 [L1][R1][L2][R2] + * SDTI2 [L3][R3][L4][R4] + * SDTI3 [L5][R5][L6][R6] + * + * [STEREO] + * Playback 2ch : SDTI1 + * Capture 2ch : SDTO1 + * + * [TDM512] + * Playback 12ch : SDTI1 + * Capture 4ch : SDTO1 + * + * [TDM256] + * Playback 12ch : SDTI1 + SDTI2 + * Playback 8ch : SDTI1 + * Capture 4ch : SDTO1 + * + * [TDM128] + * Playback 12ch : SDTI1 + SDTI2 + SDTI3 + * Playback 8ch : SDTI1 + SDTI2 + * Playback 4ch : SDTI1 + * Capture 4ch : SDTO1 + * + * + * !!! NOTE !!! + * + * Renesas is the only user of ak4613 on upstream so far, + * but the chip connection is like below. + * Thus, Renesas can't test all connection case. + * Tested TDM is very limited. + * + * +-----+ +-----------+ + * | SoC | | AK4613 | + * | |<-----|SDTO1 IN1|<-- Mic + * | | | IN2| + * | | | | + * | |----->|SDTI1 OUT1|--> Headphone + * +-----+ |SDTI2 OUT2| + * |SDTI3 OUT3| + * | OUT4| + * | OUT5| + * | OUT6| + * +-----------+ + * + * Renesas SoC can handle [2, 6,8] channels. + * Ak4613 can handle [2,4, 8,12] channels. + * + * Because of above HW connection and available channels number, + * Renesas could test are ... + * + * [STEREO] Playback 2ch : SDTI1 + * Capture 2ch : SDTO1 + * [TDM256] Playback 8ch : SDTI1 (*) + * + * (*) it used 8ch data between SoC <-> AK4613 on TDM256 mode, + * but could confirm is only first 2ch because only 1 + * Headphone is connected. + * + * see + * AK4613_ENABLE_TDM_TEST + */ #include <linux/clk.h> #include <linux/delay.h> #include <linux/i2c.h> #include <linux/slab.h> #include <linux/of_device.h> +#include <linux/of_graph.h> #include <linux/module.h> #include <linux/regmap.h> #include <sound/soc.h> @@ -78,6 +164,53 @@ /* OCTRL */ #define OCTRL_MASK (0x3F)
+/* + * configs + * + * 0x000000BA + * + * B : AK4613_CONFIG_SDTI_x + * A : AK4613_CONFIG_MODE_x + */ +#define AK4613_CONFIG_SET(priv, x) priv->configs |= AK4613_CONFIG_##x +#define AK4613_CONFIG_GET(priv, x) (priv->configs & AK4613_CONFIG_##x##_MASK) + +/* + * AK4613_CONFIG_SDTI_x + * + * It indicates how many SDTIx is connected. + */ +#define AK4613_CONFIG_SDTI_MASK (0xF << 4) +#define AK4613_CONFIG_SDTI(x) (((x) & 0xF) << 4) +#define AK4613_CONFIG_SDTI_set(priv, x) AK4613_CONFIG_SET(priv, SDTI(x)) +#define AK4613_CONFIG_SDTI_get(priv) ((AK4613_CONFIG_GET(priv, SDTI) >> 4) & 0xF) + +/* + * AK4613_CONFIG_MODE_x + * + * Same as Ctrl1 :: TDM1/TDM0 + * No shift is requested + * see + * AK4613_CTRL1_TO_MODE() + * Table 11/12/13/14 + */ +#define AK4613_CONFIG_MODE_MASK (0xF) +#define AK4613_CONFIG_MODE_STEREO (0x0) +#define AK4613_CONFIG_MODE_TDM512 (0x1) +#define AK4613_CONFIG_MODE_TDM256 (0x2) +#define AK4613_CONFIG_MODE_TDM128 (0x3) + +/* + * !!!! FIXME !!!! + * + * Because of testable HW limitation, TDM256 8ch TDM was only tested. + * This driver uses AK4613_ENABLE_TDM_TEST instead of new DT property so far. + * Don't hesitate to update driver, you don't need to care compatible + * with Renesas. + * + * #define AK4613_ENABLE_TDM_TEST + */ + struct ak4613_interface { unsigned int width; unsigned int fmt; @@ -87,12 +220,14 @@ struct ak4613_interface { struct ak4613_priv { struct mutex lock; struct snd_pcm_hw_constraint_list constraint_rates; + struct snd_pcm_hw_constraint_list constraint_channels; struct work_struct dummy_write_work; struct snd_soc_component *component; unsigned int rate; unsigned int sysclk;
unsigned int fmt; + unsigned int configs; int cnt; u8 ctrl1; u8 oc; @@ -150,6 +285,7 @@ static const struct ak4613_interface ak4613_iface[] = { AUDIO_IFACE(0x03, 24, LEFT_J), AUDIO_IFACE(0x04, 24, I2S), }; +#define AK4613_CTRL1_TO_MODE(priv) ((priv)->ctrl1 >> 6) /* AK4613_CONFIG_MODE_x */
static const struct regmap_config ak4613_regmap_cfg = { .reg_bits = 8, @@ -260,8 +396,9 @@ static void ak4613_dai_shutdown(struct snd_pcm_substream *substream, }
static void ak4613_hw_constraints(struct ak4613_priv *priv, - struct snd_pcm_runtime *runtime) + struct snd_pcm_substream *substream) { + struct snd_pcm_runtime *runtime = substream->runtime; static const unsigned int ak4613_rates[] = { 32000, 44100, @@ -272,8 +409,33 @@ static void ak4613_hw_constraints(struct ak4613_priv *priv, 176400, 192000, }; +#define AK4613_CHANNEL_2 0 +#define AK4613_CHANNEL_4 1 +#define AK4613_CHANNEL_8 2 +#define AK4613_CHANNEL_12 3 +#define AK4613_CHANNEL_NONE -1 + static const unsigned int ak4613_channels[] = { + [AK4613_CHANNEL_2] = 2, + [AK4613_CHANNEL_4] = 4, + [AK4613_CHANNEL_8] = 8, + [AK4613_CHANNEL_12] = 12, + }; +#define MODE_MAX 4 +#define SDTx_MAX 4 +#define MASK(x) (1 << AK4613_CHANNEL_##x) + static const int mask_list[MODE_MAX][SDTx_MAX] = { + /* SDTO SDTIx1 SDTIx2 SDTIx3 */ + [AK4613_CONFIG_MODE_STEREO] = { MASK(2), MASK(2), MASK(2), MASK(2)}, + [AK4613_CONFIG_MODE_TDM512] = { MASK(4), MASK(12), MASK(12), MASK(12)}, + [AK4613_CONFIG_MODE_TDM256] = { MASK(4), MASK(8), MASK(8)|MASK(12), MASK(8)|MASK(12)}, + [AK4613_CONFIG_MODE_TDM128] = { MASK(4), MASK(4), MASK(4)|MASK(8), MASK(4)|MASK(8)|MASK(12)}, + }; struct snd_pcm_hw_constraint_list *constraint; + unsigned int mask; + unsigned int mode; unsigned int fs; + int is_play = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; + int sdti_num; int i;
constraint = &priv->constraint_rates; @@ -302,6 +464,41 @@ static void ak4613_hw_constraints(struct ak4613_priv *priv,
snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, constraint); + + + sdti_num = AK4613_CONFIG_SDTI_get(priv); + if (WARN_ON(sdti_num >= SDTx_MAX)) + return; + + if (priv->cnt) { + /* + * If it was already working, + * the constraint is same as working mode. + */ + mode = AK4613_CTRL1_TO_MODE(priv); + mask = 0; /* no default */ + } else { + /* + * It is not yet working, + * the constraint is based on board configs. + * STEREO mask is default + */ + mode = AK4613_CONFIG_GET(priv, MODE); + mask = mask_list[AK4613_CONFIG_MODE_STEREO][is_play * sdti_num]; + } + + if (WARN_ON(mode >= MODE_MAX)) + return; + + /* add each mode mask */ + mask |= mask_list[mode][is_play * sdti_num]; + + constraint = &priv->constraint_channels; + constraint->list = ak4613_channels; + constraint->mask = mask; + constraint->count = sizeof(ak4613_channels); + snd_pcm_hw_constraint_list(runtime, 0, + SNDRV_PCM_HW_PARAM_CHANNELS, constraint); }
static int ak4613_dai_startup(struct snd_pcm_substream *substream, @@ -311,11 +508,10 @@ static int ak4613_dai_startup(struct snd_pcm_substream *substream, struct ak4613_priv *priv = snd_soc_component_get_drvdata(component);
mutex_lock(&priv->lock); + ak4613_hw_constraints(priv, substream); priv->cnt++; mutex_unlock(&priv->lock);
- ak4613_hw_constraints(priv, substream->runtime); - return 0; }
@@ -399,7 +595,7 @@ static int ak4613_dai_hw_params(struct snd_pcm_substream *substream, /* * FIXME * - * It doesn't support TDM at this point + * It doesn't have full TDM suppert yet */ ret = -EINVAL;
@@ -413,6 +609,15 @@ static int ak4613_dai_hw_params(struct snd_pcm_substream *substream, /* * It is not yet working, */ + unsigned int channel = params_channels(params); + u8 tdm; + + /* STEREO or TDM */ + if (channel == 2) + tdm = AK4613_CONFIG_MODE_STEREO; + else + tdm = AK4613_CONFIG_GET(priv, MODE); + for (i = ARRAY_SIZE(ak4613_iface) - 1; i >= 0; i--) { const struct ak4613_interface *iface = ak4613_iface + i;
@@ -421,9 +626,9 @@ static int ak4613_dai_hw_params(struct snd_pcm_substream *substream, * Ctrl1 * | D7 | D6 | D5 | D4 | D3 | D2 | D1 | D0 | * |TDM1|TDM0|DIF2|DIF1|DIF0|ATS1|ATS0|SMUTE| - * < iface->dif > + * < tdm > < iface->dif > */ - priv->ctrl1 = (iface->dif << 3); + priv->ctrl1 = (tdm << 6) | (iface->dif << 3); ret = 0; break; } @@ -578,14 +783,14 @@ static struct snd_soc_dai_driver ak4613_dai = { .playback = { .stream_name = "Playback", .channels_min = 2, - .channels_max = 2, + .channels_max = 12, .rates = AK4613_PCM_RATE, .formats = AK4613_PCM_FMTBIT, }, .capture = { .stream_name = "Capture", .channels_min = 2, - .channels_max = 2, + .channels_max = 4, .rates = AK4613_PCM_RATE, .formats = AK4613_PCM_FMTBIT, }, @@ -630,6 +835,7 @@ static void ak4613_parse_of(struct ak4613_priv *priv, { struct device_node *np = dev->of_node; char prop[32]; + int sdti_num; int i;
/* Input 1 - 2 */ @@ -645,6 +851,29 @@ static void ak4613_parse_of(struct ak4613_priv *priv, if (!of_get_property(np, prop, NULL)) priv->oc |= 1 << i; } + + /* + * enable TDM256 test + * + * !!! FIXME !!! + * + * It should be configured by DT or other way + * if it was full supported. + * But it is using ifdef style for now for test + * purpose. + */ +#if defined(AK4613_ENABLE_TDM_TEST) + AK4613_CONFIG_SET(priv, MODE_TDM256); +#endif + + /* + * connected STDI + */ + sdti_num = of_graph_get_endpoint_count(np); + if (WARN_ON((sdti_num > 3) || (sdti_num < 1))) + return; + + AK4613_CONFIG_SDTI_set(priv, sdti_num); }
static int ak4613_i2c_probe(struct i2c_client *i2c,
Hi Morimoto-san,
On Tue, 5 Apr 2022, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
AK4613 has STEREO/TDM512/TDM256/TDM128 mode. Renesas is the only user of ak4613 on upstream for now, and is using it as STEREO mode, because of board connection. Thus, current driver is supporting STEREO mode only, and other modes are not supported.
But I noticed that I can try first 2ch out of TDM256 mode 8ch Playback even in such a situation.
But because of board connection, I can't test full TDM256 mode, and/or other TDM mode. Thus I don't want to add new DT propaty for now. This patch enables TDM256 mode test by "ifdef style", but it has no effect to current supported STEREO mode. You can define AK4613_ENABLE_TDM_TEST to try TDM256 mode.
Please don't hesitate to break current code if you can add full TDM256 and/or other TDM mode. You don't need to care compatibility with Renesas.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Thanks for your patch, which is now commit f28dbaa958fbd8fb ("ASoC: ak4613: add TDM256 support") in v5.19-rc1.
--- a/sound/soc/codecs/ak4613.c +++ b/sound/soc/codecs/ak4613.c @@ -645,6 +851,29 @@ static void ak4613_parse_of(struct ak4613_priv *priv, if (!of_get_property(np, prop, NULL)) priv->oc |= 1 << i; }
- /*
* enable TDM256 test** !!! FIXME !!!** It should be configured by DT or other way* if it was full supported.* But it is using ifdef style for now for test* purpose.*/+#if defined(AK4613_ENABLE_TDM_TEST)
- AK4613_CONFIG_SET(priv, MODE_TDM256);
+#endif
- /*
* connected STDI*/- sdti_num = of_graph_get_endpoint_count(np);
- if (WARN_ON((sdti_num > 3) || (sdti_num < 1)))
This WARN_ON() is triggered on Ebisu-4D, as sdti_num = 0. It can be reproduced by booting renesas-devel-2022-06-07-v5.19-rc1 using renesas_defconfig.
Sorry for not noticing before.
return;- AK4613_CONFIG_SDTI_set(priv, sdti_num);
}
static int ak4613_i2c_probe(struct i2c_client *i2c,
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert
Thanks for your patch, which is now commit f28dbaa958fbd8fb ("ASoC: ak4613: add TDM256 support") in v5.19-rc1.
(snip)
This WARN_ON() is triggered on Ebisu-4D, as sdti_num = 0. It can be reproduced by booting renesas-devel-2022-06-07-v5.19-rc1 using renesas_defconfig.
Thank you for your reporting ! The patch assumed ak4613 is probed via Audio-Graph-Card, but Ebisu-4D is probing ak4613 via Simple-Audio-Card. I will post fixup patch, I hope it can solve the issue. Please check and send Tested-by to it.
Thank you for your help !!
Best regards --- Kuninori Morimoto
On Tue, 5 Apr 2022 02:05:42 +0000, Kuninori Morimoto wrote:
Renesas is the only user of ak4613 driver on upstream for now. It has STEREO/TDM512/TDM256/TDM128 mode, but STEREO only is used, because of Renesas board connection.
I noticed that I can test first 2ch out of TDM256 mode 8ch Playback even in such a situation.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/6] ASoC: ak4613: add missing mutex_lock() commit: 3407e36dc78f4a980588c9347076aee9925ca51f [2/6] ASoC: ak4613: tidyup ak4613_interface commit: f7c0e14f5717aabc5db7e4eb6324d750d415d022 [3/6] ASoC: ak4613: return error if it was setup as clock provider commit: c08673ede71fba70a10be0470565ed2470ef1fe5 [4/6] ASoC: ak4613: priv has ctrl1 instead of iface commit: e67d19a400cb12650169e4f57b8943e41266de53 [5/6] ASoC: ak4613: rename constraint to constraint_rates commit: 7bbb049c961a4e6b33520ab56d4b3abd947315ca [6/6] ASoC: ak4613: add TDM256 support commit: f28dbaa958fbd8fb7ffe40211b0e083156191f84
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (3)
-
Geert Uytterhoeven -
Kuninori Morimoto -
Mark Brown