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