Loopback mode kcontrol support #13

xiulipan · 2018-06-28T11:40:11Z

This patch add a bit field token config contains loopback mode kcontrol bit for topology.
Should work with sof and kernel PR.
work with
thesofproject/sof#26
thesofproject/linux#10

lgirdwood

These changes make this generic enough so that it will be easy to do liekwise for DMIC, HDA, SDW etc

lgirdwood · 2018-06-28T12:13:27Z

topology/sof/tokens.m4

@@ -18,6 +18,7 @@ SectionVendorTokens."sof_dai_tokens" {
 	SOF_TKN_DAI_DMAC_CONFIG			"153"
 	SOF_TKN_DAI_TYPE			"154"
 	SOF_TKN_DAI_INDEX			"155"
+	SOF_TKN_DAI_EXT_CONFIG			"157"


Is this for bespoke data ? If so, please change EXT -> BESPOKE

lgirdwood · 2018-06-28T12:42:04Z

topology/m4/dai.m4

@@ -24,6 +24,7 @@ define(`W_DAI_OUT',
 `	tokens "sof_dai_tokens"'
 `	tuples."word" {'
 `		SOF_TKN_DAI_INDEX'	$2
+`		SOF_TKN_DAI_EXT_CONFIG'	ifelse(`DAI_EXTRA_CONFIG', DAI_EXTRA_CONFIG, "0", "`DAI_EXTRA_CONFIG'")


It's probably better to

define some SSP specific data macro in SSP.m4 like SSP_CONFIG_DATA i.e. SSP_BESPOKE_DATA

Add an extra parameter to W_DAI_IN/OUT to take any bespoke DAI data

in pipe-dai-playback.m4 and friends pass in DAI_BESPOKE_DATA as last param to W_DAI_OUT

in PIPELINE_PCM_DAI_ADD() add param to pass in any BESPOKE data

OK, will follow that.

@lrgirdwo
I tried to follow the suggestion and found that add some macro like SSP_CONFIG_DATA in SSP.m4 won't be helpful. If W_DAI_OUT got SSP specific data, then DMIC and other DAIs will also need the same tuples actually this filed is not specific but the config value is specific.

I think it is better to have the token be merged in existing DAI tuples as the token. Or to have some DAI generic macro to generate that filed for all DAIs and we can pass different value form the SSP.m4 or DMIC.m4.

Can you share the patch so I can check.

xiulipan · 2018-07-03T10:33:58Z

@lgirdwood
Update the patch set for topology.
Need to pass through the config value.

lgirdwood

The idea is the we can pass vendor tuples/data or an empty macro as the last param.

lgirdwood · 2018-07-03T10:43:47Z

topology/platform/common/ssp.m4

@@ -43,4 +43,8 @@ define(`SSP_CONFIG_DATA',
 `}'
 )

+dnl SSP dai have loopback mode so set the SSP_BESPOKEN to 1
+
+define(`SSP_BESPOKEN_CONFIG', 1)


This should be SSP_BESPOKE_CONFIG and should define vendor tuples and vendor data e.g

define(SSP_BESPOKE_CONFIG', SectionVendorTuples."'N_DAI_OUT($2)_tuples_w_comp" {' tokens "sof_comp_tokens"'
tuples."word" {' SOF_TKN_COMP_PERIOD_SINK_COUNT' STR($5)
SOF_TKN_COMP_PERIOD_SOURCE_COUNT' STR($6) SOF_TKN_COMP_PRELOAD_COUNT' STR($7)
}' }'
SectionData."'N_DAI_OUT($2)_data_w_comp" {'
tuples "'N_DAI_OUT($2)_tuples_w_comp"'
`}'

Yes, the idea is good. The SectionData and SectionVendorTuples is easy to create.
If we need to add SectionData into the SectionWidget, it will need more ifelse to check if we have such SectionData.
Is it OK to have more ifelse?

yes or you could define an empty data section that can be passed for DAIs with no bespoke data, that way you dont need any conditional code.

Signed-off-by: Pan Xiuli <[email protected]>

Add DAI bespoke config data for dai_in widget, bind DAI bespoke config to DAI comp. Signed-off-by: Pan Xiuli <[email protected]>

xiulipan · 2018-07-04T05:23:40Z

@lgirdwood
New version with vendor data and tuples.

lgirdwood

Very good, but some minor changes can make this even more useful and mean it would be easy to additionally add SSP expert mode

lgirdwood · 2018-07-04T12:29:27Z

topology/platform/common/ssp.m4

+`SectionVendorTuples."$1$2.OUT_tuples_bespoke_w" {'
+`	tokens "sof_dai_tokens"'
+`	tuples."word" {'
+`		SOF_TKN_DAI_BESPOKE_CONFIG'	ifelse($3, `', "0", STR($3))


The presence of this data implies that there a bespoke config data available so there is no need for a token that says bespoke data available. I'd expect to see tokens in here things that are not generic and specific to SSP only like SSP clock source, M/N dividers, etc

You could even additionally create an advance mode SSP_BESPOKE_REGS macro that has a token for each register so that register values can be assigned here.

The user could then decide if he was using export mode or normal bespoke data, since we are always passing a macro to the DAI macro to expand.

