On Sat, Mar 17, 2012 at 7:48 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Mar 16, 2012 at 06:01:44PM -0400, Trent Piepho wrote:
On Fri, Mar 16, 2012 at 3:22 PM, Mark Brown
There's a way to do this, and it's the way that the WM8988 driver is doing it - obviously you'll have different criteria for choosing constraints but the actual application of the constraints is going to be done in the same way.
Since the constraint is based on the current configuration of another DAI, the two DAIs need some way to query each other's parameters. The
Since they're both provided by the same driver this is trivial.
It's not that trivial with the sub-streams in question aren't in the same instance of the driver, e.g. playback and record. Say you had two WM8988 codecs, how does one codec get a handle to the other? There's no global list of codecs in the driver, so you have to add something like that. Or change the device tree binding to give the slave codec a phandle to the master codec.
And as I said before, ALSA doesn't handle this 100%. There is a race if both streams are initialized at the same time. If you open one
Yes, I did reply to your previous mail about that pointing out that this is impossible...
...which didn't arrive until after I started writing my email.
I think what's necessary is for ALSA to let hw_params return an error code that means lets the driver say, "Sorry, the constraints have changed and the hw_params you have chosen are no longer valid. Here are new constraints, please try again."
Thinking about this slightly further than in my reply to Lars-Peter a few minutes ago...
I think just returning -EINVAL on bad parameters (which is pretty much what we do) is sufficient for this if the driver is good about keeping the constraints up to date. Applications should already be able to take the hint, though I imagine most of them wouldn't trust the drivers to provide accurate constraints and aren't as widely deployed as PulseAudio (which forced fixing the DMA stuff by virtue of everyone using it). Though just spinning a small number of times will probably deal with lying drivers well enough.
Does PulseAudio handle it if hw_params fails? From what I've seen, most applications just fail if hw_params fails.
I think there is still a race in the design of ALSA, in that the driver checking the parameters in its hw_params() method is not atomic with the substream being assigned those parameters, w.r.t. another substream.
The problem is in the code for snd_pcm_hw_params(),
405 err = snd_pcm_hw_params_choose(substream, params); 406 if (err < 0) 407 goto _error; 408 409 if (substream->ops->hw_params != NULL) { 410 err = substream->ops->hw_params(substream, params); 411 if (err < 0) 412 goto _error; 413 } 414 415 runtime->access = params_access(params); ... 455 runtime->status->state = SNDRV_PCM_STATE_SETUP;
At line 410 the driver is given a chance to approve the selected hw_params. It can look at any of the joint substreams, and if they are in state >= SETUP, verify that the hw_params are still valid and if not, update the constraints and return an error. But it is not until line 455 that the substream has actually finished getting the parameters and entered the SETUP state. There is no lock to prevent another substream from entering this code at the same time. To fix the race, there would need to be a critical section from line 408-455 that only one substream of the group with joint constraints can enter at one time.