[slawblauciak]

Introduces configuration IPC ABI for the MultiDAI

MultiDAI is a new logical DAI type responsible for allocation of multiple physical child DAI and transparently managing them, routing the audio data.

Signed-off-by: Slawomir Blauciak slawomir.blauciak@linux.intel.com

[plbossart]

I can't figure out what the benefits of this PR are and why we would introduce a new IPC.

[bardliao]

@slawblauciak The concept is to put/link physical dai configs to the new virtual dal config, right? My question is what is format used for? Shouldn't we use the format of each physical dais?

[slawblauciak]

@slawblauciak The concept is to put/link physical dai configs to the new virtual dal config, right? My question is what is format used for? Shouldn't we use the format of each physical dais?

If we have separate formats for each DAI, I suppose MultiDAI would be responsible for conversion?
Would we really have a use case for that?

[slawblauciak]

Changes applied. The code is now back to the initial proposal.

[bardliao]

@slawblauciak The concept is to put/link physical dai configs to the new virtual dal config, right? My question is what is format used for? Shouldn't we use the format of each physical dais?

If we have separate formats for each DAI, I suppose MultiDAI would be responsible for conversion?
Would we really have a use case for that?

No, I don't think that we have different format for each DAI use case for now. However, I don't think we have to convert the format to a single format. To me, they are still two separate DAIs. We just want to enable them at the same time.

[slawblauciak]

Changes applied partially. Rest of the code will need to be changed to follow.
@plbossart @lgirdwood is this what you had in mind?

[slawblauciak]

@lgirdwood just realized, the way I made it doesn't allow us to have multiple formats in a single config, since those are not part of the flex array. Is that okay?

[lgirdwood]

@lgirdwood just realized, the way I made it doesn't allow us to have multiple formats in a single config, since those are not part of the flex array. Is that okay?

Sorry not, following, what do you mean by multiple formats ? Do you mean a single (SSP) DAI with multiple HW formats or do you mean multiple different DAIS with different formats ? @plbossart

I would expect us to be able to send an array of DAI configs that can target multiple DAIs with multiple HW config formats/clocks/modes, although our initial use case is a single DAI with multiple configs (i.e. SSP0 with config 0 (L/R I2S stereo), SSP0 with config 1 (TDM mode, 4 channels)...

[plbossart]

Sorry not, following, what do you mean by multiple formats ? Do you mean a single (SSP) DAI with multiple HW formats or do you mean multiple different DAIS with different formats ? @plbossart

I'd like support multiple DAIs of the same type but with the same list of formats each.
The case with DAIs of different types and different number of formats each is unmanageable.

[slawblauciak]

In order to have the ability to support multiple formats in a single IPC (the same way we support multiple configs in a single IPC) we'd need to have those formats also in a separate flex array. But then, we're literally duplicating the fields in a single struct. That makes the ABI far more confusing, because then it gets all kinds of blurry as to which field is to be actually used here.

[slawblauciak]

Applied changes and rebased.

[lgirdwood]

Should be 2 commits, one for IPC changes and the other for passing config index API updates.

[plbossart]

I could use more explanations on specific points below.

[plbossart]

I am a bit lost here.

The sof_ipc_dai_config already contains a 'sof_ipc_dai_base_params'.

So in the multi-dai case, is the baseline ignored?

Also in the size of this array determined by 'num_dais'?

[slawblauciak]

The base params inside of sof_ipc_dai_config is for the DAI you're directly communicating with.
The base params inside of the multidai is for the children of the multidai. And yes, the size is determined by num_dais.

To explain a bit more what's going on here:
The union of arrays contained within sof_ipc_dai_config contain structures, which don't convey basic DAI information (dai type and index). So those need to be passed separately. That's the reason behind the additional base params array.

[plbossart]

let's take the example of a multi-dai SounDwire/ALH case.

The dai_config would contain
hdr
base_params (multi_DAI type + index)

So then the multidai array should contain the ALH information for each child DAI, right?

The problem is that I don't see anything ALH-related in the multi_dai_params...only a stream_map and a base_params (which can only mean individual DAI and type). Where would the ALH stream_id passed?

[slawblauciak]

As I understood from the previous discussion, ALH related params would be in the flex array of the parent struct, which is in sof_ipc_dai_config.

To be specific: the first element of that flex array in this case would be the sof_ipc_dai_multidai_params. And directly after that, the very next members would be the children for multidai, namely sof_ipc_dai_alh_params.

[plbossart]

sorry, I don't get how this would work.
You don't have an array of unions, you have a union of arrays.

I don't think you can make the type of structure vary between array elements?

[slawblauciak]

I'm pretty sure you can, since from the standpoint of the compiler, all union members have a size of 0 anyway. So, the union is purely cosmetic, serving as a template for the code. Now that I think about this, the union doesn't do anything at all here, might as well get rid of it.

True, another way would be to have an array of unions, but I fear this is gonna cause some issues in the long run, because of varying padding that we're gonna end up with between the elements of the array. So, elements might no longer be after each other in the memory, and the space between them might change everytime somebody modifies the size of the largest member.

[slawblauciak]

Honestly, the more I think about this, the more I realize that there's no good way of actually having multiple configs for different DAI types in a single IPC.

• As you mentioned, having a union of flex arrays suggests we can only have 1 type, doing otherwise is indeed bad practice.
• Having a flex array of unions has padding issues I mentioned earlier.
• Having multiple flex arrays, without a union also doesn't smell right

I'd honestly abandon those efforts at this point and revert to the initial proposal: not touching the DAI config, simply adding a new DAI config member with its own union of flex arrays, limited to a single type.

@lgirdwood ^

[slawblauciak]

@lgirdwood @plbossart can we come to an agreement on this one?

[lgirdwood]

@slawblauciak is the sticking point the unions or the ALH ? I defer to @plbossart for all ALH and Soundwire stuff.

[q009]

It's the unions. I don't think it's sane to try to pack all the different configurations into a single IPC inside of sof_ipc_dai_config. As I mentioned, I'd rather do this by the book and just do as in the initial proposal: a new union member in sof_ipc_dai_config, one that would have its own, similar union, except of flex arrays.

[plbossart]

are you positive that this change is compatible with the previous solution? I haven't checked the sizes and it's hard to see with the diff format.

[slawblauciak]

Yeah. I ran tests on our side as well.

[jajanusz]

Is this PR active?

[slawblauciak]

Ah, nope, it is not.