thesofproject / rimage

DSP firmware image creation and signing tool
Other
7 stars 62 forks source link

kpb: update UUID #154

Closed fkwasowi closed 1 year ago

fkwasowi commented 1 year ago

Change of uuid regarding Windows compatibility

kv2019i commented 1 year ago

@aiChaoSONG This is good to go, we are not using kpb yet in MTL topologies, so I don't see this breaking anything.

plbossart commented 1 year ago

@kv2019i Don't we use KPB in the Wake-on-Voice cases? We have such implementations on TGL and ADL.

The rule of UUID is that they NEVER change....

plbossart commented 1 year ago

And the old value was used indeed, see in tools/topology/topology1

m4/kpbm.m4:DECLARE_SOF_RT_UUID("kpb", kpb_uuid, 0xd8218443, 0x5ff3, 0x4a4c,

Meh. @lgirdwood this needs to be reworked, we can't do stuff like that.

fkwasowi commented 1 year ago

@plbossart Change does not affect TGL and ADL and IPC3 topology

plbossart commented 1 year ago

@fkwasowi what makes you say that?

m4/kpbm.m4:dnl Define macro for Key Phrase Buffer Manager(kpbm) widget
m4/kpbm.m4:DECLARE_SOF_RT_UUID("kpb", kpb_uuid, 0xd8218443, 0x5ff3, 0x4a4c,
m4/kpbm.m4:`            SOF_TKN_COMP_UUID'              STR(kpb_uuid)
sof-adl-nau8825.m4:# PCM100 <---- kpb <---- DMIC16K (dmic 16k capture)
sof-tgl-max98357a-rt5682.m4:# PCM100 <---- kpb <---- DMIC16K (dmic 16k capture)
sof-tgl-max98373-rt5682.m4:# PCM100 <---- kpb <---- DMIC16K (dmic 16k capture)
sof-tgl-sdw-max98373-rt5682.m4:# PCM12 <---- kpb <---- DMIC16k  (dmic 16k capture)
sof/pipe-kfbm-capture.m4:include(`kpbm.m4')
sof/pipe-kfbm-capture.m4:# kpbm initial parameters, aligned with struct sof_kpb_config
sof/pipe-kfbm-no-lp-capture.m4:include(`kpbm.m4')
sof/pipe-kfbm-no-lp-capture.m4:# kpbm initial parameters, aligned with struct sof_kpb_config
sof/pipe-vol-kfbm-capture.m4:include(`kpbm.m4')
sof/pipe-vol-kfbm-capture.m4:# kpbm initial parameters, aligned with struct sof_kpb_config

I see multiple TGL/ADL topologies using this UUID.

Please prove me wrong.

Edit: but wait there's more!

sof-adl-nau8825.m4:# define kfbm with volume
sof-adl-nau8825.m4:define(KFBM_TYPE, `vol-kfbm')
sof-cml-rt5682-kwd.m4:PIPELINE_PCM_DAI_ADD(sof/pipe-kfbm-capture.m4,
sof-imx8-wm8960-kwd.m4:PIPELINE_PCM_ADD(sof/pipe-kfbm-no-lp-capture.m4,
sof-imx8mp-wm8960-kwd.m4:PIPELINE_PCM_ADD(sof/pipe-kfbm-no-lp-capture.m4,
sof-tgl-max98357a-rt5682.m4:# define kfbm with volume
sof-tgl-max98357a-rt5682.m4:define(KFBM_TYPE, `vol-kfbm')
sof-tgl-max98373-rt5682.m4:# define kfbm with volume
sof-tgl-max98373-rt5682.m4:define(KFBM_TYPE, `vol-kfbm')
sof-tgl-sdw-max98373-rt5682.m4:# define kfbm with volume
sof-tgl-sdw-max98373-rt5682.m4:define(KFBM_TYPE, `vol-kfbm')
sof/pipe-kfbm-capture.m4:P_GRAPH(pipe-kfbm-capture, PIPELINE_ID,
sof/pipe-kfbm-no-lp-capture.m4:P_GRAPH(pipe-kfbm-no-lp-capture, PIPELINE_ID,
sof/pipe-vol-kfbm-capture.m4:P_GRAPH(pipe-vol-kfbm-capture, PIPELINE_ID,
plbossart commented 1 year ago

and the worry is on the IMX side, not sure if @dbaluta can confirm if the KPB component is used by NXP.

pblaszko commented 1 year ago

@fkwasowi what makes you say that?

@plbossart we totally agree UUID should be unchanged, but this is an exception - requirement to support Windows OED (transparent change from OED POV) was added on later phase of SOF project. UUID for KPB was defined in closed-source long time before SOF was initiated. Now we need to compatibility.

By not affecting ADL and TGL, @fkwasowi meant this change is done only for MTL IPC4 builds (and will be propagated for future projects). Older projects keep old UUID for KPB.

plbossart commented 1 year ago

I appreciate the explanation @pblaszko. There's still a risk that someone might try to test a MTL topology on TGL IPC4, which is still the primary test vehicle as of today.