[Sound-open-firmware] [PATCH v3 3/7] topology: add DMIC tokens

[Ranjani Sridharan]

On Thu, 2018-05-24 at 11:52 +0300, Seppo Ingalsuo wrote:
> Hi,
> 
> Sorry for some late comments, I should have commented the earlier
> RFC 
> version. Though these are quite minor.
> 
> 
> On 24.05.2018 05:47, Ranjani Sridharan wrote:
> > This patch introduces the DMIC specific config parameter tokens
> > in topology.
> > 
> > Signed-off-by: Ranjani Sridharan <ranjani.sridharan at linux.intel.com
> > >
> > ---
> >   topology/sof/tokens.m4 | 13 +++++++++++++
> >   1 file changed, 13 insertions(+)
> > 
> > diff --git a/topology/sof/tokens.m4 b/topology/sof/tokens.m4
> > index e9aa302..0dd2e00 100644
> > --- a/topology/sof/tokens.m4
> > +++ b/topology/sof/tokens.m4
> > @@ -60,3 +60,16 @@ SectionVendorTokens."sof_ssp_tokens" {
> >   	SOF_TKN_INTEL_SSP_BCLK_KEEP_ACTIVE	"501"
> >   	SOF_TKN_INTEL_SSP_FS_KEEP_ACTIVE	"502"
> >   }
> > +
> > +SectionVendorTokens."sof_dmic_tokens" {
> > +	SOF_TKN_INTEL_DMIC_DRIVER_VERSION	"600"
> 
> Would it be useful to have comments for the tokens? For the version 
> token the purpose is to ensure a topology and driver are compatible 
> versions. If the FW would return an error it is recommended to check 
> that microphone operates as specified before increasing version
> number 
> to match. The intent is to increase in FW driver the IPC version if
> the 
> clocks matching criteria would be changed.

Seppo, there are explanations for each of the tokens(or the parameters
they refer to) in the firmware. 
Would it suffice to have it there? So would you prefer adding them here
as well?

> 
> > +	SOF_TKN_INTEL_DMIC_CLK_MIN		"601"
> > +	SOF_TKN_INTEL_DMIC_CLK_MAX		"602"
> > +	SOF_TKN_INTEL_DMIC_DUTY_MIN		"603"
> > +	SOF_TKN_INTEL_DMIC_DUTY_MAX		"604"
> 
> Commenting again - The settings for these four values need to be
> looked 
> from microphone data sheet. The datasheets specify the recommended
> clock 
> ranges for various applications like speech capture, generic usage, 
> ultrasound, etc. The unit is Hertz and typically these are within 1
> MHz 
> - 5 MHz.
> 
> The unit of duty cycle is an integer 0-100%. E.g. 40% and 60% max is 
> quite typical duty cycle requirement.
> 
> If the driver fails to find a compatible mode it is recommended to
> look 
> if some min/max range can be relaxed. If still no help possibly
> consider 
> some other sample rate for application (below).
> 
> > +	SOF_TKN_INTEL_DMIC_REQUESTED_PDM_COUNT	"605"
> > +	SOF_TKN_INTEL_DMIC_PDM_0_ENABLE		"606"
> > +	SOF_TKN_INTEL_DMIC_PDM_1_ENABLE		"607"
> 
> Change proposal - The FW driver can happily work with these three 
> controls to activate the needed microphones but there would be power 
> saving opportunity by having possibility to express what active 
> microphones combination is needed for PDM0 controller.
> 
> The PDM_0 token could be split to
> 
> SOF_TKN_INTEL_DMIC_PDM_0_LEFT_ENABLE
> SOF_TKN_INTEL_DMIC_PDM_0_RIGHT_ENABLE
> 
> The driver can suspend the clocks of non-used microphones and save
> near 
> 1 mW of power for each by putting them to powered sleep state.
> 
> The PDM_COUNT token could be eliminated from topology by looking up
> the 
> enabled max. PDM_x index plus one and set it by kernel driver.
> 
> There's also new DMIC HW with support for PDM_2 and PDM_3
> controllers 
> and max. 8 microphones. Should we add those tokens now to topology? 
> Those need to be kept as zero for platforms like APL with max 4 
> microphones capability (driver will issue an error if attempted).
> 
> There's even more configuration possibility in the IPC but exposing
> all 
> of them to topology could be too much clutter while this should be 
> sufficient for typical usage. Those additional options could be left
> for 
> custom topology and kernel if need. If the kernel driver sets them
> to 
> default 0 value it's simple to spot them and customize.
> 
> Thanks,
> Seppo
> 
> 
> > +	SOF_TKN_INTEL_DMIC_SAMPLE_RATE		"608"
> > +	SOF_TKN_INTEL_DMIC_FIFO_WORD_LENGTH	"609"
> > +}
> 
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware