From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH RFC 5/9] ALSA: core: selection of audio_tstamp type and accuracy reports Date: Sun, 14 Dec 2014 16:03:58 +0100 Message-ID: References: <1418077426-8309-1-git-send-email-pierre-louis.bossart@linux.intel.com> <1418077426-8309-6-git-send-email-pierre-louis.bossart@linux.intel.com> <5488827E.4030403@linux.intel.com> <5488BF96.708@linux.intel.com> <5488D188.5030402@linux.intel.com> <548A54C0.2050604@linux.intel.com> <548B07D8.5030504@linux.intel.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 2A3E326041A for ; Sun, 14 Dec 2014 16:04:00 +0100 (CET) In-Reply-To: <548B07D8.5030504@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: Nick Stoughton , alsa-devel@alsa-project.org, Tim Cussins List-Id: alsa-devel@alsa-project.org At Fri, 12 Dec 2014 09:20:56 -0600, Pierre-Louis Bossart wrote: > > On 12/12/14, 2:37 AM, Takashi Iwai wrote: > > At Thu, 11 Dec 2014 20:36:48 -0600, > > Pierre-Louis Bossart wrote: > >> > >> > >>>> if someone used alsa-lib with the .get_wall_clock(), the new user-space > >>>> code will provide the same results as today, no change (wall clock if > >>>> supported, hw_ptr otherwise). So the library compatibility is preserved. > >>> > >>> You can't assume that all users always upgrade alsa-lib. > >>> Users may use still the old alsa-lib with the new kernel. > >>> > >>>> I don't mind adding a compatible kernel behavior for HDAudio only, but > >>>> is this really necessary? > >>> > >>> Yes, the kernel is not allowed to give any regression, if we know it > >>> would. > >> > >> ok, will add a backwards-compatible mode. no problem. > >> > >>>> I added a set of INFO defines and the matching is_supported queries in > >>>> alsa-lib. I just did a pretty dumb copy/paste/edit there, maybe we can > >>>> refactor the code here with a single routine taking a type parameter. > >>>> feedback welcome there. > >>> > >>> Right. But another concern is that this method will consume one INFO > >>> bit at each time the new tstamp type is extended. This is another > >>> concern. Exposing this information in another place would be better, > >>> IMO, if any better place is found... > >> > >> I asked about this one in late October... I mentioned that we are > >> running out of INFO (half already used) and AZX_CAPS (28 bits used) > >> fields, and for INFO it didn't seem like a big deal. > > > > The INFO bits are the public ABI and can't be changed later while > > AZX_DCAPS is the internal stuff we can change at any time. So, > > defining a new stuff for SND_PCM_INFO must be done very carefully. > > > > > >> If adding more fields to the info field is viewed as problematic, the > >> only options I can think of are: > >> - reclaim a reserved word in hw_params, e.g. rename to info1 to do > >> something like this in alsa-lib: > >> return !!(params->info1 & SNDRV_PCM_INFO1_IS_THIS_HARDWARE_BROKEN) > > > > It's OK to just add a new hw_param_mask element. Then we can handle > > up to 32 tstamp types and that should be enough. > > > > The question is whether hw_params is the best place. Usually > > hw_params is the place to select the one configuration from multiple > > choices or space. In this case, we don't choose only one, right? > > I think we are not aligned here. I am only using the INFO fields as a > means to report what the hardware can do, not for any selection. Then > the actual selection of the timestamps is done as a dynamic > configuration parameter for the STATUS ioctl. The hw_params is not a > good candidate since it's frozen after the start, we do need to be able > to query different timestamps dynamically. I somehow misunderstood you were suggesting hw_params mask extension, and the comment above was for that. Thinking of this again, IMO, we may extend hw_params by assigning one more int field from reserved area. This will be used as a read only information like info bits, just indicating the available tstamp types. > > >> - keep a 32 bit word but add a paging register in the msb to reuse lsbs. > >> Either way some more code will be required in both driver and library. > > > > Which word to reuse? Could you elaborate? > > Never mind, what I had in mind would require tons of changes in > user-space code and would break the backwards compatibility if one used > a new kernel and an older library. OK. thanks, Takashi