Re: [Sound-open-firmware] [PATCH 3/4] ASoC: SOF: Adding amd HS functionality to the sof core

[AngeloGioacchino Del Regno]

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