All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matt Flax <flatmax@flatmax.org>
To: unlisted-recipients:; (no To-header on input)
Cc: Matt Flax <flatmax@flatmax.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Ray Jui <rjui@broadcom.com>,
	Scott Branden <sbranden@broadcom.com>,
	bcm-kernel-feedback-list@broadcom.com,
	YueHaibing <yuehaibing@huawei.com>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Allison Randal <allison@lohutok.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	alsa-devel@alsa-project.org,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] ASoC: bcm2835-i2s: substream alignment now independent in hwparams
Date: Tue, 24 Mar 2020 20:08:21 +1100	[thread overview]
Message-ID: <20200324090823.20754-1-flatmax@flatmax.org> (raw)

Substream sample alignment was being set in hwparams for both
substreams at the same time. This became a problem when	the Audio
Injector isolated sound card needed to offset sample alignment
for high sample	rates. The latency difference between playback
and capture occurs because of the digital isolation chip
propagation time, particularly when the codec is master and
the DAC return is twice delayed.

This patch sets sample alignment registers  based on the substream
direction in hwparams. This gives the machine driver more control
over sample alignment in the bcm2835 i2s driver.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index e6a12e271b07..9db542699a13 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -493,11 +493,6 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	bcm2835_i2s_calc_channel_pos(&rx_ch1_pos, &rx_ch2_pos,
-		rx_mask, slot_width, data_delay, odd_slot_offset);
-	bcm2835_i2s_calc_channel_pos(&tx_ch1_pos, &tx_ch2_pos,
-		tx_mask, slot_width, data_delay, odd_slot_offset);
-
 	/*
 	 * Transmitting data immediately after frame start, eg
 	 * in left-justified or DSP mode A, only works stable
@@ -508,19 +503,26 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 			"Unstable slave config detected, L/R may be swapped");
 
 	/*
-	 * Set format for both streams.
-	 * We cannot set another frame length
-	 * (and therefore word length) anyway,
-	 * so the format will be the same.
+	 * Set format on a per stream basis.
+	 * The alignment format can be different depending on direction.
 	 */
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG, 
-		  format
-		| BCM2835_I2S_CH1_POS(rx_ch1_pos)
-		| BCM2835_I2S_CH2_POS(rx_ch2_pos));
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG, 
-		  format
-		| BCM2835_I2S_CH1_POS(tx_ch1_pos)
-		| BCM2835_I2S_CH2_POS(tx_ch2_pos));
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		bcm2835_i2s_calc_channel_pos(&rx_ch1_pos, &rx_ch2_pos,
+			rx_mask, slot_width, data_delay, odd_slot_offset);
+		regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG,
+			  format
+			| BCM2835_I2S_CH1_POS(rx_ch1_pos)
+			| BCM2835_I2S_CH2_POS(rx_ch2_pos));
+	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		bcm2835_i2s_calc_channel_pos(&tx_ch1_pos, &tx_ch2_pos,
+			tx_mask, slot_width, data_delay, odd_slot_offset);
+		regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG,
+			  format
+			| BCM2835_I2S_CH1_POS(tx_ch1_pos)
+			| BCM2835_I2S_CH2_POS(tx_ch2_pos));
+	}
 
 	/* Setup the I2S mode */
 
-- 
2.20.1


WARNING: multiple messages have this Message-ID (diff)
From: Matt Flax <flatmax@flatmax.org>
Cc: Kate Stewart <kstewart@linuxfoundation.org>,
	alsa-devel@alsa-project.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	linux-kernel@vger.kernel.org,
	Scott Branden <sbranden@broadcom.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	linux-arm-kernel@lists.infradead.org, Ray Jui <rjui@broadcom.com>,
	YueHaibing <yuehaibing@huawei.com>, Takashi Iwai <tiwai@suse.com>,
	Mark Brown <broonie@kernel.org>,
	bcm-kernel-feedback-list@broadcom.com,
	Allison Randal <allison@lohutok.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Matt Flax <flatmax@flatmax.org>,
	Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
	linux-rpi-kernel@lists.infradead.org
