From patchwork Mon Oct 2 16:04:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Charles Keepax X-Patchwork-Id: 9980983 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6A1FF60384 for ; Mon, 2 Oct 2017 16:04:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5AF511FE82 for ; Mon, 2 Oct 2017 16:04:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4FC9922A68; Mon, 2 Oct 2017 16:04:42 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 22DAC1FE82 for ; Mon, 2 Oct 2017 16:04:40 +0000 (UTC) Received: from alsa0.perex.cz (localhost [127.0.0.1]) by alsa0.perex.cz (Postfix) with ESMTP id A6E172671E3; Mon, 2 Oct 2017 18:04:36 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 111A8267235; Mon, 2 Oct 2017 18:04:36 +0200 (CEST) Received: from mx0b-001ae601.pphosted.com (mx0a-001ae601.pphosted.com [67.231.149.25]) by alsa0.perex.cz (Postfix) with ESMTP id 61756266DE1 for ; Mon, 2 Oct 2017 18:04:32 +0200 (CEST) Received: from pps.filterd (m0077473.ppops.net [127.0.0.1]) by mx0a-001ae601.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id v92FwpJa025582; Mon, 2 Oct 2017 11:04:30 -0500 Authentication-Results: ppops.net; spf=none smtp.mailfrom=ckeepax@opensource.cirrus.com Received: from mail3.cirrus.com ([87.246.76.56]) by mx0a-001ae601.pphosted.com with ESMTP id 2da8p08g7d-1; Mon, 02 Oct 2017 11:04:30 -0500 Received: from EX17.ad.cirrus.com (ex17.ad.cirrus.com [172.20.9.81]) by mail3.cirrus.com (Postfix) with ESMTP id B8A78611CE7C; Mon, 2 Oct 2017 11:07:53 -0500 (CDT) Received: from imbe.wolfsonmicro.main (198.61.95.81) by EX17.ad.cirrus.com (172.20.9.81) with Microsoft SMTP Server id 14.3.301.0; Mon, 2 Oct 2017 17:04:29 +0100 Received: from algalon.ad.cirrus.com (algalon.ad.cirrus.com [198.90.223.36]) by imbe.wolfsonmicro.main (8.14.4/8.14.4) with ESMTP id v92G4LQp016837; Mon, 2 Oct 2017 17:04:22 +0100 From: Charles Keepax To: , Date: Mon, 2 Oct 2017 17:04:21 +0100 Message-ID: <20171002160421.22696-1-ckeepax@opensource.cirrus.com> X-Mailer: git-send-email 2.11.0 MIME-Version: 1.0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1710020233 Cc: alsa-devel@alsa-project.org, vinod.koul@intel.com, patches@opensource.cirrus.com, lgirdwood@gmail.com, o-takashi@sakamocchi.jp Subject: [alsa-devel] [PATCH] ALSA: core: Add new ACCESS_TLV flag for coefficient TLV controls X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 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-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP ASoC can use the TLV mechanism to pass coefficient data for controls that exceed the normal 512 byte limit. However, there is no mechanism to inform user-space the control expects data through the TLV mechanism. Currently amixer uses a combination of the absense of non-TLV read/write permissions and the type being BYTES. This is not ideal as it is not a clear signal and prevents controls that might support both access mechanisms. Add a specific flag SNDRV_CTL_ELEM_ACCESS_TLV_COEFF to let user-space know when it is dealing with a TLV control that is being used to pass actual coefficient data. Then add this flag to the controls generated by the current users of these types of controls. Signed-off-by: Charles Keepax --- Hi, This builds on work done by Sakamoto-san [1] and the changes mentioned to amixer in the commit message are found in [2]. I think something like this should probably be added, what I am less clear on is where we go from here? Sakamoto-san's original patches also added a tag type SNDRV_CTL_TVLT_COEFF, to keep the data matching the normal format found in TLV controls. The trouble is neither of the two users really follow this convention at the moment. The ADSP code just takes the data as is with no additional headers and the Skylake code does have a tag and a length as part of the data passed through the TLV however the tag part looks like it is used for some internal purpose, so I am far from clear it would support a fixed value to identify the data without some changes as well. Changing the ADSP code is probably acceptable since we have had very few users in the field, and since this work has been done improvements in the firmware tool chains mean most firmwares are created with much smaller dedicated purpose controls rather than the large block ones that were initially the reason for using these controls on our side. Finally, since [3] tinymix has been updated to add some TLV information into control writes/reads which will break the ADSP code once it trickles down to users. So I would be keen to discuss this at this years conference and decide how we are going to handle these controls? As Mark says [4] maybe we should see if there is a better approach we should all migrate across to? Thanks, Charles [1] https://patchwork.kernel.org/patch/9304649/ [2] https://patchwork.kernel.org/patch/8147001/ [3] https://github.com/tinyalsa/tinyalsa/commit/3f813e47784674a3909fe1277bc10b70d03791e2 [4] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-September/125743.html include/sound/soc.h | 3 ++- include/uapi/sound/asound.h | 1 + sound/core/control.c | 1 + sound/soc/codecs/wm_adsp.c | 3 ++- sound/soc/intel/skylake/skl-topology.c | 1 + sound/soc/soc-ops.c | 3 +++ 6 files changed, 10 insertions(+), 2 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index d776cdee30d7..d82297a95caf 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -323,7 +323,8 @@ #define SND_SOC_BYTES_TLV(xname, xcount, xhandler_get, xhandler_put) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \ .access = SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE | \ - SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK, \ + SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK | \ + SNDRV_CTL_ELEM_ACCESS_TLV_COEFF, \ .tlv.c = (snd_soc_bytes_tlv_callback), \ .info = snd_soc_bytes_info_ext, \ .private_value = (unsigned long)&(struct soc_bytes_ext) \ diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index 1949923a40bf..4bf0b2f835e9 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -853,6 +853,7 @@ typedef int __bitwise snd_ctl_elem_iface_t; #define SNDRV_CTL_ELEM_ACCESS_TLV_WRITE (1<<5) /* TLV write is possible */ #define SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE (SNDRV_CTL_ELEM_ACCESS_TLV_READ|SNDRV_CTL_ELEM_ACCESS_TLV_WRITE) #define SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND (1<<6) /* TLV command is possible */ +#define SNDRV_CTL_ELEM_ACCESS_TLV_COEFF (1<<7) /* Coefficient data is accepted */ #define SNDRV_CTL_ELEM_ACCESS_INACTIVE (1<<8) /* control does actually nothing, but may be updated */ #define SNDRV_CTL_ELEM_ACCESS_LOCK (1<<9) /* write lock */ #define SNDRV_CTL_ELEM_ACCESS_OWNER (1<<10) /* write lock owner */ diff --git a/sound/core/control.c b/sound/core/control.c index 56b3e2d49c82..e7a74d48633b 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -266,6 +266,7 @@ struct snd_kcontrol *snd_ctl_new1(const struct snd_kcontrol_new *ncontrol, SNDRV_CTL_ELEM_ACCESS_INACTIVE | SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE | SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND | + SNDRV_CTL_ELEM_ACCESS_TLV_COEFF | SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK); err = snd_ctl_new(&kctl, count, access, NULL); diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 65c059b5ffd7..1d7fe0d8a18a 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -1156,7 +1156,8 @@ static unsigned int wmfw_convert_flags(unsigned int in, unsigned int len) wr = SNDRV_CTL_ELEM_ACCESS_TLV_WRITE; vol = SNDRV_CTL_ELEM_ACCESS_VOLATILE; - out = SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK; + out = SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK | + SNDRV_CTL_ELEM_ACCESS_TLV_COEFF; } else { rd = SNDRV_CTL_ELEM_ACCESS_READ; wr = SNDRV_CTL_ELEM_ACCESS_WRITE; diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index 27bcb62568fb..01b01c6a2bf3 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -2870,6 +2870,7 @@ static int skl_tplg_control_load(struct snd_soc_component *cmpnt, tplg_bc = container_of(hdr, struct snd_soc_tplg_bytes_control, hdr); if (kctl->access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) { + kctl->access |= SNDRV_CTL_ELEM_ACCESS_TLV_COEFF; sb = (struct soc_bytes_ext *)kctl->private_value; if (tplg_bc->priv.size) return skl_init_algo_data( diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 500f98c730b9..5ac2afe12b5d 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -776,6 +776,9 @@ int snd_soc_bytes_tlv_callback(struct snd_kcontrol *kcontrol, int op_flag, unsigned int count = size < params->max ? size : params->max; int ret = -ENXIO; + if (!(kcontrol->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_COEFF)) + return -ENXIO; + switch (op_flag) { case SNDRV_CTL_TLV_OP_READ: if (params->get)