[PATCH 1/2] ASoC: qcom: lpass: Fix hardcoded SC7810 DAI IDs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



LPASS is broken on MSM8916/DragonBoard 410c since Linux 5.10.

Attempting to play HDMI audio results in:

  ADV7533: lpass_platform_pcmops_hw_params: invalid  interface: 3
  ADV7533: lpass_platform_pcmops_trigger: invalid 3 interface
  apq8016-lpass-cpu 7708000.audio-controller: ASoC: error at soc_component_trigger on 7708000.audio-controller: -22

Attempting to record analog audio results in:

  Unable to handle kernel NULL pointer dereference at virtual address 00000000000001e4
  Internal error: Oops: 96000004 [#1] PREEMPT SMP
  CPU: 1 PID: 1568 Comm: arecord Not tainted 5.11.0-rc3 #20
  Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
  pc : regmap_write+0x14/0x80
  lr : lpass_platform_pcmops_open+0xd8/0x210 [snd_soc_lpass_platform]
  Call trace:
   regmap_write+0x14/0x80
   lpass_platform_pcmops_open+0xd8/0x210 [snd_soc_lpass_platform]
   snd_soc_component_open+0x2c/0x94
   ...

The problem was introduced in commit 7cb37b7bd0d3 ("ASoC: qcom: Add support
for lpass hdmi driver"). The mistake made there is that lpass.h now contains

  #include <dt-bindings/sound/sc7180-lpass.h>

and then the SC7810 DAI IDs are hardcoded all over lpass-cpu.c and
lpass-platform.c. But apq8016 and ipq806x have completely different
DAI IDs so now MI2S_QUATERNARY (HDMI) is invalid and
MI2S_TERTIARY (= LPASS_DP_RX in sc7180-lpass.h) is treated as HDMI port.

Therefore, LPASS is broken on all platforms except SC7810.

This commit fixes the problem by removing the include from lpass.h.
To check if a DAI is an HDMI port, we compare the dai_id with a new
"hdmi_dai" variable in lpass_variant. For SC7180 this is set to
LPASS_DP_RX (as before), for all other platform to some unrealistic
value (UINT_MAX).

Cc: Srinivasa Rao Mandadapu <srivasam@xxxxxxxxxxxxxx>
Cc: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
Fixes: 7cb37b7bd0d3 ("ASoC: qcom: Add support for lpass hdmi driver")
Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx>
---
Srinivas mentioned a potential different fix here:
https://lore.kernel.org/alsa-devel/4b34bd4f-e7bc-84f9-5e8a-b2348c17b7aa@xxxxxxxxxx/

Instead of this patch, we could change the dt-bindings for LPASS,
so that all SoCs use consistent DAI IDs.

In general, I think this might be cleaner, especially in case more special
DAIs are added in the future. However, when I made this patch (before Srinivas
mentioned it) I tried to avoid changing the dt-bindings because:

  - Changing dt-bindings after they are added is generally discouraged.

but more importantly:

  - lpass-ipq806x.c does not seem to have PRIMARY, SECONDARY, ...
    but something completely different. I know nothing about that
    platform so I don't know how to handle it.
---
 sound/soc/qcom/lpass-apq8016.c   |  1 +
 sound/soc/qcom/lpass-cpu.c       |  9 ++--
 sound/soc/qcom/lpass-ipq806x.c   |  1 +
 sound/soc/qcom/lpass-lpaif-reg.h |  2 +-
 sound/soc/qcom/lpass-platform.c  | 77 +++++++++++---------------------
 sound/soc/qcom/lpass-sc7180.c    |  1 +
 sound/soc/qcom/lpass.h           |  4 +-
 7 files changed, 39 insertions(+), 56 deletions(-)

diff --git a/sound/soc/qcom/lpass-apq8016.c b/sound/soc/qcom/lpass-apq8016.c
index 8507ef8f6679..40e9c66a74a9 100644
--- a/sound/soc/qcom/lpass-apq8016.c
+++ b/sound/soc/qcom/lpass-apq8016.c
@@ -273,6 +273,7 @@ static struct lpass_variant apq8016_data = {
 	.num_clks		= 2,
 	.dai_driver		= apq8016_lpass_cpu_dai_driver,
 	.num_dai		= ARRAY_SIZE(apq8016_lpass_cpu_dai_driver),
+	.hdmi_dai		= LPASS_HDMI_DAI_UNSUPPORTED,
 	.dai_osr_clk_names	= (const char *[]) {
 				"mi2s-osr-clk0",
 				"mi2s-osr-clk1",
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index c5e99c2d89c7..b22992050646 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -707,7 +707,8 @@ static unsigned int of_lpass_cpu_parse_sd_lines(struct device *dev,
 }
 
 static void of_lpass_cpu_parse_dai_data(struct device *dev,
-					struct lpass_data *data)
+					struct lpass_data *data,
+					struct lpass_variant *variant)
 {
 	struct device_node *node;
 	int ret, id;
@@ -724,7 +725,7 @@ static void of_lpass_cpu_parse_dai_data(struct device *dev,
 			dev_err(dev, "valid dai id not found: %d\n", ret);
 			continue;
 		}
-		if (id == LPASS_DP_RX) {
+		if (id == variant->hdmi_dai) {
 			data->hdmi_port_enable = 1;
 			dev_err(dev, "HDMI Port is enabled: %d\n", id);
 		} else {
@@ -766,7 +767,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 	drvdata->variant = (struct lpass_variant *)match->data;
 	variant = drvdata->variant;
 
-	of_lpass_cpu_parse_dai_data(dev, drvdata);
+	of_lpass_cpu_parse_dai_data(dev, drvdata, variant);
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-lpaif");
 
@@ -820,7 +821,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 
 	for (i = 0; i < variant->num_dai; i++) {
 		dai_id = variant->dai_driver[i].id;
-		if (dai_id == LPASS_DP_RX)
+		if (dai_id == variant->hdmi_dai)
 			continue;
 
 		drvdata->mi2s_osr_clk[dai_id] = devm_clk_get(dev,
diff --git a/sound/soc/qcom/lpass-ipq806x.c b/sound/soc/qcom/lpass-ipq806x.c
index 92f98b4df47f..a0c7c5eb03f1 100644
--- a/sound/soc/qcom/lpass-ipq806x.c
+++ b/sound/soc/qcom/lpass-ipq806x.c
@@ -149,6 +149,7 @@ static struct lpass_variant ipq806x_data = {
 
 	.dai_driver		= &ipq806x_lpass_cpu_dai_driver,
 	.num_dai		= 1,
+	.hdmi_dai		= LPASS_HDMI_DAI_UNSUPPORTED,
 	.dai_osr_clk_names	= (const char *[]) {
 				"mi2s-osr-clk",
 				},
diff --git a/sound/soc/qcom/lpass-lpaif-reg.h b/sound/soc/qcom/lpass-lpaif-reg.h
index 405542832e99..332f1b9ba674 100644
--- a/sound/soc/qcom/lpass-lpaif-reg.h
+++ b/sound/soc/qcom/lpass-lpaif-reg.h
@@ -133,7 +133,7 @@
 #define	LPAIF_WRDMAPERCNT_REG(v, chan)	LPAIF_WRDMA_REG_ADDR(v, 0x14, (chan))
 
 #define LPAIF_INTFDMA_REG(v, chan, reg, dai_id)  \
-		((v->dai_driver[dai_id].id ==  LPASS_DP_RX) ? \
+		((v->dai_driver[dai_id].id ==  v->hdmi_dai) ? \
 		LPAIF_HDMI_RDMA##reg##_REG(v, chan) : \
 		 LPAIF_RDMA##reg##_REG(v, chan))
 
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index d1c248590f3a..140609fe835d 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -128,7 +128,7 @@ static int lpass_platform_pcmops_open(struct snd_soc_component *component,
 		return dma_ch;
 	}
 
-	if (cpu_dai->driver->id == LPASS_DP_RX) {
+	if (cpu_dai->driver->id == v->hdmi_dai) {
 		map = drvdata->hdmiif_map;
 		drvdata->hdmi_substream[dma_ch] = substream;
 	} else {
@@ -173,7 +173,7 @@ static int lpass_platform_pcmops_close(struct snd_soc_component *component,
 	unsigned int dai_id = cpu_dai->driver->id;
 
 	data = runtime->private_data;
-	if (dai_id == LPASS_DP_RX)
+	if (dai_id == v->hdmi_dai)
 		drvdata->hdmi_substream[data->dma_ch] = NULL;
 	else
 		drvdata->substream[data->dma_ch] = NULL;
@@ -205,7 +205,7 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component,
 
 	if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
 		id = pcm_data->dma_ch;
-		if (dai_id == LPASS_DP_RX)
+		if (dai_id == v->hdmi_dai)
 			dmactl = drvdata->hdmi_rd_dmactl;
 		else
 			dmactl = drvdata->rd_dmactl;
@@ -234,8 +234,7 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component,
 		return ret;
 	}
 
-	switch (dai_id) {
-	case LPASS_DP_RX:
+	if (dai_id == v->hdmi_dai) {
 		ret = regmap_fields_write(dmactl->burst8, id,
 							LPAIF_DMACTL_BURSTEN_INCR4);
 		if (ret) {
@@ -254,9 +253,7 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component,
 			dev_err(soc_runtime->dev, "error updating dynbursten field: %d\n", ret);
 			return ret;
 		}
-		break;
-	case MI2S_PRIMARY:
-	case MI2S_SECONDARY:
+	} else {
 		ret = regmap_fields_write(dmactl->intf, id,
 						LPAIF_DMACTL_AUDINTF(dma_port));
 		if (ret) {
@@ -264,12 +261,8 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component,
 					ret);
 			return ret;
 		}
-
-		break;
-	default:
-		dev_err(soc_runtime->dev, "%s: invalid  interface: %d\n", __func__, dai_id);
-		break;
 	}
+
 	switch (bitwidth) {
 	case 16:
 		switch (channels) {
@@ -299,22 +292,22 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component,
 			regval = LPAIF_DMACTL_WPSCNT_ONE;
 			break;
 		case 2:
-			regval = (dai_id == LPASS_DP_RX ?
+			regval = (dai_id == v->hdmi_dai ?
 			LPAIF_DMACTL_WPSCNT_ONE :
 			LPAIF_DMACTL_WPSCNT_TWO);
 			break;
 		case 4:
-			regval = (dai_id == LPASS_DP_RX ?
+			regval = (dai_id == v->hdmi_dai ?
 			LPAIF_DMACTL_WPSCNT_TWO :
 			LPAIF_DMACTL_WPSCNT_FOUR);
 			break;
 		case 6:
-			regval = (dai_id == LPASS_DP_RX ?
+			regval = (dai_id == v->hdmi_dai ?
 			LPAIF_DMACTL_WPSCNT_THREE :
 			LPAIF_DMACTL_WPSCNT_SIX);
 			break;
 		case 8:
-			regval = (dai_id == LPASS_DP_RX ?
+			regval = (dai_id == v->hdmi_dai ?
 			LPAIF_DMACTL_WPSCNT_FOUR :
 			LPAIF_DMACTL_WPSCNT_EIGHT);
 			break;
@@ -354,7 +347,7 @@ static int lpass_platform_pcmops_hw_free(struct snd_soc_component *component,
 	struct regmap *map;
 	unsigned int dai_id = cpu_dai->driver->id;
 
-	if (dai_id == LPASS_DP_RX)
+	if (dai_id == v->hdmi_dai)
 		map = drvdata->hdmiif_map;
 	else
 		map = drvdata->lpaif_map;
@@ -386,7 +379,7 @@ static int lpass_platform_pcmops_prepare(struct snd_soc_component *component,
 
 	ch = pcm_data->dma_ch;
 	if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
-		if (dai_id == LPASS_DP_RX) {
+		if (dai_id == v->hdmi_dai) {
 			dmactl = drvdata->hdmi_rd_dmactl;
 			map = drvdata->hdmiif_map;
 		} else {
@@ -456,7 +449,7 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 	ch = pcm_data->dma_ch;
 	if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
 		id = pcm_data->dma_ch;
-		if (dai_id == LPASS_DP_RX) {
+		if (dai_id == v->hdmi_dai) {
 			dmactl = drvdata->hdmi_rd_dmactl;
 			map = drvdata->hdmiif_map;
 		} else {
@@ -480,8 +473,8 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 				"error writing to rdmactl reg: %d\n", ret);
 			return ret;
 		}
-		switch (dai_id) {
-		case LPASS_DP_RX:
+
+		if (dai_id == v->hdmi_dai) {
 			ret = regmap_fields_write(dmactl->dyncclk, id,
 					 LPAIF_DMACTL_DYNCLK_ON);
 			if (ret) {
@@ -504,9 +497,7 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 					LPAIF_IRQ_HDMI_REQ_ON_PRELOAD(ch) |
 					LPAIF_IRQ_HDMI_METADONE |
 					LPAIF_IRQ_HDMI_SDEEP_AUD_DIS(ch));
-			break;
-		case MI2S_PRIMARY:
-		case MI2S_SECONDARY:
+		} else {
 			reg_irqclr = LPAIF_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST);
 			val_irqclr = LPAIF_IRQ_ALL(ch);
 
@@ -514,10 +505,6 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 			reg_irqen = LPAIF_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST);
 			val_mask = LPAIF_IRQ_ALL(ch);
 			val_irqen = LPAIF_IRQ_ALL(ch);
-			break;
-		default:
-			dev_err(soc_runtime->dev, "%s: invalid %d interface\n", __func__, dai_id);
-			return -EINVAL;
 		}
 
 		ret = regmap_write(map, reg_irqclr, val_irqclr);
@@ -541,8 +528,8 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 				"error writing to rdmactl reg: %d\n", ret);
 			return ret;
 		}
-		switch (dai_id) {
-		case LPASS_DP_RX:
+
+		if (dai_id == v->hdmi_dai) {
 			ret = regmap_fields_write(dmactl->dyncclk, id,
 					 LPAIF_DMACTL_DYNCLK_OFF);
 			if (ret) {
@@ -556,16 +543,10 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 					LPAIF_IRQ_HDMI_METADONE |
 					LPAIF_IRQ_HDMI_SDEEP_AUD_DIS(ch));
 			val_irqen = 0;
-			break;
-		case MI2S_PRIMARY:
-		case MI2S_SECONDARY:
+		} else {
 			reg_irqen = LPAIF_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST);
 			val_mask = LPAIF_IRQ_ALL(ch);
 			val_irqen = 0;
-			break;
-		default:
-			dev_err(soc_runtime->dev, "%s: invalid %d interface\n", __func__, dai_id);
-			return -EINVAL;
 		}
 
 		ret = regmap_update_bits(map, reg_irqen, val_mask, val_irqen);
@@ -595,7 +576,7 @@ static snd_pcm_uframes_t lpass_platform_pcmops_pointer(
 	struct regmap *map;
 	unsigned int dai_id = cpu_dai->driver->id;
 
-	if (dai_id == LPASS_DP_RX)
+	if (dai_id == v->hdmi_dai)
 		map = drvdata->hdmiif_map;
 	else
 		map = drvdata->lpaif_map;
@@ -645,24 +626,18 @@ static irqreturn_t lpass_dma_interrupt_handler(
 	struct regmap *map;
 	unsigned int dai_id = cpu_dai->driver->id;
 
-	switch (dai_id) {
-	case LPASS_DP_RX:
+	if (dai_id == v->hdmi_dai) {
 		map = drvdata->hdmiif_map;
 		reg = LPASS_HDMITX_APP_IRQCLEAR_REG(v);
 		val = (LPAIF_IRQ_HDMI_REQ_ON_PRELOAD(chan) |
 		LPAIF_IRQ_HDMI_METADONE |
 		LPAIF_IRQ_HDMI_SDEEP_AUD_DIS(chan));
-	break;
-	case MI2S_PRIMARY:
-	case MI2S_SECONDARY:
+	} else {
 		map = drvdata->lpaif_map;
 		reg = LPAIF_IRQCLEAR_REG(v, LPAIF_IRQ_PORT_HOST);
 		val = 0;
-	break;
-	default:
-	dev_err(soc_runtime->dev, "%s: invalid  %d interface\n", __func__, dai_id);
-	return -EINVAL;
 	}
+
 	if (interrupts & LPAIF_IRQ_PER(chan)) {
 
 		rv = regmap_write(map, reg, LPAIF_IRQ_PER(chan) | val);
@@ -826,10 +801,11 @@ static void lpass_platform_pcm_free(struct snd_soc_component *component,
 static int lpass_platform_pcmops_suspend(struct snd_soc_component *component)
 {
 	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
+	struct lpass_variant *v = drvdata->variant;
 	struct regmap *map;
 	unsigned int dai_id = component->id;
 
-	if (dai_id == LPASS_DP_RX)
+	if (dai_id == v->hdmi_dai)
 		map = drvdata->hdmiif_map;
 	else
 		map = drvdata->lpaif_map;
@@ -843,10 +819,11 @@ static int lpass_platform_pcmops_suspend(struct snd_soc_component *component)
 static int lpass_platform_pcmops_resume(struct snd_soc_component *component)
 {
 	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
+	struct lpass_variant *v = drvdata->variant;
 	struct regmap *map;
 	unsigned int dai_id = component->id;
 
-	if (dai_id == LPASS_DP_RX)
+	if (dai_id == v->hdmi_dai)
 		map = drvdata->hdmiif_map;
 	else
 		map = drvdata->lpaif_map;
diff --git a/sound/soc/qcom/lpass-sc7180.c b/sound/soc/qcom/lpass-sc7180.c
index 85db650c2169..76bff06543ea 100644
--- a/sound/soc/qcom/lpass-sc7180.c
+++ b/sound/soc/qcom/lpass-sc7180.c
@@ -271,6 +271,7 @@ static struct lpass_variant sc7180_data = {
 	.num_clks		= 3,
 	.dai_driver		= sc7180_lpass_cpu_dai_driver,
 	.num_dai		= ARRAY_SIZE(sc7180_lpass_cpu_dai_driver),
+	.hdmi_dai		= LPASS_DP_RX,
 	.dai_osr_clk_names      = (const char *[]) {
 				   "mclk0",
 				   "null",
diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
index 0195372905ed..2de0632c1fb7 100644
--- a/sound/soc/qcom/lpass.h
+++ b/sound/soc/qcom/lpass.h
@@ -12,7 +12,6 @@
 #include <linux/compiler.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
-#include <dt-bindings/sound/sc7180-lpass.h>
 #include "lpass-hdmi.h"
 
 #define LPASS_AHBIX_CLOCK_FREQUENCY		131072000
@@ -20,6 +19,8 @@
 #define LPASS_MAX_DMA_CHANNELS			(8)
 #define LPASS_MAX_HDMI_DMA_CHANNELS		(4)
 
+#define LPASS_HDMI_DAI_UNSUPPORTED		UINT_MAX
+
 #define QCOM_REGMAP_FIELD_ALLOC(d, m, f, mf)    \
 	do { \
 		mf = devm_regmap_field_alloc(d, m, f);     \
@@ -245,6 +246,7 @@ struct lpass_variant {
 	/* SOC specific dais */
 	struct snd_soc_dai_driver *dai_driver;
 	int num_dai;
+	unsigned int hdmi_dai;
 	const char * const *dai_osr_clk_names;
 	const char * const *dai_bit_clk_names;
 
-- 
2.30.0




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux