diff mbox series

[10/11] ASoC: amd: acp: Use correct DAI link ID for BT codec

Message ID 20231209205351.880797-11-cristian.ciocaltea@collabora.com (mailing list archive)
State Superseded
Headers show
Series Improve SOF support for Steam Deck OLED | expand

Commit Message

Cristian Ciocaltea Dec. 9, 2023, 8:53 p.m. UTC
Commit 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
creation for I2S BT instance") added I2S BT support in ACP common
machine driver, but using a wrong BT_BE_ID, i.e. 3 instead of 2:

[ 7.799659] snd_sof_amd_vangogh 0000:04:00.5: Firmware info: version 0:0:0-7863d
[ 7.803906] snd_sof_amd_vangogh 0000:04:00.5: Firmware: ABI 3:26:0 Kernel ABI 3:23:0
[ 7.872873] snd_sof_amd_vangogh 0000:04:00.5: Topology: ABI 3:26:0 Kernel ABI 3:23:0
[ 8.508218] sof_mach nau8821-max: ASoC: physical link acp-bt-codec (id 2) not exist
[ 8.513468] sof_mach nau8821-max: ASoC: topology: could not load header: -22
[ 8.518853] snd_sof_amd_vangogh 0000:04:00.5: error: tplg component load failed -22
[ 8.524049] snd_sof_amd_vangogh 0000:04:00.5: error: failed to load DSP topology -22
[ 8.529230] snd_sof_amd_vangogh 0000:04:00.5: ASoC: error at snd_soc_component_probe on 0000:04:00.5: -22
[ 8.534465] sof_mach nau8821-max: ASoC: failed to instantiate card -22
[ 8.539820] sof_mach nau8821-max: error -EINVAL: Failed to register card(sof-nau8821-max)
[ 8.545022] sof_mach: probe of nau8821-max failed with error -22

Move BT_BE_ID to the correct position in the enum.

