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

[Cristian Ciocaltea]

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@xxxxxxxxxxxxx>
>>>> ---
>>>>    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 {