@lgirdwood
This DATA is bind to the SectionWidget for DAI COMP.
We already had some DATA for SectionBE for BE DAI to config SSP clock and other things.
Do you mean we may have other run-time reconfigurable setting to SSP?
That we can use the topology to create kcontrol for some debug use config,

Yes, the intention is that we can bind multiple data objects to the widget each containing multiple tuples (but lets start with just 1 data object today).

he "create loopback kcontrol" flag will be a boolean token in this object and so will any member of the following structure that cant be defined in the standard ALSA topology structure for BE/DAI.

struct sof_ipc_dai_ssp_params {
uint16_t mode; // FIXME: do we need this?
uint16_t mclk_id;

uint32_t mclk_rate; /* mclk frequency in Hz */ uint32_t fsync_rate; /* fsync frequency in Hz */ uint32_t bclk_rate; /* bclk frequency in Hz */ /* TDM */ uint32_t tdm_slots; uint32_t rx_slots; uint32_t tx_slots; /* data */ uint32_t sample_valid_bits; uint16_t tdm_slot_width; uint16_t reserved2; /* alignment */ /* MCLK */ uint32_t mclk_direction; uint32_t mclk_keep_active; uint32_t bclk_keep_active; uint32_t fs_keep_active; //uint32_t quirks; // FIXME: is 32 bits enough ? /* private data, e.g. for quirks */ //uint32_t pdata[10]; // FIXME: would really need ~16 u32

} attribute((packed));

e.g. checking the driver I can see we parse for SOF_TKN_INTEL_SSP_MCLK_KEEP_ACTIVE but we don't set this in any private data yet in topology. This token and any others specific to SSP should be added to this data object alongside the "create lbm kcontrol".

xiulipan · 2018-07-06T09:47:03Z

@lgirdwood
SOF_TKN_INTEL_SSP_MCLK_KEEP_ACTIVE is setting mclk_keep_active in sof_ipc_dai_ssp_params.
This token is in SectionBE and the LBM token is in SectionWidget, should they be in the same Section if we want they both in same structure?

Let me have some clarify for other issues:

we will not use bit filed flag now, a token in the tuple is needed now.
should we create a new token filed called sof_ssp_bespoke_tokens or just merge it into sof_ssp_tokens?
If we really need a new token tuple date to contain these tokens? The kernel seems did not care it.

lgirdwood · 2018-07-06T10:06:35Z

The bespoke configuration options only need to be added to the bespoke macro if they are not in any ALSA topology structure (as there is no other way to to load them).

yes.
We need a new token for every bespoke configuration option that is not in ALSA topology format.
The kernel does not care for most of the bespoke tokens, we pass the data directly to the FW in this case. The kernel should ignore tokens it does not care about i.e. ignore SSP_ENABLE_CLOCK_REQ_A, but dont ignore SSP_CREATE_LBM_KCONTROL

This was referenced Jun 28, 2018

Loopback mode kcontrol support thesofproject/linux#10

Closed

Loopback mode kcontrol support thesofproject/sof#26

Closed

lgirdwood suggested changes Jun 28, 2018

View reviewed changes

xiulipan force-pushed the lbm branch from db80584 to 765d9d9 Compare July 3, 2018 10:31

lgirdwood suggested changes Jul 3, 2018

View reviewed changes

xiulipan added 2 commits July 4, 2018 10:48

topology: Add SOF_TKN_DAI_BESPOKE_CONFIG for DAI bespoke config

9d6283e

Signed-off-by: Pan Xiuli <[email protected]>

topology: add SSP bespoke config data for SSP dais

8f27227

Add DAI bespoke config data for dai_in widget, bind DAI bespoke config to DAI comp. Signed-off-by: Pan Xiuli <[email protected]>

xiulipan force-pushed the lbm branch from 765d9d9 to 8f27227 Compare July 4, 2018 05:07

lgirdwood suggested changes Jul 4, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Loopback mode kcontrol support #13

Loopback mode kcontrol support #13

xiulipan commented Jun 28, 2018 •

edited

Loading

lgirdwood left a comment

lgirdwood Jun 28, 2018

xiulipan Jun 29, 2018

lgirdwood Jun 28, 2018

xiulipan Jun 29, 2018

xiulipan Jul 3, 2018

lgirdwood Jul 3, 2018

xiulipan commented Jul 3, 2018

lgirdwood left a comment

lgirdwood Jul 3, 2018

xiulipan Jul 3, 2018

lgirdwood Jul 3, 2018

xiulipan commented Jul 4, 2018

lgirdwood left a comment

lgirdwood Jul 4, 2018

lgirdwood Jul 4, 2018

xiulipan Jul 5, 2018

lgirdwood Jul 5, 2018

xiulipan commented Jul 6, 2018

lgirdwood commented Jul 6, 2018

Loopback mode kcontrol support #13

Are you sure you want to change the base?

Loopback mode kcontrol support #13

Conversation

xiulipan commented Jun 28, 2018 • edited Loading

lgirdwood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiulipan commented Jul 3, 2018

lgirdwood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiulipan commented Jul 4, 2018

lgirdwood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiulipan commented Jul 6, 2018

lgirdwood commented Jul 6, 2018

xiulipan commented Jun 28, 2018 •

edited

Loading