Update DMIC parsing logic #1773
Conversation
|
@juimonen FYI. I think this version makes the parser more generic to handle all widgets and all token types with no logic specific for DMIC parser. So when you add multiple tokens for the mux/demux, you will not need to change anything |
ranj063
force-pushed
the
fix/dmic_parsing
branch
from
f2022f1 to
101c307
Compare
|
closing temporarily until I fix the testcase failure issues. |
Update the logic for DMIC PDM token parsing to make it generic so that it can be used for other widgets that have multiple vendor tuples of the same type. In order to accomplish this, the signature for the sof_parse_tokens() method is modified to pass the size of the object which holds the parsed token values. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Modify the parsing logic for string/uuid type tokens to make them generic enough to handle multiple vendor tuples of the same type. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
ranj063
force-pushed
the
fix/dmic_parsing
branch
from
101c307 to
36fc48f
Compare
|
@ranj063 quickly browsed through.... I think the same comment applies here as @plbossart gave me. You could make another function like sof_parse_token_sets, so you don't have to modify code all over for changed interface of sof_parse_tokens. Also do you return the parsing when all tokens have been found? I think that's 1 nice feature in my PR. Otherwise I think this is quite the same I have, and we can do it this way or my way, if above is "fixed". |
|
@ranj063 @juimonen Regardless whether this one or #1766 gets merged in the end, I personally would find it helpful to see a topology example, that would be fixed or improved by these PRs. Are any of the existing topologies broken because of this? What in them would get fixed? Or would this allow us to use some new topologies? |
|
@lyakh In current sof master dmic pdm tokens are an example of multiple token "sets". see tools/topology/platform/intel/dmic. If you compile for example sof-apl-pcm512x.m4 to .conf file you can see them there. Now in driver side this is handled with quite "hackish" way in sof_parse_word_token. When I was starting to add demux tokens instead of binary blob (which you btw also wanted to do :) ), I noticed that this doesn't scale very well. So this effort is to generalize the parsing of multiple token "sets" already in topology by dmic and coming in the future with demux (and maybe something else also) |
@juimonen Thanks for the explanation! A small correction: I didn't necessarily want to do that myself, but I did make comments, that we needed that, so, thanks for addressing the issue! But these 2 PRs don't make that switch yet, right? Is that actually in the works somewhere? |
|
@lyakh yes correct. Sorry my wording was bad, yes you said we should change the binary blob thing, not that you would do it. I did one version of demux parsing but then noticed that the interface is about to change: With my previous driver side version that you reviewed, I didn't change anything with dmic until @ranj063 pointed out that dmic is already having several similar tuples sets. Then I noticed that, oh this doesn't maybe scale really well, and couple of other things with parsing... So all this effort is to get to generalize the multiple tuples "set" parsing and make dmic and demux use that. Once we get this generalization agreed (or not) I'll try to do the demux driver side parsing with new interface. |
@juimonen very good, thanks! I think we really need this! |
+1. This proposal remains very abstract to me as well. |
|
lets stick with @juimonen and I'll comment there |
4 participants
This is a replacement for #1766