Skip to content

DMIC: Fixed length IPC #2610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgirdwood merged 3 commits into from
Apr 8, 2020
Merged

DMIC: Fixed length IPC #2610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c9d321
DMIC: Change IPC in dmic_set_config() to use a fixed length struct
singalsu Mar 20, 2020
34cccc3
DMIC: Fix a typo in header file comment text
singalsu Mar 20, 2020
f948977
DMIC: Rename DMIC IPC struct field reserved_2
singalsu Mar 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions src/drivers/intel/cavs/dmic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1147,9 +1147,7 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config)
* "prm" is initialized with default params for all HW controllers
*/
if (!dmic_prm[0]) {
size = sizeof(struct sof_ipc_dai_dmic_params)
+ DMIC_HW_CONTROLLERS
* sizeof(struct sof_ipc_dai_dmic_pdm_ctrl);
size = sizeof(struct sof_ipc_dai_dmic_params);
dmic_prm[0] = rzalloc(SOF_MEM_ZONE_SYS_RUNTIME, 0,
SOF_MEM_CAPS_RAM,
DMIC_HW_FIFOS * size);
Expand All @@ -1162,11 +1160,11 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config)
((uint8_t *)dmic_prm[i - 1] + size);
}

/* Copy the new DMIC params to persistent. The last request
* determines the parameters.
/* Copy the new DMIC params header (all but not pdm[]) to persistent.
* The last arrived request determines the parameters.
*/
ret = memcpy_s(dmic_prm[di], sizeof(*dmic_prm[di]), &config->dmic,
sizeof(struct sof_ipc_dai_dmic_params));
offsetof(struct sof_ipc_dai_dmic_params, pdm));
assert(!ret);

/* copy the pdm controller params from ipc */
Expand Down
15 changes: 10 additions & 5 deletions src/include/ipc/dai-intel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@
/* bclk idle */
#define SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_IDLE_HIGH BIT(5)

/* DMIC max. four controllers for eight microphone channels */
#define SOF_DAI_INTEL_DMIC_NUM_CTRL 4

/* SSP Configuration Request - SOF_IPC_DAI_SSP_CONFIG */
struct sof_ipc_dai_ssp_params {
uint32_t reserved0;
Expand Down Expand Up @@ -143,7 +146,7 @@ struct sof_ipc_dai_dmic_pdm_ctrl {
* 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.
Copy link
Collaborator

@ranj063 ranj063 Mar 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

@singalsu singalsu Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

@singalsu singalsu Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgirdwood @kv2019i Should I fix this?

Copy link
Collaborator

@ranj063 ranj063 Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

@singalsu singalsu Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

* of the microphone.
*
* Note: The microphone clock (pdmclk_min, pdmclk_max, duty_min, duty_max)
* parameters need to be set as defined in microphone data sheet. E.g. clock
Expand All @@ -168,6 +171,7 @@ struct sof_ipc_dai_dmic_pdm_ctrl {
* met. The unit for both is microseconds (us). Exceed of 100 ms will be
* treated as an error.
*/

struct sof_ipc_dai_dmic_params {
uint32_t reserved0;
uint32_t driver_ipc_version; /**< Version (1..N) */
Expand All @@ -178,12 +182,13 @@ struct sof_ipc_dai_dmic_params {
uint32_t fifo_fs; /**< FIFO sample rate in Hz (8000..96000) */
uint32_t reserved_1; /**< Reserved */
uint16_t fifo_bits; /**< FIFO word length (16 or 32) */
uint16_t reserved_2; /**< Reserved */
uint16_t fifo_bits_b; /**< Deprecated since firmware ABI 3.0.1 */

uint16_t duty_min; /**< Min. mic clock duty cycle in % (20..80) */
uint16_t duty_max; /**< Max. mic clock duty cycle in % (min..80) */

uint32_t num_pdm_active; /**< Number of active pdm controllers */
uint32_t num_pdm_active; /**< Number of active pdm controllers. */
/**< Range is 1..SOF_DAI_INTEL_DMIC_NUM_CTRL */

uint32_t wake_up_time; /**< Time from clock start to data (us) */
uint32_t min_clock_on_time; /**< Min. time that clk is kept on (us) */
Expand All @@ -192,8 +197,8 @@ struct sof_ipc_dai_dmic_params {
/* reserved for future use */
uint32_t reserved[5];

/**< variable number of pdm controller config */
struct sof_ipc_dai_dmic_pdm_ctrl pdm[0];
/**< PDM controllers configuration */
struct sof_ipc_dai_dmic_pdm_ctrl pdm[SOF_DAI_INTEL_DMIC_NUM_CTRL];
Copy link
Member

@lgirdwood lgirdwood Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

@singalsu singalsu Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, need to add.

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

Copy link
Collaborator Author

@singalsu singalsu Mar 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

@singalsu singalsu Mar 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

@singalsu singalsu Mar 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} __attribute__((packed));

#endif /* __IPC_DAI_INTEL_H__ */