Skip to content

Conversation

@ujfalusi
Copy link
Collaborator

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.

The create pipeline message can carry the target code_id which is set to
0 at the moment.

Add macros to set the core_id in the message extension.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Token SOF_TKN_SCHED_CORE in topology file can specify the target core for
the pipeline, if it is missing it is going to be 0 (as it is right now).

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

plbossart commented Nov 29, 2022

Sorry, I don't understand this PR and the matching SOF PR at all.

In IPC4 DP modules are supposed to be independently pinned to specific cores. The pipeline information is not used for any scheduling.
In IPC4 LL modules are handled in a round-robin manner with the priority defined by the pipeline ID. I don't see how the multi-core helps with the round-robin implementation.

So what are we gaining with this PR? @kv2019i @lgirdwood this is missing a lot of context I am afraid.

@ranj063
Copy link
Collaborator

ranj063 commented Nov 29, 2022

Sorry, I don't understand this PR and the matching SOF PR at all.

In IPC4 DP modules are supposed to be independently pinned to specific cores. The pipeline information is not used for any scheduling. In IPC4 LL modules are handled in a round-robin manner with the priority defined by the pipeline ID. I don't see how the multi-core helps with the round-robin implementation.

So what are we gaining with this PR? @kv2019i @lgirdwood this is missing a lot of context I am afraid.

@plbossart IIUC this is where we deviate from the multi-core implementation in the reference FW. We do not have the notion of DP modules in the SOF firmware yet. But we want to keep the notion of running entire pipelines on a secondary core much like what we had in IPC3. But I am afraid that if we go down this path, we'll have to stop testing the reference FW with multi-core. @kv2019i @lgirdwood please correct me if I am wrong.

@ujfalusi
Copy link
Collaborator Author

Looking at this with a fresh eyes, I think we are still missing something...
There is a swidget->core as well which is sent via the MOD_INIT_INSTANCE message and it is queried in topology.c via SOF_TKN_COMP_CORE_ID token for the given widget.
I'll move the PR to Draft while I figure how to handle this.
Either copy the core_id from the swidget to pipeline or the other way or to verify that the pipeline core and the widgets in it have the same core ID.

@ujfalusi ujfalusi marked this pull request as draft November 30, 2022 09:35
@plbossart
Copy link
Member

plbossart commented Nov 30, 2022

he pipeline core and the widgets in it have the same core ID.

That is NOT the long-term direction @ujfalusi. The plan is to have core-assignment at the module instance level. I don't understand the piecemeal/tactical changes that are going-on in firmware-land, but this notion of multi-core with LL pipelines doesn't seem fully baked. We should do kernel changes that will be upstreamed, if they will be invalidated later and are incompatible with the reference firmware they belong in an engineering/experimental branch, not merged on sof-dev.

@ujfalusi
Copy link
Collaborator Author

@plbossart, OK, I will close this PR.

@ujfalusi ujfalusi closed this Nov 30, 2022
@lgirdwood
Copy link
Member

Long term FW will support (on any core)

  1. LL alongside another like DP, TwB, EDF, simple thread preemption.
  2. No LL, but one of DP, TwB, EDF or thread preemption.

@ujfalusi please reopen this PR.
@plbossart this is now baked into IPC ABI and only fwd compatible changes will be permited .

@plbossart
Copy link
Member

I think there's still a need to clarify if the driver should run a pass to verify that pipelines using coreX only include components using coreX, or if the firmware will check for this. it's a problem in general when the information is spread in two locations. Someone will get it wrong - as in 50% of humanity.

@ujfalusi
Copy link
Collaborator Author

ujfalusi commented Dec 8, 2022

I think there's still a need to clarify if the driver should run a pass to verify that pipelines using coreX only include components using coreX, or if the firmware will check for this. it's a problem in general when the information is spread in two locations. Someone will get it wrong - as in 50% of humanity.

I was looking at ways to do the verification (which does not exists on IPC3 either), but it is a bit tricky, but I'll try to come up with something which might work: when a path is set up we will check that all involved widget, pipeline have the same core_id, if not, we will fail with a meaningful message.

@ujfalusi
Copy link
Collaborator Author

ujfalusi commented Dec 8, 2022

I can not re-open this PR, I will create a new one.

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.

6 participants