topology: fix bad parameters in generic dmic dai add #2089

juimonen · 2019-11-12T18:48:39Z

Intel generic dmic pipelines have bad extra parameters in
their dai add macros, so remove them.

Signed-off-by: Jaska Uimonen jaska.uimonen@intel.com

plbossart

comments below

plbossart · 2019-11-12T19:21:52Z

tools/topology/platform/intel/intel-generic-dmic.m4

@@ -46,14 +46,14 @@ dnl     deadline, priority, core, time_domain)
 DAI_ADD(sof/pipe-dai-capture.m4,
 	DMIC_PIPELINE_48k_ID, DMIC, 0, dmic01,
 	concat(`PIPELINE_SINK_', DMIC_PIPELINE_48k_ID), 2, s32le,
-	1000, 0, 0, 48000, 48000, 48000)


those parameters are obviously wrong, but shouldn't we use the TIMER now?
The DMIC is clock master so there's no real benefit in using DMA interrupts here, is there?

hmm, ok someone needs to jump in here, because I don't know this area.... maybe before the wrong value 16000 something has been just interpreted as != 0 -> so timer?

@tlauda @lgirdwood see above?

@juimonen these are wrong, please validate against the DAI_ADD defn. It looks like we are also missing some macro defs for Time domain, these can be added too in order to avoid ambiguous definitions.

dnl DAI_ADD(pipeline, dnl pipe id, dai type, dai_index, dai_be, dnl buffer, periods, format, dnl period , priority, core, time_domain) define(`DAI_ADD', `undefine(`PIPELINE_ID')' `undefine(`DAI_TYPE')' `undefine(`DAI_INDEX')' `undefine(`DAI_BE')' `undefine(`DAI_BUF')' `undefine(`DAI_PERIODS')' `undefine(`DAI_FORMAT')' `undefine(`SCHEDULE_PERIOD')' `undefine(`SCHEDULE_PRIORITY')' `undefine(`SCHEDULE_CORE')' `undefine(`SCHEDULE_TIME_DOMAIN')' `define(`PIPELINE_ID', $2)' `define(`DAI_TYPE', STR($3))' `define(`DAI_INDEX', STR($4))' `define(`DAI_BE', $5)' `define(`DAI_BUF', $6)' `define(`DAI_NAME', $3$4)' `define(`DAI_PERIODS', $7)' `define(`DAI_FORMAT', $8)' `define(`SCHEDULE_PERIOD', $9)' `define(`SCHEDULE_PRIORITY', $10)' `define(`SCHEDULE_CORE', $11)' `define(`SCHEDULE_TIME_DOMAIN', $12)' `include($1)' `DEBUG_DAI($3, $4)'

@lgirdwood Pierre's questions was should this be TIMER or DMA...? And I guess it is TIMER as I see only that defined anywhere. And then I see DAI_ADD macros without either TIMER or DMA that compiles to "" in conf files, which is alsp probably not good.... So should those then be DMA?

and forget last comment about not using, was not grepping for tokens... so question is should the empty "" be DOMAIN_DMA?

So in below topologies the TIME_DOMAIN is not specified so it is "":

sof-cht-max98090.m4
sof-cht-nocodec.m4
sof-cht-rt5682.m4
sof-imx8qxp-nocodec.m4
sof-imx8qxp-nocodec-sai.m4
sof-imx8qxp-cs42888.m4
sof-imx8qxp-wm8960.m4
sof-hsw-rt5640.m4
sof-bdw-codec.m4
sof-cht-src-50khz-pcm512x.m4
platform/intel/intel-generic-dmic.m4
sof-byt-codec.m4

Any objections I just change it to TIMER in all?

@juimonen The imx8qxp ones are 100% DMA-driven, so TIMER might/will break them. No words on the other ones though.

@juimonen all topologies use the SOC clock master so there's no problem using the TIMER
the two cases where the codec is master is wm8804 and some versions of pcm512x, those would need to remain DMA based.

In general when the DAI uses 'codec_slave' we can use the TIMER.

sof-apl-wm8804.m4: SSP_CLOCK(bclk, 3072000, codec_master), sof-apl-wm8804.m4: SSP_CLOCK(fsync, 48000, codec_master), sof-imx8qxp-wm8960.m4: SAI_CLOCK(bclk, 3072000, codec_master), sof-imx8qxp-wm8960.m4: SAI_CLOCK(fsync, 48000, codec_master),

@tlauda any comment here ?

juimonen · 2019-11-14T18:06:19Z

@plbossart @paulstelian97 thanks for the info and PR updated. All imx ones should have DMA domain now. I now explicitly defined either TIMER or DMA everywhere. I also found couple of related issues which are separate commits... I have no idea how those topologies have ever worked with all pipelines.

ranj063 · 2019-11-14T19:22:43Z

@juimonen Would you mind splitting up the commit per tplg file? It is easy to figure out which commit caused issues when debugging that way

juimonen · 2019-11-15T09:30:31Z

@ranj063 @plbossart @paulstelian97 PR now divided per topology file basis. @plbossart I needed to change byt and cht based topology timers to DMA, because CI just went crazy otherwise...

tools/topology/sof-imx8qxp-cs42888.m4

juimonen · 2019-11-15T10:47:57Z

@paulstelian97 updated, hope it is more clear now.

paulstelian97

Alright, looks good to me now.

plbossart · 2019-11-15T14:36:21Z

@ranj063 @plbossart @paulstelian97 PR now divided per topology file basis. @plbossart I needed to change byt and cht based topology timers to DMA, because CI just went crazy otherwise...

yes, my baytrail device went in the weeds with your PR.
I think we should change from DMA to TIMER one platform at a time, separately, with tests done by different people, not just CI.

plbossart

let's keep broadwell the way it is and remove haswell

plbossart · 2019-11-22T21:40:09Z

tools/topology/sof-bdw-codec.m4


 # capture DAI is SSP0 using 2 periods
 # Buffers use s24le format, 1000us deadline on core 0 with priority 0
 DAI_ADD(sof/pipe-dai-capture.m4,
 	2, SSP, 0, Codec,
 	PIPELINE_SINK_2, 2, s24le,
-	1000, 0, 0)
+	1000, 0, 0, SCHEDULE_TIME_DOMAIN_TIMER)


probably better to keep DMAs on broadwell, this is difficult enough with DMAs alone and IIRC there are no external timers?

plbossart · 2019-11-22T21:41:34Z

tools/topology/sof-hsw-rt5640.m4


 # capture DAI is SSP0 using 2 periods
 # Buffers use s24le format, 1000us deadline on core 0 with priority 0
 DAI_ADD(sof/pipe-dai-capture.m4,
 	2, SSP, 0, Codec,
 	PIPELINE_SINK_2, 2, s24le,
-	1000, 0, 0)
+	1000, 0, 0, SCHEDULE_TIME_DOMAIN_TIMER)


keep as DMA.
we need to remove this topology anyways, hsw is not supported

plbossart · 2019-11-22T21:42:27Z

tools/topology/sof-bdw-codec.m4

-	8000, 96000, 48000)
+	8000, 96000, 48000,
+	SCHEDULE_TIME_DOMAIN_TIMER,
+	PIPELINE_PLAYBACK_SCHED_COMP_1)


leave broadwell with DMA

plbossart · 2019-11-22T21:42:51Z

tools/topology/sof-hsw-rt5640.m4

-	8000, 96000, 48000)
+	8000, 96000, 48000,
+	SCHEDULE_TIME_DOMAIN_TIMER,
+	PIPELINE_PLAYBACK_SCHED_COMP_1)


remove this file, not supported

Fix errorneous parameters in DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Add time domain DMA explicitly to DAI_ADD and pipeline. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Add time domain DMA explicitly to DAI_ADD and pipeline. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

sof-cml-rt5682 dmic definition has some extra parameters that should be removed. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Media pipe is missing the scheduling component, so move it after the dai definition and add the scheduling component. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Remove as unsupported platform. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

juimonen · 2019-11-25T10:53:44Z

@plbossart updated. bdw changed to dma, hsw removed.

plbossart

Thanks @juimonen

lgirdwood · 2019-11-25T16:12:08Z

Internal CI test passed - all green.

juimonen requested review from lbetlej, lgirdwood, mmaka1 and plbossart as code owners November 12, 2019 18:48

juimonen mentioned this pull request Nov 12, 2019

IPC error + kernel oops on WHL w/ HDA/DMIC and 1.3 signed firmware thesofproject/linux#1417

Closed

plbossart reviewed Nov 12, 2019

View reviewed changes

juimonen force-pushed the dmic_dai_fix branch from be48808 to 614b5ec Compare November 14, 2019 18:03

juimonen requested review from ranj063 and xiulipan as code owners November 14, 2019 18:03

juimonen force-pushed the dmic_dai_fix branch from 614b5ec to 68c35f5 Compare November 14, 2019 19:14

juimonen force-pushed the dmic_dai_fix branch 3 times, most recently from a2fbe54 to 179d639 Compare November 15, 2019 08:48

paulstelian97 reviewed Nov 15, 2019

View reviewed changes

tools/topology/sof-imx8qxp-cs42888.m4 Show resolved Hide resolved

juimonen force-pushed the dmic_dai_fix branch from 179d639 to 7ed5cc7 Compare November 15, 2019 10:46

paulstelian97 approved these changes Nov 15, 2019

View reviewed changes

plbossart requested changes Nov 22, 2019

View reviewed changes

Jaska Uimonen added 8 commits November 25, 2019 11:25

topology: intel-generic-dmic.m4: fix dai parameters

a438feb

Fix errorneous parameters in DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-bdw-codec.m4: add time domain DMA

46afa5a

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-cht-nocodec.m4: add time domain DMA

13c2de7

Add time domain DMA explicitly to DAI_ADD and pipeline. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-cht-rt5682.m4: add time domain DMA

a8c73ea

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-byt-codec.m4: add time domain DMA

d542258

Add time domain DMA explicitly to DAI_ADD and pipeline. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-cht-max98090.m4: add time domain DMA

5f4aaf4

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-cht-src-50khz-pcm512x.m4: add time domain DMA

e0f67a6

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-imx8qxp-cs42888.m4: add time domain DMA

5b9e6f7

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

Jaska Uimonen added 6 commits November 25, 2019 11:28

topology: sof-imx8qxp-nocodec-sai.m4: add time domain DMA

b4b66b8

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-imx8qxp-nocodec.m4: add time domain DMA

64cf2dc

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-imx8qxp-wm8960.m4: add time domain DMA

912459b

Add time domain DMA explicitly to DAI_ADD. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-cml-rt5682.m4: remove extra parameters

6bccee4

sof-cml-rt5682 dmic definition has some extra parameters that should be removed. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-bdw-codec.m4: add scheduling dai to media pipe

1ec17c9

Media pipe is missing the scheduling component, so move it after the dai definition and add the scheduling component. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

topology: sof-hsw-rt5640.m4: remove topology

6882461

Remove as unsupported platform. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>

juimonen force-pushed the dmic_dai_fix branch 2 times, most recently from 6ed3d9a to 5bf2207 Compare November 25, 2019 09:34

juimonen requested a review from jajanusz as a code owner November 25, 2019 09:34

juimonen force-pushed the dmic_dai_fix branch from 5bf2207 to 6882461 Compare November 25, 2019 09:45

plbossart approved these changes Nov 25, 2019

View reviewed changes

lgirdwood merged commit db7671a into thesofproject:master Nov 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

topology: fix bad parameters in generic dmic dai add #2089

topology: fix bad parameters in generic dmic dai add #2089

juimonen commented Nov 12, 2019

plbossart left a comment

plbossart Nov 12, 2019 •

edited

Loading

juimonen Nov 12, 2019

juimonen Nov 13, 2019

lgirdwood Nov 14, 2019

juimonen Nov 14, 2019

juimonen Nov 14, 2019

juimonen Nov 14, 2019

paulstelian97 Nov 14, 2019

plbossart Nov 14, 2019

lgirdwood Nov 15, 2019

juimonen commented Nov 14, 2019

ranj063 commented Nov 14, 2019

juimonen commented Nov 15, 2019

juimonen commented Nov 15, 2019

paulstelian97 left a comment

plbossart commented Nov 15, 2019

plbossart left a comment

plbossart Nov 22, 2019

plbossart Nov 22, 2019

plbossart Nov 22, 2019

plbossart Nov 22, 2019

juimonen commented Nov 25, 2019

plbossart left a comment

lgirdwood commented Nov 25, 2019

topology: fix bad parameters in generic dmic dai add #2089

topology: fix bad parameters in generic dmic dai add #2089

Conversation

juimonen commented Nov 12, 2019

plbossart left a comment

Choose a reason for hiding this comment

plbossart Nov 12, 2019 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juimonen commented Nov 14, 2019

ranj063 commented Nov 14, 2019

juimonen commented Nov 15, 2019

juimonen commented Nov 15, 2019

paulstelian97 left a comment

Choose a reason for hiding this comment

plbossart commented Nov 15, 2019

plbossart 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

juimonen commented Nov 25, 2019

plbossart left a comment

Choose a reason for hiding this comment

lgirdwood commented Nov 25, 2019

plbossart Nov 12, 2019 •

edited

Loading