ASoC: SOF: ipc3-topology: overwrite ALH dai index

[bardliao]

ALH dai index should be (sdw link id * 256 + data port id). Currently,
we get ALH dai index from topology. The sdw_params_stream callback
provides ALH dai index and we set dai_index to config->dai_index, but
not comp_dai->dai_index.
This patch sets comp_dai->dai_index = config->dai_index, too. So that
we don't need to rely on topology's dai index.

Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com

[ujfalusi]

How we are not counting dai indexes with this change? This patch does something else than the commit message tells?

[bardliao]

How we are not counting dai indexes with this change? This patch does something else than the commit message tells?

Currently, we calculate dai index in topology, like eval(UAJ_LINK * 256 + 2). That is to match the formula data.dai_index = (params_data->link_id << 8) | d->id; in sdw_params_stream. With this commit, we will get dai_index from sdw_params_stream and that's why we don't need to calculate dai index in topology.

[ujfalusi]

Right, but I don't see this calculation. Is it in the firmware's topology section or where?
It is confusing to mention something which is not visible.

Can there be disconnect (and issue) if the somewhere calculated dai_index is not matching with dai_index calculated and stored via sdw_params_stream?
And btw, I don't see any sdw_params_stream in this patch either..

[bardliao]

Right, but I don't see this calculation. Is it in the firmware's topology section or where? It is confusing to mention something which is not visible.

Yes, the calculation is in topology. We use link id and data port id to calculate the dai index and the formula is dai_index = (link_id * 256 + data_port_id)
It is not visible in the patch because the calculation is already on kernel side but we didn't use it.
I will try to explain it more in the commit message.

Can there be disconnect (and issue) if the somewhere calculated dai_index is not matching with dai_index calculated and stored via sdw_params_stream? And btw, I don't see any sdw_params_stream in this patch either..

Yes, the firmware will be disconnected if the dai index is not correct. The firmware needs dai index to get SoundWire link id and data port. Just like SSP dai index matches SSP port.
sdw_params_stream is in sound/soc/sof/intel/hda.c, and the patch doesn't touch it.

[ujfalusi]

@bardliao, hrm, so this was pretty much broken before, right?
I mean currently we use two numbers as dai_index, coming from independent sources and they are identical "by luck" (I know, it is a bit more than luck).
It would have been correct if:

struct sof_dai_private_data *dai_data = dai->private;
struct sof_ipc_comp_dai *comp_dai = dai_data->comp_dai;

config->dai_index = comp_dai->dai_index;
config->alh.stream_id = data->dai_data;

So taking the dai_index for both struct from the same source.

I would further clarify the commit message to:

ASoC: SOF: ipc3-topology: Ignore the dai_index for ALH from topology

Overwrite the dai_index in the struct sof_ipc_comp_dai from the topology file
with the platform code provided one, which is already used for the DAI config
IPC message.

With this change we can be sure that the dai_index is coming from the same
source.

or something.

[bardliao]

I would further clarify the commit message to:

ASoC: SOF: ipc3-topology: Ignore the dai_index for ALH from topology

Overwrite the dai_index in the struct sof_ipc_comp_dai from the topology file
with the platform code provided one, which is already used for the DAI config
IPC message.

With this change we can be sure that the dai_index is coming from the same
source.

or something.

Commit message updated.

[ranj063]

@bardliao I'd still argue that this change is not really needed. But if you insist, I'd go one step further and also remove the dai_index setting on line 1540 to absolutely make sure that we do not use the index coming from topology.

[plbossart]

@bardliao it would help to get a bit more context on what we do on topology.

I see that we use the arithmetic in multiple places

DAI_ADD(sof/pipe-dai-playback.m4,
	3, ALH, eval(AMP_1_LINK * 256 + 2),

DAI_CONFIG(ALH, eval(UAJ_LINK * 256 + 2), 0, `SDW'eval(UAJ_LINK)`-Playback',
	ALH_CONFIG(ALH_CONFIG_DATA(ALH, eval(UAJ_LINK * 256 + 2), 48000, 2)))

I have no idea which ones of those evals are required....

[bardliao]

@bardliao it would help to get a bit more context on what we do on topology.

I see that we use the arithmetic in multiple places

DAI_ADD(sof/pipe-dai-playback.m4,
	3, ALH, eval(AMP_1_LINK * 256 + 2),

DAI_CONFIG(ALH, eval(UAJ_LINK * 256 + 2), 0, `SDW'eval(UAJ_LINK)`-Playback',
	ALH_CONFIG(ALH_CONFIG_DATA(ALH, eval(UAJ_LINK * 256 + 2), 48000, 2)))

I have no idea which ones of those evals are required....

 DAI_ADD(sof/pipe-dai-playback.m4,
 	3, ALH, eval(AMP_1_LINK * 256 + 2),

The above is for dai index, so it is required.

And the other seems to be not required. I changed it and the topology still worked.

[bardliao]

@ujfalusi The commit message is updated.

[ranj063]

@bardliao do we really need this change? There won't be any new IPC3 topologies and I doubt we will be removing the index calculation from the existing topologies either.

[bardliao]

@bardliao do we really need this change? There won't be any new IPC3 topologies and I doubt we will be removing the index calculation from the existing topologies either.

I won't change the existing topologies. But this change will let people know where does the dai index from, and be consistent between IPC3 and IPC4.