All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
@ 2018-11-01 12:38 Rohit kumar
  2018-11-02  7:42   ` Takashi Iwai
  2018-12-13 18:10   ` Mark Brown
  0 siblings, 2 replies; 14+ messages in thread
From: Rohit kumar @ 2018-11-01 12:38 UTC (permalink / raw)
  To: plai, bgoswami, lgirdwood, broonie, perex, tiwai, alsa-devel,
	linux-kernel, rohkumar, srinivas.kandagatla
  Cc: Rohit kumar

Remove no_pcm check to invoke pcm_new() for backend dai-links
too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
while accessing chmap_info struct. chmap_info struct memory is
allocated in pcm_new() of hdmi codec driver which is not invoked
in case of DPCM when hdmi codec driver is part of backend dai-link.

Below is the crash stack:

[   61.635493] Unable to handle kernel NULL pointer dereference at virtual address 00000018
..
[   61.666696]   CM = 0, WnR = 1
[   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd = ffffffc0d6633000
[   61.676526] [0000000000000018] *pgd=0000000153fc8003, *pud=0000000153fc8003, *pmd=0000000000000000
[   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
[   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
..
[   61.740269] PC is at hdmi_codec_startup+0x124/0x164
[   61.745308] LR is at hdmi_codec_startup+0xe4/0x164

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 sound/soc/soc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 6ddcf12..abdc460 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1467,7 +1467,7 @@ static int soc_link_dai_pcm_new(struct snd_soc_dai **dais, int num_dais,
 	for (i = 0; i < num_dais; ++i) {
 		struct snd_soc_dai_driver *drv = dais[i]->driver;
 
-		if (!rtd->dai_link->no_pcm && drv->pcm_new)
+		if (drv->pcm_new)
 			ret = drv->pcm_new(rtd, dais[i]);
 		if (ret < 0) {
 			dev_err(dais[i]->dev,
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
  2018-11-01 12:38 [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link Rohit kumar
@ 2018-11-02  7:42   ` Takashi Iwai
  2018-12-13 18:10   ` Mark Brown
  1 sibling, 0 replies; 14+ messages in thread
From: Takashi Iwai @ 2018-11-02  7:42 UTC (permalink / raw)
  To: Rohit kumar
  Cc: plai, bgoswami, lgirdwood, broonie, perex, tiwai, alsa-devel,
	linux-kernel, rohkumar, srinivas.kandagatla

On Thu, 01 Nov 2018 13:38:49 +0100,
Rohit kumar wrote:
> 
> Remove no_pcm check to invoke pcm_new() for backend dai-links
> too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
> while accessing chmap_info struct. chmap_info struct memory is
> allocated in pcm_new() of hdmi codec driver which is not invoked
> in case of DPCM when hdmi codec driver is part of backend dai-link.
> 
> Below is the crash stack:
> 
> [   61.635493] Unable to handle kernel NULL pointer dereference at virtual address 00000018
> ..
> [   61.666696]   CM = 0, WnR = 1
> [   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd = ffffffc0d6633000
> [   61.676526] [0000000000000018] *pgd=0000000153fc8003, *pud=0000000153fc8003, *pmd=0000000000000000
> [   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
> [   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
> ..
> [   61.740269] PC is at hdmi_codec_startup+0x124/0x164
> [   61.745308] LR is at hdmi_codec_startup+0xe4/0x164
> 
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>

Did you check whether all drivers have no side-effect by this change?
The hdmi-codec isn't the only driver that has pcm_new ops, so we have
to make sure that such a fundamental change wouldn't bring any
regressions.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
@ 2018-11-02  7:42   ` Takashi Iwai
  0 siblings, 0 replies; 14+ messages in thread
From: Takashi Iwai @ 2018-11-02  7:42 UTC (permalink / raw)
  To: Rohit kumar
  Cc: rohkumar, alsa-devel, bgoswami, linux-kernel, plai, tiwai,
	lgirdwood, broonie, srinivas.kandagatla

On Thu, 01 Nov 2018 13:38:49 +0100,
Rohit kumar wrote:
> 
> Remove no_pcm check to invoke pcm_new() for backend dai-links
> too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
> while accessing chmap_info struct. chmap_info struct memory is
> allocated in pcm_new() of hdmi codec driver which is not invoked
> in case of DPCM when hdmi codec driver is part of backend dai-link.
> 
> Below is the crash stack:
> 
> [   61.635493] Unable to handle kernel NULL pointer dereference at virtual address 00000018
> ..
> [   61.666696]   CM = 0, WnR = 1
> [   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd = ffffffc0d6633000
> [   61.676526] [0000000000000018] *pgd=0000000153fc8003, *pud=0000000153fc8003, *pmd=0000000000000000
> [   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
> [   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
> ..
> [   61.740269] PC is at hdmi_codec_startup+0x124/0x164
> [   61.745308] LR is at hdmi_codec_startup+0xe4/0x164
> 
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>

Did you check whether all drivers have no side-effect by this change?
The hdmi-codec isn't the only driver that has pcm_new ops, so we have
to make sure that such a fundamental change wouldn't bring any
regressions.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
  2018-11-02  7:42   ` Takashi Iwai
  (?)
@ 2018-11-02 12:06   ` Rohit Kumar
  2018-11-05 11:13       ` Arnaud Pouliquen
  -1 siblings, 1 reply; 14+ messages in thread
From: Rohit Kumar @ 2018-11-02 12:06 UTC (permalink / raw)
  To: Takashi Iwai, olivier.moysan
  Cc: plai, bgoswami, lgirdwood, broonie, perex, tiwai, alsa-devel,
	linux-kernel, rohkumar, srinivas.kandagatla


On 11/2/2018 1:12 PM, Takashi Iwai wrote:
> On Thu, 01 Nov 2018 13:38:49 +0100,
> Rohit kumar wrote:
>> Remove no_pcm check to invoke pcm_new() for backend dai-links
>> too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
>> while accessing chmap_info struct. chmap_info struct memory is
>> allocated in pcm_new() of hdmi codec driver which is not invoked
>> in case of DPCM when hdmi codec driver is part of backend dai-link.
>>
>> Below is the crash stack:
>>
>> [   61.635493] Unable to handle kernel NULL pointer dereference at virtual address 00000018
>> ..
>> [   61.666696]   CM = 0, WnR = 1
>> [   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd = ffffffc0d6633000
>> [   61.676526] [0000000000000018] *pgd=0000000153fc8003, *pud=0000000153fc8003, *pmd=0000000000000000
>> [   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
>> [   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
>> ..
>> [   61.740269] PC is at hdmi_codec_startup+0x124/0x164
>> [   61.745308] LR is at hdmi_codec_startup+0xe4/0x164
>>
>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
> Did you check whether all drivers have no side-effect by this change?
> The hdmi-codec isn't the only driver that has pcm_new ops, so we have
> to make sure that such a fundamental change wouldn't bring any
> regressions.
>
Below are the drivers calling pcm_new() other than hdmi codec driver.
sound/soc/meson/axg-frddr.c
sound/soc/meson/axg-toddr.c
These two drivers are frontend DAI drivers and should not be impacted
because of this.

Other than this, pcm_new() is called from sound/soc/stm/stm32_sai_sub.c
I could not get much info about this driver. However, it is just adding
kcontrols in pcm_new() which uses internal private structs in get()/put().
Olivier Moysan can too confirm on this.

Thanks,
Rohit
> thanks,
>
> Takashi

-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
  2018-11-02 12:06   ` [alsa-devel] " Rohit Kumar
@ 2018-11-05 11:13       ` Arnaud Pouliquen
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaud Pouliquen @ 2018-11-05 11:13 UTC (permalink / raw)
  To: Rohit Kumar, Takashi Iwai, olivier.moysan
  Cc: rohkumar, alsa-devel, bgoswami, linux-kernel, plai, tiwai,
	lgirdwood, broonie, srinivas.kandagatla

Hello Rohit,

On 11/2/18 1:06 PM, Rohit Kumar wrote:
> 
> On 11/2/2018 1:12 PM, Takashi Iwai wrote:
>> On Thu, 01 Nov 2018 13:38:49 +0100,
>> Rohit kumar wrote:
>>> Remove no_pcm check to invoke pcm_new() for backend dai-links
>>> too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
>>> while accessing chmap_info struct. chmap_info struct memory is
>>> allocated in pcm_new() of hdmi codec driver which is not invoked
>>> in case of DPCM when hdmi codec driver is part of backend dai-link.
>>>
>>> Below is the crash stack:
>>>
>>> [   61.635493] Unable to handle kernel NULL pointer dereference at
>>> virtual address 00000018
>>> ..
>>> [   61.666696]   CM = 0, WnR = 1
>>> [   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd =
>>> ffffffc0d6633000
>>> [   61.676526] [0000000000000018] *pgd=0000000153fc8003,
>>> *pud=0000000153fc8003, *pmd=0000000000000000
>>> [   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
>>> [   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
>>> ..
>>> [   61.740269] PC is at hdmi_codec_startup+0x124/0x164
>>> [   61.745308] LR is at hdmi_codec_startup+0xe4/0x164
>>>
>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>> Did you check whether all drivers have no side-effect by this change?
>> The hdmi-codec isn't the only driver that has pcm_new ops, so we have
>> to make sure that such a fundamental change wouldn't bring any
>> regressions.
>>
> Below are the drivers calling pcm_new() other than hdmi codec driver.
> sound/soc/meson/axg-frddr.c
> sound/soc/meson/axg-toddr.c
> These two drivers are frontend DAI drivers and should not be impacted
> because of this.
> 
> Other than this, pcm_new() is called from sound/soc/stm/stm32_sai_sub.c
> I could not get much info about this driver. However, it is just adding
> kcontrols in pcm_new() which uses internal private structs in get()/put().
> Olivier Moysan can too confirm on this.
First, i'm answering  for Olivier: no regression identified for the SAI
driver, it is not a DPCM driver.

Then i have a concern about the call of pcm_new for a no-PCM backend.
Does it make sense? In DPCM concept, the backend is not linked to the
PCM device...

Instead, I would suggest that you add protection in HDMI_codec on
chmap_info pointer.

The drawback would be that the control is no more available...do you
need it?

Regards
Arnaud

> 
> Thanks,
> Rohit
>> thanks,
>>
>> Takashi
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
@ 2018-11-05 11:13       ` Arnaud Pouliquen
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaud Pouliquen @ 2018-11-05 11:13 UTC (permalink / raw)
  To: Rohit Kumar, Takashi Iwai, olivier.moysan
  Cc: rohkumar, alsa-devel, bgoswami, lgirdwood, plai, linux-kernel,
	tiwai, broonie, srinivas.kandagatla

Hello Rohit,

On 11/2/18 1:06 PM, Rohit Kumar wrote:
> 
> On 11/2/2018 1:12 PM, Takashi Iwai wrote:
>> On Thu, 01 Nov 2018 13:38:49 +0100,
>> Rohit kumar wrote:
>>> Remove no_pcm check to invoke pcm_new() for backend dai-links
>>> too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
>>> while accessing chmap_info struct. chmap_info struct memory is
>>> allocated in pcm_new() of hdmi codec driver which is not invoked
>>> in case of DPCM when hdmi codec driver is part of backend dai-link.
>>>
>>> Below is the crash stack:
>>>
>>> [   61.635493] Unable to handle kernel NULL pointer dereference at
>>> virtual address 00000018
>>> ..
>>> [   61.666696]   CM = 0, WnR = 1
>>> [   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd =
>>> ffffffc0d6633000
>>> [   61.676526] [0000000000000018] *pgd=0000000153fc8003,
>>> *pud=0000000153fc8003, *pmd=0000000000000000
>>> [   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
>>> [   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
>>> ..
>>> [   61.740269] PC is at hdmi_codec_startup+0x124/0x164
>>> [   61.745308] LR is at hdmi_codec_startup+0xe4/0x164
>>>
>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>> Did you check whether all drivers have no side-effect by this change?
>> The hdmi-codec isn't the only driver that has pcm_new ops, so we have
>> to make sure that such a fundamental change wouldn't bring any
>> regressions.
>>
> Below are the drivers calling pcm_new() other than hdmi codec driver.
> sound/soc/meson/axg-frddr.c
> sound/soc/meson/axg-toddr.c
> These two drivers are frontend DAI drivers and should not be impacted
> because of this.
> 
> Other than this, pcm_new() is called from sound/soc/stm/stm32_sai_sub.c
> I could not get much info about this driver. However, it is just adding
> kcontrols in pcm_new() which uses internal private structs in get()/put().
> Olivier Moysan can too confirm on this.
First, i'm answering  for Olivier: no regression identified for the SAI
driver, it is not a DPCM driver.

Then i have a concern about the call of pcm_new for a no-PCM backend.
Does it make sense? In DPCM concept, the backend is not linked to the
PCM device...

Instead, I would suggest that you add protection in HDMI_codec on
chmap_info pointer.

The drawback would be that the control is no more available...do you
need it?

Regards
Arnaud

> 
> Thanks,
> Rohit
>> thanks,
>>
>> Takashi
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
  2018-11-05 11:13       ` Arnaud Pouliquen
  (?)
@ 2018-11-05 18:14       ` Rohit Kumar
  2018-11-06 15:41           ` Arnaud Pouliquen
  -1 siblings, 1 reply; 14+ messages in thread
From: Rohit Kumar @ 2018-11-05 18:14 UTC (permalink / raw)
  To: Arnaud Pouliquen, Takashi Iwai, olivier.moysan
  Cc: rohkumar, alsa-devel, bgoswami, linux-kernel, plai, tiwai,
	lgirdwood, broonie, srinivas.kandagatla

Hello Arnaud,


On 11/5/2018 4:43 PM, Arnaud Pouliquen wrote:
> Hello Rohit,
>
> On 11/2/18 1:06 PM, Rohit Kumar wrote:
>> On 11/2/2018 1:12 PM, Takashi Iwai wrote:
>>> On Thu, 01 Nov 2018 13:38:49 +0100,
>>> Rohit kumar wrote:
>>>> Remove no_pcm check to invoke pcm_new() for backend dai-links
>>>> too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
>>>> while accessing chmap_info struct. chmap_info struct memory is
>>>> allocated in pcm_new() of hdmi codec driver which is not invoked
>>>> in case of DPCM when hdmi codec driver is part of backend dai-link.
>>>>
>>>> Below is the crash stack:
>>>>
>>>> [   61.635493] Unable to handle kernel NULL pointer dereference at
>>>> virtual address 00000018
>>>> ..
>>>> [   61.666696]   CM = 0, WnR = 1
>>>> [   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd =
>>>> ffffffc0d6633000
>>>> [   61.676526] [0000000000000018] *pgd=0000000153fc8003,
>>>> *pud=0000000153fc8003, *pmd=0000000000000000
>>>> [   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
>>>> [   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
>>>> ..
>>>> [   61.740269] PC is at hdmi_codec_startup+0x124/0x164
>>>> [   61.745308] LR is at hdmi_codec_startup+0xe4/0x164
>>>>
>>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>>> Did you check whether all drivers have no side-effect by this change?
>>> The hdmi-codec isn't the only driver that has pcm_new ops, so we have
>>> to make sure that such a fundamental change wouldn't bring any
>>> regressions.
>>>
>> Below are the drivers calling pcm_new() other than hdmi codec driver.
>> sound/soc/meson/axg-frddr.c
>> sound/soc/meson/axg-toddr.c
>> These two drivers are frontend DAI drivers and should not be impacted
>> because of this.
>>
>> Other than this, pcm_new() is called from sound/soc/stm/stm32_sai_sub.c
>> I could not get much info about this driver. However, it is just adding
>> kcontrols in pcm_new() which uses internal private structs in get()/put().
>> Olivier Moysan can too confirm on this.
> First, i'm answering  for Olivier: no regression identified for the SAI
> driver, it is not a DPCM driver.
>
> Then i have a concern about the call of pcm_new for a no-PCM backend.
> Does it make sense? In DPCM concept, the backend is not linked to the
> PCM device...
>
> Instead, I would suggest that you add protection in HDMI_codec on
> chmap_info pointer.
>
> The drawback would be that the control is no more available...do you
> need it?
I don't need chmap_info, but ELD kcontrol is also defined in pcm_new() which
we need. We should probably update the driver to make it compatible with
DPCM. Any suggestions?
> Regards
> Arnaud
>
>> Thanks,
>> Rohit
>>> thanks,
>>>
>>> Takashi
Thanks,
Rohit

-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
  2018-11-05 18:14       ` [alsa-devel] " Rohit Kumar
@ 2018-11-06 15:41           ` Arnaud Pouliquen
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaud Pouliquen @ 2018-11-06 15:41 UTC (permalink / raw)
  To: Rohit Kumar, Takashi Iwai, olivier.moysan
  Cc: rohkumar, alsa-devel, bgoswami, lgirdwood, plai, linux-kernel,
	tiwai, broonie, srinivas.kandagatla

Hello Rohit

On 11/5/18 7:14 PM, Rohit Kumar wrote:
> Hello Arnaud,
> 
> 
> On 11/5/2018 4:43 PM, Arnaud Pouliquen wrote:
>> Hello Rohit,
>>
>> On 11/2/18 1:06 PM, Rohit Kumar wrote:
>>> On 11/2/2018 1:12 PM, Takashi Iwai wrote:
>>>> On Thu, 01 Nov 2018 13:38:49 +0100,
>>>> Rohit kumar wrote:
>>>>> Remove no_pcm check to invoke pcm_new() for backend dai-links
>>>>> too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
>>>>> while accessing chmap_info struct. chmap_info struct memory is
>>>>> allocated in pcm_new() of hdmi codec driver which is not invoked
>>>>> in case of DPCM when hdmi codec driver is part of backend dai-link.
>>>>>
>>>>> Below is the crash stack:
>>>>>
>>>>> [   61.635493] Unable to handle kernel NULL pointer dereference at
>>>>> virtual address 00000018
>>>>> ..
>>>>> [   61.666696]   CM = 0, WnR = 1
>>>>> [   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd =
>>>>> ffffffc0d6633000
>>>>> [   61.676526] [0000000000000018] *pgd=0000000153fc8003,
>>>>> *pud=0000000153fc8003, *pmd=0000000000000000
>>>>> [   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
>>>>> [   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
>>>>> ..
>>>>> [   61.740269] PC is at hdmi_codec_startup+0x124/0x164
>>>>> [   61.745308] LR is at hdmi_codec_startup+0xe4/0x164
>>>>>
>>>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>>>> Did you check whether all drivers have no side-effect by this change?
>>>> The hdmi-codec isn't the only driver that has pcm_new ops, so we have
>>>> to make sure that such a fundamental change wouldn't bring any
>>>> regressions.
>>>>
>>> Below are the drivers calling pcm_new() other than hdmi codec driver.
>>> sound/soc/meson/axg-frddr.c
>>> sound/soc/meson/axg-toddr.c
>>> These two drivers are frontend DAI drivers and should not be impacted
>>> because of this.
>>>
>>> Other than this, pcm_new() is called from sound/soc/stm/stm32_sai_sub.c
>>> I could not get much info about this driver. However, it is just adding
>>> kcontrols in pcm_new() which uses internal private structs in
>>> get()/put().
>>> Olivier Moysan can too confirm on this.
>> First, i'm answering  for Olivier: no regression identified for the SAI
>> driver, it is not a DPCM driver.
>>
>> Then i have a concern about the call of pcm_new for a no-PCM backend.
>> Does it make sense? In DPCM concept, the backend is not linked to the
>> PCM device...
>>
>> Instead, I would suggest that you add protection in HDMI_codec on
>> chmap_info pointer.
>>
>> The drawback would be that the control is no more available...do you
>> need it?
> I don't need chmap_info, but ELD kcontrol is also defined in pcm_new()
> which
> we need. We should probably update the driver to make it compatible with
> DPCM. Any suggestions?

I would say force device to 0 if no_pcm (need probably to create the
control in hdmi_of_xlate_dai_id instead of hdmi_codec_pcm_new).
But keep in mind that solution has to work in case of multi HDMI codec
instances, perhaps using prefix...
Anyway, i'm not a DPCM expert so perhaps more elegant solutions exist.

Regards
Arnaud

>> Regards
>> Arnaud
>>
>>> Thanks,
>>> Rohit
>>>> thanks,
>>>>
>>>> Takashi
> Thanks,
> Rohit
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
@ 2018-11-06 15:41           ` Arnaud Pouliquen
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaud Pouliquen @ 2018-11-06 15:41 UTC (permalink / raw)
  To: Rohit Kumar, Takashi Iwai, olivier.moysan
  Cc: rohkumar, alsa-devel, bgoswami, plai, tiwai, lgirdwood,
	linux-kernel, broonie, srinivas.kandagatla

Hello Rohit

On 11/5/18 7:14 PM, Rohit Kumar wrote:
> Hello Arnaud,
> 
> 
> On 11/5/2018 4:43 PM, Arnaud Pouliquen wrote:
>> Hello Rohit,
>>
>> On 11/2/18 1:06 PM, Rohit Kumar wrote:
>>> On 11/2/2018 1:12 PM, Takashi Iwai wrote:
>>>> On Thu, 01 Nov 2018 13:38:49 +0100,
>>>> Rohit kumar wrote:
>>>>> Remove no_pcm check to invoke pcm_new() for backend dai-links
>>>>> too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
>>>>> while accessing chmap_info struct. chmap_info struct memory is
>>>>> allocated in pcm_new() of hdmi codec driver which is not invoked
>>>>> in case of DPCM when hdmi codec driver is part of backend dai-link.
>>>>>
>>>>> Below is the crash stack:
>>>>>
>>>>> [   61.635493] Unable to handle kernel NULL pointer dereference at
>>>>> virtual address 00000018
>>>>> ..
>>>>> [   61.666696]   CM = 0, WnR = 1
>>>>> [   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd =
>>>>> ffffffc0d6633000
>>>>> [   61.676526] [0000000000000018] *pgd=0000000153fc8003,
>>>>> *pud=0000000153fc8003, *pmd=0000000000000000
>>>>> [   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
>>>>> [   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
>>>>> ..
>>>>> [   61.740269] PC is at hdmi_codec_startup+0x124/0x164
>>>>> [   61.745308] LR is at hdmi_codec_startup+0xe4/0x164
>>>>>
>>>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>>>> Did you check whether all drivers have no side-effect by this change?
>>>> The hdmi-codec isn't the only driver that has pcm_new ops, so we have
>>>> to make sure that such a fundamental change wouldn't bring any
>>>> regressions.
>>>>
>>> Below are the drivers calling pcm_new() other than hdmi codec driver.
>>> sound/soc/meson/axg-frddr.c
>>> sound/soc/meson/axg-toddr.c
>>> These two drivers are frontend DAI drivers and should not be impacted
>>> because of this.
>>>
>>> Other than this, pcm_new() is called from sound/soc/stm/stm32_sai_sub.c
>>> I could not get much info about this driver. However, it is just adding
>>> kcontrols in pcm_new() which uses internal private structs in
>>> get()/put().
>>> Olivier Moysan can too confirm on this.
>> First, i'm answering  for Olivier: no regression identified for the SAI
>> driver, it is not a DPCM driver.
>>
>> Then i have a concern about the call of pcm_new for a no-PCM backend.
>> Does it make sense? In DPCM concept, the backend is not linked to the
>> PCM device...
>>
>> Instead, I would suggest that you add protection in HDMI_codec on
>> chmap_info pointer.
>>
>> The drawback would be that the control is no more available...do you
>> need it?
> I don't need chmap_info, but ELD kcontrol is also defined in pcm_new()
> which
> we need. We should probably update the driver to make it compatible with
> DPCM. Any suggestions?

I would say force device to 0 if no_pcm (need probably to create the
control in hdmi_of_xlate_dai_id instead of hdmi_codec_pcm_new).
But keep in mind that solution has to work in case of multi HDMI codec
instances, perhaps using prefix...
Anyway, i'm not a DPCM expert so perhaps more elegant solutions exist.

Regards
Arnaud

>> Regards
>> Arnaud
>>
>>> Thanks,
>>> Rohit
>>>> thanks,
>>>>
>>>> Takashi
> Thanks,
> Rohit
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
  2018-11-06 15:41           ` Arnaud Pouliquen
@ 2018-11-07 16:14             ` Mark Brown
  -1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2018-11-07 16:14 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: Rohit Kumar, Takashi Iwai, olivier.moysan, rohkumar, alsa-devel,
	bgoswami, lgirdwood, plai, linux-kernel, tiwai,
	srinivas.kandagatla

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

On Tue, Nov 06, 2018 at 04:41:23PM +0100, Arnaud Pouliquen wrote:

> I would say force device to 0 if no_pcm (need probably to create the
> control in hdmi_of_xlate_dai_id instead of hdmi_codec_pcm_new).
> But keep in mind that solution has to work in case of multi HDMI codec
> instances, perhaps using prefix...
> Anyway, i'm not a DPCM expert so perhaps more elegant solutions exist.

Liam, any thoughts on how to handle this?  I've still not really worked
with DPCM systems!

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
@ 2018-11-07 16:14             ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2018-11-07 16:14 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: rohkumar, alsa-devel, bgoswami, olivier.moysan, Takashi Iwai,
	plai, tiwai, lgirdwood, linux-kernel, Rohit Kumar,
	srinivas.kandagatla


[-- Attachment #1.1: Type: text/plain, Size: 479 bytes --]

On Tue, Nov 06, 2018 at 04:41:23PM +0100, Arnaud Pouliquen wrote:

> I would say force device to 0 if no_pcm (need probably to create the
> control in hdmi_of_xlate_dai_id instead of hdmi_codec_pcm_new).
> But keep in mind that solution has to work in case of multi HDMI codec
> instances, perhaps using prefix...
> Anyway, i'm not a DPCM expert so perhaps more elegant solutions exist.

Liam, any thoughts on how to handle this?  I've still not really worked
with DPCM systems!

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link
  2018-11-07 16:14             ` Mark Brown
  (?)
@ 2018-11-21  7:38             ` Rohit Kumar
  -1 siblings, 0 replies; 14+ messages in thread
From: Rohit Kumar @ 2018-11-21  7:38 UTC (permalink / raw)
  To: Mark Brown, Arnaud Pouliquen
  Cc: rohkumar, alsa-devel, bgoswami, olivier.moysan, vinod.koul,
	Takashi Iwai, linux-kernel, plai, tiwai, lgirdwood,
	srinivas.kandagatla, ajitp


On 11/7/2018 9:44 PM, Mark Brown wrote:
> On Tue, Nov 06, 2018 at 04:41:23PM +0100, Arnaud Pouliquen wrote:
>
>> I would say force device to 0 if no_pcm (need probably to create the
>> control in hdmi_of_xlate_dai_id instead of hdmi_codec_pcm_new).
>> But keep in mind that solution has to work in case of multi HDMI codec
>> instances, perhaps using prefix...
>> Anyway, i'm not a DPCM expert so perhaps more elegant solutions exist.
> Liam, any thoughts on how to handle this?  I've still not really worked
> with DPCM systems!
>
Liam/Mark, Do you have any suggestion on this?

>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Thanks,
Rohit

-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Applied "ASoC: core: Invoke pcm_new() for all DAI-link" to the asoc tree
  2018-11-01 12:38 [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link Rohit kumar
@ 2018-12-13 18:10   ` Mark Brown
  2018-12-13 18:10   ` Mark Brown
  1 sibling, 0 replies; 14+ messages in thread
From: Mark Brown @ 2018-12-13 18:10 UTC (permalink / raw)
  To: Rohit kumar
  Cc: Mark Brown, plai, bgoswami, lgirdwood, broonie, perex, tiwai,
	alsa-devel, linux-kernel, rohkumar, srinivas.kandagatla,
	alsa-devel

The patch

   ASoC: core: Invoke pcm_new() for all DAI-link

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From de17f14ea576d8a0f2932404467fa916542da94d Mon Sep 17 00:00:00 2001
From: Rohit kumar <rohitkr@codeaurora.org>
Date: Thu, 1 Nov 2018 18:08:49 +0530
Subject: [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link

Remove no_pcm check to invoke pcm_new() for backend dai-links
too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
while accessing chmap_info struct. chmap_info struct memory is
allocated in pcm_new() of hdmi codec driver which is not invoked
in case of DPCM when hdmi codec driver is part of backend dai-link.

Below is the crash stack:

[   61.635493] Unable to handle kernel NULL pointer dereference at virtual address 00000018
..
[   61.666696]   CM = 0, WnR = 1
[   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd = ffffffc0d6633000
[   61.676526] [0000000000000018] *pgd=0000000153fc8003, *pud=0000000153fc8003, *pmd=0000000000000000
[   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
[   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
..
[   61.740269] PC is at hdmi_codec_startup+0x124/0x164
[   61.745308] LR is at hdmi_codec_startup+0xe4/0x164

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b0db59e6339d..0462b3ec977a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1467,7 +1467,7 @@ static int soc_link_dai_pcm_new(struct snd_soc_dai **dais, int num_dais,
 	for (i = 0; i < num_dais; ++i) {
 		struct snd_soc_dai_driver *drv = dais[i]->driver;
 
-		if (!rtd->dai_link->no_pcm && drv->pcm_new)
+		if (drv->pcm_new)
 			ret = drv->pcm_new(rtd, dais[i]);
 		if (ret < 0) {
 			dev_err(dais[i]->dev,
-- 
2.19.0.rc2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Applied "ASoC: core: Invoke pcm_new() for all DAI-link" to the asoc tree
@ 2018-12-13 18:10   ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2018-12-13 18:10 UTC (permalink / raw)
  To: Rohit kumar
  Cc: rohkumar, alsa-devel, bgoswami, linux-kernel, plai, tiwai,
	lgirdwood, broonie, srinivas.kandagatla

The patch

   ASoC: core: Invoke pcm_new() for all DAI-link

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From de17f14ea576d8a0f2932404467fa916542da94d Mon Sep 17 00:00:00 2001
From: Rohit kumar <rohitkr@codeaurora.org>
Date: Thu, 1 Nov 2018 18:08:49 +0530
Subject: [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link

Remove no_pcm check to invoke pcm_new() for backend dai-links
too. This fixes crash in hdmi codec driver during hdmi_codec_startup()
while accessing chmap_info struct. chmap_info struct memory is
allocated in pcm_new() of hdmi codec driver which is not invoked
in case of DPCM when hdmi codec driver is part of backend dai-link.

Below is the crash stack:

[   61.635493] Unable to handle kernel NULL pointer dereference at virtual address 00000018
..
[   61.666696]   CM = 0, WnR = 1
[   61.669778] user pgtable: 4k pages, 39-bit VAs, pgd = ffffffc0d6633000
[   61.676526] [0000000000000018] *pgd=0000000153fc8003, *pud=0000000153fc8003, *pmd=0000000000000000
[   61.685793] Internal error: Oops: 96000046 [#1] PREEMPT SMP
[   61.722955] CPU: 7 PID: 2238 Comm: aplay Not tainted 4.14.72 #21
..
[   61.740269] PC is at hdmi_codec_startup+0x124/0x164
[   61.745308] LR is at hdmi_codec_startup+0xe4/0x164

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/soc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b0db59e6339d..0462b3ec977a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1467,7 +1467,7 @@ static int soc_link_dai_pcm_new(struct snd_soc_dai **dais, int num_dais,
 	for (i = 0; i < num_dais; ++i) {
 		struct snd_soc_dai_driver *drv = dais[i]->driver;
 
-		if (!rtd->dai_link->no_pcm && drv->pcm_new)
+		if (drv->pcm_new)
 			ret = drv->pcm_new(rtd, dais[i]);
 		if (ret < 0) {
 			dev_err(dais[i]->dev,
-- 
2.19.0.rc2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-12-13 18:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-01 12:38 [PATCH] ASoC: core: Invoke pcm_new() for all DAI-link Rohit kumar
2018-11-02  7:42 ` [alsa-devel] " Takashi Iwai
2018-11-02  7:42   ` Takashi Iwai
2018-11-02 12:06   ` [alsa-devel] " Rohit Kumar
2018-11-05 11:13     ` Arnaud Pouliquen
2018-11-05 11:13       ` Arnaud Pouliquen
2018-11-05 18:14       ` [alsa-devel] " Rohit Kumar
2018-11-06 15:41         ` Arnaud Pouliquen
2018-11-06 15:41           ` Arnaud Pouliquen
2018-11-07 16:14           ` [alsa-devel] " Mark Brown
2018-11-07 16:14             ` Mark Brown
2018-11-21  7:38             ` Rohit Kumar
2018-12-13 18:10 ` Applied "ASoC: core: Invoke pcm_new() for all DAI-link" to the asoc tree Mark Brown
2018-12-13 18:10   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.