Skip to content

Conversation

@ujfalusi
Copy link
Contributor

@ujfalusi ujfalusi commented Jan 3, 2024

…mple

We need to keep the topology files for different ADSP generations separate because they can include NHLT blobs and the blobs are not compatible among different versions.

While this only affects generically named topology files (sof-hda-generic*.tplg), it is cleaner to expect the topology files to be stored under different directories rather than having some in sof-ipc4-tplg root and some under directories.
Keeping the generational topology files in separate directory also enables us to offer backwards compatibility for deployed MTL installs with older kernels.

The new default topology path shall be:
/lib/firmware/intel/sof-ipc4-tplg/PLAT/sof-CONFIG.tplg

With a practical note that the PLAT is symlinked to a common directory for the generation they belopng to:
.
└── intel
├── sof-ace-tplg -> sof-ipc4-tplg/ACE1
└── sof-ipc4-tplg
├── ACE1
├── CAVS2.5
├── mtl -> ACE1
├── tgl -> CAVS2.5
└── tgl-h -> CAVS2.5

…mple

We need to keep the topology files for different ADSP generations separate
because they can include NHLT blobs and the blobs are not compatible among
different versions.

While this only affects generically named topology files
(sof-hda-generic*.tplg), it is cleaner to expect the topology files to be
stored under different directories rather than having some in sof-ipc4-tplg
root and some under directories.
Keeping the generational topology files in separate directory also enables
us to offer backwards compatibility for deployed MTL installs with older
kernels.

The new default topology path shall be:
/lib/firmware/intel/sof-ipc4-tplg/PLAT/sof-CONFIG.tplg

With a practical note that the PLAT is symlinked to a common directory for
the generation they belopng to:
.
└── intel
    ├── sof-ace-tplg -> sof-ipc4-tplg/ACE1
    └── sof-ipc4-tplg
        ├── ACE1
        ├── CAVS2.5
        ├── mtl -> ACE1
        ├── tgl -> CAVS2.5
        └── tgl-h -> CAVS2.5

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
@plbossart
Copy link
Member

How would LunarLake be handled? With an ACE2.0 directory? If yes, we are going to have multiple files that are really the same under different directories?

Also why would we expose tgl-h at the topology level? This is too detailed IMHO.

@ujfalusi
Copy link
Contributor Author

ujfalusi commented Jan 3, 2024

How would LunarLake be handled? With an ACE2.0 directory? If yes, we are going to have multiple files that are really the same under different directories?
ACE2 is symlinked to ACE1 unless we need a separate directory for some reason, optionally we can just symlink lnl to ACE1

ACE2 -> ACE1
lnl -> ACE2

or

lnl -> ACE1

Also why would we expose tgl-h at the topology level? This is too detailed IMHO.

tgl-h symlinked to the same CAVS2.5 directory, please see the patch with the example.
The reason why I have proposed this is that in this way the default topology path can be generated in core based on vendor/platform name alone (a reworked/updated thesofproject/linux#4603).
If we don't want to have different path profiles per IPC versions then we could just use sof-ipc4-tplg/cavs2.5, sof-ipc4-tplg/ace1 and sof-ipc4-tplg/ace2 (ace2 -> ace1) and no platform symlinks, but we need to constantly update the platform names for each generations in sof-docs also to avoid going out of sync with reality.

@plbossart
Copy link
Member

my brain is already hurting. We have different types of platforms...
a) different architecture (cAVS/ACE)
b) different DSP skus with more/fewer cores and hence potential topology differences
c) different IO IP versions (e.g. DMIC for MTL+ different from MTL-)
d) artificial differences due to signature requirements.

Which ones do we want to track?

@ujfalusi
Copy link
Contributor Author

ujfalusi commented Jan 3, 2024

my brain is already hurting. We have different types of platforms...
a) different architecture (cAVS/ACE)
b) different DSP skus with more/fewer cores and hence potential topology differences
c) different IO IP versions (e.g. DMIC for MTL+ different from MTL-)
d) artificial differences due to signature requirements.

Which ones do we want to track?

We must separate the architectures (cAVS/ACE).
This is linux firmware location, so I'm using the same platform definitions we have for platforms within architectures, like we do with the firmware files.

As I said we could have just architecture folders and no platform:

.
└── intel
    ├── sof-ace-tplg -> sof-ipc4-tplg/ace1
    └── sof-ipc4-tplg
        ├── ace1
        │   ├── sof-hda-generic-2ch.tplg
        │   ├── sof-hda-generic-4ch.tplg
        │   ├── sof-hda-generic-idisp.tplg
        │   ├── sof-hda-generic.tplg
        │   ├── sof-lnl-rt711-4ch.tplg
        │   ├── sof-lnl-rt711-l0-rt1316-l23-rt714-l1.tplg
        │   ├── sof-mtl-cs42l43-l0-cs35l56-l12.tplg
        │   ├── sof-mtl-es83x6-ssp1-hdmi-ssp02.tplg
        │   ├── sof-mtl-max98357a-rt5682-ssp2-ssp0-2ch-pdm1.tplg
        │   ├── sof-mtl-max98357a-rt5682-ssp2-ssp0.tplg
        │   ├── sof-mtl-max98357a-rt5682.tplg
        │   ├── sof-mtl-rt1019-rt5682.tplg
        │   ├── sof-mtl-rt1318-l12-rt714-l0.tplg
        │   ├── sof-mtl-rt5650-dts.tplg
        │   ├── sof-mtl-rt711-4ch.tplg
        │   ├── sof-mtl-rt711-l0-rt1316-l23-rt714-l1.tplg
        │   ├── sof-mtl-rt712-l0-rt1712-l3.tplg
        │   ├── sof-mtl-rt713-l0-rt1316-l12-rt1713-l3.tplg
        │   ├── sof-mtl-rt713-l0-rt1316-l12.tplg
        │   ├── sof-mtl-rt722-l0.tplg
        │   └── sof-mtl-sdw-cs42l42-l0-max98363-l2.tplg
        ├── ace2 -> ace1
        └── cavs2.5
            ├── sof-adl-rt711-4ch.tplg
            ├── sof-adl-rt711-l0-rt1316-l12-rt714-l3.tplg
            ├── sof-hda-generic-2ch.tplg
            ├── sof-hda-generic-4ch.tplg
            ├── sof-hda-generic-idisp.tplg
            ├── sof-hda-generic.tplg
            ├── sof-tgl-cs42l43-l3-cs35l56-l01.tplg
            ├── sof-tgl-rt711-rt1308-4ch.tplg
            ├── sof-tgl-rt711-rt1308-rt715.tplg
            ├── sof-tgl-rt711-rt1316-rt714.tplg
            ├── sof-tgl-rt712.tplg
            └── sof-tgl-rt715-rt711-rt1308-mono.tplg

But that would make the generic path generation in core impossible or awkward since on top of the vendor and platform strings we would need architecture/generation string to be able to guess which platform belongs to which architecture.

@kv2019i
Copy link
Contributor

kv2019i commented Jan 3, 2024

Option C added to thesofproject/sof#8683 (comment)

@ujfalusi
Copy link
Contributor Author

ujfalusi commented Jan 3, 2024

my brain is already hurting. We have different types of platforms...

The reason that I have opted with the definition I have in this PR is to avoid my brain hurting and to not try to introduce complex rules, conditions, parameters on where to look for a topology file.
In essence we need this for the sof-hda-generic-2/4ch.tplg, right? But what if we will have something else? If we only special case this two topology files than it is going to be a pain to document and get it right, especially that we need to have sof-ace-tplg compatibility with older kernels...

We will maximum duplicate one file in this way (sof-hda-generic.tplg), in theory we could survive with a single one for all platforms. In theory.


Practical notes on IPC4 deployment
- In practice the topology files should be grouped by generations and platforms should be symlinked to the matching generation
- For compatibility reasons for **Meteor Lake**'s ``/lib/firmware/intel/sof-ace-tplg`` must be symlinked to ``/lib/firmware/intel/sof-ipc4-tplg/ACE1``
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- For compatibility reasons for **Meteor Lake**'s ``/lib/firmware/intel/sof-ace-tplg`` must be symlinked to ``/lib/firmware/intel/sof-ipc4-tplg/ACE1``
- For compatibility reasons with respect to **Meteor Lake**, ``/lib/firmware/intel/sof-ace-tplg`` must be symlinked to ``/lib/firmware/intel/sof-ipc4-tplg/ACE1``

