[PATCH 0/7] ASoC: adds new .get_fmt support

[PATCH 0/7] ASoC: adds new .get_fmt support

[Kuninori Morimoto]

Hi Mark

These are a little bit challenging patch-set.

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.
If we can use it, Sound Card Driver user no longer need to select it,
and I want to use this style on new audio-graph-card2.

This patch-set adds new .get_fmt callback which indicate available
dai_fmt to ALSA SoC Framework.
Of course Sound Card Driver can still select dai_link->dai_fmt, same as before.
If Sound Card Driver didn't set it, and if both CPU / Codec had
.get_fmt callback, dai_link->dai_fmt will be automatically selected.

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       |  35 ++++++
 sound/soc/codecs/ak4613.c     |  10 ++
 sound/soc/codecs/hdmi-codec.c |  19 ++++
 sound/soc/codecs/pcm3168a.c   |  13 +++
 sound/soc/sh/fsi.c            |  13 +++
 sound/soc/sh/rcar/core.c      |  16 +++
 sound/soc/soc-core.c          | 195 +++++++++++++++++++++++-----------
 sound/soc/soc-dai.c           |  19 ++++
 sound/soc/soc-utils.c         |  26 +++++
 9 files changed, 284 insertions(+), 62 deletions(-)

-- 
2.25.1

[PATCH 1/7] ASoC: soc-core: move snd_soc_runtime_set_dai_fmt() to upside

[Kuninori Morimoto]

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>
---
 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.
-- 
2.25.1

[PATCH 2/7] ASoC: soc-core: add snd_soc_runtime_get_dai_fmt()

[Kuninori Morimoto]

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() and
.get_dai_fmt callback for it.

We can still select it via Sound Card driver same as before.
If both CPU / Codec driver had .get_dai_fmt callback,
dai_fmt will be automatically selected from each driver.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc-dai.h | 35 ++++++++++++++++++++
 sound/soc/soc-core.c    | 71 +++++++++++++++++++++++++++++++++++++++++
 sound/soc/soc-dai.c     | 19 +++++++++++
 sound/soc/soc-utils.c   | 26 +++++++++++++++
 4 files changed, 151 insertions(+)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 1358a0ceb4d0..0a4afb84f12e 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -36,6 +36,17 @@ 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 */
+#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 +56,12 @@ 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 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_CONT		(0x1ULL	<< SND_SOC_POSSIBLE_DAIFMT_CLOCK_SHIFT)
+#define SND_SOC_POSSIBLE_DAIFMT_GATED		(0x0ULL	<< SND_SOC_POSSIBLE_DAIFMT_CLOCK_SHIFT)
+
 /*
  * DAI hardware signal polarity.
  *
@@ -71,6 +88,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 +114,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 +164,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 +270,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..621e82adda90 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1054,6 +1054,76 @@ 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 */
+	for (i = 63; i >= 0; i--) {
+		pos = 1ULL << i;
+		switch (possible_fmt & pos) {
+		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;
+		case SND_SOC_POSSIBLE_DAIFMT_CONT:
+			dai_fmt |= SND_SOC_DAIFMT_CONT;
+			break;
+		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;
+		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;
+		}
+	}
+
+	/* use original dai_fmt if sound card specify */
+	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_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 +1202,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..109a562ebeb9 100644
--- a/sound/soc/soc-dai.c
+++ b/sound/soc/soc-dai.c
@@ -134,6 +134,25 @@ 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;
+}
+EXPORT_SYMBOL_GPL(snd_soc_dai_get_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..b92b1d07ec83 100644
--- a/sound/soc/soc-utils.c
+++ b/sound/soc/soc-utils.c
@@ -97,6 +97,31 @@ 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)
+{
+	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_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	|
+		SND_SOC_POSSIBLE_DAIFMT_CBP_CFP	|
+		SND_SOC_POSSIBLE_DAIFMT_CBC_CFP	|
+		SND_SOC_POSSIBLE_DAIFMT_CBP_CFC	|
+		SND_SOC_POSSIBLE_DAIFMT_CBC_CFC;
+}
+
+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 +147,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)
-- 
2.25.1

Re: [PATCH 2/7] ASoC: soc-core: add snd_soc_runtime_get_dai_fmt()

[Mark Brown]

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

On Thu, Apr 22, 2021 at 10:53:44AM +0900, Kuninori Morimoto wrote:

> +/* Describes the possible PCM format */
> +#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)

I'm not 100% sure I get why we have the separate _POSSIBLE_ macros here?
One thing we'll have to take account of is that there's some DAIs that
have some restrictions on what options they can combine - for example
only doing I2S with one format of clock but allowing clock inversion in
DSP mode.  It might be safer (if tedious for driver authors without some
help...) to just have arrays of fully specified daifmt values.

>  /* Digital Audio interface formatting */
> +u64 snd_soc_dai_get_fmt(struct snd_soc_dai *dai);

Like I said on the cover letter I think we need to be able to specify at
least two levels of preference here.  How about

	int snd_soc_dai_get_fmt(struct snd_soc_dai *dai,
				u64 *preferred, u64 *supported);

or something?  Just thinking off the top of my head, that's a bit ugly
and doesn't scale to multiple levels so I don't know if I'm *super*
happy with that interface.  But we might be better off using just arrays
of daifmt values like I say, if we do that perhaps just one array but
sorting it might do?

> +	/* use original dai_fmt if sound card specify */
> +	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_link->dai_fmt | (dai_fmt & mask);

If the sound card specifies something why not just use it verbatim
instead of trying to merge?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/7] ASoC: soc-core: add snd_soc_runtime_get_dai_fmt()

[Kuninori Morimoto]

Hi Mark

Thank you for your feedback

> the other edge) but there are other examples.  This means we probably
> want drivers to be able to list at least two levels of preferences for
> formats separately then try to ideally use one they both like, but it
> gets a little tricky.  We probably also want to have some priority list
> in the kernel where we try to rank how likely formats are to have bugs
> in the hardware to maximise the chances that things will work.
(snip)
> I'm not 100% sure I get why we have the separate _POSSIBLE_ macros here?
> One thing we'll have to take account of is that there's some DAIs that
> have some restrictions on what options they can combine - for example
> only doing I2S with one format of clock but allowing clock inversion in
> DSP mode.  It might be safer (if tedious for driver authors without some
> help...) to just have arrays of fully specified daifmt values.
(snip)
> > +	/* use original dai_fmt if sound card specify */
> > +	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_link->dai_fmt | (dai_fmt & mask);
> 
> If the sound card specifies something why not just use it verbatim
> instead of trying to merge?

I'm thinking the way that for example select "format" automatically,
but specify others from Card, etc (= not all settings).
Because as you are concerning, some driver might has complex limitations for some parts.
We want to manually and directly select it from Card in such case.

My idea was very simple, but indeed I could understand your concern.
I have no good idea so far, but want to consider about it more.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/7] ASoC: soc-core: add snd_soc_runtime_get_dai_fmt()

[Mark Brown]

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

On Mon, Apr 26, 2021 at 03:13:55PM +0900, Kuninori Morimoto wrote:

> > If the sound card specifies something why not just use it verbatim
> > instead of trying to merge?

> I'm thinking the way that for example select "format" automatically,
> but specify others from Card, etc (= not all settings).
> Because as you are concerning, some driver might has complex limitations for some parts.
> We want to manually and directly select it from Card in such case.

> My idea was very simple, but indeed I could understand your concern.
> I have no good idea so far, but want to consider about it more.

Yeah, the only thing I can think of is big lists of configurations.
That's not the end of the world but it's potentially annoying, we should
be able to get helpers that make life easier I guess.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/7] ASoC: soc-core: add snd_soc_runtime_get_dai_fmt()

[Kuninori Morimoto]

Hi Mark

> > I'm thinking the way that for example select "format" automatically,
> > but specify others from Card, etc (= not all settings).
> > Because as you are concerning, some driver might has complex limitations for some parts.
> > We want to manually and directly select it from Card in such case.
> 
> > My idea was very simple, but indeed I could understand your concern.
> > I have no good idea so far, but want to consider about it more.
> 
> Yeah, the only thing I can think of is big lists of configurations.
> That's not the end of the world but it's potentially annoying, we should
> be able to get helpers that make life easier I guess.

Thanks.
I think I need to explain above my idea in patch/comment.
And I noticed that my previous patch had some issues.
I will fixup these, and re-post it.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

[PATCH 3/7] ASoC: ak4613: add .get_fmt support

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

ak4613 supports .get_fmt by this patch

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/codecs/ak4613.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/soc/codecs/ak4613.c b/sound/soc/codecs/ak4613.c
index fe208cfdd3ba..c63da8e32945 100644
--- a/sound/soc/codecs/ak4613.c
+++ b/sound/soc/codecs/ak4613.c
@@ -322,6 +322,15 @@ 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)
+{
+	return	SND_SOC_POSSIBLE_DAIFMT_I2S	|
+		SND_SOC_POSSIBLE_DAIFMT_LEFT_J	|
+		SND_SOC_POSSIBLE_DAIFMT_CBP_CFP	|
+		SND_SOC_POSSIBLE_DAIFMT_CBC_CFC	|
+		SND_SOC_POSSIBLE_DAIFMT_NB_NF;
+}
+
 static int ak4613_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
 	struct snd_soc_component *component = dai->component;
