DMIC: Fixed length IPC #2610

singalsu · 2020-03-23T16:07:19Z

No description provided.

singalsu · 2020-03-23T16:29:38Z

There's an open with ABI minor version. Seems like firmware is ahead by one in minor version and I think the FW and kernel side PRs for DMIC IPC change should propose the same ABI minor version.

singalsu · 2020-03-23T17:02:02Z

There's good feedback for kernel side PR. I'll soon update both PRs the same way.

singalsu · 2020-03-23T17:44:56Z

New version to sync with kernel. I changed the macro name to SOF_DAI_INTEL_DMIC_NUM_CTRL. It helps to avoid to confuse this with microphones or channels count.

lgirdwood · 2020-03-24T10:23:00Z

src/include/ipc/dai-intel.h

Whats the rational behind this change, it's missing from commit message.

Yep, need to add.

There's fair amount of comments on kernel side and I'll do the same updates for firmware too.

Commit text is now updated and header file comments are in sync with kernel.

Sorry, diffing revealed a difference of one "." so new version again. There's also lot of other changes but I'll complete my part.

Done. Finally same DMIC configuration headers as in kernel PR version.

src/include/kernel/abi.h

lgirdwood

Needs ABI classification

This patch was created due to kernel driver code enhancement. The DMIC was the only DAI type that used variable length arrays for configuration IPC. It complicated the driver topology parsing more than necessary. Since the header files for IPC are common also the firmware side needs similar changes. The flexible array member pdm[] is changed into a fixed array of four that is the max. number of stereo PDM controllers in the current Intel platforms. The DMIC driver code needs minor changes to correctly handle the changed IPC struct size. The IPC logic remains as before: The IPC PDM entries do not correspond to hardware PDM controllers. They are applied per provided index in each entry. Depending on number of PDM controllers there is now zeros data after the last defined controller that will be omitted. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This patch fixes the topo in word "microphone". Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Since the SOF kernel and firmware headers need to be the same the rename is also done on firmware side. The fifo_bits_b parameter was deprecated in firmware ABI version 3.0.1 and the value has been ignored. Since the kernel sets this in case of ABI version 3.0.0 or earlier the name reserved_2 is not appropriate. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

lgirdwood · 2020-03-26T16:48:29Z

SOFCI TEST

ranj063 · 2020-03-26T17:57:13Z

src/include/ipc/dai-intel.h

 * version number used in configuration data is checked vs. version used by
 * device driver src/drivers/dmic.c need to match. It is incremented from
 * initial value 1 if updates done for the to driver would alter the operation
- * of the microhone.


This I found hilarious :) you left a typo in the word typo itself in the commit message

Haha, I only now noticed it!! Can I still fix this without restarting the approval process?

@lgirdwood @kv2019i Should I fix this?

@singalsu no need I think. its just the commit message.

Good, I'll leave this funny bit into the history.

ranj063

looks good @singalsu. I commented on a minor typo on the commit message in the second patch. But no biggie!

lgirdwood · 2020-03-26T19:25:13Z

Jenkins all green

lgirdwood · 2020-04-03T10:20:55Z

SOFCI TEST

lgirdwood · 2020-04-03T10:23:27Z

@singalsu if both this PR and kernel one are approved we can merge at 15 ? Providing bot PRs dont change the ABI minor (which is already at 15). @kv2019i kernel PR good for you at 15 too ?

kv2019i · 2020-04-03T10:28:10Z

@lgirdwood wrote:

@singalsu if both this PR and kernel one are approved we can merge at 15 ? Providing bot PRs dont change the ABI minor (which is already at 15). @kv2019i kernel PR good for you at 15 too ?

Ack, the kernel side PR (thesofproject/linux#1924) is all good to go. Just needs to be refreshed on latest sof-dev.
UPDATE: @lgirdwood kernel PR now updated to ABI15 and ready to go after tests complete.

kv2019i · 2020-04-08T07:46:21Z

@lgirdwood Are we good to go? Kernel side is approved and ready thesofproject/linux#1924

lgirdwood · 2020-04-08T08:57:04Z

SOFCI TEST

lgirdwood · 2020-04-08T08:58:20Z

@kv2019i sorry, it looks like this one was missing from full CI tests. rerunning.

lgirdwood · 2020-04-08T18:30:47Z

Travis all green, but showing pending. Jenkins known issues. @kv2019i merged, good for kernel side now.

kv2019i · 2020-04-09T08:25:50Z

Travis all green, but showing pending. Jenkins known issues. @kv2019i merged, good for kernel side now.

Thanks @lgirdwood , kernel merged now as well.

lgirdwood · 2020-04-09T08:27:55Z

@kv2019i great :) I will tag v1.5-rc1 now.

singalsu requested review from lgirdwood, mmaka1, paulstelian97, plbossart and ranj063 March 23, 2020 16:07

singalsu requested a review from a team as a code owner March 23, 2020 16:07

singalsu mentioned this pull request Mar 23, 2020

Remove variable-length arrays for DMIC params thesofproject/linux#1801

Closed

singalsu added the ABI ABI change involved label Mar 23, 2020

singalsu force-pushed the dmic_fixed_length_ipc branch from 7c445bd to 93c20d8 Compare March 23, 2020 17:41

lgirdwood reviewed Mar 24, 2020

View reviewed changes

src/include/kernel/abi.h Outdated Show resolved Hide resolved

lgirdwood requested changes Mar 24, 2020

View reviewed changes

singalsu force-pushed the dmic_fixed_length_ipc branch 2 times, most recently from b5dbb3a to b87002e Compare March 25, 2020 17:38

singalsu added 3 commits March 25, 2020 19:49

DMIC: Fix a typo in header file comment text

34cccc3

This patch fixes the topo in word "microphone". Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

singalsu force-pushed the dmic_fixed_length_ipc branch from b87002e to f948977 Compare March 25, 2020 17:52

singalsu requested a review from lgirdwood March 25, 2020 17:56

kv2019i mentioned this pull request Mar 26, 2020

ASoC: SOF: DMIC: Fixed length IPC thesofproject/linux#1924

Merged

ranj063 reviewed Mar 26, 2020

View reviewed changes

ranj063 approved these changes Mar 26, 2020

View reviewed changes

lgirdwood approved these changes Mar 26, 2020

View reviewed changes

lgirdwood merged commit 37850f5 into thesofproject:master Apr 8, 2020

singalsu deleted the dmic_fixed_length_ipc branch September 15, 2022 13:14

DMIC: Fixed length IPC #2610

DMIC: Fixed length IPC #2610

Uh oh!

Conversation

singalsu commented Mar 23, 2020

Uh oh!

singalsu commented Mar 23, 2020

Uh oh!

singalsu commented Mar 23, 2020

Uh oh!

singalsu commented Mar 23, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Mar 26, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranj063 left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Mar 26, 2020

Uh oh!

lgirdwood commented Apr 3, 2020

Uh oh!

lgirdwood commented Apr 3, 2020

Uh oh!

kv2019i commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kv2019i commented Apr 8, 2020

Uh oh!

lgirdwood commented Apr 8, 2020

Uh oh!

lgirdwood commented Apr 8, 2020

Uh oh!

lgirdwood commented Apr 8, 2020

Uh oh!

kv2019i commented Apr 9, 2020

Uh oh!

lgirdwood commented Apr 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kv2019i commented Apr 3, 2020 •

edited

Loading