alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: Samuel Holland <samuel@sholland.org>
To: Mark Brown <broonie@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Maxime Ripard <mripard@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>
Cc: Ondrej Jirman <megous@megous.com>,
	devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	Samuel Holland <samuel@sholland.org>,
	linux-kernel@vger.kernel.org,
	Vasily Khoruzhick <anarsoul@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/7] ASoC: sun8i-codec: Fix DAPM to match the hardware topology
Date: Sat, 25 Jul 2020 20:25:52 -0500	[thread overview]
Message-ID: <20200726012557.38282-3-samuel@sholland.org> (raw)
In-Reply-To: <20200726012557.38282-1-samuel@sholland.org>

The A33/A64 digital codec has 4 physical inputs and 4 physical outputs:
3 AIFs/DAIs and one ADC/DAC pair. Internal routing is accomplished by
a 4-channel mixer connected to each output.

The analog and digital sides of the ADC/DAC are in separate ASoC
components, so card-level DAPM routes (provided in the device tree) are
necessary to connect them together. Currently, these routes are wrong.

For AIF1 Playback, the correct topology is:

         ||<<============ sun8i-codec ===========>>||
         ||                                        ||
 CPU DAI -> AIF1 DA0 -> DAC Mixer -> DAC (digital) -> DAC (analog)
         ||                                        ||

but the driver and device trees currently describe:

         ||                                        ||
 CPU DAI -> AIF1 DA0 -------------------------------> DAC (analog)
         ||     \--> DAC Mixer -> ??? [dead end]   ||

For AIF1 Capture, there is an additional problem, because the Mixer
route is backward. The topology should be:

              ||                                             ||
 ADC (analog) -> ADC (digital) -> AIF1 AD0 Mixer -> AIF1 AD0 -> CPU DAI
              ||                                             ||

but the driver and device trees currently describe:

              ||                                             ||
 ADC (analog) -> AIF1 AD0 ------------------------------------> CPU DAI
              ||     \--> ADC Mixer -> ??? [dead end]        ||

The ADC/DAC are only powered because AIF1 AD0 (capture) has supply
routes from the ADC, and AIF1 DA0 (playback) has supply routes from the
DAC. However, neither set of supply routes matches the hardware
topology. Audio can be routed among AIF1/2/3 without using the ADC or
DAC at all; and audio can be routed from the ADC to the DAC without
using any AIFs (via the "ADC Digital DAC Playback Switch"). Because the
DAPM routes are wrong, both of these use cases are currently broken.

This commit adds the necessary widgets and routes to represent the real
hardware topology, with functionality equivalent to the current driver.

For the existing "allwinner,sun8i-a33-codec" compatible, widgets with
the old names are kept as wrappers around the new widgets, so existing
device trees will continue to work. For "allwinner,sun50i-a64-codec",
the old widgets can be omitted, because no device trees yet use that
compatible.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 sound/soc/sunxi/sun8i-codec.c | 120 +++++++++++++++++++++++++++-------
 1 file changed, 95 insertions(+), 25 deletions(-)

diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
index ca51af114419..ffeac150c086 100644
--- a/sound/soc/sunxi/sun8i-codec.c
+++ b/sound/soc/sunxi/sun8i-codec.c
@@ -13,6 +13,7 @@
 #include <linux/delay.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/log2.h>
@@ -85,10 +86,15 @@
 #define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV_MASK	GENMASK(8, 6)
 #define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK	GENMASK(12, 9)
 
+struct sun8i_codec_quirks {
+	bool legacy_widgets	: 1;
+};
+
 struct sun8i_codec {
-	struct regmap	*regmap;
-	struct clk	*clk_module;
-	struct clk	*clk_bus;
+	struct regmap			*regmap;
+	struct clk			*clk_module;
+	struct clk			*clk_bus;
+	const struct sun8i_codec_quirks	*quirks;
 };
 
 static int sun8i_codec_runtime_resume(struct device *dev)
@@ -388,22 +394,30 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY("ADC", SUN8I_ADC_DIG_CTRL, SUN8I_ADC_DIG_CTRL_ENDA,
 			    0, NULL, 0),
 
-	/* Analog DAC AIF */
-	SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left", "Playback", 0,
+	/* AIF "DAC" Inputs */
+	SND_SOC_DAPM_AIF_IN("AIF1 DA0L", "Playback", 0,
 			    SUN8I_AIF1_DACDAT_CTRL,
 			    SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA, 0),
