ASoC: SOF: ipc4: Add support for core_id in create pipeline message

[ujfalusi]

Hi,

the recently merged thesofproject/sof#6637 added support for specifying the target core_id for the pipeline to be created by carving out 4 bits from the reserved section of the message extension.

The core_id can be specified in topology with SOF_TKN_SCHED_CORE token and the kernel will set it to the message.
The core_id will remain 0 (as it is now) when the token is not present and since these bits were ignored, old firmware should with new topology (which is unsupported combination) should not cause regressions, issues either.

replaces PR#4047

Added one new patch to fix a NULL pointer dereference if I force the core_id of the pipelines to 1 which will fail in firmware and the pipeline list allocated, but we have not stored any pipeline in the array, so on stop we are hitting a NULL pointer on the first spipe.

[ujfalusi]

Changes since v1:

• typo fixed in the first patch's commit message
• check added to validate the core_id consistency along the path:
• checking each connection core_id for pipelines and widgets which will eventually going to find out if any component uses different core_id.

[plbossart]

I don't think the protection rules are correct.

[plbossart]

I don't think this rule is correct. It's perfectly fine to have a route between components that are handled by separate pipelines on separate cores. This would only allow for connected pipelines to be on the same core, and that's a short-term limitation we absolutely don't want to upstream.

[plbossart]

same on the source side.

[ujfalusi]

right, so no limitation on the connected pipelines and widgets.
The only limitation is that a widget's core must match with the pipeline's core that it is part of?

[ujfalusi]

iow, this is not valid:

if (src_pipeline->core_id != src_widget->core) {
...
}

if (sink_pipeline->core_id != sink_widget->core) {
...
}

[plbossart]

The only limitation is that a widget's core must match with the pipeline's core that it is part of?

Even that part is not clear. When we have DP modules, we will be able to have chained modules on the same pipeline use different cores. So I don't know what the pipeline core would mean, it's an irrelevant notion when DP is used.

[ujfalusi]

right, so if all combinations are fine then what to check?

[ujfalusi]

and most importantly, how the DP is going to be used? Is it going to have some mark in tplg?
Without DP we do have limitation, no? We should have the limitation checked now (we don't have DP support afaik) and make it conditional when DP is available?

[plbossart]

IIRC there is nothing advertized anywhere in kernel-accessible manifests or topology that a module is LL or DP, so it could even be the case that only firmware can check for invalid configurations. @lgirdwood can we have the firmware folks confirm what errors cases we could/need to handled in the driver?

[ujfalusi]

fwiw, if you send the CREATE_PIPELINE message with core_id=1, the firmware will reply with "invalid core id"

[ujfalusi]

Changes since v2:

• Drop the core id verification from sof_ipc4_route_setup()
• the check is going to happen in the firmware and in case of error it is going to be communicated to the kernel which we handle gracefully. This will allow policy and changes in firmware without a need for a synchronized kernel change.

[plbossart]

I would add a comment in the commit message to reflect the direction on error check

Firmware will double-check all information retrieved by topology and report errors if required. This will allow policy and changes in topologies without a need for a synchronized kernel change.

[ujfalusi]

Changes since v3:

• Commit message updated for the last (core_id support) patch.