On Tue, 13 Mar 2018 18:31:55 +0100,
Oleksandr Andrushchenko wrote:
> On 03/13/2018 06:31 PM, Takashi Iwai wrote:
> > On Tue, 13 Mar 2018 12:49:00 +0100,
> > Oleksandr Andrushchenko wrote:
> >> So, I tried to make a POC to stress the protocol changes and see
> >> what implementation of the HW parameter negotiation would look like.
> >> Please find protocol changes at :
> >> - add XENSND_OP_HW_PARAM_QUERY request to read/update
> >> configuration space for the parameter given: request passes
> >> desired parameter interval and the response to this request
> >> returns min/max interval for the parameter to be used.
> >> Parameters supported by this request:
> >> - frame rate
> >> - sample rate
> >> - number of channels
> >> - buffer size
> >> - period size
> >> - add minimum buffer size to XenStore configuration
> >> From the previous changes to the protocol which I posted earlier I see
> >> that XENSND_OP_HW_PARAM_SET is not really needed - removed.
> >> The implementation in the PV frontend driver is at .
> >> Takashi, could you please take a look at the above if it meets your
> >> expectations
> >> so I can move forward?
> > This looks almost good through a quick glance.
> > But the mixture of SNDRV_PCM_HW_PARAM_PERIOD_SIZE and
> > SNDRV_PCM_HW_PARAM_BUFFER_BYTES are likely confusing.
> > The *_SIZE means in frames unit while *_BYTES means in bytes.
> > You should align both PERIOD_ and BUFFER_ to the same units,
> > i.e. either use SNDRV_PCM_HW_PARAM_PERIOD_BYTES and *_BUFFER_BYTES,
> > or SNDRV_PCM_HW_PARAM_PERIOD_SIZE and *_BUFFER_SIZE.
> You are correct, fixed this at 
> > Also, a slightly remaining concern is the use-case where hw_params is
> > called multiple times. An application may call hw_free and hw_params
> > freely, or even hw_params calls multiple times, in order to change the
> > parameter.
> > If the backend needs to resolve some dependency between parameters
> > (e.g. the available period size depends on the sample rate), the
> > backend has to remember the previously passed parameters.
> > So, instead of passing a single parameter, you may extend the protocol
> > always to pass the full (five) parameters, too.
> > OTOH, this can be considered to be a minor case, and the backend
> > (e.g. PA) can likely support every possible combinations, so maybe a
> > simpler code may be a better solution in the end.
> Yes, let's have it step by step.
> If you are ok with what we have at the moment then, after I implement both
> backend and frontend changes and confirm that protocol works,
> I will send v3 of the series (protocol changes).
> Still there some questions:
> 1. Do we really need min buffer value as configuration ? I see no
> way it can be used,
> for instance at , we only have snd_pcm_hardware.buffer_bytes_max,
> but not min.
> So, I feel I can drop that
Actually with the hw_param query mechanism, this setup is moot.
You can pass a fixed value that should be enough large for all cases
> 2. Can I assume that min buffer size == period size and add such a
> in the frontend driver?
The buffer sie == period size is a special case, i.e. periods=1, and
this won't work most likely. It's used only for a case like PA
deployment without the period interrupt. And it needs a special
hw_params flag your driver doesn't deal with.
So for the sane setup, you can safely assume min_periods=2.
> 3. On backend side (ALSA), with current changes in the protocol I will
> call something like
> int snd_pcm_hw_params_set_channels_minmax(snd_pcm_t *pcm,
> snd_pcm_hw_params_t *params, unsigned int *min, unsigned int *max)
> instead of
> int snd_pcm_hw_params_set_channels(snd_pcm_t *pcm, snd_pcm_hw_params_t
> *params, unsigned int val)
> while servicing
> XENSND_OP_HW_PARAM_QUERY.XENSND_OP_HW_PARAM_CHANNELS. Does this make
Yeah, that's better, I suppose.
Xen-devel mailing list