@ujfalusi ujfalusi marked this pull request as draft January 5, 2024 09:00
@ujfalusi
Copy link
Contributor Author

ujfalusi commented Jan 5, 2024

Moving to draft to avoid ninja-merging it.
We might not need to do an update at all if the solution I'm working on is accepted.

Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

I've lost the plot @ujfalusi, the cure seems worse than the initial disease...

├── rpl-s -> CAVS2.5
├── tgl -> CAVS2.5
└── tgl-h -> CAVS2.5

Copy link
Member

Choose a reason for hiding this comment

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

Sorry @ujfalusi, this list looks too exhaustive and to some extent more confusing than the initial naming.

Apart from naming conventions that can be argued on forever, what is wrong with the existing layout and can we not keep sof-ace-tplg for MTL/LNL, and change the layout as needed for future platforms?

Is there really anything broken with the current layout for MTL/LNL?

Copy link
Contributor Author

@ujfalusi ujfalusi Jan 10, 2024

Choose a reason for hiding this comment

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

Sorry @ujfalusi, this list looks too exhaustive and to some extent more confusing than the initial naming.

I have put this to draft as imho thesofproject/sof#8712 would scale correctly without complexity.

Apart from naming conventions that can be argued on forever, what is wrong with the existing layout

Right, so how does things looks today with linux-firmware and sof-firmware installed?

# cd /lib/firmware/intel/
/lib/firmware/intel# ls -l | grep -E 'avs|sof'
drwxr-xr-x 5 root root   4096 20.11. 03:52 avs
lrwxrwxrwx 1 root root     26 11.12. 18:21 dsp_fw_bxtn.bin.zst -> avs/apl/dsp_basefw.bin.zst
lrwxrwxrwx 1 root root     26 11.12. 18:21 dsp_fw_cnl.bin.zst -> avs/cnl/dsp_basefw.bin.zst
lrwxrwxrwx 1 root root     26 11.12. 18:21 dsp_fw_glk.bin.zst -> avs/apl/dsp_basefw.bin.zst
lrwxrwxrwx 1 root root     26 11.12. 18:21 dsp_fw_kbl.bin.zst -> avs/skl/dsp_basefw.bin.zst
lrwxrwxrwx 1 root root     26 11.12. 18:21 dsp_fw_release.bin.zst -> avs/skl/dsp_basefw.bin.zst
drwxr-xr-x 4 root root   4096  2. 1. 14:23 sof
drwxr-xr-x 2 root root   4096  2. 1. 14:23 sof-ace-tplg
drwxr-xr-x 3 root root   4096 21.11. 17:37 sof-ipc4
drwxr-xr-x 2 root root  20480  2. 1. 14:23 sof-tplg

The IPC3 firmware files are under sof and the corresponding topology files are under sof-tplg
The IPC4 firmware files are under sof-ipc4 and the corresponding topology files are under sof-ace-tplg

and can we not keep sof-ace-tplg for MTL/LNL, and change the layout as needed for future platforms?

Yes, we can think after a new IPC4 architecture is released to public on where it is going to end up, my aim with thesofproject/sof#8712 is to avoid that completely.
Bu let's say we release CAV2.5 files, the kernel now follows the sof-docs specification so CAVS2.5 will expect firmware files under sof-ipc4 and the corresponding topology files under sof-ipc4-tplg. Then we release other platforms for a new architecture, likely colliding with existing "generic" topology files (sof-hda-generic-2/4ch), so:

Directory Content
sof IPC3 firmware files
sof-tplg IPC3 topology files
sof-ipc4 IPC4 firmware files (ACE1/ACE2/CAVS2.5/some_arch/some_other_arch)
sof-ace-tplg IPC4 topology files (ACE1/ACE2)
sof-ipc4-tplg IPC4 topology files (for CAV2.5 platforms)
sof-some_arch-tplg IPC4 topology files (for some_arch only)
sof-some_other_arch-tplg IPC4 topology files (for some_other_arch only)

