Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASoC: SOF: update gain ipc msg definition to align with fw #4071

Merged

Conversation

RanderWang
Copy link

@RanderWang RanderWang commented Dec 5, 2022

Fw uses latest 64 bits curve_duration in gain ipc msg definition so update kernel driver side. Curve_duration is in number of hundreds nanoseconds, and only 32 bits int can support up to 400s volume ramp, which is enough for ramp feature, so we don't need to support 64 bits read by topology. Now divide 64 bits curve_duration to two parts: low part set by topology and high part zeroed in driver.

fix #4026 (IPC4 volume windows ramp was broken with latest sof-dev kernel and sof firmware)

@RanderWang
Copy link
Author

RanderWang commented Dec 5, 2022

The discussion is in #4026 . The gain ipc msg was first supported based on closed source TGL branch in 2020. At that time it was 32 bits. It was updated to 64bits later but kernel side is not updated

bardliao
bardliao previously approved these changes Dec 5, 2022
@ranj063
Copy link
Collaborator

ranj063 commented Dec 5, 2022

The discussion is in #4026 . The gain ipc msg was first supported based on closed source TGL branch in 2020. At that time it was 32 bits. It was updated to 64bits later but kernel side is not updated

@RanderWang will this break TGL with the tester?

@RanderWang
Copy link
Author

The discussion is in #4026 . The gain ipc msg was first supported based on closed source TGL branch in 2020. At that time it was 32 bits. It was updated to 64bits later but kernel side is not updated

@RanderWang will this break TGL with the tester?

I validated on TGL with Closed source FW. Please check the original definition, a u32 sized memory is padded by compiler to align 8. Why FW is crashed ? because we don't zeroed "struct sof_ipc4_gain_data" in volume update, so the padded u32 is full of garbage, so FW get a 64bit data with high part of unexpected data. Now we change the padded u32 to explicitly defined high part and zero it.

struct sof_ipc4_gain_data {
	uint32_t channels;
	uint32_t init_val;
	uint32_t curve_type;
	uint32_t reserved;
	uint32_t curve_duration;
        // compiler will allocate more u32 memory to aligned(8)
} __aligned(8);

Recent firmware changes modified the curve duration from 32 to 64 bits,
which breaks volume ramps. A simple solution would be to change the
definition, but unfortunately the ASoC topology framework only supports
up to 32 bit tokens.

This patch suggests breaking the 64 bit value in low and high parts, with
only the low-part extracted from topology and high-part only zeroes. Since
the curve duration is represented in hundred of nanoseconds, we can still
represent a 400s ramp, which is just fine. The defacto ABI change has no
effect on existing users since the IPC4 firmware has not been released just
yet.

Link: thesofproject#4026

Signed-off-by: Rander Wang <[email protected]>
@plbossart plbossart merged commit 642ad26 into thesofproject:topic/sof-dev Dec 7, 2022
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.

[BUG] IPC4 volume windows ramp was broken with latest sof-dev kernel and sof firmware
5 participants