intel_adsp: dmic: Refactoring of blob parsing function #60172
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
softwarecki requested review from kv2019i, marc-hb, serhiy-katsyuba-intel and tmleman softwarecki requested review from abonislawski, andyross, dcpleung, marcinszkudlinski and nashif as code owners 
zephyrbot requested review from MaureenHelm, aborisovich, ceolin, juimonen, lgirdwood and lyakh softwarecki requested review from jxstelter and removed request for MaureenHelm, aborisovich, ceolin, juimonen, lgirdwood and lyakh softwarecki force-pushed the dmic-refactor-pr branch from ef37a85 to 2d33c77 Compare 
singalsu reviewed Aug 16, 2023
drivers/dai/intel/dmic/dmic_nhlt.c Outdated
There was a problem hiding this comment.
I agree that using a reserved bit could be just warning. But exceed of a max value like BFTH feels like it should be an error because it results to incorrect operation.
drivers/dai/intel/dmic/dmic.c Outdated
There was a problem hiding this comment.
I don't think this is a code improvement. Are you sure?
There was a problem hiding this comment.
Introduction of the PDM_CHANNEL_REGS_SIZE allows to remove a few switches, so I think this is an improvement.
drivers/dai/intel/dmic/dmic.c Outdated
There was a problem hiding this comment.
Instead of two variables with the same value, we have one, so this is a simplification. In fact, double negation is unnecessary because the comparison returns a boolean value. I will fix it.
Definitions of a configuration blob structures were separated from the main drivers header file and moved to a dedicated file to improve code readability. Removed unnecessary nhlt_pdm_fir_coeffs structure. The nhlt_pdm_ctrl_cfg structure was extended with nhlt_pdm_ctrl_fir_cfg and fir coefficients. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Moved code fragments responsible for logging and verification of the configuration register values from the dai_dmic_set_config_nhlt function to a separate functions. Behavior of the code verifying the correctness of register values has been changed so that it only displays warnings. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
All PDM controllers have the same set of registers. Their definitions have been merged to simplify the code. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Added reading of a necessary register values in dai_nhlt_dmic_dai_params_get function to simplify its parameter list. The code that calculates dai_params has been moved to it to simplify the dai_dmic_set_config_nhlt function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
The while loop in the code fragments waiting for a bit to be cleared has been replaced with the WAIT_FOR macro call. Added a warning in the case of timeout. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Created dai_dmic_start_fifo_packers function corresponding to an already existing dai_dmic_stop_fifo_packers. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
All fir filters have an identical set of registers so their definitions were combined to simplify the code. From the dai_dmic_set_config_nhlt function, a duplicate piece of code responsible for configuring fir was separated into a new function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
More verbose variable pdm_idx was used instead of n. The series of references to the array of pdm base addresses has been replaced with a pdm_base variable. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Added reading of a necessary register values in dai_nhlt_dmic_dai_params_get function to simplify its parameter list. The code that calculates dai_params has been moved to it to simplify the dai_dmic_set_config_nhlt function. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Created set of new functions for configure fir coefficients with support for packed format. This allowed to make the dai_dmic_set_config_nhlt function simpler. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
softwarecki force-pushed the dmic-refactor-pr branch from d3bd045 to cf7621e Compare 
|
kv2019i approved these changes Aug 22, 2023
There was a problem hiding this comment.
Thanks @softwarecki , looks good and test PR also passes. I'd like @singalsu to approve this series before merge, otherwise good to go.
tmleman approved these changes Sep 1, 2023
marc-hb reviewed Jan 11, 2024
| p_mfir = FIELD_GET(FIR_CONFIG_FIR_DECIMATION, val) + 1; | ||
| | ||
| rate_div = p_clkdiv * p_mcic * p_mfir; | ||
| LOG_ERR("dai_index = %d, rate_div = %d, p_clkdiv = %d, p_mcic = %d, p_mfir = %d", |
There was a problem hiding this comment.
@softwarecki why are these LOG_ERR()? They seem printed every time:
This delays thesofproject/sof-test#1075 once more...
There was a problem hiding this comment.
@marc-hb oops, it should be LOG_INF
There was a problem hiding this comment.
ops, it should be LOG_INF
Fix submitted: #67567
marc-hb added a commit to marc-hb/zephyr that referenced this pull request Jan 12, 2024
Fix the log level of two LOG_ERR() statements which should have always been LOG_INF(). As confirmed by the author Adrian in zephyrproject-rtos#60172 Fixes commit 3fbaed4 ("dai: intel: ace: dmic: Refactor of dai_nhlt_dmic_dai_params_get function") Signed-off-by: Marc Herbert <marc.herbert@intel.com>
kv2019i pushed a commit to kv2019i/zephyr that referenced this pull request Mar 4, 2024
Fix the log level of two LOG_ERR() statements which should have always been LOG_INF(). As confirmed by the author Adrian in zephyrproject-rtos#60172 Fixes commit 3fbaed4 ("dai: intel: ace: dmic: Refactor of dai_nhlt_dmic_dai_params_get function") Signed-off-by: Marc Herbert <marc.herbert@intel.com> (cherry picked from commit 8042e73)
kv2019i pushed a commit to thesofproject/zephyr that referenced this pull request Mar 5, 2024
Fix the log level of two LOG_ERR() statements which should have always been LOG_INF(). As confirmed by the author Adrian in zephyrproject-rtos#60172 Fixes commit 3fbaed4 ("dai: intel: ace: dmic: Refactor of dai_nhlt_dmic_dai_params_get function") Signed-off-by: Marc Herbert <marc.herbert@intel.com> (cherry picked from commit 8042e73)
kartben added a commit to kartben/zephyr that referenced this pull request Sep 24, 2025
juimonen hasn't reviewed any PRs or engaged in any Zephyr activity since July 2023. Recent activities: - review: Review on 'drivers: dai: doc: Cleanup Doxygen documentation' zephyrproject-rtos#60514 (2023-07-18T11:01:00Z) - review: Review on 'intel_adsp: dmic: Refactoring of blob parsing function' zephyrproject-rtos#60172 (2023-07-18T10:35:52Z) - review: Review on 'drivers: intel: ssp: delay initialization after dma' zephyrproject-rtos#60304 (2023-07-18T10:01:58Z) Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
kartben added a commit to kartben/zephyr that referenced this pull request Sep 24, 2025
juimonen hasn't reviewed any PRs or engaged in any Zephyr activity since July 2023. Recent activities: - review: Review on 'drivers: dai: doc: Cleanup Doxygen documentation' zephyrproject-rtos#60514 (2023-07-18T11:01:00Z) - review: Review on 'intel_adsp: dmic: Refactoring of blob parsing function' zephyrproject-rtos#60172 (2023-07-18T10:35:52Z) - review: Review on 'drivers: intel: ssp: delay initialization after dma' zephyrproject-rtos#60304 (2023-07-18T10:01:58Z) Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
cfriedt pushed a commit that referenced this pull request Sep 27, 2025
juimonen hasn't reviewed any PRs or engaged in any Zephyr activity since July 2023. Recent activities: - review: Review on 'drivers: dai: doc: Cleanup Doxygen documentation' #60514 (2023-07-18T11:01:00Z) - review: Review on 'intel_adsp: dmic: Refactoring of blob parsing function' #60172 (2023-07-18T10:35:52Z) - review: Review on 'drivers: intel: ssp: delay initialization after dma' #60304 (2023-07-18T10:01:58Z) Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Definitions of a configuration blob structures were moved from the main drivers header file to a dedicated file.
Moved code fragments responsible for logging and verification of the configuration register values from the dai_dmic_set_config_nhlt function to a separate functions. Behavior of the code verifying the correctness of register values has been changed so that it only displays warnings.
All PDM controllers have the same set of registers. Their definitions have been merged to simplify the code. The same was done for FIR filters registers.
Code responsible for configuring fir was separated into a new function. Created set of new functions for configure fir coefficients with support for packed format.
Added reading of a necessary register values in dai_nhlt_dmic_dai_params_get function to simplify its parameter list. The code that calculates dai_params has been moved to it to simplify the dai_dmic_set_config_nhlt function.