Is there really anything broken with the current layout for MTL/LNL?

Apart from not scalable and going to be a confusing mess over time, no, nothing.
But let's say we will transition to a new and better IPC version, let's say IPC5 and we are fortunate that ACE can be supported by it. Then what? How sof-ace-tplg is not for IPC5 but only IPC4?
The new IPC version is hypothetical, just for highlighting the non scalability of what we have.

What is wrong with my proposal?

Directory Content
sof IPC3 firmware files
sof-tplg IPC3 topology files
sof-ipc4 IPC4 firmware files (ACE1/ACE2/CAVS2.5/some_arch/some_other_arch)
sof-ipc4-tplg IPC4 topology files (ACE1/ACE2/CAVS2.5/some_arch/some_other_arch)
sof-ace-tplg symlink to sof-ipc4-tplg for backwards compatibility (to be kept for few years)

Copy link
Member

Choose a reason for hiding this comment

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

"sof-ace-tplg symlink to sof-ipc4-tplg for backwards compatibility (to be kept for few years)"

To be clear, we have NO PLANS to change the IPC4 default path in the kernel files, do we? That would mean that the sof-ace-tplg path will have to be maintained forever

We also have NO PLANS to change the IPC type for MTL/LNL, so I am not sure that the scalability issue is a problem we need to solve.

What we do know is that we have to avoid a conflict for the next generations, but that doesn't mean we need to align and risk breaking existing stuff with late changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"sof-ace-tplg symlink to sof-ipc4-tplg for backwards compatibility (to be kept for few years)"

To be clear, we have NO PLANS to change the IPC4 default path in the kernel files, do we? That would mean that the sof-ace-tplg path will have to be maintained forever

I would change the path in kernel after the second sof-bin release with the sof-ipc4-tplg + sof-ace-tplg symlink.
Distros update the sof-firmware and in case of issue we always ask for updated sof-bin.

We also have NO PLANS to change the IPC type for MTL/LNL, so I am not sure that the scalability issue is a problem we need to solve.

That was hypothetical note.

What we do know is that we have to avoid a conflict for the next generations, but that doesn't mean we need to align and risk breaking existing stuff with late changes.

We can have a consistent location with minimal effort today, tomorrow it is not going to be possible and we will have an inconsistent mess to manage for eternity.

Copy link
Member

@plbossart plbossart Jan 10, 2024

Choose a reason for hiding this comment

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

why do you need to change the path in kernel? That's asking for trouble with multiple versions of the kernel pointing to different locations.

Also why would we need to track all versions of SOC at the topology level? We should only do so if there's a good reason where the compatibility with previous solution needs to be broken.

My counter proposal would be

sof-ipc4-tplg/
   all existing cavs stuff without any further redirections
   ace/  all topologies for MTL and LNL (i.e this covers ace1.x and ace2.x)
   aceN/ topologies for the first 'N' version that is no longer compatible with the 'ace' topologies
sof-ace-tplg/ symlink for sof-ipc4-tplg/ace

We have a similar handling for the PCI devices, we group them in the same files until we can't because they require different handling. No need to track each SOC separately if we can avoid doing so.

@marc-hb
Copy link
Collaborator

marc-hb commented Jan 9, 2024

The reason that I have opted with the definition I have in this PR is to avoid my brain hurting and to not try to introduce complex rules, conditions, parameters on where to look for a topology file.

Complexity is never good but this affects only the Linux kernel, correct? I don't think these rules are duplicated anywhere else. sof-test is still grepping the kernel logs (until thesofproject/linux#3867) and CI may have some hardcoded tables but no logic.

EDIT: these rules are "somewhat" duplicated in the sof/tools/topology/ tree structure of course.

@ujfalusi
Copy link
Contributor Author

Closing, replaced by a simple text update: #483

@ujfalusi ujfalusi closed this Jan 10, 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.

5 participants