From: Matt Flax <flatmax@flatmax.org>
To: Mark Brown <broonie@kernel.org>
Cc: alsa-devel@alsa-project.org,
Stephen Warren <swarren@wwwdotorg.org>,
Lee Jones <lee@kernel.org>,
phil@raspberrypi.org, Eric Anholt <eric@anholt.net>,
Florian Meier <florian.meier@koalo.de>,
linux-rpi-kernel@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] ASoC: bcm2835: Increase channels_max to 8
Date: Sun, 5 Feb 2017 12:26:31 +1100 [thread overview]
Message-ID: <a7f5de09-b07b-638d-466f-525e68ebf0f9@flatmax.org> (raw)
In-Reply-To: <20170204154944.timxhmmibc4kngly@sirena.org.uk>
On 05/02/17 02:49, Mark Brown wrote:
> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>
>> switch (params_channels(params)) {
>> case 2:
>> + case 8:
>> format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>> format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>> format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> I can't understand how this works. We're programming the hardware in an
> identical fashion for both channel counts, that means that what we're
> sending will be indistinguishable from a garbled stereo stream. How
> does this produce working clocks or manage to sync up reliably with
> external clocks?
>
I am not surprised it is confusing, to my knowledge this is the first
time that someone has demonstrated this type of I2S signalling.
I will attempt to explain why nothing is garbled by talking about what
the BCM2835 silicon does.
In this case BCM2835 is a clock slave and not concerned with generating
the I2S clocks, only receiving - think of all clocks and synchrony being
generated/managed in hardware on the sound card.
In short the BCM2835 silicon is only concerned with shifting bits into
two hardware registers which (after DMA) are interpreted by ALSA
according to the specified channel count.
Let me be a little more explicit. Regarding the BCM2835 I2S silicon :
The BCM2835 I2S silicon is concerned with shifting data from I2S bit
streams into hardware registers. The BCM2835 is only concerned with the
word format for the hardware registers. The BCM2835 silicon doesn't care
about channel count. This is why we use format=* in this part of the
bcm2835-i2s.c driver. Specifically we are concerned with the protocol's
word format (e.g. BCM2835_I2S_CH1(format)) and the location of the word
onset in the bit stream (e.g. BCM2835_I2S_CHPOS(ch1pos)).
This is (to my knowledge) the first time someone has tried this concept
and proven that it is successful and robust. My gut feeling is that it
will work will most/all silicon. In my opinion it is a little
evolution/revolution in I2S signalling.
Personally, I think it would be better if the code read as follows :
// if the channel count is even
if ( fmod((float)params_channels(params)) != 1 ) {
format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
} else
return -EINVAL;
I would even prefer to get rid of the channel count guard as strictly
speaking the format of the I2S bit stream has nothing to do with the
channel count !
thanks
Matt
WARNING: multiple messages have this Message-ID (diff)
From: flatmax@flatmax.org (Matt Flax)
To: linux-arm-kernel@lists.infradead.org
Subject: [alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8
Date: Sun, 5 Feb 2017 12:26:31 +1100 [thread overview]
Message-ID: <a7f5de09-b07b-638d-466f-525e68ebf0f9@flatmax.org> (raw)
In-Reply-To: <20170204154944.timxhmmibc4kngly@sirena.org.uk>
On 05/02/17 02:49, Mark Brown wrote:
> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>
>> switch (params_channels(params)) {
>> case 2:
>> + case 8:
>> format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>> format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>> format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> I can't understand how this works. We're programming the hardware in an
> identical fashion for both channel counts, that means that what we're
> sending will be indistinguishable from a garbled stereo stream. How
> does this produce working clocks or manage to sync up reliably with
> external clocks?
>
I am not surprised it is confusing, to my knowledge this is the first
time that someone has demonstrated this type of I2S signalling.
I will attempt to explain why nothing is garbled by talking about what
the BCM2835 silicon does.
In this case BCM2835 is a clock slave and not concerned with generating
the I2S clocks, only receiving - think of all clocks and synchrony being
generated/managed in hardware on the sound card.
In short the BCM2835 silicon is only concerned with shifting bits into
two hardware registers which (after DMA) are interpreted by ALSA
according to the specified channel count.
Let me be a little more explicit. Regarding the BCM2835 I2S silicon :
The BCM2835 I2S silicon is concerned with shifting data from I2S bit
streams into hardware registers. The BCM2835 is only concerned with the
word format for the hardware registers. The BCM2835 silicon doesn't care
about channel count. This is why we use format=* in this part of the
bcm2835-i2s.c driver. Specifically we are concerned with the protocol's
word format (e.g. BCM2835_I2S_CH1(format)) and the location of the word
onset in the bit stream (e.g. BCM2835_I2S_CHPOS(ch1pos)).
This is (to my knowledge) the first time someone has tried this concept
and proven that it is successful and robust. My gut feeling is that it
will work will most/all silicon. In my opinion it is a little
evolution/revolution in I2S signalling.
Personally, I think it would be better if the code read as follows :
// if the channel count is even
if ( fmod((float)params_channels(params)) != 1 ) {
format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
} else
return -EINVAL;
I would even prefer to get rid of the channel count guard as strictly
speaking the format of the I2S bit stream has nothing to do with the
channel count !
thanks
Matt
next prev parent reply other threads:[~2017-02-05 1:26 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-01 23:37 [PATCH] ASoC: bcm2835: Increase channels_max to 8 Matt Flax
2017-02-01 23:37 ` Matt Flax
2017-02-04 15:49 ` Mark Brown
2017-02-04 15:49 ` Mark Brown
2017-02-05 1:26 ` Matt Flax [this message]
2017-02-05 1:26 ` [alsa-devel] " Matt Flax
2017-02-05 10:20 ` Florian Kauer
2017-02-05 10:20 ` Florian Kauer
2017-02-05 10:55 ` Matt Flax
2017-02-05 10:55 ` [alsa-devel] " Matt Flax
2017-02-05 16:34 ` Mark Brown
2017-02-05 16:34 ` [alsa-devel] " Mark Brown
2017-02-05 20:37 ` Matt Flax
2017-02-05 20:37 ` [alsa-devel] " Matt Flax
2017-02-05 22:49 ` Matt Flax
2017-02-05 22:49 ` [alsa-devel] " Matt Flax
2017-02-06 16:43 ` Mark Brown
2017-02-06 16:43 ` [alsa-devel] " Mark Brown
2017-02-06 20:25 ` Matt Flax
2017-02-06 20:25 ` [alsa-devel] " Matt Flax
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=a7f5de09-b07b-638d-466f-525e68ebf0f9@flatmax.org \
--to=flatmax@flatmax.org \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=eric@anholt.net \
--cc=florian.meier@koalo.de \
--cc=lee@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-rpi-kernel@lists.infradead.org \
--cc=phil@raspberrypi.org \
--cc=swarren@wwwdotorg.org \
/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.