[alsa-devel] [PATCH 0/2] Introduce dmic mode switch delay parameter

[Vinod Koul]

On 21-10-18, 22:42, Pierre-Louis Bossart wrote:
> 
> On 10/2/18 12:57 AM, Jenny TC wrote:
> > Some DMICs need clock to be running for a specified duration as per the
> > DMIC spec to complete the mode transition like sleep to mormal, off to normal
> > etc.
> > 
> > Jenny TC (2):
> >    ASoC: dmic: Enable ACPI device entry
> >    ASoC: dmic: Introduce mode switch delay
> > 
> >   sound/soc/codecs/dmic.c | 38 ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 38 insertions(+)
> > 
> Sorry for the late feedback.
> 
> Allowing some timing adjustments for the clock transitions is a good thing.
> The way it's done is questionable and raises a number of concerns.
> 
> First, there was an Intel internal discussion before my extended break on
> why this 'dmic-codec' is needed on Intel platforms. To the best of my
> knowledge we don't control the mics with GPIOs, which was the initial
> purpose of this driver. We have experimental evidence on ApolloLake and
> GeminiLake that using the soc-dummy/soc-dummy-dai definitions are enough,
> and it may be a good thing to agree on the direction here. If you want a
> parameter, you can still use a machine driver DMI-based kernel quirk and/or
> pass a kernel parameter, the need to extend this dmic-codec is far from
> obvious to me.

I think in this case you can go with dummy codec. You are modelling an
endpoint here, nothing else

> Assuming you still want to use this codec, then there are still major
> concerns about the ACPI directions. As Mark noted it, "DMIC" does not follow
> any of the guidelines or accepted definitions with an unambiguous vendor and
> part ID. We know we already have conflicts between Intel-defined ACPI IDs,
> e.g. for RT298 on multiple platforms, let's be careful here, shall we?
> 
> And I am coming to my last point. The Skylake driver already contains code
> to create the dmic devices by hand (see below the git grep results). So I
> wonder what happens if you use both ACPI-based enumeration AND manually
> create the dmic device - I view these solutions as mutually incompatible.
> Either you have not tested against the upstream code or something is missing
> from your patchset. What am I missing?

That is very valid point. Also DMIC clock is generated by Audio block
and not really a system clock, so why shouldn't this be described in
audio block.

Furthermore I recall we have NHLT table and some description for DMICs.
The driver currently parses that information to get number of dmics to
use (see skl_get_dmic_geo). I would think that adding this delay to NHLT
would be more sensible and pass it on to whatever entity wishes to use
it.

-- 
~Vinod