Repository navigation
[RFC]selector: need treat selector as endpoint for command handling - #1365
Conversation
For hw_params(), prepare(), trigger(), reset() commands, we should treat seletor as endpoint(don't propagate them upstream anymore), otherwise, those upstream components(e.g. kpb and dai) will complain about double handling and status might be wrong, errors like "source component buffer has not enough data available" will happen. This is only RFC to raise attention of this issue, it is not good fix, please help comment. Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
lgirdwood
left a comment
There was a problem hiding this comment.
Not following, can you describe use case where this happens.
@lgirdwood for the detect pipeline(pipeline 9), those commands(e.g. hw_params, trigger, reset) should stop before KPB(that is, at selector), otherwise, it will interact the running of capture pipeline(pipeline 8), here the solution to fix it is not decent, I draft the RFC patch here which verified work, to try to get better solution of it especially from @tlauda who have done refinement for pipeline recently. Hi @tlauda any comment? |
|
@keyonjie ok, this is how mixer and mux work, they will stop path walk at any unconnected paths. So your patch is using the correct constructs as the selector actually determines which sink paths are valid. |
The slight difference here is that the path is actually connected, so... |
|
@keyonjie @lgirdwood We need to think about some more generic solution, because there is no guarantee that the selector will always be the first component in the pipeline. This is specific for our current topology. The best and most error-proof would be to treat every pipeline in FW as separate entity, but then kernel would need to "know" about backend pipelines. |
That's what I concern exactly, thanks @tlauda for your appearance. :) |
|
@keyonjie This is one of the things I wanted to discuss for a while now. Currently some of the parameters are propagated to connected pipelines and some aren't and with so many different types of topologies our logic is not good enough. We have mixer, mux and kwd topologies and every one of them expects different behavior from FW. And also I don't like the idea of putting this logic inside of the module, because it takes away from our flexibility. I still think that controlling every pipeline separately from the kernel would be the best solution. Simultaneous trigger of pipelines can be handled using compound IPCs. |
I can totally feel your feeling, I talked this to @lbetlej in Gdansk also, this might need discussion among wider range(e.g. at least @plbossart need to be involved), and the good solution with it will be our long-term goal(won't be available in days). So here, I prefix it with [RFC] to try to get alignment for short-term solution firstly. |
|
@tlauda @lgirdwood as we can expect we won't have solution for this in short time, I suggest we take the solution in my PR firstly and refine it to long-term solution once that is available, here the change is only inside selector component, the risk here is more controlable comparing to any pipeline short-term fixes IMO. |
|
@keyonjie @tlauda please see #1280 on propagating params through pipeline. |
@lgirdwood I totally appreciate what you described on #1280, what I concern only is when will we get this feature supported in the FW, now it is block for both KWD and demux what @juimonen is working on. |
|
@zrombel please check the CI if it's a false negative report on the CodeCheck? |
For hw_params(), prepare(), trigger(), reset() commands, we should treat
seletor as endpoint(don't propagate them upstream anymore), otherwise,
those upstream components(e.g. kpb and dai) will complain about double
handling and status might be wrong, errors like "source component buffer
has not enough data available" will happen.
This is only RFC to raise attention of this issue, it is not good fix,
please help comment.
Signed-off-by: Keyon Jie yang.jie@linux.intel.com