On Thu, Feb 20, 2020 at 12:04:45PM +0530, Sameer Pujar wrote:No, the HW default is 0x2. Also, we are repurposing the LP filter for applying gain.
+++ b/sound/soc/tegra/tegra210_dmic.cPlease make the entire comment a C++ one so things look more
@@ -0,0 +1,515 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * tegra210_dmic.c - Tegra210 DMIC driver
+ *
+ * Copyright (c) 2020 NVIDIA CORPORATION. All rights reserved.
intentional.
+ /* Below enables all filters - DCR, LP and SC */So this isn't the hardware default?
+ { TEGRA210_DMIC_DBG_CTRL, 0xe },
Yes, hw_params is where it is taken from. The additional override is optional and would not usually need to be set by user. However, we have certain other modules, like multiplexer and demultiplexer (proposed to be upstreamed in the near future), where no. of channels get changed at the output. When one or more such modules are connected in the path, hw_params does not reflect the channels post multiplex/demultiplex, hence this override would be needed.
+ srate = params_rate(params);How does this work for userspace? If we just ignore the sample rate we
+ if (dmic->srate_override)
+ srate = dmic->srate_override;
were asked for I'd expect that the application would get upset.
+ if (strstr(kcontrol->id.name, "Boost Gain"))Volume controls should end in "Volume".
+ dmic->boost_gain = value;
+ else if (strstr(kcontrol->id.name, "Audio Channels"))This is something that would usually come from hw_params?
+ dmic->audio_ch_override = value;
No, these are not board wiring. OSR, polarity, etc. are configurable within the DMIC (within Tegra). Of course, these controls are optional and the default works just fine. Also, to clarify here, 'runtime selectable' does not mean these parameters can be modified while a recording session is in progress. These parameters (optional) need to set up before a session is started. User can close the recording, reconfigure the parameters, then start a new session, without needing reboot.
+ else if (strstr(kcontrol->id.name, "LR Polarity Select"))This and some of the others look like they're describing details of how
+ dmic->lrsel = value;
the board is wired up so I'd not expect them to be runtime selectable?
It is not necessary for an actual mic to be connected to the pin. Recording will work even when the pin is left open, capture being silence. All the drivers, whether for playback or for capture, work OK even without any physical end-point, hence the "dummy" I/O.
+ SND_SOC_DAPM_MIC("Dummy Input", NULL),This is just the microphone that happens to be attached, isn't it? If
so that's a weird name.
These are finer controls and I would guess are outside what can be described by DAPM. E.g. - say the user wants mono capture, but the board has stereo mics connected. Default configuration is that the Left channel would be propagated. The stereo_to_mono option offers additional control - to select the Right channel, or mix L and R into one. As mentioned earlier, much of the controls here are not necessary to be set for a basic use case to work. These are advanced settings.
+static const char * const tegra210_dmic_mono_conv_text[] = {It'd be more idiomatic for ALSA to write these as Zero and Copy.
+ "ZERO", "COPY",
+};
+ SOC_ENUM_EXT("Channel Select", tegra210_dmic_ch_enum,I'd expect these to be in DAPM.
+ tegra210_dmic_get_control, tegra210_dmic_put_control),
+ SOC_ENUM_EXT("Mono To Stereo",
+ tegra210_dmic_mono_conv_enum, tegra210_dmic_get_control,
+ tegra210_dmic_put_control),
+ SOC_ENUM_EXT("Stereo To Mono",
+ tegra210_dmic_stereo_conv_enum, tegra210_dmic_get_control,
+ tegra210_dmic_put_control),