@@ -543,6 +552,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,
-- 
2.25.1

[PATCH 4/7] ASoC: pcm3168a: add .get_fmt support

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

pcm3168a supports .get_fmt by this patch

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/codecs/pcm3168a.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/sound/soc/codecs/pcm3168a.c b/sound/soc/codecs/pcm3168a.c
index 821e7395f90f..91295b813687 100644
--- a/sound/soc/codecs/pcm3168a.c
+++ b/sound/soc/codecs/pcm3168a.c
@@ -353,6 +353,18 @@ static void pcm3168a_update_fixup_pcm_stream(struct snd_soc_dai *dai)
 	}
 }
 
+static u64 pcm3168a_get_dai_fmt(struct snd_soc_dai *dai)
+{
+	return	SND_SOC_POSSIBLE_DAIFMT_LEFT_J	|
+		SND_SOC_POSSIBLE_DAIFMT_I2S	|
+		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_CBP_CFP	|
+		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 +586,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,
-- 
2.25.1

[PATCH 5/7] ASoC: rsnd: add .get_fmt support

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

rsnd supports .get_fmt by this patch

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 8696a993c478..958865c3951d 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -754,6 +754,21 @@ 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)
+{
+	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_CBP_CFP	|
+		SND_SOC_POSSIBLE_DAIFMT_CBC_CFC	|
+		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 rsnd_soc_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
 	struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
@@ -1047,6 +1062,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,
-- 
2.25.1

[PATCH 6/7] ASoC: fsi: add .get_fmt support

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

fsi supports .get_fmt by this patch

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/fsi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 3c574792231b..3a841f72ba5f 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -1627,6 +1627,18 @@ static int fsi_set_fmt_spdif(struct fsi_priv *fsi)
 	return 0;
 }
 
+static u64 fsi_dai_get_fmt(struct snd_soc_dai *dai)
+{
+	return	SND_SOC_POSSIBLE_DAIFMT_I2S	|
+		SND_SOC_POSSIBLE_DAIFMT_LEFT_J	|
+		SND_SOC_POSSIBLE_DAIFMT_CBP_CFP	|
+		SND_SOC_POSSIBLE_DAIFMT_CBC_CFC	|
+		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 +1710,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,
 };
-- 
2.25.1

[PATCH 7/7] ASoC: hdmi-codec: add .get_fmt support

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

hdmi-codec supports .get_fmt by this patch

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/codecs/hdmi-codec.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 1567ba196ab9..9d2ceed3778a 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -493,6 +493,24 @@ 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)
+{
+	return	SND_SOC_POSSIBLE_DAIFMT_CBP_CFP	|
+		SND_SOC_POSSIBLE_DAIFMT_CBC_CFP	|
+		SND_SOC_POSSIBLE_DAIFMT_CBP_CFC	|
+		SND_SOC_POSSIBLE_DAIFMT_CBC_CFC	|
+		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 +602,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,
 };
-- 
2.25.1

Re: [PATCH 0/7] ASoC: adds new .get_fmt support

[Mark Brown]

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

On Thu, Apr 22, 2021 at 10:52:57AM +0900, Kuninori Morimoto wrote:

> But, it can be automatically selected if both CPU and Codec drivers
> indicate it to ALSA SoC Framework, somehow.
> If we can use it, Sound Card Driver user no longer need to select it,
> and I want to use this style on new audio-graph-card2.

It could even be done with the existing ones if we just make the format
property optional.  The automatic selection is something we really ought
to at least try to do so thanks for working on this!

> This patch-set adds new .get_fmt callback which indicate available
> dai_fmt to ALSA SoC Framework.
> Of course Sound Card Driver can still select dai_link->dai_fmt, same as before.
> If Sound Card Driver didn't set it, and if both CPU / Codec had
> .get_fmt callback, dai_link->dai_fmt will be automatically selected.

One of the biggest things to consider here is that there's a bunch of
devices that have varying degrees of support for different formats so
that while they *can* do some formats it's only in specific setups.  The
most common example is generic serial port devices which can often cope
with I2S so long as there are no extra clock cycles but don't *really*
fully support I2S (they clock off the first edge on LRCLK and ignore
the other edge) but there are other examples.  This means we probably
want drivers to be able to list at least two levels of preferences for
formats separately then try to ideally use one they both like, but it
gets a little tricky.  We probably also want to have some priority list
in the kernel where we try to rank how likely formats are to have bugs
in the hardware to maximise the chances that things will work.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]