[PATCH 1/2] ASoC: qcom: lpass: Fix hardcoded SC7810 DAI IDs
Stephan Gerhold
stephan at gerhold.net
Thu Jan 14 10:46:14 CET 2021
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 at codeaurora.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla at linaro.org>
Fixes: 7cb37b7bd0d3 ("ASoC: qcom: Add support for lpass hdmi driver")
Signed-off-by: Stephan Gerhold <stephan at gerhold.net>
---
Srinivas mentioned a potential different fix here:
https://lore.kernel.org/alsa-devel/4b34bd4f-e7bc-84f9-5e8a-b2348c17b7aa@linaro.org/
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
More information about the Alsa-devel
mailing list