Skip to content

[BUG] Platform-specific IPC4 code in platform-independent source files #7549

Closed

Description

@andyross

Recent IPC4 changes have been introducing Intel DSP details into the generic SOF source layer.

The src/audio/base_fw.c file (despite the name, this is an IPC4-only source file) is implementing a protocol that seems directly related to Intel hardware details. In particular the handling in basefw_mem_state_info() is doing direct register reads to retrieve hardware state. It seems like there's a facility in place to structure the output of this function via "tuple" key/data pairs, but there's no documentation on the format it nor indirection API available for platforms.

Similarly the component driver module handling in ipc4/helper.c:ipc4_get_comp_drv() assumes that there is a Intel rimage-generated manifest format in memory from which to load modules. There is a "LIBRARY_MANAGER" feature that seems to be intended to replace this, I think? As of right now neither mechanism is in use in the main tree.

All hardware-specific features like this need to be provided behind an API that allows for other platforms to redefine it as needed, and ideally be defined/documented in a way that makes as few assumptions as possible about the underlying hardware.

See also comments and commit messages is PR #7531

Activity

  1. added
    bugSomething isn't working as expected
    on May 2, 2023
  2. paulstelian97 commented on May 3, 2023

    @paulstelian97
    Collaborator

    Some documentation on what this required component is doing is also useful for porting other platforms (we want to switch to IPC4 at NXP, and in my quick look this was one of the places where I stopped, not knowing how to progress further). Why is it even required in the first place no matter what?

  3. added this to the v2.7 milestone on Jul 4, 2023
  4. modified the milestones: v2.7, v2.8 on Aug 16, 2023
  5. kv2019i commented on Oct 26, 2023

    @kv2019i
    Collaborator

    Tracking the basefw part in #8391

  6. kv2019i commented on Nov 20, 2023

    @kv2019i
    Collaborator

    No assigned person and one week to go before branch point, kicking to v2.9.

  7. modified the milestones: v2.8, v2.9 on Nov 20, 2023
  8. lgirdwood commented on Nov 23, 2023

    @lgirdwood
    Member

    @andyross @cujomalainey we will tackle this in Q1, there is ongoing work already to split IPC logic out of modules as 1st step so we have module-common.c, module-ipc3.c and module-ipc4.c. We can then move this into other places so that common codebase is generic.

  9. kv2019i commented on Mar 4, 2024

    @kv2019i
    Collaborator

    Stable-v2.9 branched, this didn't make the cut, bumping to 2.10.

  10. modified the milestones: v2.9, v2.10 on Mar 4, 2024
  11. self-assigned this
    on Apr 9, 2024
  12. kv2019i commented on Apr 24, 2024

    @kv2019i
    Collaborator

    With #9060 merged, we can close this. All vendor specific code has been now removed. Only exception is SOF_TELEMETRY, but that is behind separate ifdefs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working as expected

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions