diff mbox

ASoC: codecs-ac97: Remove rate constraints

Message ID 554D27A6.4040602@maciej.szmigiero.name (mailing list archive)
State New, archived
Headers show

Commit Message

Maciej S. Szmigiero May 8, 2015, 9:16 p.m. UTC
Remove rate constraints from generic ASoC AC'97 CODEC and make
it selectable in config.

Supported rates should be detected and constrained anyway by
AC'97 generic code - was tested with VT1613 CODEC and iMX6 SSI
controller.

This way this driver can be used for platforms which don't need
specialized AC'97 CODEC drivers while at the same avoiding
code duplication from implementing equivalent functionality in
a controller driver.

Resending due to no response received.

Signed-off-by: Maciej Szmigiero <mail@maciej.szmigiero.name>

Comments

Fabio Estevam May 8, 2015, 9:32 p.m. UTC | #1
On Fri, May 8, 2015 at 6:16 PM, Maciej S. Szmigiero
<mail@maciej.szmigiero.name> wrote:
> Remove rate constraints from generic ASoC AC'97 CODEC and make
> it selectable in config.

Shouldn't this be split in two patches?

>
> Supported rates should be detected and constrained anyway by
> AC'97 generic code - was tested with VT1613 CODEC and iMX6 SSI
> controller.

Nice, I would like to test this on a Udoo board. Care to share the dts
changes? (I know this is off topic for this list ;-) Apart from the
dts changes: are there still missing patches in linux-next to make
audio work in Udoo?

> This way this driver can be used for platforms which don't need
> specialized AC'97 CODEC drivers while at the same avoiding
> code duplication from implementing equivalent functionality in
> a controller driver.
>
> Resending due to no response received.

No need to put this in the commit log.

Thanks
Maciej S. Szmigiero May 8, 2015, 11:14 p.m. UTC | #2
W dniu 08.05.2015 23:32, Fabio Estevam pisze:
> On Fri, May 8, 2015 at 6:16 PM, Maciej S. Szmigiero
> <mail@maciej.szmigiero.name> wrote:
>> Remove rate constraints from generic ASoC AC'97 CODEC and make
>> it selectable in config.
> 
> Shouldn't this be split in two patches?

I've submitted it as one patch because they are two trivial changes
to make the generic ASoC AC'97 CODEC usable for me outside existing
platform files.

But naturally, I can split them and submit them separately
if that would be better.

>> Supported rates should be detected and constrained anyway by
>> AC'97 generic code - was tested with VT1613 CODEC and iMX6 SSI
>> controller.
> 
> Nice, I would like to test this on a Udoo board. Care to share the dts
> changes? (I know this is off topic for this list ;-) Apart from the
> dts changes: are there still missing patches in linux-next to make
> audio work in Udoo?

I currently have audio running on this board at kernel based on
vanilla 3.19 and porting required changes part-by-part to linux-next.

Changes required on vanilla 3.19 to have it working are:
* AC'97 audio support needs to be added to fsl-asoc-card,

* AC'97 CODEC platform device needs to be instantiated in fsl_ssi,

* IPG clock needs to be enabled in fsl_ssi AC'97 mode,
so AC'97 regs can be accessed,

* Few small fixes for AC'97 mode in fsl_ssi (missing switch label
for format, missing  fsl_ssi_dai_probe entry in fsl_ssi_ac97_dai,
etc.).

There also is a problem with this CODEC that it seems to pull samples
for S/PDIF output from time to time even if S/PDIF output is disabled.

By default this requests samples in AC'97 slots 10/11 via SLOTREQ,
which in turn causes SSI to enable these slots in SACCST register
and start sending half of the sound samples there.

The end result is that audio suddenly starts to play two times
too fast.

Currently, I have a workaround of setting S/PDIF slot assignment
in CODEC to first front pair so at least it doesn't affect
playback rate.

If you like to have these changes or DT file diff then naturally
I can share them, just they aren't production-quality as of now.

>> This way this driver can be used for platforms which don't need
>> specialized AC'97 CODEC drivers while at the same avoiding
>> code duplication from implementing equivalent functionality in
>> a controller driver.
>>
>> Resending due to no response received.
> 
> No need to put this in the commit log.
>
> Thanks

Thanks for looking into patch and best regards,
Maciej Szmigiero
Fabio Estevam May 8, 2015, 11:47 p.m. UTC | #3
Hi Maciej,

