[alsa-devel] ASoC: PCM formats for AC97
Tell me if this is right and I'll do a patch....
----------------------------------------------------------- The current mpc5200 code has:
static const struct snd_pcm_hardware psc_dma_hardware = { .formats = SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_BE | SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S32_BE,
struct snd_soc_dai psc_ac97_dai[] = { { .playback = { .formats = SNDRV_PCM_FMTBIT_S32_BE,
I think I have this backwards, the ac97 driver can take 8/16/24/32 it's the Bestcomm program that doesn't know about anything except 32b. So if we wrote new Bestcomm programs that expanded bytes from memory out into 32b DMA register writes, we could support 8/16b formats.
The correct formats would be: static const struct snd_pcm_hardware psc_dma_hardware = { .formats = SNDRV_PCM_FMTBIT_S32_BE,
struct snd_soc_dai psc_ac97_dai[] = { { .playback = { .formats = SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_BE | SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S32_BE,
After implementing new Bestcomm programs....
static const struct snd_pcm_hardware psc_dma_hardware = { .formats = SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_BE | SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S32_BE,
----------------------------------------------------------- A related issue in soc-dai.h:
#define SND_SOC_STD_AC97_FMTS (SNDRV_PCM_FMTBIT_S16_LE |\ SNDRV_PCM_FMTBIT_S32_LE |\ SNDRV_PCM_FMTBIT_S32_BE)
shouldn't this be #define SND_SOC_STD_AC97_FMTS (SNDRV_PCM_FMTBIT_S16 |\ SNDRV_PCM_FMTBIT_S32)
The first form says the AC97 hardware can take both endians, but it doesn't take both endians it takes the endian matching the machine it is attached to. Don't the standard AC97 formats also include 8 and 24b?
On Sat, Aug 08, 2009 at 02:44:48PM -0400, Jon Smirl wrote:
I think I have this backwards, the ac97 driver can take 8/16/24/32 it's the Bestcomm program that doesn't know about anything except 32b.
Remember, you need to consider the I2S driver as well.
#define SND_SOC_STD_AC97_FMTS (SNDRV_PCM_FMTBIT_S16_LE |\ SNDRV_PCM_FMTBIT_S32_LE |\ SNDRV_PCM_FMTBIT_S32_BE) shouldn't this be
#define SND_SOC_STD_AC97_FMTS (SNDRV_PCM_FMTBIT_S16 |\ SNDRV_PCM_FMTBIT_S32)
The first form says the AC97 hardware can take both endians, but it doesn't take both endians it takes the endian matching the machine it is attached to.
No, it can take any endianess - the wire format is fixed. The layout of the data in memory is of little interest to the CODEC. The formats used will be restricted by the CPU driver, the values there are essentially functioning as a wildcard match.
Don't the standard AC97 formats also include 8 and 24b?
They'll include whatever the controller is happy to render down onto the bus. That's just the ones that people have needed so far.
On Sun, Aug 9, 2009 at 6:48 AM, Mark Brownbroonie@sirena.org.uk wrote:
On Sat, Aug 08, 2009 at 02:44:48PM -0400, Jon Smirl wrote:
I think I have this backwards, the ac97 driver can take 8/16/24/32 it's the Bestcomm program that doesn't know about anything except 32b.
Remember, you need to consider the I2S driver as well.
#define SND_SOC_STD_AC97_FMTS (SNDRV_PCM_FMTBIT_S16_LE |\ SNDRV_PCM_FMTBIT_S32_LE |\ SNDRV_PCM_FMTBIT_S32_BE) shouldn't this be
#define SND_SOC_STD_AC97_FMTS (SNDRV_PCM_FMTBIT_S16 |\ SNDRV_PCM_FMTBIT_S32)
The first form says the AC97 hardware can take both endians, but it doesn't take both endians it takes the endian matching the machine it is attached to.
No, it can take any endianess - the wire format is fixed. The layout of the data in memory is of little interest to the CODEC. The formats used will be restricted by the CPU driver, the values there are essentially functioning as a wildcard match.
I agree with you, but I think the second form better indicates the wildcard than the first form. It makes it look like endianess is not a factor.
If you had used the second form I wouldn't have had a problem bringing up a powerpc driver.
Don't the standard AC97 formats also include 8 and 24b?
They'll include whatever the controller is happy to render down onto the bus. That's just the ones that people have needed so far.
We could add 8/24b now so to make thing easier on the next driver developer.
On Sun, Aug 09, 2009 at 09:32:22AM -0400, Jon Smirl wrote:
I agree with you, but I think the second form better indicates the wildcard than the first form. It makes it look like endianess is not a factor.
You need to specify both - the controller may be able to handle data in either endianness, not just the native format.
On Sun, Aug 9, 2009 at 10:37 AM, Mark Brownbroonie@opensource.wolfsonmicro.com wrote:
On Sun, Aug 09, 2009 at 09:32:22AM -0400, Jon Smirl wrote:
I agree with you, but I think the second form better indicates the wildcard than the first form. It makes it look like endianess is not a factor.
You need to specify both - the controller may be able to handle data in either endianness, not just the native format.
I thought endianess was fixed in the wire protocol standard. It's the host AC97 controller that is converting the endianess.
I struggled with this when first writing the host driver for my BE powerpc chip. I kept trying to encode the AC97 wire protocol definition into the AC97 codec. The trick is that you don't want to encode it since it is fixed by the standard.
If we declared the AC97 codecs like this, which they really are: .formats = SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE,
Then I would need a way to say my powerpc host AC97 controller needs BE on the input and that it converts it to LE on the output. ALSA doesn't have a way to say that.
Leaving the endianess off from the AC97 codec definition makes it implicit. .formats = SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_S24 | SNDRV_PCM_FMTBIT_S32, We don't need to declare an endianess because it is fixed in the standard. Now the ifdefs in the h file make this work for both BE and LE.
This also hides another implicit conversion. The AC97 codecs only take 24b. Again it is the host controller that is converting 8/16/32 to 24b.
On Sun, Aug 09, 2009 at 11:10:24AM -0400, Jon Smirl wrote:
We don't need to declare an endianess because it is fixed in the standard. Now the ifdefs in the h file make this work for both BE and LE.
The ifdefs in the header file define only the native endianness. There's no reason why an AC97 controller can't do byte swapping for itself but if we use the non-specific constants only native endianness will be advertised by the CODEC.
This also hides another implicit conversion. The AC97 codecs only take 24b. Again it is the host controller that is converting 8/16/32 to 24b.
This is exactly why we're defining this standard format for AC97 devices. The formats are just placeholders to make the standard matching stuff work without having to teach it about AC97, they have no influence on the behaviour of the CODEC.
On Sun, Aug 9, 2009 at 3:03 PM, Mark Brownbroonie@opensource.wolfsonmicro.com wrote:
On Sun, Aug 09, 2009 at 11:10:24AM -0400, Jon Smirl wrote:
We don't need to declare an endianess because it is fixed in the standard. Now the ifdefs in the h file make this work for both BE and LE.
The ifdefs in the header file define only the native endianness. There's no reason why an AC97 controller can't do byte swapping for itself but if we use the non-specific constants only native endianness will be advertised by the CODEC.
So the BE/LE are there to support the case of an AC97 host that can handle both endians. I think the mpc5200 can do that for I2S but not AC97.
This also hides another implicit conversion. The AC97 codecs only take 24b. Again it is the host controller that is converting 8/16/32 to 24b.
This is exactly why we're defining this standard format for AC97 devices. The formats are just placeholders to make the standard matching stuff work without having to teach it about AC97, they have no influence on the behaviour of the CODEC.
participants (3)
-
Jon Smirl -
Mark Brown -
Mark Brown