samples: convert smart amp test module to not expect the blob during init with IPC4 #7042

ujfalusi · 2023-02-03T21:54:35Z

Hi,

the module initialization for IPC4 does not contain binary (configuration) blob, only base_cfg and optional bae_cfg_ext.
The configuration blob is sent as a large_config message separately.

lgirdwood · 2023-02-06T13:26:53Z

@wszypelt @lrudyX good to go here ? Dont think this flow is tested in CI yet ?

wszypelt · 2023-02-06T14:01:25Z

@lgirdwood 3 tests on KD was failed, I think it's not good to merge

ujfalusi · 2023-02-06T14:11:16Z

@lgirdwood 3 tests on KD was failed, I think it's not good to merge

To be fair, most of the PRs are failing in a same way, if I read the logs right, no?

wszypelt · 2023-02-06T14:16:29Z

@ujfalusi a lot of PR has a problem with one test and right now I'm trying to understand why, but here the whole module fails.
so I'm not sure if it's the same problem as others ;/

wszypelt · 2023-02-06T14:17:42Z

@ujfalusi @lgirdwood I'm taking FW and will check these tests on another machine, in a moment I will write how it went.

ujfalusi · 2023-02-06T14:21:13Z

@ujfalusi @lgirdwood I'm taking FW and will check these tests on another machine, in a moment I will write how it went.

Right, so, with this patch the configuration blob is not expected within MOD_INIT_INSTANCE message, a separate LARGE_CONFIG should be use to send the configuration (param_id = 2) and the blob itself should be the exact same payload as it was with IPC3, So in the abi header you need to change the magic to 0x34464F53, set they type to 2 and send that with Linux.

ujfalusi · 2023-02-06T14:23:15Z

I was not aware that we had any tests for detect_test at all, like we did not have for the smart_amp_test neither.

mwasko · 2023-02-06T15:18:06Z

@iganakov please review proposed changes

wszypelt · 2023-02-06T15:30:57Z

@ujfalusi @lgirdwood
As for the current tests
Unfortunately, all three KD tests fail on this PR. I checked on the same settings, compared it to FW from main. those tests pass there, I also confirmed that the 16bit16bit test is unstable, so I will exclude it from the scope and rerun other PR. The other two KD tests remain.

mszleszy · 2023-02-06T15:58:13Z

@lgirdwood
I'll update tests asap but I need to clarify few things.
KdCfgBlobHeader is being removed from init instance message and it should be send in separate LargeConfigSet.
Should I use IPC4_DETECT_TEST_SET_CONFIG param?
If so the message will contain base config + KdCfgBlobHeader - do we want to keep it this way, change IPC4_DETECT_TEST_SET_CONFIG requirements or use different kd param ?

softwarecki · 2023-02-06T15:17:18Z

src/include/sof/samples/audio/smart_amp_test.h

ipc4 repeated

Thanks for spotting it, I will use sof_smart_amp_ipc4_config

iganakov

Looks good to me except unused field load_memory_size in sof_detect_test_config. Probably, should be removed.

aiChaoSONG · 2023-02-07T02:22:02Z

src/samples/audio/smart_amp_test.c

We have the macros defined, use them here?

#define SMART_AMP_NUM_IN_PINS 2 #define SMART_AMP_NUM_OUT_PINS 1

OK, this is why I added the defines ;)

aiChaoSONG · 2023-02-07T07:35:03Z

The smart amp change in this PR is verified with #6996 and thesofproject/linux#4160, need more time to test the WoV change.

ujfalusi · 2023-02-07T07:48:39Z

I will drop the detect_test change from the PR for now and open a new one for it to get the smart amp feature enabled faster.

During the component create (module init) we are not receiving the blob containing the sof_smart_amp_config, only the base_cfg and base_cfg_ext is received. The sof_smart_amp_config content will be sent via large_config_set at a later time. Introduce a new IPC4 only struct to store the base and pin configurations and handle the IPC3 / IPC4 init differently. The blob received via large_config_set must be the same format as the blob with IPC3. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ujfalusi · 2023-02-07T08:02:31Z

Changes since v1:

drop the detect_test patch
use the defines in place for the number of pins number
remove duplicated 'ipc4' from the IPC4 config struct name

ujfalusi · 2023-02-07T08:11:35Z

@lgirdwood I'll update tests asap but I need to clarify few things. KdCfgBlobHeader is being removed from init instance message and it should be send in separate LargeConfigSet. Should I use IPC4_DETECT_TEST_SET_CONFIG param? If so the message will contain base config + KdCfgBlobHeader - do we want to keep it this way, change IPC4_DETECT_TEST_SET_CONFIG requirements or use different kd param ?

