Skip to content

Conversation

@ktrzcinx
Copy link
Member

@ktrzcinx ktrzcinx commented Jun 2, 2020

A few UUID entries, related with components from topology,
should be loaded into SRAM to allow runtime comparison with
value in IPC message during component creation. UUID of the
components whose are not created by IPC message are not needed
in runtime, so there should not be there to reduce memory
consumtion.

Signed-off-by: Karol Trzcinski karolx.trzcinski@linux.intel.com

It's a part of #2920
@keyonjie please check if I choose 'RT` UUID for all needed components.

@paulstelian97
Copy link
Collaborator

We'll wait until we're sure this won't cause any Out Of Memory issues in DSP SRAM.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep the DECLARE_SOF_UUID()/SOF_UUID() for logging-only case unchanged. And introduce DECLARE_SOF_RT_UUID()/SOF_RT_UUID() for run-time identification.

@ktrzcinx
Copy link
Member Author

ktrzcinx commented Jun 3, 2020

From CI:

0.13s$ ./scripts/host-testbench.sh
==========================================================
test volume with ./volume_run.sh 16 16 48000 zeros_in.raw volume_out.raw
volume test failed!

@singalsu maybe you have a suggestion what is going on ?

@singalsu
Copy link
Collaborator

singalsu commented Jun 3, 2020

From CI:

0.13s$ ./scripts/host-testbench.sh
==========================================================
test volume with ./volume_run.sh 16 16 48000 zeros_in.raw volume_out.raw
volume test failed!

@singalsu maybe you have a suggestion what is going on ?

OK, I'll see what causes that.

@singalsu
Copy link
Collaborator

singalsu commented Jun 3, 2020

@ktrzcinx It segfaults to this line

0x00007ffff7e4e8bd in comp_new (comp=comp@entry=0x7fffffffd4a0) at /home/singalsu/work/current/sof/sof/src/audio/component.c:79

tr_info(&comp_tr, "comp new %s type %d id %d.%d", drv->tctx->uuid_p, comp->type, comp->pipeline_id, comp->id);
drv->tctx is NULL

Where is it set normally? Probably when updating this it has been forgotten to add to testbench runtime too. The trace output in testbench has been somewhat broken for a while already due to left without updates but this is fist time that it segfaults.

ktrzcinx added 3 commits June 3, 2020 16:13
Each component driver should have assigned UUID value and trace
contecxt, to allow generic component creation routine usage.
It's espiecially important for trace context, because it's
accessed by pointer so dereferencing NULL pointer will lead to
segmentation fault.

Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
UUID entries, related with components from topology,
should be loaded into SRAM to allow runtime comparison with
value in IPC message during component creation.
UUID of the components whose are not created by IPC message
are not needed in runtime, so there should not be there to
reduce memory consumtion - as it is now. It's untouched.

Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
It allows to comparison this full UUID value with value in IPC message,
what is important step to create new component from topology by their
UUID value.

Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
@ktrzcinx
Copy link
Member Author

ktrzcinx commented Jun 3, 2020

@singalsu Thanks, comp_file_host and comp_file_dai didn't have assigned any pointer to .tctx, which one I start to use in comp_new(). So I added one more commit and now should be ok, I guess.

@singalsu
Copy link
Collaborator

singalsu commented Jun 3, 2020

@singalsu Thanks, comp_file_host and comp_file_dai didn't have assigned any pointer to .tctx, which one I start to use in comp_new(). So I added one more commit and now should be ok, I guess.

Superb, thanks!! Yes I just ran the process_test and everything worked again.

If it's possible, each line with comment to struct field
should be aligned, to keep code style consistent.

Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
Copy link

@mmaka1 mmaka1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implements and closes #769

@lgirdwood
Copy link
Member

Jenkins DUT failure.

@lgirdwood lgirdwood merged commit 014499c into thesofproject:master Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants