ipc4: Ensure pipeline component directions are synchronized #8969
Conversation
tmleman
requested review from
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
tmleman
requested review from
abonislawski,
dnikodem,
marcinszkudlinski and
softwarecki
| * mark the source's direction as set. This ensures that the pipeline's flow | ||
| * direction is consistent and correctly configured for the traversal process. | ||
| */ | ||
| if (!ppl_icd->pipeline->source_comp->direction_set && |
There was a problem hiding this comment.
Identical code and logic is already done in ipc_comp_connect(). It would be interesting to understand better why this can happen in some case. If this cannot be avoided, can we have a shared helped used by both this function and ipc_comp_connect() ?
There was a problem hiding this comment.
the choice of a function to place this code into isn't very obvious. Is this function called for each new pipeline before it's activated? Maybe some more obvious place like when a streaming attempt is made would be clearer?
There was a problem hiding this comment.
@lyakh I had the same concern, but looks like it's not so easy to pick another spot. This is called from multiple places, from the state machine implementing pipeline state changes. I'm happy with the extended git commit message to explain the change.
tmleman
force-pushed
the
topic/upstream/pr/ipc4/ppl_prepare/comp_prepare/direction_set
branch
2 times, most recently
from
b4cb1eb
to
4db629f
Compare
|
@kv2019i please take another look. |
|
Ready to go once mandatory CI checks are green. |
This patch addresses an issue where audio output could be silent due to the direction property of pipeline components not being set. The problem manifests when the pipeline is initialized in the sequence: Init -> Reset -> Pause -> Ready In this scenario, the direction property may remain unset, leading to incorrect pipeline behavior. In flow of transitions: Init -> Pause -> Ready, this issue does not occur because the firmware attempts to set the directions in pipe components during the transition from Init to Pause. This step is skipped if Pause is done after Reset, which is the scenario that this patch addresses. The added code ensures that if the source component's direction is unset but the sink's direction is set, or vice versa, the direction is copied from the set component to the unset one. This synchronization of the direction property guarantees that the pipeline's data flow is correctly established. This synchronization of the direction property guarantees that the pipeline's data flow is correctly established, allowing the `pipeline_for_each_comp` function to traverse and process all components as intended, thus resolving the silent audio output problem. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
| @@ -251,6 +251,28 @@ static struct ipc_comp_dev *pipeline_get_host_dev(struct ipc_comp_dev *ppl_icd) | |||
| struct ipc *ipc = ipc_get(); | |||
There was a problem hiding this comment.
nitpick: in commit message
This synchronization of the
direction property guarantees that the pipeline's data flow is correctly
established.This synchronization of the direction property guarantees that the
pipeline's data flow is correctly established
looks like a duplication
| * mark the source's direction as set. This ensures that the pipeline's flow | ||
| * direction is consistent and correctly configured for the traversal process. | ||
| */ | ||
| if (!ppl_icd->pipeline->source_comp->direction_set && |
There was a problem hiding this comment.
the choice of a function to place this code into isn't very obvious. Is this function called for each new pipeline before it's activated? Maybe some more obvious place like when a streaming attempt is made would be clearer?
|
No further comments, so proceeding with merge. |
This patch addresses an issue where audio output could be silent due to the direction property of pipeline components not being set. The added code ensures that if the source component's direction is unset but the sink's direction is set, or vice versa, the direction is copied from the set component to the unset one.
This synchronization of the direction property guarantees that the pipeline's data flow is correctly established, allowing the
pipeline_for_each_compfunction to traverse and process all components as intended, thus resolving the silent audio output problem.