Fixes: 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink creation for I2S BT instance")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 sound/soc/amd/acp/acp-mach.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Venkata Prasad Potturu Dec. 10, 2023, 3:24 a.m. UTC | #1
On 12/10/23 02:23, Cristian Ciocaltea wrote:
> Commit 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
> creation for I2S BT instance") added I2S BT support in ACP common
> machine driver, but using a wrong BT_BE_ID, i.e. 3 instead of 2:
>
> [ 7.799659] snd_sof_amd_vangogh 0000:04:00.5: Firmware info: version 0:0:0-7863d
> [ 7.803906] snd_sof_amd_vangogh 0000:04:00.5: Firmware: ABI 3:26:0 Kernel ABI 3:23:0
> [ 7.872873] snd_sof_amd_vangogh 0000:04:00.5: Topology: ABI 3:26:0 Kernel ABI 3:23:0
> [ 8.508218] sof_mach nau8821-max: ASoC: physical link acp-bt-codec (id 2) not exist
> [ 8.513468] sof_mach nau8821-max: ASoC: topology: could not load header: -22
> [ 8.518853] snd_sof_amd_vangogh 0000:04:00.5: error: tplg component load failed -22
> [ 8.524049] snd_sof_amd_vangogh 0000:04:00.5: error: failed to load DSP topology -22
> [ 8.529230] snd_sof_amd_vangogh 0000:04:00.5: ASoC: error at snd_soc_component_probe on 0000:04:00.5: -22
> [ 8.534465] sof_mach nau8821-max: ASoC: failed to instantiate card -22
> [ 8.539820] sof_mach nau8821-max: error -EINVAL: Failed to register card(sof-nau8821-max)
> [ 8.545022] sof_mach: probe of nau8821-max failed with error -22
>
> Move BT_BE_ID to the correct position in the enum.
>
> Fixes: 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink creation for I2S BT instance")
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>   sound/soc/amd/acp/acp-mach.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/amd/acp/acp-mach.h b/sound/soc/amd/acp/acp-mach.h
> index a48546d8d407..0c18ccd29305 100644
> --- a/sound/soc/amd/acp/acp-mach.h
> +++ b/sound/soc/amd/acp/acp-mach.h
> @@ -27,8 +27,8 @@
>   enum be_id {
>   	HEADSET_BE_ID = 0,
>   	AMP_BE_ID,
> -	DMIC_BE_ID,
>   	BT_BE_ID,
> +	DMIC_BE_ID,
This will break the other platforms as this same enum used in topology 
to create dailink.
>   };
>   
>   enum cpu_endpoints {
Cristian Ciocaltea Dec. 10, 2023, 9:06 a.m. UTC | #2
On 12/10/23 05:24, Venkata Prasad Potturu wrote:
> 
> On 12/10/23 02:23, Cristian Ciocaltea wrote:
>> Commit 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
>> creation for I2S BT instance") added I2S BT support in ACP common
>> machine driver, but using a wrong BT_BE_ID, i.e. 3 instead of 2:
>>
>> [ 7.799659] snd_sof_amd_vangogh 0000:04:00.5: Firmware info: version
>> 0:0:0-7863d
>> [ 7.803906] snd_sof_amd_vangogh 0000:04:00.5: Firmware: ABI 3:26:0
>> Kernel ABI 3:23:0
>> [ 7.872873] snd_sof_amd_vangogh 0000:04:00.5: Topology: ABI 3:26:0
>> Kernel ABI 3:23:0
>> [ 8.508218] sof_mach nau8821-max: ASoC: physical link acp-bt-codec (id
>> 2) not exist
>> [ 8.513468] sof_mach nau8821-max: ASoC: topology: could not load
>> header: -22
>> [ 8.518853] snd_sof_amd_vangogh 0000:04:00.5: error: tplg component
>> load failed -22
>> [ 8.524049] snd_sof_amd_vangogh 0000:04:00.5: error: failed to load
>> DSP topology -22
>> [ 8.529230] snd_sof_amd_vangogh 0000:04:00.5: ASoC: error at
>> snd_soc_component_probe on 0000:04:00.5: -22
>> [ 8.534465] sof_mach nau8821-max: ASoC: failed to instantiate card -22
>> [ 8.539820] sof_mach nau8821-max: error -EINVAL: Failed to register
>> card(sof-nau8821-max)
>> [ 8.545022] sof_mach: probe of nau8821-max failed with error -22
>>
>> Move BT_BE_ID to the correct position in the enum.
>>
>> Fixes: 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
>> creation for I2S BT instance")
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>   sound/soc/amd/acp/acp-mach.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/amd/acp/acp-mach.h b/sound/soc/amd/acp/acp-mach.h
>> index a48546d8d407..0c18ccd29305 100644
>> --- a/sound/soc/amd/acp/acp-mach.h
>> +++ b/sound/soc/amd/acp/acp-mach.h
>> @@ -27,8 +27,8 @@
>>   enum be_id {
>>       HEADSET_BE_ID = 0,
>>       AMP_BE_ID,
>> -    DMIC_BE_ID,
>>       BT_BE_ID,
>> +    DMIC_BE_ID,
> This will break the other platforms as this same enum used in topology
> to create dailink.

If I understand this correctly, there is no consistency across firmware
regarding the IDs used for DAI link identification.  What would be the
suggested solution in this case?

Thanks,
Cristian

>>   };
>>     enum cpu_endpoints {
Venkata Prasad Potturu Dec. 10, 2023, 10:05 a.m. UTC | #3
On 12/10/23 14:36, Cristian Ciocaltea wrote:
> On 12/10/23 05:24, Venkata Prasad Potturu wrote:
>> On 12/10/23 02:23, Cristian Ciocaltea wrote:
>>> Commit 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
>>> creation for I2S BT instance") added I2S BT support in ACP common
>>> machine driver, but using a wrong BT_BE_ID, i.e. 3 instead of 2:
>>>
>>> [ 7.799659] snd_sof_amd_vangogh 0000:04:00.5: Firmware info: version
>>> 0:0:0-7863d
>>> [ 7.803906] snd_sof_amd_vangogh 0000:04:00.5: Firmware: ABI 3:26:0
>>> Kernel ABI 3:23:0
>>> [ 7.872873] snd_sof_amd_vangogh 0000:04:00.5: Topology: ABI 3:26:0
>>> Kernel ABI 3:23:0
>>> [ 8.508218] sof_mach nau8821-max: ASoC: physical link acp-bt-codec (id
>>> 2) not exist
>>> [ 8.513468] sof_mach nau8821-max: ASoC: topology: could not load
>>> header: -22
>>> [ 8.518853] snd_sof_amd_vangogh 0000:04:00.5: error: tplg component
>>> load failed -22
>>> [ 8.524049] snd_sof_amd_vangogh 0000:04:00.5: error: failed to load
>>> DSP topology -22
>>> [ 8.529230] snd_sof_amd_vangogh 0000:04:00.5: ASoC: error at
>>> snd_soc_component_probe on 0000:04:00.5: -22
>>> [ 8.534465] sof_mach nau8821-max: ASoC: failed to instantiate card -22
>>> [ 8.539820] sof_mach nau8821-max: error -EINVAL: Failed to register
>>> card(sof-nau8821-max)
>>> [ 8.545022] sof_mach: probe of nau8821-max failed with error -22
>>>
>>> Move BT_BE_ID to the correct position in the enum.
>>>
>>> Fixes: 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
>>> creation for I2S BT instance")
>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>> ---
>>>    sound/soc/amd/acp/acp-mach.h | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/amd/acp/acp-mach.h b/sound/soc/amd/acp/acp-mach.h
>>> index a48546d8d407..0c18ccd29305 100644
>>> --- a/sound/soc/amd/acp/acp-mach.h
>>> +++ b/sound/soc/amd/acp/acp-mach.h
>>> @@ -27,8 +27,8 @@
>>>    enum be_id {
>>>        HEADSET_BE_ID = 0,
>>>        AMP_BE_ID,
>>> -    DMIC_BE_ID,
>>>        BT_BE_ID,
>>> +    DMIC_BE_ID,
>> This will break the other platforms as this same enum used in topology
>> to create dailink.
> If I understand this correctly, there is no consistency across firmware
> regarding the IDs used for DAI link identification.  What would be the
> suggested solution in this case?

These id values should be same in machine driver and topology file, then 
only dailink can create without an error.
Always new be_id should add at the end only.

In this case BT_BE_ID should be at the end.

   enum be_id {
       HEADSET_BE_ID = 0,
       AMP_BE_ID,
       DMIC_BE_ID,
       BT_BE_ID,
   }




>
> Thanks,
> Cristian
>
>>>    };
>>>      enum cpu_endpoints {
Cristian Ciocaltea Dec. 10, 2023, 10:32 a.m. UTC | #4
On 12/10/23 12:05, Venkata Prasad Potturu wrote:
> 
> On 12/10/23 14:36, Cristian Ciocaltea wrote:
>> On 12/10/23 05:24, Venkata Prasad Potturu wrote:
>>> On 12/10/23 02:23, Cristian Ciocaltea wrote:
>>>> Commit 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
>>>> creation for I2S BT instance") added I2S BT support in ACP common
>>>> machine driver, but using a wrong BT_BE_ID, i.e. 3 instead of 2:
>>>>
>>>> [ 7.799659] snd_sof_amd_vangogh 0000:04:00.5: Firmware info: version
>>>> 0:0:0-7863d
>>>> [ 7.803906] snd_sof_amd_vangogh 0000:04:00.5: Firmware: ABI 3:26:0
>>>> Kernel ABI 3:23:0
>>>> [ 7.872873] snd_sof_amd_vangogh 0000:04:00.5: Topology: ABI 3:26:0
>>>> Kernel ABI 3:23:0
>>>> [ 8.508218] sof_mach nau8821-max: ASoC: physical link acp-bt-codec (id
>>>> 2) not exist
>>>> [ 8.513468] sof_mach nau8821-max: ASoC: topology: could not load
>>>> header: -22
>>>> [ 8.518853] snd_sof_amd_vangogh 0000:04:00.5: error: tplg component
>>>> load failed -22
>>>> [ 8.524049] snd_sof_amd_vangogh 0000:04:00.5: error: failed to load
>>>> DSP topology -22
>>>> [ 8.529230] snd_sof_amd_vangogh 0000:04:00.5: ASoC: error at
>>>> snd_soc_component_probe on 0000:04:00.5: -22
>>>> [ 8.534465] sof_mach nau8821-max: ASoC: failed to instantiate card -22
>>>> [ 8.539820] sof_mach nau8821-max: error -EINVAL: Failed to register
>>>> card(sof-nau8821-max)
>>>> [ 8.545022] sof_mach: probe of nau8821-max failed with error -22
>>>>
>>>> Move BT_BE_ID to the correct position in the enum.
>>>>
>>>> Fixes: 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
>>>> creation for I2S BT instance")
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>> ---
>>>>    sound/soc/amd/acp/acp-mach.h | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/sound/soc/amd/acp/acp-mach.h
>>>> b/sound/soc/amd/acp/acp-mach.h
>>>> index a48546d8d407..0c18ccd29305 100644
>>>> --- a/sound/soc/amd/acp/acp-mach.h
>>>> +++ b/sound/soc/amd/acp/acp-mach.h
>>>> @@ -27,8 +27,8 @@
>>>>    enum be_id {
>>>>        HEADSET_BE_ID = 0,
>>>>        AMP_BE_ID,
>>>> -    DMIC_BE_ID,
>>>>        BT_BE_ID,
>>>> +    DMIC_BE_ID,
>>> This will break the other platforms as this same enum used in topology
>>> to create dailink.
>> If I understand this correctly, there is no consistency across firmware
>> regarding the IDs used for DAI link identification.  What would be the
>> suggested solution in this case?
> 
> These id values should be same in machine driver and topology file, then
> only dailink can create without an error.

Yes, my point was that some topology files seem to require different IDs
for the same DAI link types.  In this case the topology expects ID 2 for
BT, but other topologies would interpret that as DMIC.

> Always new be_id should add at the end only.
> 
> In this case BT_BE_ID should be at the end.
> 
>   enum be_id {
>       HEADSET_BE_ID = 0,
>       AMP_BE_ID,
>       DMIC_BE_ID,
>       BT_BE_ID,
>   }

So you are basically stating the firmware is broken and needs an update
to use ID 3 for BT, and there is nothing we can do about it on driver's
side.  Is that correct?

> 
> 
>>
>> Thanks,
>> Cristian
>>
>>>>    };
>>>>      enum cpu_endpoints {
Venkata Prasad Potturu Dec. 11, 2023, 5:48 a.m. UTC | #5
On 12/10/23 16:02, Cristian Ciocaltea wrote:
> On 12/10/23 12:05, Venkata Prasad Potturu wrote:
>> On 12/10/23 14:36, Cristian Ciocaltea wrote:
>>> On 12/10/23 05:24, Venkata Prasad Potturu wrote:
>>>> On 12/10/23 02:23, Cristian Ciocaltea wrote:
>>>>> Commit 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
>>>>> creation for I2S BT instance") added I2S BT support in ACP common
>>>>> machine driver, but using a wrong BT_BE_ID, i.e. 3 instead of 2:
>>>>>
>>>>> [ 7.799659] snd_sof_amd_vangogh 0000:04:00.5: Firmware info: version
>>>>> 0:0:0-7863d
>>>>> [ 7.803906] snd_sof_amd_vangogh 0000:04:00.5: Firmware: ABI 3:26:0
>>>>> Kernel ABI 3:23:0
>>>>> [ 7.872873] snd_sof_amd_vangogh 0000:04:00.5: Topology: ABI 3:26:0
>>>>> Kernel ABI 3:23:0
>>>>> [ 8.508218] sof_mach nau8821-max: ASoC: physical link acp-bt-codec (id
>>>>> 2) not exist
>>>>> [ 8.513468] sof_mach nau8821-max: ASoC: topology: could not load
>>>>> header: -22
>>>>> [ 8.518853] snd_sof_amd_vangogh 0000:04:00.5: error: tplg component
>>>>> load failed -22
>>>>> [ 8.524049] snd_sof_amd_vangogh 0000:04:00.5: error: failed to load
>>>>> DSP topology -22
>>>>> [ 8.529230] snd_sof_amd_vangogh 0000:04:00.5: ASoC: error at
>>>>> snd_soc_component_probe on 0000:04:00.5: -22
>>>>> [ 8.534465] sof_mach nau8821-max: ASoC: failed to instantiate card -22
>>>>> [ 8.539820] sof_mach nau8821-max: error -EINVAL: Failed to register
>>>>> card(sof-nau8821-max)
>>>>> [ 8.545022] sof_mach: probe of nau8821-max failed with error -22
>>>>>
>>>>> Move BT_BE_ID to the correct position in the enum.
>>>>>
>>>>> Fixes: 671dd2ffbd8b ("ASoC: amd: acp: Add new cpu dai and dailink
>>>>> creation for I2S BT instance")
>>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>>> ---
>>>>>     sound/soc/amd/acp/acp-mach.h | 2 +-
>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/sound/soc/amd/acp/acp-mach.h
>>>>> b/sound/soc/amd/acp/acp-mach.h
>>>>> index a48546d8d407..0c18ccd29305 100644
>>>>> --- a/sound/soc/amd/acp/acp-mach.h
>>>>> +++ b/sound/soc/amd/acp/acp-mach.h
>>>>> @@ -27,8 +27,8 @@
>>>>>     enum be_id {
>>>>>         HEADSET_BE_ID = 0,
>>>>>         AMP_BE_ID,
>>>>> -    DMIC_BE_ID,
>>>>>         BT_BE_ID,
>>>>> +    DMIC_BE_ID,
>>>> This will break the other platforms as this same enum used in topology
>>>> to create dailink.
>>> If I understand this correctly, there is no consistency across firmware
>>> regarding the IDs used for DAI link identification.  What would be the
>>> suggested solution in this case?
>> These id values should be same in machine driver and topology file, then
>> only dailink can create without an error.
> Yes, my point was that some topology files seem to require different IDs
> for the same DAI link types.  In this case the topology expects ID 2 for
> BT, but other topologies would interpret that as DMIC.
>
>> Always new be_id should add at the end only.
>>
>> In this case BT_BE_ID should be at the end.
>>
>>    enum be_id {
>>        HEADSET_BE_ID = 0,
>>        AMP_BE_ID,
>>        DMIC_BE_ID,
>>        BT_BE_ID,
>>    }
> So you are basically stating the firmware is broken and needs an update
> to use ID 3 for BT, and there is nothing we can do about it on driver's
> side.  Is that correct?
Yes, id 3 should be used for BT_BE_ID in topology file.
>
>>
>>> Thanks,
>>> Cristian
>>>
>>>>>     };
>>>>>       enum cpu_endpoints {
diff mbox series

Patch

diff --git a/sound/soc/amd/acp/acp-mach.h b/sound/soc/amd/acp/acp-mach.h
index a48546d8d407..0c18ccd29305 100644
--- a/sound/soc/amd/acp/acp-mach.h
+++ b/sound/soc/amd/acp/acp-mach.h
@@ -27,8 +27,8 @@ 
 enum be_id {
 	HEADSET_BE_ID = 0,
 	AMP_BE_ID,
-	DMIC_BE_ID,
 	BT_BE_ID,
+	DMIC_BE_ID,
 };
 
 enum cpu_endpoints {