Re: [alsa-devel] [PATCH v3 5/5] ALSA - hda: Add support for link audio time reporting

On Wed, 03 Aug 2016 18:06:14 +0200, Vinod Koul wrote:

+static int azx_get_sync_time(ktime_t *device,

• struct system_counterval_t *system, void *ctx)
  

+{

• struct snd_pcm_substream *substream = (struct snd_pcm_substream *)ctx;

Superfluous cast.

• struct azx_dev *azx_dev = get_azx_dev(substream);
• struct azx_pcm *apcm = snd_pcm_substream_chip(substream);
• struct azx *chip = apcm->chip;
• struct snd_pcm_runtime *runtime;
• u64 ll_counter, ll_counter_l, ll_counter_h;
• u64 tsc_counter, tsc_counter_l, tsc_counter_h;
• u32 wallclk_ctr, wallclk_cycles;
• bool direction;
• u32 dma_select;
• u32 timeout = 200;
• u32 retry_count = 0;
• runtime = substream->runtime;
• if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)

• direction = 1;
  

• else

• direction = 0;
  

• /* 0th stream tag is not used, so DMA ch 0 is for 1st stream tag */
• do {

• timeout = 100;
  

• dma_select = (direction << GTSCC_CDMAS_DMA_DIR_SHIFT) |
  

• 			(azx_dev->core.stream_tag - 1);
  

• _snd_hdac_chip_write(l, azx_bus(chip), AZX_REG_GTSCC,
  

• 				dma_select);
  

You can use snd_hdac_chip_writel(azx_bus(chip), GTSCC, dma_select);

The use of _snd_hdac_chip_write() is for non-constant registers. When you pass AZX_REG_XXX, you can use the standard macro. For example:

• /* Enable the capture */
  

• _snd_hdac_chip_write(l, azx_bus(chip), AZX_REG_GTSCC,
  

• 		     _snd_hdac_chip_read(l, azx_bus(chip),
  

• 			AZX_REG_GTSCC) | GTSCC_TSCCI_MASK);
  

This can be simplified with snd_hdac_chip_updatel().

• while (timeout) {
  

• 	if (_snd_hdac_chip_read(l, azx_bus(chip),
  

• 		AZX_REG_GTSCC) & GTSCC_TSCCD_MASK)
  

• 		break;
  

• 	timeout--;
  

• }
  

• if (!timeout) {
  

• 	dev_err(chip->card->dev, "GTSCC capture Timedout!\n");
  

• 	return -EIO;
  

• }
  

• /* Read wall clock counter */
  

• wallclk_ctr = _snd_hdac_chip_read(l, azx_bus(chip),
  

• 				  AZX_REG_WALFCC);
  

• /* Read TSC counter */
  

• tsc_counter_l = _snd_hdac_chip_read(l, azx_bus(chip),
  

• 				    AZX_REG_TSCCL);
  

• tsc_counter_h = _snd_hdac_chip_read(l, azx_bus(chip),
  

• 				    AZX_REG_TSCCU);
  

• /* Read Link counter */
  

• ll_counter_l = _snd_hdac_chip_read(l, azx_bus(chip),
  

• 				   AZX_REG_LLPCL);
  

• ll_counter_h = _snd_hdac_chip_read(l, azx_bus(chip),
  

• 				   AZX_REG_LLPCU);
  

• /* Ack: registers read done */
  

• _snd_hdac_chip_write(l, azx_bus(chip),
  

• 		     AZX_REG_GTSCC,
  

• 		     (0x1 << GTSCC_TSCCD_SHIFT));
  

• tsc_counter = (tsc_counter_h << TSCCU_CCU_SHIFT) |
  

• 				tsc_counter_l;
  

• ll_counter = (ll_counter_h << LLPC_CCU_SHIFT) |	ll_counter_l;
  

• wallclk_cycles = wallclk_ctr & WALFCC_CIF_MASK;
  

• /*
  

•  * An error occurs near frame "rollover". The clocks in
  

•  * frame value indicates whether this error may have
  

•  * occurred. Here we use the value of 10 i.e.,
  

•  * HDA_MAX_CYCLE_OFFSET
  

•  */
  

• if (wallclk_cycles < HDA_MAX_CYCLE_VALUE - HDA_MAX_CYCLE_OFFSET
  

•     && wallclk_cycles > HDA_MAX_CYCLE_OFFSET)
  

• 	break;
  

• /*
  

•  * Sleep before we read again, else we may again get
  

•  * value near to MAX_CYCLE. Try to sleep for different
  

•  * amount of time so we dont hit the same number again
  

•  */
  

• udelay(retry_count++);
  

• } while (retry_count != HDA_MAX_CYCLE_READ_RETRY);
• if (retry_count == HDA_MAX_CYCLE_READ_RETRY) {

• dev_err(chip->card->dev, "Error in WALFCC cycle count\n");
  

Hrm, this has a danger to spew huge amount of error messages, since this gets called so often.

• return -EIO;
  

• }
• *device = ns_to_ktime(azx_scale64(ll_counter,

• 		NSEC_PER_SEC, runtime->rate));
  