On Fri, May 8, 2015 at 8:14 PM, Maciej S. Szmigiero
<mail@maciej.szmigiero.name> wrote:

> I currently have audio running on this board at kernel based on
> vanilla 3.19 and porting required changes part-by-part to linux-next.
>
> Changes required on vanilla 3.19 to have it working are:
> * AC'97 audio support needs to be added to fsl-asoc-card,
>
> * AC'97 CODEC platform device needs to be instantiated in fsl_ssi,
>
> * IPG clock needs to be enabled in fsl_ssi AC'97 mode,
> so AC'97 regs can be accessed,
>
> * Few small fixes for AC'97 mode in fsl_ssi (missing switch label
> for format, missing  fsl_ssi_dai_probe entry in fsl_ssi_ac97_dai,
> etc.).
>
> There also is a problem with this CODEC that it seems to pull samples
> for S/PDIF output from time to time even if S/PDIF output is disabled.
>
> By default this requests samples in AC'97 slots 10/11 via SLOTREQ,
> which in turn causes SSI to enable these slots in SACCST register
> and start sending half of the sound samples there.
>
> The end result is that audio suddenly starts to play two times
> too fast.
>
> Currently, I have a workaround of setting S/PDIF slot assignment
> in CODEC to first front pair so at least it doesn't affect
> playback rate.
>
> If you like to have these changes or DT file diff then naturally
> I can share them, just they aren't production-quality as of now.

Good job!

Please keep me on Cc when you submit further ac97 patches / udoo dts,
so that I can help testing them.

Thanks,

Fabio Estevam
Maciej S. Szmigiero May 9, 2015, 10:06 p.m. UTC | #4
Hi Fabio,

W dniu 09.05.2015 01:47, Fabio Estevam pisze:
> Hi Maciej,
> 
(..)
> 
> Please keep me on Cc when you submit further ac97 patches / udoo dts,
> so that I can help testing them.
>
> Thanks,
> 
> Fabio Estevam

Thank you for your kind words,
naturally I will keep you CCed.

Best regards,
Maciej Szmigiero
diff mbox

Patch

diff --git a/sound/soc/codecs/ac97.c b/sound/soc/codecs/ac97.c
index d0ac723..5b3224c 100644
--- a/sound/soc/codecs/ac97.c
+++ b/sound/soc/codecs/ac97.c
@@ -44,10 +44,6 @@  static int ac97_prepare(struct snd_pcm_substream *substream,
 	return snd_ac97_set_rate(ac97, reg, substream->runtime->rate);
 }
 
-#define STD_AC97_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 |\
-		SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_44100 |\
-		SNDRV_PCM_RATE_48000)
-
 static const struct snd_soc_dai_ops ac97_dai_ops = {
 	.prepare	= ac97_prepare,
 };
@@ -58,13 +54,13 @@  static struct snd_soc_dai_driver ac97_dai = {
 		.stream_name = "AC97 Playback",
 		.channels_min = 1,
 		.channels_max = 2,
-		.rates = STD_AC97_RATES,
+		.rates = SNDRV_PCM_RATE_KNOT,
 		.formats = SND_SOC_STD_AC97_FMTS,},
 	.capture = {
 		.stream_name = "AC97 Capture",
 		.channels_min = 1,
 		.channels_max = 2,
-		.rates = STD_AC97_RATES,
+		.rates = SNDRV_PCM_RATE_KNOT,
 		.formats = SND_SOC_STD_AC97_FMTS,},
 	.ops = &ac97_dai_ops,
 };
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 061c465..84cad9a 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -16,7 +16,7 @@  config SND_SOC_ALL_CODECS
 	select SND_SOC_88PM860X if MFD_88PM860X
 	select SND_SOC_L3
 	select SND_SOC_AB8500_CODEC if ABX500_CORE
-	select SND_SOC_AC97_CODEC if SND_SOC_AC97_BUS
+	select SND_SOC_AC97_CODEC
 	select SND_SOC_AD1836 if SPI_MASTER
 	select SND_SOC_AD193X_SPI if SPI_MASTER
 	select SND_SOC_AD193X_I2C if I2C
@@ -211,8 +211,9 @@  config SND_SOC_AB8500_CODEC
 	tristate
 
 config SND_SOC_AC97_CODEC
-	tristate
+	tristate "Build generic ASoC AC97 CODEC driver"
 	select SND_AC97_CODEC
+	select SND_SOC_AC97_BUS
 
 config SND_SOC_AD1836
 	tristate