-	SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right", "Playback", 0,
+	SND_SOC_DAPM_AIF_IN("AIF1 DA0R", "Playback", 0,
 			    SUN8I_AIF1_DACDAT_CTRL,
 			    SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA, 0),
 
-	/* Analog ADC AIF */
-	SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left ADC", "Capture", 0,
+	/* AIF "ADC" Outputs */
+	SND_SOC_DAPM_AIF_IN("AIF1 AD0L", "Capture", 0,
 			    SUN8I_AIF1_ADCDAT_CTRL,
 			    SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA, 0),
-	SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right ADC", "Capture", 0,
+	SND_SOC_DAPM_AIF_IN("AIF1 AD0R", "Capture", 0,
 			    SUN8I_AIF1_ADCDAT_CTRL,
 			    SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA, 0),
 
+	/* ADC Inputs (connected to analog codec DAPM context) */
+	SND_SOC_DAPM_ADC("ADCL", NULL, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_ADC("ADCR", NULL, SND_SOC_NOPM, 0, 0),
+
+	/* DAC Outputs (connected to analog codec DAPM context) */
+	SND_SOC_DAPM_DAC("DACL", NULL, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_DAC("DACR", NULL, SND_SOC_NOPM, 0, 0),
+
 	/* DAC and ADC Mixers */
 	SOC_MIXER_ARRAY("Left Digital DAC Mixer", SND_SOC_NOPM, 0, 0,
 			sun8i_dac_mixer_controls),
@@ -449,40 +463,86 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = {
 	/* Clock Routes */
 	{ "AIF1", NULL, "SYSCLK AIF1" },
 	{ "AIF1 PLL", NULL, "AIF1" },
-	{ "RST AIF1", NULL, "AIF1 PLL" },
+	{ "SYSCLK", NULL, "AIF1 PLL" },
+
+	{ "RST AIF1", NULL, "SYSCLK" },
 	{ "MODCLK AFI1", NULL, "RST AIF1" },
-	{ "DAC", NULL, "MODCLK AFI1" },
-	{ "ADC", NULL, "MODCLK AFI1" },
+	{ "AIF1 AD0L", NULL, "MODCLK AFI1" },
+	{ "AIF1 AD0R", NULL, "MODCLK AFI1" },
+	{ "AIF1 DA0L", NULL, "MODCLK AFI1" },
+	{ "AIF1 DA0R", NULL, "MODCLK AFI1" },
 
 	{ "RST DAC", NULL, "SYSCLK" },
 	{ "MODCLK DAC", NULL, "RST DAC" },
 	{ "DAC", NULL, "MODCLK DAC" },
+	{ "DACL", NULL, "DAC" },
+	{ "DACR", NULL, "DAC" },
 
 	{ "RST ADC", NULL, "SYSCLK" },
 	{ "MODCLK ADC", NULL, "RST ADC" },
 	{ "ADC", NULL, "MODCLK ADC" },
+	{ "ADCL", NULL, "ADC" },
+	{ "ADCR", NULL, "ADC" },
 
 	/* DAC Routes */
-	{ "AIF1 Slot 0 Right", NULL, "DAC" },
-	{ "AIF1 Slot 0 Left", NULL, "DAC" },
+	{ "DACL", NULL, "Left Digital DAC Mixer" },
+	{ "DACR", NULL, "Right Digital DAC Mixer" },
 
 	/* DAC Mixer Routes */
-	{ "Left Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch",
-	  "AIF1 Slot 0 Left"},
-	{ "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch",
-	  "AIF1 Slot 0 Right"},
+	{ "Left Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 DA0L" },
+	{ "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 DA0R" },
 
 	/* ADC Routes */
-	{ "AIF1 Slot 0 Right ADC", NULL, "ADC" },
-	{ "AIF1 Slot 0 Left ADC", NULL, "ADC" },
+	{ "AIF1 AD0L", NULL, "Left Digital ADC Mixer" },
+	{ "AIF1 AD0R", NULL, "Right Digital ADC Mixer" },
 
 	/* ADC Mixer Routes */
-	{ "Left Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch",
-	  "AIF1 Slot 0 Left ADC" },
-	{ "Right Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch",
-	  "AIF1 Slot 0 Right ADC" },
+	{ "Left Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", "ADCL" },
+	{ "Right Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", "ADCR" },
+};
+
+static const struct snd_soc_dapm_widget sun8i_codec_legacy_widgets[] = {
+	/* Legacy ADC Inputs (connected to analog codec DAPM context) */
+	SND_SOC_DAPM_ADC("AIF1 Slot 0 Left ADC", NULL, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_ADC("AIF1 Slot 0 Right ADC", NULL, SND_SOC_NOPM, 0, 0),
+
+	/* Legacy DAC Outputs (connected to analog codec DAPM context) */
+	SND_SOC_DAPM_DAC("AIF1 Slot 0 Left", NULL, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_DAC("AIF1 Slot 0 Right", NULL, SND_SOC_NOPM, 0, 0),
+};
+
+static const struct snd_soc_dapm_route sun8i_codec_legacy_routes[] = {
+	/* Legacy ADC Routes */
+	{ "ADCL", NULL, "AIF1 Slot 0 Left ADC" },
+	{ "ADCR", NULL, "AIF1 Slot 0 Right ADC" },
+
+	/* Legacy DAC Routes */
+	{ "AIF1 Slot 0 Left", NULL, "DACL" },
+	{ "AIF1 Slot 0 Right", NULL, "DACR" },
 };
 
+static int sun8i_codec_component_probe(struct snd_soc_component *component)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
+	struct sun8i_codec *scodec = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	/* Add widgets for backward compatibility with old device trees. */
+	if (scodec->quirks->legacy_widgets) {
+		ret = snd_soc_dapm_new_controls(dapm, sun8i_codec_legacy_widgets,
+						ARRAY_SIZE(sun8i_codec_legacy_widgets));
+		if (ret)
+			return ret;
+
+		ret = snd_soc_dapm_add_routes(dapm, sun8i_codec_legacy_routes,
+					      ARRAY_SIZE(sun8i_codec_legacy_routes));
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static const struct snd_soc_dai_ops sun8i_codec_dai_ops = {
 	.hw_params = sun8i_codec_hw_params,
 	.set_fmt = sun8i_set_fmt,
@@ -566,6 +626,8 @@ static int sun8i_codec_probe(struct platform_device *pdev)
 		return PTR_ERR(scodec->regmap);
 	}
 
+	scodec->quirks = of_device_get_match_data(&pdev->dev);
+
 	platform_set_drvdata(pdev, scodec);
 
 	pm_runtime_enable(&pdev->dev);
@@ -603,8 +665,16 @@ static int sun8i_codec_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct sun8i_codec_quirks sun8i_a33_quirks = {
+	.legacy_widgets	= true,
+};
+
+static const struct sun8i_codec_quirks sun50i_a64_quirks = {
+};
+
 static const struct of_device_id sun8i_codec_of_match[] = {
-	{ .compatible = "allwinner,sun8i-a33-codec" },
+	{ .compatible = "allwinner,sun8i-a33-codec", .data = &sun8i_a33_quirks },
+	{ .compatible = "allwinner,sun50i-a64-codec", .data = &sun50i_a64_quirks },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sun8i_codec_of_match);
-- 
2.26.2


  parent reply	other threads:[~2020-07-26  1:27 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-26  1:25 [PATCH 0/7] Allwinner A64 digital audio codec fixes Samuel Holland
2020-07-26  1:25 ` [PATCH 1/7] ASoC: dt-bindings: Add a new compatible for the A64 codec Samuel Holland
2020-07-31 20:20   ` Rob Herring
2020-07-26  1:25 ` Samuel Holland [this message]
2020-07-26  1:25 ` [PATCH 3/7] ASoC: sun8i-codec: Add missing mixer routes Samuel Holland
2020-07-26  1:25 ` [PATCH 4/7] ASoC: sun8i-codec: Add a quirk for LRCK inversion Samuel Holland
2020-07-26  1:25 ` [PATCH 5/7] ARM: dts: sun8i: a33: Update codec widget names Samuel Holland
2020-07-26  1:25 ` [PATCH 6/7] arm64: dts: allwinner: a64: " Samuel Holland
2020-07-26  1:25 ` [PATCH 7/7] arm64: dts: allwinner: a64: Update the audio codec compatible Samuel Holland
2020-08-18 16:54 ` [PATCH 0/7] Allwinner A64 digital audio codec fixes Mark Brown
2020-08-24 14:03   ` Maxime Ripard
2020-08-25 15:36     ` Mark Brown
2020-08-28 10:08       ` Maxime Ripard
2020-08-28 10:49 ` Maxime Ripard
2020-08-28 11:05   ` Ricard Wanderlof

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200726012557.38282-3-samuel@sholland.org \
    --to=samuel@sholland.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=anarsoul@gmail.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=megous@megous.com \
    --cc=mripard@kernel.org \
    --cc=perex@perex.cz \
    --cc=robh+dt@kernel.org \
    --cc=tiwai@suse.com \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).