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

audio: base_fw: add platform layer to fw_config data #9059

Closed
wants to merge 2 commits into from

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Apr 18, 2024

Build on foundation set in #9012

kv2019i added 2 commits April 18, 2024 12:37
Fix spelling in platform_basefw_hw_config() documentation.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Some of the IPC4 FW_CONFIG fields are platform specific and cannot be
filled with generic code.

Handle this data by adding a call to platform_basefw_fw_config()
and add an implementation for all current platforms with IPC4
support.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
{
struct sof_tlv *tuple = (struct sof_tlv *)data;

tlv_value_uint32_set(tuple, IPC4_SLOW_CLOCK_FREQ_HZ_FW_CFG, IPC4_ALH_CAVS_1_8);
Copy link
Contributor

Choose a reason for hiding this comment

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

This cannot be right, right?
see the original code:

	tuple = tlv_next(tuple);
	tlv_value_uint32_set(tuple,
			     IPC4_SLOW_CLOCK_FREQ_HZ_FW_CFG,
			     clock_get_freq(CPU_LOWEST_FREQ_IDX));

	tuple = tlv_next(tuple);
	tlv_value_uint32_set(tuple, IPC4_SLOW_CLOCK_FREQ_HZ_FW_CFG, IPC4_ALH_CAVS_1_8);

The first set of IPC4_SLOW_CLOCK_FREQ_HZ_FW_CFG looks valid, the second (and the one which got moved) is not.

Copy link
Contributor

@ujfalusi ujfalusi Apr 18, 2024

Choose a reason for hiding this comment

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

It is introduced by
3efd780 ("ipc4: add basefw component support")

so it is introduced alongside of the file itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ujfalusi I'm not touching the values here, just moving them around as-is. That certainly looks wrong, but haven't dug into details yet. Linux driver is not using this, so hard to say much about this.

@kv2019i kv2019i requested review from tobonex and softwarecki April 18, 2024 11:51
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Lets do the code move 1st, we can then followup if we thing data is wrong (as its passing CI).
@softwarecki pls review.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Apr 18, 2024

There's more incoming, now still as draft in #9060

@kv2019i
Copy link
Collaborator Author

kv2019i commented Apr 18, 2024

Let's go directly with #9060 , closing this one.

@kv2019i kv2019i closed this Apr 18, 2024
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.

3 participants