[alsa-devel] [PATCH 0/2] Introduce dmic mode switch delay parameter
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Mon Oct 22 16:31:10 CEST 2018
On 10/21/18 10:42 PM, 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.
>
> 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?
I forgot to add another open on ACPI support: what would be the scope of
the "DMIC" device? With ACPI we typically have a parent-child
relationship, e.g. we put audio codec below the relevant I2C/SPI
controller in the DSDT definitions. In the absence of a DMIC bus, you
need to be careful how the DMIC device is added in the DSDT - it'd
likely need to be below the scope of the HDaudio controller.
>
> -Pierre
>
> skl.c:static int skl_dmic_device_register(struct skl *skl)
> skl.c: /* SKL has one dmic port, so allocate dmic device for this */
> skl.c: pdev = platform_device_alloc("dmic-codec", -1);
> skl.c: dev_err(bus->dev, "failed to allocate dmic device\n");
> skl.c: dev_err(bus->dev, "failed to add dmic device: %d\n",
> ret);
> skl.c: skl->dmic_dev = pdev;
> skl.c:static void skl_dmic_device_unregister(struct skl *skl)
> skl.c: if (skl->dmic_dev)
> skl.c: platform_device_unregister(skl->dmic_dev);
> skl.c: /* create device for soc dmic */
> skl.c: err = skl_dmic_device_register(skl);
> skl.c: skl_dmic_device_unregister(skl);
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
More information about the Alsa-devel
mailing list