• *device = ktime_add_ns(*device, (wallclk_cycles * NSEC_PER_SEC) /

• 	       ((HDA_MAX_CYCLE_VALUE + 1) * runtime->rate));
  

• *system = convert_art_to_tsc(tsc_counter);
• return 0;

+}

+#else +static int azx_get_sync_time(ktime_t *device,

• struct system_counterval_t *system, void *ctx)
  

+{

• return -ENXIO;

+} +#endif

+static int azx_get_crosststamp(struct snd_pcm_substream *substream,

• 	      struct system_device_crosststamp *xtstamp)
  

+{

• return get_device_system_crosststamp(azx_get_sync_time,

• 			substream, NULL, xtstamp);
  

+}

static int azx_get_time_info(struct snd_pcm_substream *substream, struct timespec *system_ts, struct timespec *audio_ts, struct snd_pcm_audio_tstamp_config *audio_tstamp_config, struct snd_pcm_audio_tstamp_report *audio_tstamp_report) { struct azx_dev *azx_dev = get_azx_dev(substream);

• struct snd_pcm_runtime *runtime = substream->runtime;

• struct system_device_crosststamp xtstamp; u64 nsec;

  if ((substream->runtime->hw.info & SNDRV_PCM_INFO_HAS_LINK_ATIME) &&

@@ -361,8 +524,38 @@ static int azx_get_time_info(struct snd_pcm_substream *substream, audio_tstamp_report->accuracy_report = 1; /* rest of structure is valid */ audio_tstamp_report->accuracy = 42; /* 24 MHz WallClock == 42ns resolution */

• } else

• } else if ((runtime->hw.info &

• 	SNDRV_PCM_INFO_HAS_LINK_SYNCHRONIZED_ATIME) &&
  

• 	(audio_tstamp_config->type_requested ==
  

• 	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK_SYNCHRONIZED)) {
  

The indentation is hard to follow here...

• azx_get_crosststamp(substream, &xtstamp);
  

No error check?

thanks,

Takashi

• switch (runtime->tstamp_type) {
  

• case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC:
  

• 	return -EINVAL;
  

• case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW:
  

• 	*system_ts = ktime_to_timespec(xtstamp.sys_monoraw);
  

• 	break;
  

• default:
  

• 	*system_ts = ktime_to_timespec(xtstamp.sys_realtime);
  

• 	break;
  

• }
  

• *audio_ts = ktime_to_timespec(xtstamp.device);
  

• audio_tstamp_report->actual_type =
  

• 	SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK_SYNCHRONIZED;
  

• audio_tstamp_report->accuracy_report = 1;
  

• /* 24 MHz WallClock == 42ns resolution */
  

• audio_tstamp_report->accuracy = 42;
  

• } else { audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT;

• }

  return 0;

}

1.9.1