Subject: [PATCH] ASoC: bcm2835-i2s: substream alignment now independent in hwparams
Date: Tue, 24 Mar 2020 20:08:21 +1100	[thread overview]
Message-ID: <20200324090823.20754-1-flatmax@flatmax.org> (raw)

Substream sample alignment was being set in hwparams for both
substreams at the same time. This became a problem when	the Audio
Injector isolated sound card needed to offset sample alignment
for high sample	rates. The latency difference between playback
and capture occurs because of the digital isolation chip
propagation time, particularly when the codec is master and
the DAC return is twice delayed.

This patch sets sample alignment registers  based on the substream
direction in hwparams. This gives the machine driver more control
over sample alignment in the bcm2835 i2s driver.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index e6a12e271b07..9db542699a13 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -493,11 +493,6 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	bcm2835_i2s_calc_channel_pos(&rx_ch1_pos, &rx_ch2_pos,
-		rx_mask, slot_width, data_delay, odd_slot_offset);
-	bcm2835_i2s_calc_channel_pos(&tx_ch1_pos, &tx_ch2_pos,
-		tx_mask, slot_width, data_delay, odd_slot_offset);
-
 	/*
 	 * Transmitting data immediately after frame start, eg
 	 * in left-justified or DSP mode A, only works stable
@@ -508,19 +503,26 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 			"Unstable slave config detected, L/R may be swapped");
 
 	/*
-	 * Set format for both streams.
-	 * We cannot set another frame length
-	 * (and therefore word length) anyway,
-	 * so the format will be the same.
+	 * Set format on a per stream basis.
+	 * The alignment format can be different depending on direction.
 	 */
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG, 
-		  format
-		| BCM2835_I2S_CH1_POS(rx_ch1_pos)
-		| BCM2835_I2S_CH2_POS(rx_ch2_pos));
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG, 
-		  format
-		| BCM2835_I2S_CH1_POS(tx_ch1_pos)
-		| BCM2835_I2S_CH2_POS(tx_ch2_pos));
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		bcm2835_i2s_calc_channel_pos(&rx_ch1_pos, &rx_ch2_pos,
+			rx_mask, slot_width, data_delay, odd_slot_offset);
+		regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG,
+			  format
+			| BCM2835_I2S_CH1_POS(rx_ch1_pos)
+			| BCM2835_I2S_CH2_POS(rx_ch2_pos));
+	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		bcm2835_i2s_calc_channel_pos(&tx_ch1_pos, &tx_ch2_pos,
+			tx_mask, slot_width, data_delay, odd_slot_offset);
+		regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG,
+			  format
+			| BCM2835_I2S_CH1_POS(tx_ch1_pos)
+			| BCM2835_I2S_CH2_POS(tx_ch2_pos));
+	}
 
 	/* Setup the I2S mode */
 
-- 
2.20.1


WARNING: multiple messages have this Message-ID (diff)
From: Matt Flax <flatmax@flatmax.org>
Cc: Kate Stewart <kstewart@linuxfoundation.org>,
	alsa-devel@alsa-project.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	linux-kernel@vger.kernel.org,
	Scott Branden <sbranden@broadcom.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	linux-arm-kernel@lists.infradead.org, Ray Jui <rjui@broadcom.com>,
	YueHaibing <yuehaibing@huawei.com>, Takashi Iwai <tiwai@suse.com>,
	Jaroslav Kysela <perex@perex.cz>, Mark Brown <broonie@kernel.org>,
	bcm-kernel-feedback-list@broadcom.com,
	Allison Randal <allison@lohutok.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Matt Flax <flatmax@flatmax.org>,
	Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
	linux-rpi-kernel@lists.infradead.org
Subject: [PATCH] ASoC: bcm2835-i2s: substream alignment now independent in hwparams
Date: Tue, 24 Mar 2020 20:08:21 +1100	[thread overview]
Message-ID: <20200324090823.20754-1-flatmax@flatmax.org> (raw)

