From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vinod Koul Subject: Re: [PATCH 0/2] Introduce dmic mode switch delay parameter Date: Thu, 25 Oct 2018 17:08:50 +0100 Message-ID: <20181025160850.GA827@vkoul-mobl> References: <1538459851-17066-1-git-send-email-jenny.tc@intel.com> <102a22bc-a054-b07d-23c8-95118d69450f@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by alsa0.perex.cz (Postfix) with ESMTP id EC31726759E for ; Thu, 25 Oct 2018 18:08:55 +0200 (CEST) Content-Disposition: inline In-Reply-To: <102a22bc-a054-b07d-23c8-95118d69450f@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Pierre-Louis Bossart Cc: alsa-devel@alsa-project.org, Kuninori Morimoto , Mark Brown , Takashi Iwai , Liam Girdwood , Matthias Kaehlcke , Jenny TC List-Id: alsa-devel@alsa-project.org 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