ASoC: SOF: Validate and correct the core id against the number of cores of the DSP #5600
Conversation
ujfalusi
requested review from
bardliao,
dbaluta,
kv2019i,
lgirdwood,
plbossart and
ranj063
as code owners
sound/soc/sof/ipc3-topology.c
Outdated
| } else if (pipeline->core > sdev->num_cores - 1) { | ||
| dev_dbg(scomp->dev, "core id %d is out of range, changing it to %d\n", | ||
| pipeline->core, sdev->num_cores ? (sdev->num_cores - 1) : 0); | ||
| pipeline->core = sdev->num_cores ? (sdev->num_cores - 1) : 0; |
There was a problem hiding this comment.
Im ok with this, although Id prefer switching all pipelines with invalid cores IDs to run on core 0 instead of Core N
There was a problem hiding this comment.
@ranj063, OK, I will move them to core0 and change the dev_dbg() to dev_info() to be visible in logs for us to decide if we need to address something or not.
There was a problem hiding this comment.
@ranj063 @ujfalusi you are the master, I'm ok with core0 as well, although I worry this allows topology develops to slip in bad topologies (as kernel driver is the primary end2end test bed when making topology changes). So a fatal error would be very useful to raise for tplg developers. But not blocking on this.
There was a problem hiding this comment.
We will print in info level to raise awareness, but at the same time I don't think it is better to have a back and forth to get a working system (does not work -> set 0x20 in sof_debug -> new topology for testing -> user test -> sof_debug kept until sof-bin release)
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Minor notes and an opinion about whether this should be adjusted or an error. But nothing blocking.
sound/soc/sof/ipc3-topology.c
Outdated
| } else if (pipeline->core > sdev->num_cores - 1) { | ||
| dev_dbg(scomp->dev, "core id %d is out of range, changing it to %d\n", | ||
| pipeline->core, sdev->num_cores ? (sdev->num_cores - 1) : 0); | ||
| pipeline->core = sdev->num_cores ? (sdev->num_cores - 1) : 0; |
There was a problem hiding this comment.
@ranj063 @ujfalusi you are the master, I'm ok with core0 as well, although I worry this allows topology develops to slip in bad topologies (as kernel driver is the primary end2end test bed when making topology changes). So a fatal error would be very useful to raise for tplg developers. But not blocking on this.
ujfalusi
force-pushed
the
peter/sof/pr/validate_core_id
branch
from
4dc8b84 to
5a54b1d
Compare
|
Changes since v1:
|
sound/soc/sof/ipc3-topology.c
Outdated
| } else if (pipeline->core > sdev->num_cores - 1) { | ||
| dev_info(scomp->dev, "out of range core id for %s, moving it %d -> 0\n", | ||
| swidget->widget->name, pipeline->core); | ||
| pipeline->core = 0; |
ujfalusi
force-pushed
the
peter/sof/pr/validate_core_id
branch
2 times, most recently
from
cc9b22a to
1c47651
Compare
|
Changes since v2:
|
…ipelines SOF_DBG_DISABLE_MULTICORE is handled for swidgets in topology.c but the pipeline's core is not changed to primary core if the flag is set. Check the flag and if it is set, force the pipeline core to primary core. Cc: [email protected] # 6.7+ Signed-off-by: Peter Ujfalusi <[email protected]>
…es of the DSP Generic development topologies can reference core id outside of the range of the number of DSP cores the device might have. Product families have different number of cores, for example: Intel TGL has 4, TGL-H has 2, ADL has 4, ADL-S has 2, etc The development topologies are tuned for the higher end devices and in this case they will fail on DSP with less number of cores. Override the out of range core id from topology to primary core and inform the user about it. Signed-off-by: Peter Ujfalusi <[email protected]>
4 participants
Generic development topologies can reference core id outside of the range
of the number of DSP cores the device might have.
Product families have different number of cores, for example:
TGL has 4, TGL-H has 2, ADL has 4, ADL-S has 2, etc
The development topologies are tuned for the higher end devices and in this
case they will fail on DSP with less number of cores.