[PATCH v2 0/7] ASoC: adds new .get_fmt support
Hi Mark
These are v2 of .get_fmt support. This is a little bit challenging patch-set. The idea/code is almost same as v1.
We need to set dai_link->dai_fmt to select CPU/Codec settings, and it is selected by Sound Card Driver, today.
Because of it, Sound Card user need to know both CPU / Codec available dai_fmt, and needs to select it. For example simple-card / audio-graph case, it is selected by "format" and "bitclock/frame-master/inversion" on DT.
But, it can be automatically selected if both CPU and Codec drivers indicate it to ALSA SoC Framework, somehow.
By this patch, dai_fmt can be automatically selected from each driver if both CPU / Codec driver had .get_dai_fmt callback. Automatically selectable *field* is depends on each drivers.
For example, some driver want to select format "automatically", but want to select other fields "manually", because of complex limitation. Or other example, in case of both CPU and Codec are possible to be clock provider, but the quality was different. In these case, user need/want to *manually* select each fields from Sound Card driver.
It uses Sound Card specified fields preferentially, and try to select non-specific fields from CPU and Codec driver settings if driver had callbacks. In other words, we can select all dai_fmt via Sound Card driver same as before.
Select dai_fmt 100% automatically is very difficult and will be very complex, but select automatically some fields only is very easy, I guess. This patch-set is based on such assumption.
v1 -> v2 - Add more detail explanation on git-log, code, comment. - Possible to be Clock/Frame provider is depends on driver's situation.
Link: https://lore.kernel.org/r/871rb3hypy.wl-kuninori.morimoto.gx@renesas.com
Kuninori Morimoto (7): ASoC: soc-core: move snd_soc_runtime_set_dai_fmt() to upside ASoC: soc-core: add snd_soc_runtime_get_dai_fmt() ASoC: ak4613: add .get_fmt support ASoC: pcm3168a: add .get_fmt support ASoC: rsnd: add .get_fmt support ASoC: fsi: add .get_fmt support ASoC: hdmi-codec: add .get_fmt support
include/sound/soc-dai.h | 45 +++++++ sound/soc/codecs/ak4613.c | 14 ++ sound/soc/codecs/hdmi-codec.c | 22 ++++ sound/soc/codecs/pcm3168a.c | 19 +++ sound/soc/sh/fsi.c | 19 +++ sound/soc/sh/rcar/adg.c | 8 ++ sound/soc/sh/rcar/core.c | 24 +++- sound/soc/sh/rcar/rsnd.h | 1 + sound/soc/soc-core.c | 234 +++++++++++++++++++++++++--------- sound/soc/soc-dai.c | 18 +++ sound/soc/soc-utils.c | 32 +++++ 11 files changed, 372 insertions(+), 64 deletions(-)
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
This patch moves snd_soc_runtime_set_dai_fmt() to upside. This is prepare to support snd_soc_runtime_get_dai_fmt().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2 - no change
sound/soc/soc-core.c | 124 +++++++++++++++++++++---------------------- 1 file changed, 62 insertions(+), 62 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 1c0904acb935..e241c35fb63a 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1054,6 +1054,68 @@ int snd_soc_add_pcm_runtime(struct snd_soc_card *card, } EXPORT_SYMBOL_GPL(snd_soc_add_pcm_runtime);
+/** + * snd_soc_runtime_set_dai_fmt() - Change DAI link format for a ASoC runtime + * @rtd: The runtime for which the DAI link format should be changed + * @dai_fmt: The new DAI link format + * + * This function updates the DAI link format for all DAIs connected to the DAI + * link for the specified runtime. + * + * Note: For setups with a static format set the dai_fmt field in the + * corresponding snd_dai_link struct instead of using this function. + * + * Returns 0 on success, otherwise a negative error code. + */ +int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, + unsigned int dai_fmt) +{ + struct snd_soc_dai *cpu_dai; + struct snd_soc_dai *codec_dai; + unsigned int inv_dai_fmt; + unsigned int i; + int ret; + + for_each_rtd_codec_dais(rtd, i, codec_dai) { + ret = snd_soc_dai_set_fmt(codec_dai, dai_fmt); + if (ret != 0 && ret != -ENOTSUPP) + return ret; + } + + /* + * Flip the polarity for the "CPU" end of a CODEC<->CODEC link + * the component which has non_legacy_dai_naming is Codec + */ + inv_dai_fmt = dai_fmt & ~SND_SOC_DAIFMT_MASTER_MASK; + switch (dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) { + case SND_SOC_DAIFMT_CBM_CFM: + inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFS; + break; + case SND_SOC_DAIFMT_CBM_CFS: + inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFM; + break; + case SND_SOC_DAIFMT_CBS_CFM: + inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFS; + break; + case SND_SOC_DAIFMT_CBS_CFS: + inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFM; + break; + } + for_each_rtd_cpu_dais(rtd, i, cpu_dai) { + unsigned int fmt = dai_fmt; + + if (cpu_dai->component->driver->non_legacy_dai_naming) + fmt = inv_dai_fmt; + + ret = snd_soc_dai_set_fmt(cpu_dai, fmt); + if (ret != 0 && ret != -ENOTSUPP) + return ret; + } + + return 0; +} +EXPORT_SYMBOL_GPL(snd_soc_runtime_set_dai_fmt); + static int soc_init_pcm_runtime(struct snd_soc_card *card, struct snd_soc_pcm_runtime *rtd) { @@ -1402,68 +1464,6 @@ static void soc_remove_aux_devices(struct snd_soc_card *card) } }
-/** - * snd_soc_runtime_set_dai_fmt() - Change DAI link format for a ASoC runtime - * @rtd: The runtime for which the DAI link format should be changed - * @dai_fmt: The new DAI link format - * - * This function updates the DAI link format for all DAIs connected to the DAI - * link for the specified runtime. - * - * Note: For setups with a static format set the dai_fmt field in the - * corresponding snd_dai_link struct instead of using this function. - * - * Returns 0 on success, otherwise a negative error code. - */ -int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, - unsigned int dai_fmt) -{ - struct snd_soc_dai *cpu_dai; - struct snd_soc_dai *codec_dai; - unsigned int inv_dai_fmt; - unsigned int i; - int ret; - - for_each_rtd_codec_dais(rtd, i, codec_dai) { - ret = snd_soc_dai_set_fmt(codec_dai, dai_fmt); - if (ret != 0 && ret != -ENOTSUPP) - return ret; - } - - /* - * Flip the polarity for the "CPU" end of a CODEC<->CODEC link - * the component which has non_legacy_dai_naming is Codec - */ - inv_dai_fmt = dai_fmt & ~SND_SOC_DAIFMT_MASTER_MASK; - switch (dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) { - case SND_SOC_DAIFMT_CBM_CFM: - inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFS; - break; - case SND_SOC_DAIFMT_CBM_CFS: - inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFM; - break; - case SND_SOC_DAIFMT_CBS_CFM: - inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFS; - break; - case SND_SOC_DAIFMT_CBS_CFS: - inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFM; - break; - } - for_each_rtd_cpu_dais(rtd, i, cpu_dai) { - unsigned int fmt = dai_fmt; - - if (cpu_dai->component->driver->non_legacy_dai_naming) - fmt = inv_dai_fmt; - - ret = snd_soc_dai_set_fmt(cpu_dai, fmt); - if (ret != 0 && ret != -ENOTSUPP) - return ret; - } - - return 0; -} -EXPORT_SYMBOL_GPL(snd_soc_runtime_set_dai_fmt); - #ifdef CONFIG_DMI /* * If a DMI filed contain strings in this blacklist (e.g.
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
ASoC is using dai_link which specify DAI format (= dai_link->dai_fmt), and it is selected by "Sound Card" driver in corrent implementation. In other words, Sound Card *needs* to setup it. But, it should be possible to automatically selected from CPU and Codec driver settings.
This patch adds new snd_soc_runtime_get_dai_fmt() callback to help it.
By this patch, dai_fmt can be automatically selected from each driver if both CPU / Codec driver had .get_dai_fmt callback. Automatically selectable *field* is depends on each drivers.
For example, some driver want to select format "automatically", but want to select other fields "manually", because of complex limitation. Or other example, in case of both CPU and Codec are possible to be clock provider, but the quality was different. In these case, user need/want to *manually* select each fields from Sound Card driver.
It uses Sound Card specified fields preferentially, and try to select non-specific fields from CPU and Codec driver settings if driver had callbacks. In other words, we can select all dai_fmt via Sound Card driver same as before.
Link: https://lore.kernel.org/r/871rb3hypy.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2 - add more detail explanation on git-log, comment - don't specify dummy clock/frame settings.
include/sound/soc-dai.h | 45 ++++++++++++++++ sound/soc/soc-core.c | 110 ++++++++++++++++++++++++++++++++++++++++ sound/soc/soc-dai.c | 18 +++++++ sound/soc/soc-utils.c | 32 ++++++++++++ 4 files changed, 205 insertions(+)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 0bc29c4516e7..13ea51df283a 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -36,6 +36,22 @@ struct snd_compr_stream; #define SND_SOC_DAIFMT_MSB SND_SOC_DAIFMT_LEFT_J #define SND_SOC_DAIFMT_LSB SND_SOC_DAIFMT_RIGHT_J
+/* Describes the possible PCM format */ +/* + * use SND_SOC_DAI_FORMAT_xx as eash shift. + * see + * snd_soc_runtime_get_dai_fmt() + */ +#define SND_SOC_POSSIBLE_DAIFMT_FORMAT_SHIFT 0 +#define SND_SOC_POSSIBLE_DAIFMT_FORMAT_MASK (0xFFFF << SND_SOC_POSSIBLE_DAIFMT_FORMAT_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_I2S (1 << SND_SOC_DAI_FORMAT_I2S) +#define SND_SOC_POSSIBLE_DAIFMT_RIGHT_J (1 << SND_SOC_DAI_FORMAT_RIGHT_J) +#define SND_SOC_POSSIBLE_DAIFMT_LEFT_J (1 << SND_SOC_DAI_FORMAT_LEFT_J) +#define SND_SOC_POSSIBLE_DAIFMT_DSP_A (1 << SND_SOC_DAI_FORMAT_DSP_A) +#define SND_SOC_POSSIBLE_DAIFMT_DSP_B (1 << SND_SOC_DAI_FORMAT_DSP_B) +#define SND_SOC_POSSIBLE_DAIFMT_AC97 (1 << SND_SOC_DAI_FORMAT_AC97) +#define SND_SOC_POSSIBLE_DAIFMT_PDM (1 << SND_SOC_DAI_FORMAT_PDM) + /* * DAI Clock gating. * @@ -45,6 +61,17 @@ struct snd_compr_stream; #define SND_SOC_DAIFMT_CONT (1 << 4) /* continuous clock */ #define SND_SOC_DAIFMT_GATED (0 << 4) /* clock is gated */
+/* Describes the possible PCM format */ +/* + * define GATED -> CONT. GATED will be selected if both are selected. + * see + * snd_soc_runtime_get_dai_fmt() + */ +#define SND_SOC_POSSIBLE_DAIFMT_CLOCK_SHIFT 16 +#define SND_SOC_POSSIBLE_DAIFMT_CLOCK_MASK (0xFFFF << SND_SOC_POSSIBLE_DAIFMT_CLOCK_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_GATED (0x1ULL << SND_SOC_POSSIBLE_DAIFMT_CLOCK_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_CONT (0x2ULL << SND_SOC_POSSIBLE_DAIFMT_CLOCK_SHIFT) + /* * DAI hardware signal polarity. * @@ -71,6 +98,14 @@ struct snd_compr_stream; #define SND_SOC_DAIFMT_IB_NF (3 << 8) /* invert BCLK + nor FRM */ #define SND_SOC_DAIFMT_IB_IF (4 << 8) /* invert BCLK + FRM */
+/* Describes the possible PCM format */ +#define SND_SOC_POSSIBLE_DAIFMT_INV_SHIFT 32 +#define SND_SOC_POSSIBLE_DAIFMT_INV_MASK (0xFFFFULL << SND_SOC_POSSIBLE_DAIFMT_INV_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_NB_NF (0x1ULL << SND_SOC_POSSIBLE_DAIFMT_INV_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_NB_IF (0x2ULL << SND_SOC_POSSIBLE_DAIFMT_INV_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_IB_NF (0x4ULL << SND_SOC_POSSIBLE_DAIFMT_INV_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_IB_IF (0x8ULL << SND_SOC_POSSIBLE_DAIFMT_INV_SHIFT) + /* * DAI hardware clock providers/consumers * @@ -89,6 +124,14 @@ struct snd_compr_stream; #define SND_SOC_DAIFMT_CBM_CFS SND_SOC_DAIFMT_CBP_CFC #define SND_SOC_DAIFMT_CBS_CFS SND_SOC_DAIFMT_CBC_CFC
+/* Describes the possible PCM format */ +#define SND_SOC_POSSIBLE_DAIFMT_CLOCK_PROVIDER_SHIFT 48 +#define SND_SOC_POSSIBLE_DAIFMT_CLOCK_PROVIDER_MASK (0xFFFFULL << SND_SOC_POSSIBLE_DAIFMT_CLOCK_PROVIDER_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_CBP_CFP (0x1ULL << SND_SOC_POSSIBLE_DAIFMT_CLOCK_PROVIDER_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_CBC_CFP (0x2ULL << SND_SOC_POSSIBLE_DAIFMT_CLOCK_PROVIDER_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_CBP_CFC (0x4ULL << SND_SOC_POSSIBLE_DAIFMT_CLOCK_PROVIDER_SHIFT) +#define SND_SOC_POSSIBLE_DAIFMT_CBC_CFC (0x8ULL << SND_SOC_POSSIBLE_DAIFMT_CLOCK_PROVIDER_SHIFT) + #define SND_SOC_DAIFMT_FORMAT_MASK 0x000f #define SND_SOC_DAIFMT_CLOCK_MASK 0x00f0 #define SND_SOC_DAIFMT_INV_MASK 0x0f00 @@ -131,6 +174,7 @@ int snd_soc_dai_set_pll(struct snd_soc_dai *dai, int snd_soc_dai_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio);
/* Digital Audio interface formatting */ +u64 snd_soc_dai_get_fmt(struct snd_soc_dai *dai); int snd_soc_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt);
int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai, @@ -236,6 +280,7 @@ struct snd_soc_dai_ops { * DAI format configuration * Called by soc_card drivers, normally in their hw_params. */ + u64 (*get_fmt)(struct snd_soc_dai *dai); int (*set_fmt)(struct snd_soc_dai *dai, unsigned int fmt); int (*xlate_tdm_slot_mask)(unsigned int slots, unsigned int *tx_mask, unsigned int *rx_mask); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index e241c35fb63a..194bf576b44d 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1054,6 +1054,115 @@ int snd_soc_add_pcm_runtime(struct snd_soc_card *card, } EXPORT_SYMBOL_GPL(snd_soc_add_pcm_runtime);
+static void snd_soc_runtime_get_dai_fmt(struct snd_soc_pcm_runtime *rtd) +{ + struct snd_soc_dai_link *dai_link = rtd->dai_link; + struct snd_soc_dai *dai; + u64 pos, possible_fmt = ULLONG_MAX; + unsigned int mask = 0, dai_fmt = 0; + int i; + + for_each_rtd_dais(rtd, i, dai) + possible_fmt &= snd_soc_dai_get_fmt(dai); + + if (!possible_fmt) + return; + + /* + * convert POSSIBLE_DAIFMT to DAIFMT + * + * Some basic/default settings on each is defined as 0. + * see + * SND_SOC_DAIFMT_NB_NF + * SND_SOC_DAIFMT_GATED + * + * SND_SOC_DAIFMT_xxx_MASK can't notice it if Sound Card specify + * these value, and will be overwrite to auto selected value. + * + * To avoid such issue, loop from 63 to 0 here. + * Small number of SND_SOC_POSSIBLE_xxx will be Hi priority. + * Basic/Default settings of each part and aboves are defined + * as Hi priority (= small number) of SND_SOC_POSSIBLE_xxx. + */ + for (i = 63; i >= 0; i--) { + pos = 1ULL << i; + switch (possible_fmt & pos) { + /* + * for format + */ + case SND_SOC_POSSIBLE_DAIFMT_I2S: + case SND_SOC_POSSIBLE_DAIFMT_RIGHT_J: + case SND_SOC_POSSIBLE_DAIFMT_LEFT_J: + case SND_SOC_POSSIBLE_DAIFMT_DSP_A: + case SND_SOC_POSSIBLE_DAIFMT_DSP_B: + case SND_SOC_POSSIBLE_DAIFMT_AC97: + case SND_SOC_POSSIBLE_DAIFMT_PDM: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_FORMAT_MASK) | i; + break; + /* + * for clock + */ + case SND_SOC_POSSIBLE_DAIFMT_CONT: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_CLOCK_MASK) | SND_SOC_DAIFMT_CONT; + break; + case SND_SOC_POSSIBLE_DAIFMT_GATED: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_CLOCK_MASK) | SND_SOC_DAIFMT_GATED; + break; + /* + * for clock invert + */ + case SND_SOC_POSSIBLE_DAIFMT_NB_NF: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_INV_MASK) | SND_SOC_DAIFMT_NB_NF; + break; + case SND_SOC_POSSIBLE_DAIFMT_NB_IF: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_INV_MASK) | SND_SOC_DAIFMT_NB_IF; + break; + case SND_SOC_POSSIBLE_DAIFMT_IB_NF: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_INV_MASK) | SND_SOC_DAIFMT_IB_NF; + break; + case SND_SOC_POSSIBLE_DAIFMT_IB_IF: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_INV_MASK) | SND_SOC_DAIFMT_IB_IF; + break; + /* + * for clock provider / consumer + */ + case SND_SOC_POSSIBLE_DAIFMT_CBP_CFP: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) | SND_SOC_DAIFMT_CBP_CFP; + break; + case SND_SOC_POSSIBLE_DAIFMT_CBC_CFP: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) | SND_SOC_DAIFMT_CBC_CFP; + break; + case SND_SOC_POSSIBLE_DAIFMT_CBP_CFC: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) | SND_SOC_DAIFMT_CBP_CFC; + break; + case SND_SOC_POSSIBLE_DAIFMT_CBC_CFC: + dai_fmt = (dai_fmt & ~SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) | SND_SOC_DAIFMT_CBC_CFC; + break; + } + } + + /* + * Some driver might have very complex limitation. + * In such case, user want to auto-select non-limitation part, + * and want to manually specify complex part. + * + * Or for example, if both CPU and Codec can be clock provider, + * but because of its quality, user want to specify it manually. + * + * Use manually specified settings if sound card did. + */ + if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK)) + mask |= SND_SOC_DAIFMT_FORMAT_MASK; + if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_CLOCK_MASK)) + mask |= SND_SOC_DAIFMT_CLOCK_MASK; + if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_INV_MASK)) + mask |= SND_SOC_DAIFMT_INV_MASK; + if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK)) + mask |= SND_SOC_DAIFMT_MASTER_MASK; + + dai_link->dai_fmt |= (dai_fmt & mask); +} + /** * snd_soc_runtime_set_dai_fmt() - Change DAI link format for a ASoC runtime * @rtd: The runtime for which the DAI link format should be changed @@ -1132,6 +1241,7 @@ static int soc_init_pcm_runtime(struct snd_soc_card *card, if (ret < 0) return ret;
+ snd_soc_runtime_get_dai_fmt(rtd); if (dai_link->dai_fmt) { ret = snd_soc_runtime_set_dai_fmt(rtd, dai_link->dai_fmt); if (ret) diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c index 080fbe053fc5..762ae5251244 100644 --- a/sound/soc/soc-dai.c +++ b/sound/soc/soc-dai.c @@ -134,6 +134,24 @@ int snd_soc_dai_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio) } EXPORT_SYMBOL_GPL(snd_soc_dai_set_bclk_ratio);
+/** + * snd_soc_dai_get_fmt - get enable DAI hardware audio format. + * @dai: DAI + * @fmt: SND_SOC_POSSIBLE_DAIFMT_* format value. + * + * Configures the DAI hardware format and clocking. + */ +u64 snd_soc_dai_get_fmt(struct snd_soc_dai *dai) +{ + u64 fmt = 0; + + if (dai->driver->ops && + dai->driver->ops->get_fmt) + fmt = dai->driver->ops->get_fmt(dai); + + return fmt; +} + /** * snd_soc_dai_set_fmt - configure DAI hardware audio format. * @dai: DAI diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 98383fd76224..ac53dc8c9a4c 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -97,6 +97,37 @@ static const struct snd_soc_component_driver dummy_codec = { SNDRV_PCM_FMTBIT_S32_LE | \ SNDRV_PCM_FMTBIT_U32_LE | \ SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE) + +static u64 dummy_dai_get_fmt(struct snd_soc_dai *dai) +{ + /* + * dummy can be both CPU and Codec. + * Thus, it can't specify Clock/Frame provider/consumer here. + * (= SND_SOC_POSSIBLE_DAIFMT_CBP_CFP + * SND_SOC_POSSIBLE_DAIFMT_CBP_CFC + * SND_SOC_POSSIBLE_DAIFMT_CBC_CFP + * SND_SOC_POSSIBLE_DAIFMT_CBC_CFC) + */ + + return SND_SOC_POSSIBLE_DAIFMT_I2S | + SND_SOC_POSSIBLE_DAIFMT_RIGHT_J | + SND_SOC_POSSIBLE_DAIFMT_LEFT_J | + SND_SOC_POSSIBLE_DAIFMT_DSP_A | + SND_SOC_POSSIBLE_DAIFMT_DSP_B | + SND_SOC_POSSIBLE_DAIFMT_AC97 | + SND_SOC_POSSIBLE_DAIFMT_PDM | + SND_SOC_POSSIBLE_DAIFMT_GATED | + SND_SOC_POSSIBLE_DAIFMT_CONT | + SND_SOC_POSSIBLE_DAIFMT_NB_NF | + SND_SOC_POSSIBLE_DAIFMT_NB_IF | + SND_SOC_POSSIBLE_DAIFMT_IB_NF | + SND_SOC_POSSIBLE_DAIFMT_IB_IF; +} + +static const struct snd_soc_dai_ops dummy_dai_ops = { + .get_fmt = dummy_dai_get_fmt, +}; + /* * The dummy CODEC is only meant to be used in situations where there is no * actual hardware. @@ -122,6 +153,7 @@ static struct snd_soc_dai_driver dummy_dai = { .rates = STUB_RATES, .formats = STUB_FORMATS, }, + .ops = &dummy_dai_ops, };
int snd_soc_dai_is_dummy(struct snd_soc_dai *dai)
On Wed, May 12, 2021 at 09:45:33AM +0900, Kuninori Morimoto wrote:
Hi Morimoto-san,
First up sorry it's taken me a while to respond to this - I wanted to get it clear in my head what I thought the best approach is here. I think your code here is good with a couple of documentation tweaks:
For example, some driver want to select format "automatically", but want to select other fields "manually", because of complex limitation. Or other example, in case of both CPU and Codec are possible to be clock provider, but the quality was different. In these case, user need/want to *manually* select each fields from Sound Card driver.
It uses Sound Card specified fields preferentially, and try to select non-specific fields from CPU and Codec driver settings if driver had callbacks. In other words, we can select all dai_fmt via Sound Card driver same as before.
I mentioned last time around the idea of having first and second preferences for the DAIs, for things like "this will work but only if you get the configuration exactly right". These patches don't support that, they just treat everything the DAI reports as being equally valid. I've actually come round to thinking that might be OK for now but...
+/**
- snd_soc_dai_get_fmt - get enable DAI hardware audio format.
s/get enable DAI hardware/get supported/
- @dai: DAI
- @fmt: SND_SOC_POSSIBLE_DAIFMT_* format value.
- Configures the DAI hardware format and clocking.
...I think here we should say something like
This should return only formats implemented with high quality by the DAI so that the core can configure a format which will work well with other devices. For example devices which don't support both edges of the LRCLK signal in I2S style formats should only list DSP modes. This will mean that sometimes fewer formats are reported here than are supported by set_fmt().
so that instead of worrying about formats that aren't quite supported properly we just tell drivers not to list them for now so if the system is relying on the framework to pick the DAI format then we know it's one that's robust, we're not going to choose one that the hardware in one of the devices isn't very good at. Does that make sense to you?
If we did do first and second choices we'd get an algorithm like:
1. Try to match both DAI's 1st choice, if match use that. 2. Pick one DAI A, try it's 2nd choice with the other DAI B's 1st choice. 3. Swap and try 1st choice of A and 2nd choice of B. 4. Try 2nd choices of both DAIs.
but I think that might get too complicated and we'd end up stuck if we ever wanted to change the algorithm since DTs that don't specify a format may break if a different format is picked. If we go with ranking every format individually it gets even more complicated for both driver authors and the core.
I think with the documentation tweak above your idea is a better one for now, we can always go back later with more experience.
Hi Mark
First up sorry it's taken me a while to respond to this - I wanted to get it clear in my head what I thought the best approach is here. I think your code here is good with a couple of documentation tweaks:
Thank you for your feedback / review, and no worry about taken times. This is very challeng patch, and I know you are always busy :)
For example, some driver want to select format "automatically", but want to select other fields "manually", because of complex limitation. Or other example, in case of both CPU and Codec are possible to be clock provider, but the quality was different. In these case, user need/want to *manually* select each fields from Sound Card driver.
It uses Sound Card specified fields preferentially, and try to select non-specific fields from CPU and Codec driver settings if driver had callbacks. In other words, we can select all dai_fmt via Sound Card driver same as before.
I mentioned last time around the idea of having first and second preferences for the DAIs, for things like "this will work but only if you get the configuration exactly right". These patches don't support that, they just treat everything the DAI reports as being equally valid. I've actually come round to thinking that might be OK for now but...
(snip)
so that instead of worrying about formats that aren't quite supported properly we just tell drivers not to list them for now so if the system is relying on the framework to pick the DAI format then we know it's one that's robust, we're not going to choose one that the hardware in one of the devices isn't very good at. Does that make sense to you?
If we did do first and second choices we'd get an algorithm like:
- Try to match both DAI's 1st choice, if match use that.
- Pick one DAI A, try it's 2nd choice with the other DAI B's 1st choice.
- Swap and try 1st choice of A and 2nd choice of B.
- Try 2nd choices of both DAIs.
but I think that might get too complicated and we'd end up stuck if we ever wanted to change the algorithm since DTs that don't specify a format may break if a different format is picked. If we go with ranking every format individually it gets even more complicated for both driver authors and the core.
I think with the documentation tweak above your idea is a better one for now, we can always go back later with more experience.
Yes, we want to have robust code, and indeed it might be issue in the future if something happen around algorithm.
To avoid such case, indeed having "priority" is nice idea, and indicating such things is indeed nesessary.
OK, thank you for your feedback, I will update code, and repost it.
Thank you for your help !!
Best regards --- Kuninori Morimoto
Hi Mark again
- Try to match both DAI's 1st choice, if match use that.
- Pick one DAI A, try it's 2nd choice with the other DAI B's 1st choice.
- Swap and try 1st choice of A and 2nd choice of B.
- Try 2nd choices of both DAIs.
About this, Because we might use Multi-CPU/Codec, step 3 is difficult. Except it I can implement.
Thank you for your help !!
Best regards --- Kuninori Morimoto
On Thu, May 27, 2021 at 07:54:04AM +0900, Kuninori Morimoto wrote:
- Try to match both DAI's 1st choice, if match use that.
- Pick one DAI A, try it's 2nd choice with the other DAI B's 1st choice.
- Swap and try 1st choice of A and 2nd choice of B.
- Try 2nd choices of both DAIs.
About this, Because we might use Multi-CPU/Codec, step 3 is difficult. Except it I can implement.
Yeah, I think it's probably too hard to implement the whole thing and may be safer to simply go with what you've implemented already (with the documentation changes) as a first step. Like I said I'm a little worried about what happens if we change the algorithm later.
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
ak4613 supports .get_fmt by this patch
Link: https://lore.kernel.org/r/871rb3hypy.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2 - possible to be clock/frame master is depends on board
sound/soc/codecs/ak4613.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/sound/soc/codecs/ak4613.c b/sound/soc/codecs/ak4613.c index fe208cfdd3ba..6f2b285b864b 100644 --- a/sound/soc/codecs/ak4613.c +++ b/sound/soc/codecs/ak4613.c @@ -322,6 +322,19 @@ static int ak4613_dai_set_sysclk(struct snd_soc_dai *codec_dai, return 0; }
+static u64 ak4613_dai_get_fmt(struct snd_soc_dai *dai) +{ + struct snd_soc_component *component = dai->component; + struct ak4613_priv *priv = snd_soc_component_get_drvdata(component); + u64 cbp_bfp = (priv->sysclk) ? SND_SOC_POSSIBLE_DAIFMT_CBP_CFP : 0; + + return SND_SOC_POSSIBLE_DAIFMT_I2S | + SND_SOC_POSSIBLE_DAIFMT_LEFT_J | + SND_SOC_POSSIBLE_DAIFMT_CBC_CFC | + SND_SOC_POSSIBLE_DAIFMT_NB_NF | + cbp_bfp; +} + static int ak4613_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) { struct snd_soc_component *component = dai->component; @@ -543,6 +556,7 @@ static const struct snd_soc_dai_ops ak4613_dai_ops = { .startup = ak4613_dai_startup, .shutdown = ak4613_dai_shutdown, .set_sysclk = ak4613_dai_set_sysclk, + .get_fmt = ak4613_dai_get_fmt, .set_fmt = ak4613_dai_set_fmt, .trigger = ak4613_dai_trigger, .hw_params = ak4613_dai_hw_params,
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
pcm3168a supports .get_fmt by this patch
Link: https://lore.kernel.org/r/871rb3hypy.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2 - possible to be clock/frame master is depends on board, and has picky limitation. don't select it here.
sound/soc/codecs/pcm3168a.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/sound/soc/codecs/pcm3168a.c b/sound/soc/codecs/pcm3168a.c index 821e7395f90f..c6c327241e26 100644 --- a/sound/soc/codecs/pcm3168a.c +++ b/sound/soc/codecs/pcm3168a.c @@ -353,6 +353,24 @@ static void pcm3168a_update_fixup_pcm_stream(struct snd_soc_dai *dai) } }
+static u64 pcm3168a_get_dai_fmt(struct snd_soc_dai *dai) +{ + /* + * It can be clock/frame provider, + * (= SND_SOC_POSSIBLE_DAIFMT_CBP_CFP) + * but has picky limitation. + * Select it manually if you want, not automatically. + */ + + return SND_SOC_POSSIBLE_DAIFMT_I2S | + SND_SOC_POSSIBLE_DAIFMT_LEFT_J | + SND_SOC_POSSIBLE_DAIFMT_RIGHT_J | + SND_SOC_POSSIBLE_DAIFMT_DSP_A | + SND_SOC_POSSIBLE_DAIFMT_DSP_B | + SND_SOC_POSSIBLE_DAIFMT_CBC_CFC | + SND_SOC_POSSIBLE_DAIFMT_NB_NF; +} + static int pcm3168a_set_dai_fmt(struct snd_soc_dai *dai, unsigned int format) { struct snd_soc_component *component = dai->component; @@ -574,6 +592,7 @@ static int pcm3168a_hw_params(struct snd_pcm_substream *substream, }
static const struct snd_soc_dai_ops pcm3168a_dai_ops = { + .get_fmt = pcm3168a_get_dai_fmt, .set_fmt = pcm3168a_set_dai_fmt, .set_sysclk = pcm3168a_set_dai_sysclk, .hw_params = pcm3168a_hw_params,
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
rsnd supports .get_fmt by this patch
Link: https://lore.kernel.org/r/871rb3hypy.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2 - possible to be clock/frame master is depends on board settings.
sound/soc/sh/rcar/adg.c | 8 ++++++++ sound/soc/sh/rcar/core.c | 24 ++++++++++++++++++++++-- sound/soc/sh/rcar/rsnd.h | 1 + 3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c index 0b8ae3eee148..f7773c41085b 100644 --- a/sound/soc/sh/rcar/adg.c +++ b/sound/soc/sh/rcar/adg.c @@ -557,6 +557,14 @@ static void rsnd_adg_get_clkout(struct rsnd_priv *priv, adg->rbgb = rbgb; }
+int rsnd_adg_clk_can_be_provider(struct rsnd_priv *priv) +{ + struct rsnd_adg *adg = rsnd_priv_to_adg(priv); + + return (adg->rbga_rate_for_441khz && + adg->rbgb_rate_for_48khz); +} + #ifdef DEBUG static void rsnd_adg_clk_dbg_info(struct rsnd_priv *priv, struct rsnd_adg *adg) { diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c index 8696a993c478..11b1e6f3ae40 100644 --- a/sound/soc/sh/rcar/core.c +++ b/sound/soc/sh/rcar/core.c @@ -754,16 +754,35 @@ static int rsnd_soc_dai_trigger(struct snd_pcm_substream *substream, int cmd, return ret; }
+static u64 rsnd_soc_dai_get_fmt(struct snd_soc_dai *dai) +{ + struct rsnd_priv *priv = rsnd_dai_to_priv(dai); + u64 cbc_cfc = rsnd_adg_clk_can_be_provider(priv) ? + SND_SOC_POSSIBLE_DAIFMT_CBC_CFC : 0; + + return SND_SOC_POSSIBLE_DAIFMT_I2S | + SND_SOC_POSSIBLE_DAIFMT_RIGHT_J | + SND_SOC_POSSIBLE_DAIFMT_LEFT_J | + SND_SOC_POSSIBLE_DAIFMT_DSP_A | + SND_SOC_POSSIBLE_DAIFMT_DSP_B | + SND_SOC_POSSIBLE_DAIFMT_NB_NF | + SND_SOC_POSSIBLE_DAIFMT_NB_IF | + SND_SOC_POSSIBLE_DAIFMT_IB_NF | + SND_SOC_POSSIBLE_DAIFMT_IB_IF | + SND_SOC_POSSIBLE_DAIFMT_CBP_CFP | + cbc_cfc; +} + static int rsnd_soc_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) { struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
/* set clock master for audio interface */ switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { - case SND_SOC_DAIFMT_CBM_CFM: + case SND_SOC_DAIFMT_CBP_CFP: rdai->clk_master = 0; break; - case SND_SOC_DAIFMT_CBS_CFS: + case SND_SOC_DAIFMT_CBC_CFC: rdai->clk_master = 1; /* cpu is master */ break; default: @@ -1047,6 +1066,7 @@ static const struct snd_soc_dai_ops rsnd_soc_dai_ops = { .startup = rsnd_soc_dai_startup, .shutdown = rsnd_soc_dai_shutdown, .trigger = rsnd_soc_dai_trigger, + .get_fmt = rsnd_soc_dai_get_fmt, .set_fmt = rsnd_soc_dai_set_fmt, .set_tdm_slot = rsnd_soc_set_dai_tdm_slot, .prepare = rsnd_soc_dai_prepare, diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h index 1255a85151db..c8c7691f7426 100644 --- a/sound/soc/sh/rcar/rsnd.h +++ b/sound/soc/sh/rcar/rsnd.h @@ -610,6 +610,7 @@ int rsnd_adg_set_cmd_timsel_gen2(struct rsnd_mod *cmd_mod, #define rsnd_adg_clk_enable(priv) rsnd_adg_clk_control(priv, 1) #define rsnd_adg_clk_disable(priv) rsnd_adg_clk_control(priv, 0) void rsnd_adg_clk_control(struct rsnd_priv *priv, int enable); +int rsnd_adg_clk_can_be_provider(struct rsnd_priv *priv);
/* * R-Car sound priv
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
fsi supports .get_fmt by this patch
Link: https://lore.kernel.org/r/871rb3hypy.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2 - don't select clock/frame master possibility, because it is depends on complex board settings.
sound/soc/sh/fsi.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 3c574792231b..caa07056e973 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -1627,6 +1627,24 @@ static int fsi_set_fmt_spdif(struct fsi_priv *fsi) return 0; }
+static u64 fsi_dai_get_fmt(struct snd_soc_dai *dai) +{ + /* + * It can be clock/frame master, + * (= SND_SOC_POSSIBLE_DAIFMT_CBC_CFC) + * but needs xck/ick/div, and these are depends on cpg. + * Select it manually if you want, not automatically. + */ + + return SND_SOC_POSSIBLE_DAIFMT_I2S | + SND_SOC_POSSIBLE_DAIFMT_LEFT_J | + SND_SOC_POSSIBLE_DAIFMT_CBP_CFP | + SND_SOC_POSSIBLE_DAIFMT_NB_NF | + SND_SOC_POSSIBLE_DAIFMT_NB_IF | + SND_SOC_POSSIBLE_DAIFMT_IB_NF | + SND_SOC_POSSIBLE_DAIFMT_IB_IF; +} + static int fsi_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) { struct fsi_priv *fsi = fsi_get_priv_frm_dai(dai); @@ -1698,6 +1716,7 @@ static const struct snd_soc_dai_ops fsi_dai_ops = { .startup = fsi_dai_startup, .shutdown = fsi_dai_shutdown, .trigger = fsi_dai_trigger, + .get_fmt = fsi_dai_get_fmt, .set_fmt = fsi_dai_set_fmt, .hw_params = fsi_dai_hw_params, };
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
hdmi-codec supports .get_fmt by this patch
Link: https://lore.kernel.org/r/871rb3hypy.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2 - don't select clock/frame master possibility
sound/soc/codecs/hdmi-codec.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c index 1567ba196ab9..b3dcf28ad6ee 100644 --- a/sound/soc/codecs/hdmi-codec.c +++ b/sound/soc/codecs/hdmi-codec.c @@ -493,6 +493,27 @@ static int hdmi_codec_hw_params(struct snd_pcm_substream *substream, cf, &hp); }
+static u64 hdmi_codec_i2s_get_fmt(struct snd_soc_dai *dai) +{ + /* + * This driver can select all SND_SOC_DAIFMT_CBx_CFx, + * but need to be selected from Sound Card, not be auto selected. + * Because it might be used from other driver. + * For example, + * ${LINUX}/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c + */ + return SND_SOC_POSSIBLE_DAIFMT_NB_NF | + SND_SOC_POSSIBLE_DAIFMT_NB_IF | + SND_SOC_POSSIBLE_DAIFMT_IB_NF | + SND_SOC_POSSIBLE_DAIFMT_IB_IF | + SND_SOC_POSSIBLE_DAIFMT_I2S | + SND_SOC_POSSIBLE_DAIFMT_DSP_A | + SND_SOC_POSSIBLE_DAIFMT_DSP_B | + SND_SOC_POSSIBLE_DAIFMT_RIGHT_J | + SND_SOC_POSSIBLE_DAIFMT_LEFT_J | + SND_SOC_POSSIBLE_DAIFMT_AC97; +} + static int hdmi_codec_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) { @@ -584,6 +605,7 @@ static const struct snd_soc_dai_ops hdmi_codec_i2s_dai_ops = { .startup = hdmi_codec_startup, .shutdown = hdmi_codec_shutdown, .hw_params = hdmi_codec_hw_params, + .get_fmt = hdmi_codec_i2s_get_fmt, .set_fmt = hdmi_codec_i2s_set_fmt, .mute_stream = hdmi_codec_mute, };
participants (2)
-
Kuninori Morimoto -
Mark Brown