From mboxrd@z Thu Jan 1 00:00:00 1970 From: Moise Gergaud Subject: Re: [PATCH 1/3] ALSA: pcm: add IEC958 channel status update Date: Tue, 8 Dec 2015 15:30:22 +0100 Message-ID: <5666E97E.7020002@st.com> References: <1449066126-19431-1-git-send-email-moise.gergaud@st.com> <1449066126-19431-2-git-send-email-moise.gergaud@st.com> <20151202190036.GD8644@n2100.arm.linux.org.uk> <56602046.8070307@st.com> <20151203111304.GL8644@n2100.arm.linux.org.uk> <5665474A.9030906@st.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mx07-00178001.pphosted.com (mx08-00178001.pphosted.com [91.207.212.93]) by alsa0.perex.cz (Postfix) with ESMTP id 5CE45266097 for ; Tue, 8 Dec 2015 15:30:26 +0100 (CET) In-Reply-To: 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: Russell King - ARM Linux Cc: Takashi Iwai , alsa-devel@alsa-project.org, broonie@kernel.org, arnaud.pouliquen@st.com, lgirdwood@gmail.com List-Id: alsa-devel@alsa-project.org On 12/07/2015 10:51 AM, Takashi Iwai wrote: > On Mon, 07 Dec 2015 09:46:02 +0100, > Moise Gergaud wrote: >> >> On 12/03/2015 01:03 PM, Takashi Iwai wrote: >>> On Thu, 03 Dec 2015 12:13:04 +0100, >>> Russell King - ARM Linux wrote: >>>> >>>> On Thu, Dec 03, 2015 at 11:58:14AM +0100, Moise Gergaud wrote: >>>>> On 12/02/2015 08:00 PM, Russell King - ARM Linux wrote: >>>>>> Again, I'm not happy with this change. The intention here is that we >>>>>> validate all arguments before we change anything. This breaks that >>>>>> principle. >>>>>> >>>>> We propose this change for compressed mode (IEC61937/non linear PCM). >>>>> In case of compressed mode, iec958 channel status byte 0 bit1 = 1, then we >>>>> cannot use snd_pcm_create_iec958_consumer. >>>> >>>> Why can you not use snd_pcm_create_iec958_consumer() for this case? You >>>> are allowed to modify the values you get afterwards in order to handle >>>> this case if you so wish to toggle this bit. >>>> >>>> However, if you are using data mode, you really should be using the AES >>>> bytes passed by the application through alsalib and into the kernel via >>>> the IEC958 controls. There are three controls specified for this: >>>> >>>> SNDRV_CTL_NAME_IEC958("", PLAYBACK, CON_MASK) - the bits which can be >>>> changed for consumer mode. >>>> SNDRV_CTL_NAME_IEC958("", PLAYBACK, PRO_MASK) - the bits which can be >>>> changed for professional mode. >>>> SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT) - the value of the changable >>>> bits. >>>> >>>> When using this, it is userspace's responsibility to set the sample rate >>>> appropriately for the stream. >>>> >>>> The only case to use snd_pcm_create_iec958_consumer() is to generate the >>>> status bytes for PCM audio, where the userspace API doesn't facilitiate >>>> generating and/or passing the AES status bytes. >>> >>> That's true. OTOH, in most drivers, we try to be kind not to be too >>> strict for this requirement and adjust the status bits accordingly. >>> This comes from the lessons we learned how user-space doesn't care the >>> things. >>> >>> So, if we want to keep that good uncle attitude, this change doesn't >>> look so bad to me. >>> >> shall I deliver a new version of this patch with header correction or >> shall I abandon this patch? > > It's up to you whether you want further convincing Russell :) > I'd happily merge this once when we get his ack. > > > thanks, > > Takashi > I share Takashi's view; unfortunately, we cannot always trust user space. And so, for robustness reason, I think drivers shall ensure that channel status params are correct. For that purpose, the update helper function could be useful.