Re: [Sound-open-firmware] [PATCH 3/4] ASoC: SOF: Adding amd HS functionality to the sof core
Il 13/09/22 16:43, V sujith kumar Reddy ha scritto:
Add I2S HS control instance to the sof core. This will help the amd topology to use the I2S HS Dai.
Signed-off-by: V sujith kumar Reddy Vsujithkumar.Reddy@amd.com
Hello,
Since this patch was merged, SoundOpenFirmware stopped working on MediaTek MT8195, as it fails on DAI component creation (firmware side)... check below...
include/sound/sof/dai.h | 2 ++ sound/soc/sof/ipc3-pcm.c | 9 +++++++++ sound/soc/sof/ipc3-topology.c | 33 +++++++++++++++++++++++++++++++++ sound/soc/sof/topology.c | 1 + 4 files changed, 45 insertions(+)
diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h index 21d98f31a9ca..83fd81c82e4c 100644 --- a/include/sound/sof/dai.h +++ b/include/sound/sof/dai.h @@ -84,6 +84,7 @@ enum sof_ipc_dai_type { SOF_DAI_AMD_BT, /**< AMD ACP BT*/ SOF_DAI_AMD_SP, /**< AMD ACP SP */ SOF_DAI_AMD_DMIC, /**< AMD ACP DMIC */
- SOF_DAI_AMD_HS, /**< Amd HS */ SOF_DAI_MEDIATEK_AFE, /**< Mediatek AFE */
Adding SOF_DAI_AMD_HS before SOF_DAI_MEDIATEK_AFE desynced this enumeration so the DAI type is now 11 and not 10 anymore, leading to a failure in firmware at IPC3 helper function `dai_get()`, as when `dai_find_type()` is called, the DAI type that the firmware expects doesn't match with the one that gets sent in the request message from the kernel.
As a local test, I tried moving SOF_DAI_AMD_HS after SOF_DAI_MEDIATEK_AFE and this has restored full functionality on my MT8195 platform (Tomato Chromebook).
If SOF is supposed to guarantee backwards compatibility (and I believe it is), this commit breaks that.
I would be tempted to send a commit that moves SOF_DAI_AMD_HS to the end, but that would break the already compiled firmware for AMD platforms, so I am not sure how to proceed.
So... how can we solve that? Any ideas?
P.S.: Sharing some logs at the end of this email, just for completeness.
Best regards, Angelo
.. Relevant firmware and kernel trace/debug log lines ..
Log from Xtensa DSP: [ 828266.737921] ( 3.125000) c0 ipc src/ipc/ipc3/handler.c:1579 INFO ipc: new cmd 0x30010000 [ 828273.404587] ( 6.666667) c0 component src/ipc/ipc3/helper.c:296 INFO comp new dai <c2b00d27-ffbc-4150-a51a-245c79c5e54b> type 2 id 4.22 [ 828284.342087] ( 10.937500) c0 dai src/audio/dai.c:177 ERROR dai_new(): dai_get() failed to create DAI. [ 828291.321253] ( 6.979167) c0 dai src/ipc/ipc3/helper.c:303 ERROR comp_new(): unable to create the new component [ 828295.383753] ( 4.062500) c0 ipc src/ipc/ipc3/helper.c:624 ERROR ipc_comp_new(): component cd = NULL [ 828299.654586] ( 4.270833) c0 ipc src/ipc/ipc3/handler.c:1248 ERROR ipc: pipe 4 comp 22 creation failed -22
Kernel log: [ 15.011677] sof-audio-of-mt8195 10803000.dsp: request_firmware mediatek/sof/sof-mt8195.ri successful ............... [ 15.021452] sof-audio-of-mt8195 10803000.dsp: Firmware info: version 2:0:0-df141 [ 15.039661] sof-audio-of-mt8195 10803000.dsp: Firmware: ABI 3:21:0 Kernel ABI 3:23:0 [ 15.039663] sof-audio-of-mt8195 10803000.dsp: found sof_ext_man header type 2 size 0x70 [ 15.039665] sof-audio-of-mt8195 10803000.dsp: Firmware info: used compiler XCC 12:0:8 <RI-2019.1-linux> used optimization flags -O2
...........
[ 15.107660] sof-audio-of-mt8195 10803000.dsp: Firmware: DBG_ABI 5:3:0 [ 15.292019] sof-audio-of-mt8195 10803000.dsp: booting DSP firmware [ 15.292025] sof-audio-of-mt8195 10803000.dsp: HIFIxDSP boot from base : 0x40000000 [ 15.297257] sof-audio-of-mt8195 10803000.dsp: ipc rx: 0x70000000: FW_READY [ 15.363305] sof-audio-of-mt8195 10803000.dsp: DSP is ready 0x70000000 offset 0x800000 [ 15.363319] sof-audio-of-mt8195 10803000.dsp: Firmware info: version 2:0:0-df141 [ 15.383651] sof-audio-of-mt8195 10803000.dsp: Firmware: ABI 3:21:0 Kernel ABI 3:23:0
............
[ 16.336460] sof-audio-of-mt8195 10803000.dsp: loaded host PCM16P [ 16.336461] sof-audio-of-mt8195 10803000.dsp: config: periods snk 2 src 0 fmt 0 [ 16.336466] sof-audio-of-mt8195 10803000.dsp: ipc tx: 0x30100000: GLB_TPLG_MSG: PIPE_NEW [ 16.336603] sof-audio-of-mt8195 10803000.dsp: widget PIPELINE.4.AFE3.IN setup complete [ 16.336607] sof-audio-of-mt8195 10803000.dsp: ipc tx: 0x30010000: GLB_TPLG_MSG: COMP_NEW [ 16.336663] sof-audio-of-mt8195 10803000.dsp: ipc tx error for 0x30010000 (msg/reply size: 96/20): -22 [ 16.336665] sof-audio-of-mt8195 10803000.dsp: Failed to setup widget AFE3.IN [ 16.336670] sof-audio-of-mt8195 10803000.dsp: ipc tx: 0x30110000: GLB_TPLG_MSG: PIPE_FREE [ 16.336778] sof-audio-of-mt8195 10803000.dsp: widget PIPELINE.4.AFE3.IN freed [ 16.336887] sof-audio-of-mt8195 10803000.dsp: error: tplg component load failed -22 [ 16.336899] sof-audio-of-mt8195 10803000.dsp: error: failed to load DSP topology -22 [ 16.336900] sof-audio-of-mt8195 10803000.dsp: ASoC: error at snd_soc_component_probe on 10803000.dsp: -22 [ 16.336983] mt8195_mt6359 mt8195-sound: ASoC: failed to instantiate card -22 [ 16.340339] mt8195_mt6359: probe of mt8195-sound failed with error -22
diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h index 21d98f31a9ca..83fd81c82e4c 100644 --- a/include/sound/sof/dai.h +++ b/include/sound/sof/dai.h @@ -84,6 +84,7 @@ enum sof_ipc_dai_type { SOF_DAI_AMD_BT, /**< AMD ACP BT*/ SOF_DAI_AMD_SP, /**< AMD ACP SP */ SOF_DAI_AMD_DMIC, /**< AMD ACP DMIC */ + SOF_DAI_AMD_HS, /**< Amd HS */ SOF_DAI_MEDIATEK_AFE, /**< Mediatek AFE */
Adding SOF_DAI_AMD_HS before SOF_DAI_MEDIATEK_AFE desynced this enumeration so the DAI type is now 11 and not 10 anymore, leading to a failure in firmware at IPC3 helper function `dai_get()`, as when `dai_find_type()` is called, the DAI type that the firmware expects doesn't match with the one that gets sent in the request message from the kernel.
As a local test, I tried moving SOF_DAI_AMD_HS after SOF_DAI_MEDIATEK_AFE and this has restored full functionality on my MT8195 platform (Tomato Chromebook).
If SOF is supposed to guarantee backwards compatibility (and I believe it is), this commit breaks that.
I would be tempted to send a commit that moves SOF_DAI_AMD_HS to the end, but that would break the already compiled firmware for AMD platforms, so I am not sure how to proceed.
D'oh. Yes this breaks backwards-compatibility and this is a clear mistake. I think your suggestion to add the AMD_HS at the end is the only practical solution indeed - this would need to be done for both kernel and SOF version of dai.h.
Il 16/11/22 16:04, Pierre-Louis Bossart ha scritto:
diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h index 21d98f31a9ca..83fd81c82e4c 100644 --- a/include/sound/sof/dai.h +++ b/include/sound/sof/dai.h @@ -84,6 +84,7 @@ enum sof_ipc_dai_type { SOF_DAI_AMD_BT, /**< AMD ACP BT*/ SOF_DAI_AMD_SP, /**< AMD ACP SP */ SOF_DAI_AMD_DMIC, /**< AMD ACP DMIC */ + SOF_DAI_AMD_HS, /**< Amd HS */ SOF_DAI_MEDIATEK_AFE, /**< Mediatek AFE */
Adding SOF_DAI_AMD_HS before SOF_DAI_MEDIATEK_AFE desynced this enumeration so the DAI type is now 11 and not 10 anymore, leading to a failure in firmware at IPC3 helper function `dai_get()`, as when `dai_find_type()` is called, the DAI type that the firmware expects doesn't match with the one that gets sent in the request message from the kernel.
As a local test, I tried moving SOF_DAI_AMD_HS after SOF_DAI_MEDIATEK_AFE and this has restored full functionality on my MT8195 platform (Tomato Chromebook).
If SOF is supposed to guarantee backwards compatibility (and I believe it is), this commit breaks that.
I would be tempted to send a commit that moves SOF_DAI_AMD_HS to the end, but that would break the already compiled firmware for AMD platforms, so I am not sure how to proceed.
D'oh. Yes this breaks backwards-compatibility and this is a clear mistake. I think your suggestion to add the AMD_HS at the end is the only practical solution indeed - this would need to be done for both kernel and SOF version of dai.h.
Okay, I will send a commit tomorrow :-)
Thanks, Angelo
On 11/16/22 10:33, AngeloGioacchino Del Regno wrote:
Il 16/11/22 16:04, Pierre-Louis Bossart ha scritto:
diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h index 21d98f31a9ca..83fd81c82e4c 100644 --- a/include/sound/sof/dai.h +++ b/include/sound/sof/dai.h @@ -84,6 +84,7 @@ enum sof_ipc_dai_type { SOF_DAI_AMD_BT, /**< AMD ACP BT*/ SOF_DAI_AMD_SP, /**< AMD ACP SP */ SOF_DAI_AMD_DMIC, /**< AMD ACP DMIC */ + SOF_DAI_AMD_HS, /**< Amd HS */ SOF_DAI_MEDIATEK_AFE, /**< Mediatek AFE */
Adding SOF_DAI_AMD_HS before SOF_DAI_MEDIATEK_AFE desynced this enumeration so the DAI type is now 11 and not 10 anymore, leading to a failure in firmware at IPC3 helper function `dai_get()`, as when `dai_find_type()` is called, the DAI type that the firmware expects doesn't match with the one that gets sent in the request message from the kernel.
As a local test, I tried moving SOF_DAI_AMD_HS after SOF_DAI_MEDIATEK_AFE and this has restored full functionality on my MT8195 platform (Tomato Chromebook).
If SOF is supposed to guarantee backwards compatibility (and I believe it is), this commit breaks that.
I would be tempted to send a commit that moves SOF_DAI_AMD_HS to the end, but that would break the already compiled firmware for AMD platforms, so I am not sure how to proceed.
D'oh. Yes this breaks backwards-compatibility and this is a clear mistake. I think your suggestion to add the AMD_HS at the end is the only practical solution indeed - this would need to be done for both kernel and SOF version of dai.h.
Okay, I will send a commit tomorrow :-)
I sent those two GitHub pull requests already:
https://github.com/thesofproject/linux/pull/4017 https://github.com/thesofproject/sof/pull/6616
Il 16/11/22 17:55, Pierre-Louis Bossart ha scritto:
On 11/16/22 10:33, AngeloGioacchino Del Regno wrote:
Il 16/11/22 16:04, Pierre-Louis Bossart ha scritto:
diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h index 21d98f31a9ca..83fd81c82e4c 100644 --- a/include/sound/sof/dai.h +++ b/include/sound/sof/dai.h @@ -84,6 +84,7 @@ enum sof_ipc_dai_type { SOF_DAI_AMD_BT, /**< AMD ACP BT*/ SOF_DAI_AMD_SP, /**< AMD ACP SP */ SOF_DAI_AMD_DMIC, /**< AMD ACP DMIC */ + SOF_DAI_AMD_HS, /**< Amd HS */ SOF_DAI_MEDIATEK_AFE, /**< Mediatek AFE */
Adding SOF_DAI_AMD_HS before SOF_DAI_MEDIATEK_AFE desynced this enumeration so the DAI type is now 11 and not 10 anymore, leading to a failure in firmware at IPC3 helper function `dai_get()`, as when `dai_find_type()` is called, the DAI type that the firmware expects doesn't match with the one that gets sent in the request message from the kernel.
As a local test, I tried moving SOF_DAI_AMD_HS after SOF_DAI_MEDIATEK_AFE and this has restored full functionality on my MT8195 platform (Tomato Chromebook).
If SOF is supposed to guarantee backwards compatibility (and I believe it is), this commit breaks that.
I would be tempted to send a commit that moves SOF_DAI_AMD_HS to the end, but that would break the already compiled firmware for AMD platforms, so I am not sure how to proceed.
D'oh. Yes this breaks backwards-compatibility and this is a clear mistake. I think your suggestion to add the AMD_HS at the end is the only practical solution indeed - this would need to be done for both kernel and SOF version of dai.h.
Okay, I will send a commit tomorrow :-)
I sent those two GitHub pull requests already:
https://github.com/thesofproject/linux/pull/4017 https://github.com/thesofproject/sof/pull/6616
Thanks for the fast action.
Regards, Angelo
participants (2)
-
AngeloGioacchino Del Regno -
Pierre-Louis Bossart