Re: Sound not working after commit bbf7d3b1c4f40eb02dd1dffb500ba00b0bff0303 on Amlogic A311D device
Wow. So there's already a problem with a config and we made it worse...
Can you try with this hack attached, just to see what causes the kernel oops? Thanks!
Hello friends) I modified your hack a little. And I don't understanding what is fe and be, but I see what problem apearing due to be_substream points to 0. The "if (!fe_substream->pcm->nonatomic && be_substream->pcm->nonatomic) {" at result gives the error...
* I am sorry what delete recipients in the previous send((
On 7/1/22 10:47, Alex Natalsson wrote:
Wow. So there's already a problem with a config and we made it worse...
Can you try with this hack attached, just to see what causes the kernel oops? Thanks!
Hello friends) I modified your hack a little. And I don't understanding what is fe and be, but I see what problem apearing due to be_substream points to 0. The "if (!fe_substream->pcm->nonatomic && be_substream->pcm->nonatomic) {" at result gives the error...
- I am sorry what delete recipients in the previous send((
Interesting, thanks for the test results!
[ 108.090732] dpcm_be_connect: start
[ 108.090734] dpcm_be_connect: 1
[ 108.090735] dpcm_be_connect: 3
[ 108.090737] dpcm_be_connect: 3.1
[ 108.090738] dpcm_be_connect: 3.1 fe_substream_addr=128378368
[ 108.090740] dpcm_be_connect: 3.1 be_substream_addr=0
[ 108.090750] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
Indeed it looks like we are de-referencing a NULL pointer, be_substream isn't initialized.
we could add a simple error check as in the diff below but I don't know what the root cause might be
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index a827cc3c158a..093b98b0e394 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1203,6 +1203,15 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe,
fe_substream = snd_soc_dpcm_get_substream(fe, stream);
be_substream = snd_soc_dpcm_get_substream(be, stream);
+ if (!fe_substream) {
+ dev_err(fe->dev, "%s: fe_substream not initialized\n", __func__);
+ return -EINVAL;
+ }
+ if (!be_substream) {
+ dev_err(be->dev, "%s: be_substream not initialized\n", __func__);
+ return -EINVAL;
+ }
+
if (!fe_substream->pcm->nonatomic && be_substream->pcm->nonatomic) {
dev_err(be->dev, "%s: FE is atomic but BE is nonatomic, invalid configuration\n",
__func__);
if (!fe_substream) {dev_err(fe->dev, "%s: fe_substream not initialized\n",__func__);
return -EINVAL;}if (!be_substream) {dev_err(be->dev, "%s: be_substream not initialized\n",__func__);
return -EINVAL;}
Good) USB-audio is working) May be and A311D internal sound card probably to repear too?)) Big Thanks!
if (!fe_substream) {dev_err(fe->dev, "%s: fe_substream not initialized\n",__func__);
return -EINVAL;}if (!be_substream) {dev_err(be->dev, "%s: be_substream not initialized\n",__func__);
return -EINVAL;}
Will be this in upstream or needing bugzilla reporting message?
On 7/9/22 06:19, Alex Natalsson wrote:
if (!fe_substream) {dev_err(fe->dev, "%s: fe_substream not initialized\n",__func__);
return -EINVAL;}if (!be_substream) {dev_err(be->dev, "%s: be_substream not initialized\n",__func__);
return -EINVAL;}Will be this in upstream or needing bugzilla reporting message?
I created a patch based on this, see https://github.com/thesofproject/linux/pull/3735
I am not sure however if this is the 'right' fix. There was a comment from Peter Ujfalusi that a BE substream may be initialized later, but if that's the case then the atomicity check that was introduced is done in the wrong location.
Takashi, we could use your guidance here. Thanks -Pierre
On 11/07/2022 17:33, Pierre-Louis Bossart wrote:
On 7/9/22 06:19, Alex Natalsson wrote:
if (!fe_substream) {dev_err(fe->dev, "%s: fe_substream not initialized\n",__func__);
return -EINVAL;}if (!be_substream) {dev_err(be->dev, "%s: be_substream not initialized\n",__func__);
return -EINVAL;}Will be this in upstream or needing bugzilla reporting message?
I created a patch based on this, see https://github.com/thesofproject/linux/pull/3735
I am not sure however if this is the 'right' fix. There was a comment from Peter Ujfalusi that a BE substream may be initialized later, but if that's the case then the atomicity check that was introduced is done in the wrong location.
fwiw, the dpcm_apply_symmetry() have this check at line 1822:
/* A backend may not have the requested substream */ if (!be_substream) continue;
both dpcm_be_connect() and dpcm_apply_symmetry() are called via dpcm_fe_dai_open() line 2736-2739:
/* calculate valid and active FE <-> BE dpcms */ dpcm_process_paths(fe, stream, &list, 1);
ret = dpcm_fe_dai_startup(fe_substream);
dpcm_fe_dai_open -> dpcm_process_paths -> dpcm_add_paths > dpcm_be_connect
dpcm_fe_dai_open -> dpcm_fe_dai_startup -> dpcm_apply_symmetry
If the check was added by 6246f283d5e02 ("ASoC: dpcm: skip missing substream while applying symmetry")
It looks like that it is not uncommon to not have be_substream at this point...
Takashi, we could use your guidance here. Thanks -Pierre
On Mon, 11 Jul 2022 16:33:14 +0200, Pierre-Louis Bossart wrote:
On 7/9/22 06:19, Alex Natalsson wrote:
if (!fe_substream) {dev_err(fe->dev, "%s: fe_substream not initialized\n",__func__);
return -EINVAL;}if (!be_substream) {dev_err(be->dev, "%s: be_substream not initialized\n",__func__);
return -EINVAL;}Will be this in upstream or needing bugzilla reporting message?
I created a patch based on this, see https://github.com/thesofproject/linux/pull/3735
I am not sure however if this is the 'right' fix. There was a comment from Peter Ujfalusi that a BE substream may be initialized later, but if that's the case then the atomicity check that was introduced is done in the wrong location.
Takashi, we could use your guidance here.
I guess that the check of fe_substream there is superfluous. It must be never NULL. And, in principle, passing the invalid BE to this function itself sounds already wrong. That said, if any check is needed, it should be done beforehand at choosing the right BE.
A fix like below might work instead? (Totally untested)
thanks,
Takashi
-- 8< -- --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1318,6 +1318,9 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card, if (!be->dai_link->no_pcm) continue;
+ if (!snd_soc_dpcm_get_substream(be, stream)) + continue; + for_each_rtd_dais(be, i, dai) { w = snd_soc_dai_get_widget(dai, stream);
participants (4)
-
Alex Natalsson -
Pierre-Louis Bossart -
Péter Ujfalusi -
Takashi Iwai