@mszleszy, the detect_test at init will receive the struct ipc4_base_module_cfg only. The configuration is going to arrive via set_large_config in a form of (an unmodified, IPC3 compatible) struct sof_detect_test_config and the param ID for this config_large is IPC4_DETECT_TEST_SET_CONFIG (2).

Fwiw, this is how the blob will look like in topology:

Object.Base.data."detect_test_init" {
	bytes "
	0x53,0x4f,0x46,0x34,0x02,0x00,0x00,0x00,	# magic + param_id
	0x20,0x00,0x00,0x00,0x00,0x00,0x00,0x00,	# size + abi version
	0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,	# unused
	0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,	# unused 

	0x20,0x00,0x00,0x00,0x00,0x00,0x00,0x00,	# the blob itself (struct sof_detect_test_config)
	0x00,0x00,0x00,0x00,0x34,0x08,0x00,0x00,
	0x03,0x00,0x18,0x00,0x00,0x00,0x00,0x00,
	0xD0,0x07,0x00,0x00,0x00,0x00,0x00,0x00"
)

ujfalusi · 2023-02-07T08:29:38Z

Looks good to me except unused field load_memory_size in sof_detect_test_config. Probably, should be removed.

@iganakov, I have intentionally kept the original struct sof_detect_test_config intact in order to be able to re-use the same blob from IPC3 in IPC4 with only needing to update the sof_abi_hdr in topology and in user space.
We can not drop members from the struct from the middle without breaking existing blobs.

lgirdwood · 2023-02-07T16:34:46Z

@iganakov @softwarecki updates OK for you ?

ujfalusi requested review from aiChaoSONG, juimonen, kv2019i, ranj063 and singalsu February 3, 2023 21:54

ujfalusi requested review from dbaluta, lbetlej, lgirdwood, mmaka1 and plbossart as code owners February 3, 2023 21:54

kv2019i approved these changes Feb 6, 2023

View reviewed changes

kv2019i requested a review from libinyang February 6, 2023 09:50

lgirdwood approved these changes Feb 6, 2023

View reviewed changes

mwasko requested a review from iganakov February 6, 2023 15:18

softwarecki requested changes Feb 6, 2023

View reviewed changes

iganakov requested changes Feb 6, 2023

View reviewed changes

aiChaoSONG reviewed Feb 7, 2023

View reviewed changes

ujfalusi force-pushed the peter/pr/test_module_updates_no_blob_at_init_01 branch from 9ce454b to 9adad6f Compare February 7, 2023 08:01

ujfalusi changed the title ~~samples: convert smart amp and wov test modules to not expect the blob during init with IPC4~~ samples: convert smart amp test module to not expect the blob during init with IPC4 Feb 7, 2023

ranj063 approved these changes Feb 7, 2023

View reviewed changes

aiChaoSONG approved these changes Feb 8, 2023

View reviewed changes

libinyang approved these changes Feb 8, 2023

View reviewed changes

softwarecki approved these changes Feb 8, 2023

View reviewed changes

iganakov approved these changes Feb 8, 2023

View reviewed changes

lgirdwood merged commit bcc1407 into thesofproject:main Feb 8, 2023

ujfalusi deleted the peter/pr/test_module_updates_no_blob_at_init_01 branch August 17, 2023 06:04

samples: convert smart amp test module to not expect the blob during init with IPC4 #7042

samples: convert smart amp test module to not expect the blob during init with IPC4 #7042

Uh oh!

Conversation

ujfalusi commented Feb 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood commented Feb 6, 2023

Uh oh!

wszypelt commented Feb 6, 2023

Uh oh!

ujfalusi commented Feb 6, 2023

Uh oh!

wszypelt commented Feb 6, 2023

Uh oh!

wszypelt commented Feb 6, 2023

Uh oh!

ujfalusi commented Feb 6, 2023

Uh oh!

ujfalusi commented Feb 6, 2023

Uh oh!

mwasko commented Feb 6, 2023

Uh oh!

wszypelt commented Feb 6, 2023

Uh oh!

mszleszy commented Feb 6, 2023

Uh oh!

softwarecki Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

ujfalusi Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

iganakov left a comment

Choose a reason for hiding this comment

Uh oh!

aiChaoSONG Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

ujfalusi Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

aiChaoSONG commented Feb 7, 2023

Uh oh!

ujfalusi commented Feb 7, 2023

Uh oh!

ujfalusi commented Feb 7, 2023

Uh oh!

ujfalusi commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ujfalusi commented Feb 7, 2023

Uh oh!

lgirdwood commented Feb 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

ujfalusi commented Feb 3, 2023 •

edited

Loading

ujfalusi commented Feb 7, 2023 •

edited

Loading