[alsa-devel] [PATCH 05/11] ALSA: dice: enable to change current sampling transmission frequency

[Takashi Sakamoto]

Hi,

On Apr 25 2018 08:42, Takashi Sakamoto wrote:
> This is a preparation for userspace applications to change current sampling
> transmission frequency via ALSA PCM interface.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
> ---
>   sound/firewire/dice/dice-stream.c | 44 +++++++++++++++++++++++++++++++++------
>   1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
> index e22896aaf346..92f3c345fa59 100644
> --- a/sound/firewire/dice/dice-stream.c
> +++ b/sound/firewire/dice/dice-stream.c
> @@ -52,19 +52,32 @@ int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
>    * This operation has an effect to synchronize GLOBAL_STATUS/GLOBAL_SAMPLE_RATE
>    * to GLOBAL_STATUS. Especially, just after powering on, these are different.
>    */
> -static int ensure_phase_lock(struct snd_dice *dice)
> +static int ensure_phase_lock(struct snd_dice *dice, unsigned int rate)
>   {
>   	__be32 reg, nominal;
> +	u32 data;
> +	int i;
>   	int err;
>   
>   	err = snd_dice_transaction_read_global(dice, GLOBAL_CLOCK_SELECT,
>   					       &reg, sizeof(reg));
>   	if (err < 0)
>   		return err;
> +	data = be32_to_cpu(reg);
> +
> +	data &= ~CLOCK_RATE_MASK;
> +	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); ++i) {
> +		if (snd_dice_rates[i] == rate)
> +			break;
> +	}
> +	if (i == ARRAY_SIZE(snd_dice_rates))
> +		return -EINVAL;
> +	data |= i << CLOCK_RATE_SHIFT;
>   
>   	if (completion_done(&dice->clock_accepted))
>   		reinit_completion(&dice->clock_accepted);
>   
> +	reg = cpu_to_be32(data);
>   	err = snd_dice_transaction_write_global(dice, GLOBAL_CLOCK_SELECT,
>   						&reg, sizeof(reg));
>   	if (err < 0)
> @@ -210,6 +223,7 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
>   			 unsigned int rate, struct reg_params *params)
>   {
>   	__be32 reg[2];
> +	unsigned int mode;
>   	unsigned int i, pcm_chs, midi_ports;
>   	struct amdtp_stream *streams;
>   	struct fw_iso_resources *resources;
> @@ -224,12 +238,23 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
>   		resources = dice->rx_resources;
>   	}
>   
> +	err = snd_dice_stream_get_rate_mode(dice, rate, &mode);
> +	if (err < 0)
> +		return err;
> +
>   	for (i = 0; i < params->count; i++) {
> +		unsigned int pcm_cache;
> +		unsigned int midi_cache;
> +
>   		if (dir == AMDTP_IN_STREAM) {
> +			pcm_cache = dice->tx_channels[i][mode];
> +			midi_cache = dice->tx_midi_ports[i];
>   			err = snd_dice_transaction_read_tx(dice,
>   					params->size * i + TX_NUMBER_AUDIO,
>   					reg, sizeof(reg));
>   		} else {
> +			pcm_cache = dice->rx_channels[i][mode];
> +			midi_cache = dice->rx_midi_ports[i];
>   			err = snd_dice_transaction_read_rx(dice,
>   					params->size * i + RX_NUMBER_AUDIO,
>   					reg, sizeof(reg));
> @@ -239,6 +264,14 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
>   		pcm_chs = be32_to_cpu(reg[0]);
>   		midi_ports = be32_to_cpu(reg[1]);
>   
> +		/* These are important for developer of this driver. */
> +		if (pcm_chs != pcm_cache || midi_ports != midi_cache) {
> +			dev_info(&dice->unit->device,
> +				 "cache mismatch: pcm: %u:%u, midi: %u:%u\n",
> +				 pcm_chs, pcm_cache, midi_ports, midi_cache);
> +			return -EPROTO;
> +		}
> +
>   		err = keep_resources(dice, dir, i, rate, pcm_chs, midi_ports);
>   		if (err < 0)
>   			return err;
> @@ -300,10 +333,9 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
>   			"fail to get sampling rate\n");
>   		return err;
>   	}
> -	if (rate == 0)
> +
> +	if (rate != 0)
>   		rate = curr_rate;
> -	if (rate != curr_rate)
> -		return -EINVAL;
>   
>   	/* Judge to need to restart streams. */
>   	for (i = 0; i < MAX_STREAMS; i++) {
> @@ -318,7 +350,7 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
>   				break;
>   		}
>   	}
> -	need_to_start = (i < MAX_STREAMS);
> +	need_to_start = (rate != curr_rate || i < MAX_STREAMS);
>   
>   	if (need_to_start) {
>   		/* Stop transmission. */
> @@ -327,7 +359,7 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
>   		stop_streams(dice, AMDTP_OUT_STREAM, &rx_params);
>   		release_resources(dice);
>   
> -		err = ensure_phase_lock(dice);
> +		err = ensure_phase_lock(dice, rate);
>   		if (err < 0) {
>   			dev_err(&dice->unit->device,
>   				"fail to ensure phase lock\n");

Additionally, I realized this patch includes a bug that 'tx_params' and 
'rx_params' are reused after changing state of sampling transmission 
frequency in target unit. After the change, stream formats are changed 
and content of the 'tx_params' and 'rx_params' can differ...

I'll post revised patchset in this weekend.


Thanks

Takashi Sakamoto