Substream sample alignment was being set in hwparams for both
substreams at the same time. This became a problem when	the Audio
Injector isolated sound card needed to offset sample alignment
for high sample	rates. The latency difference between playback
and capture occurs because of the digital isolation chip
propagation time, particularly when the codec is master and
the DAC return is twice delayed.

This patch sets sample alignment registers  based on the substream
direction in hwparams. This gives the machine driver more control
over sample alignment in the bcm2835 i2s driver.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index e6a12e271b07..9db542699a13 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -493,11 +493,6 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	bcm2835_i2s_calc_channel_pos(&rx_ch1_pos, &rx_ch2_pos,
-		rx_mask, slot_width, data_delay, odd_slot_offset);
-	bcm2835_i2s_calc_channel_pos(&tx_ch1_pos, &tx_ch2_pos,
-		tx_mask, slot_width, data_delay, odd_slot_offset);
-
 	/*
 	 * Transmitting data immediately after frame start, eg
 	 * in left-justified or DSP mode A, only works stable
@@ -508,19 +503,26 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 			"Unstable slave config detected, L/R may be swapped");
 
 	/*
-	 * Set format for both streams.
-	 * We cannot set another frame length
-	 * (and therefore word length) anyway,
-	 * so the format will be the same.
+	 * Set format on a per stream basis.
+	 * The alignment format can be different depending on direction.
 	 */
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG, 
-		  format
-		| BCM2835_I2S_CH1_POS(rx_ch1_pos)
-		| BCM2835_I2S_CH2_POS(rx_ch2_pos));
-	regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG, 
-		  format
-		| BCM2835_I2S_CH1_POS(tx_ch1_pos)
-		| BCM2835_I2S_CH2_POS(tx_ch2_pos));
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		bcm2835_i2s_calc_channel_pos(&rx_ch1_pos, &rx_ch2_pos,
+			rx_mask, slot_width, data_delay, odd_slot_offset);
+		regmap_write(dev->i2s_regmap, BCM2835_I2S_RXC_A_REG,
+			  format
+			| BCM2835_I2S_CH1_POS(rx_ch1_pos)
+			| BCM2835_I2S_CH2_POS(rx_ch2_pos));
+	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		bcm2835_i2s_calc_channel_pos(&tx_ch1_pos, &tx_ch2_pos,
+			tx_mask, slot_width, data_delay, odd_slot_offset);
+		regmap_write(dev->i2s_regmap, BCM2835_I2S_TXC_A_REG,
+			  format
+			| BCM2835_I2S_CH1_POS(tx_ch1_pos)
+			| BCM2835_I2S_CH2_POS(tx_ch2_pos));
+	}
 
 	/* Setup the I2S mode */
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

             reply	other threads:[~2020-03-24  9:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24  9:08 Matt Flax [this message]
2020-03-24  9:08 ` [PATCH] ASoC: bcm2835-i2s: substream alignment now independent in hwparams Matt Flax
2020-03-24  9:08 ` Matt Flax
2020-03-26 23:56 ` Matt Flax
2020-03-26 23:56   ` Matt Flax
2020-03-26 23:56   ` Matt Flax
2020-03-27  0:30   ` Matt Flax
2020-03-27 13:23     ` Matthias Reichl
2020-03-27 21:50       ` Matt Flax
2020-03-28 11:59         ` Matthias Reichl
2020-03-28 23:47           ` Matt Flax
2020-03-29 11:01             ` Matthias Reichl
2020-03-27 13:07   ` Mark Brown
2020-03-27 13:07     ` Mark Brown
2020-03-27 13:07     ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200324090823.20754-1-flatmax@flatmax.org \
    --to=flatmax@flatmax.org \
    --cc=allison@lohutok.net \
    --cc=alsa-devel@alsa-project.org \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=broonie@kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kstewart@linuxfoundation.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=nsaenzjulienne@suse.de \
    --cc=perex@perex.cz \
    --cc=rjui@broadcom.com \
    --cc=sbranden@broadcom.com \
    --cc=tglx@linutronix.de \
    --cc=tiwai@suse.com \
    --cc=yuehaibing@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.