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: move platform specific code from base_fw.c #9060

Merged

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Apr 18, 2024

Continue from #9059 (first 2 patches here are from #9059) and move more implementation out from the generic file.
UPDATE: 9059 closed, let's review the whole set in one go.

Addressing #8391 and #7549

@kv2019i
Copy link
Collaborator Author

kv2019i commented Apr 18, 2024

Fuzzing fails to apt get problem -> https://github.com/thesofproject/sof/actions/runs/8739094489/job/23979620908?pr=9060

Checkpatch fail is from existing code I'm moving around -> https://github.com/thesofproject/sof/actions/runs/8739094514/job/23979621326?pr=9060

@kv2019i kv2019i requested review from gbernatxintel and removed request for RanderWang April 18, 2024 16:37
@kv2019i
Copy link
Collaborator Author

kv2019i commented Apr 22, 2024

@abonislawski @softwarecki @tmleman @tobonex can you take a look?

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.

LGTM, but we need to remove src/platform/intel/ and put this data into a src/ipc/ipc4/manifest.c, src/ipc/ipc4/intel.c etc - maybe just agit mv as the final step here.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Apr 23, 2024

@lgirdwood wrote:

LGTM, but we need to remove src/platform/intel/ and put this data into a src/ipc/ipc4/manifest.c, src/ipc/ipc4/intel.c etc - maybe just agit mv as the final step here.

I thought we concluded on this already, this is essentially one of the options I proposed in #9012 (comment) (but not the one that was merged). Oh well, I can make a follow-up PR that folds the Intel specific code and drops the platform abstractions.

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>
Add support to handle platform specific extensions with
platform_basefw_get_large_config() and
platform_basefw_set_large_config(). These functions are called when
get/set handled is called with a parameter type not handled
in generic code.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Move the Intel specific L1_EXIT control handling from base_fw.c
to Intel specific platform code.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The implementation for IPC4_MEMORY_STATE_INFO_GET requires
reading direct register contents to fill out the structure,
so this is by definition platform specific code. Move this
to platform implementation.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Move implementation of IPC4_EXTENDED_SYSTEM_TIME from generic
to platform specific code. The implementation depends on specific
ART counter that is not available on all platforms, so it is better
to handle this in platform specific code.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add missing copyright statement to file header.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i force-pushed the 202404-basefw-getset-platform-impl branch from 1faa2a1 to 09bf0bc Compare April 23, 2024 16:55
@kv2019i
Copy link
Collaborator Author

kv2019i commented Apr 23, 2024

V2:

  • rebased due to conflicts (multiple other PRs merged)
  • no other changes

Copy link
Contributor

@gbernatxintel gbernatxintel left a comment

Choose a reason for hiding this comment

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

Looks good for me

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