From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E2A61C433DB for ; Thu, 11 Feb 2021 17:16:08 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 42C9764E87 for ; Thu, 11 Feb 2021 17:16:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 42C9764E87 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id ED40A16F1; Thu, 11 Feb 2021 18:15:14 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz ED40A16F1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1613063765; bh=kfQxjMHYTcfSrV8LGixo5kdcTe1kUM0/DDmwFu9RcKc=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=I+j7mXayQhgBCOtl7UNDf5u7zChfQe0/4cfn/6zm03UIg3LFskSGtB7nPxmnaMSYq 5F/kQGOETJEqw4lEpIlkJJwnz+4gOGHvctKDHBXFq0W6ug4S43osodVAZIirV/+UqF nNXrxEi64YFwkqQ/1Zkvuks0rNkmbMfjAsoAACbM= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 6F646F8014D; Thu, 11 Feb 2021 18:15:14 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 8FA9BF801D5; Thu, 11 Feb 2021 18:15:12 +0100 (CET) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 09A4EF8014D for ; Thu, 11 Feb 2021 18:15:09 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 09A4EF8014D X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 1D970B130; Thu, 11 Feb 2021 17:15:08 +0000 (UTC) Date: Thu, 11 Feb 2021 18:15:07 +0100 Message-ID: From: Takashi Iwai To: Jaroslav Kysela Subject: Re: [PATCH 0/5] ALSA: control - add generic LED trigger code In-Reply-To: <20210211111400.1131020-1-perex@perex.cz> References: <20210211111400.1131020-1-perex@perex.cz> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Cc: Hans de Goede , ALSA development , Perry Yuan X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Thu, 11 Feb 2021 12:13:55 +0100, Jaroslav Kysela wrote: > > This patchset tries to resolve the diversity in the audio LED > control among the ALSA drivers. > > A new control layer registration is introduced which allows > to run additional operations on top of the elementary ALSA > sound controls. > > A new control access group (three bits in the access flags) > was introduced to carry the LED group information for > the sound controls. The low-level sound drivers can just > mark those controls using this access group. This information > is exported to the user space and eventually the user space > can create sound controls which can belong to a LED group. > > The actual state ('route') evaluation is really easy > (the minimal value check for all channels / controls / cards). > If there's more complicated logic for a given hardware, > the card driver may eventually export a new read-only > sound control for the LED group and do the logic itself. > > The new LED trigger control code is completely separated > and possibly optional (there's no symbol dependency). > The full code separation allows eventually to move this > LED trigger control to the user space in future. > Actually it replaces the already present functionality > in the kernel space (HDA drivers) and allows a quick adoption > for the recent hardware (SoundWire ASoC codecs). > > # lsmod | grep snd_ctl_led > snd_ctl_led 16384 0 > > The sound driver implementation is really easy: > > 1) call snd_ctl_led_request() when control LED layer should be > automatically activated > / it calls module_request("snd-ctl-led") on demand / > 2) mark all related kcontrols with > SNDRV_CTL_ELEM_ACCESS_SPK_LED or > SNDRV_CTL_ELEM_ACCESS_MIC_LED > > Original RFC: https://lore.kernel.org/alsa-devel/20210207201157.869972-1-perex@perex.cz/ > > Cc: Hans de Goede > Cc: Perry Yuan > > Jaroslav Kysela (5): > ALSA: control - introduce snd_ctl_notify_one() helper > ALSA: control - add layer registration routines > ALSA: control - add generic LED trigger module as the new control > layer > ALSA: HDA - remove the custom implementation for the audio LED trigger > ALSA: control - add sysfs support to the LED trigger module Thanks for the patch. I'm afraid that it's a bit too late for 5.12, as the merge window will be likely closed soon. For 5.13, we'll have enough time for get a consensus about the design. Whether this is the best way to go, or we should rather consider user-space solution as Sakamoto-san mentioned: that has to be decided. Back to the design: your new implementation allows the separation and the dynamic opt-in, which is nice. Thanks for that. This looks generic and may be extended for other purposes in future, too. One thing I still miss from the picture is how to deal with the case like AMD ACP. It has no mixer control to bundle with the LED trigger. Your idea is to make a (dummy) user element and tie the LED trigger with it? Another slight concern is the possible regression: by moving the mute-LED mode enum stuff into the sysfs, user will get incompatibilities after the kernel update. And it's not that trivial to change the sysfs entry as default for each user. It needs some detailed documentation or some temporary workaround (e.g. keep providing the controls for now but warns if the value is changed from the default value via the controls). thanks, Takashi