LinuxLists.cc - Re: looping S/PDIF data

[permalink] [raw]

Subject: Re: looping S/PDIF data

On 17-06-08 21:55, Pierre Ossman wrote:

> Ping
>
> On Sat, 31 May 2008 11:26:06 +0200
> Pierre Ossman <[email protected]> wrote:
>
>> I have a minor annoyance with the snd_trident driver that I was hoping
>> you might have a fix for:
>>
>> Pausing the output does not seem to take on the S/PDIF output. Analog
>> outputs properly stop in their tracks, but the S/PDIF port keeps
>> looping the current data buffer, which gives a rather unpleasant end
>> result. :)

Try the alsa-devel list...

Rene.

2008-06-19 10:05:53

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

At Wed, 18 Jun 2008 09:24:59 +0200,
Rene Herman wrote:
>
> On 17-06-08 21:55, Pierre Ossman wrote:
>
> > Ping
> >
> > On Sat, 31 May 2008 11:26:06 +0200
> > Pierre Ossman <[email protected]> wrote:
> >
> >> I have a minor annoyance with the snd_trident driver that I was hoping
> >> you might have a fix for:
> >>
> >> Pausing the output does not seem to take on the S/PDIF output. Analog
> >> outputs properly stop in their tracks, but the S/PDIF port keeps
> >> looping the current data buffer, which gives a rather unpleasant end
> >> result. :)
>
> Try the alsa-devel list...

Does the patch below help?

Takashi

diff --git a/sound/pci/trident/trident_main.c b/sound/pci/trident/trident_main.c
index bbcee2c..916d4b8 100644
--- a/sound/pci/trident/trident_main.c
+++ b/sound/pci/trident/trident_main.c
@@ -1593,7 +1593,11 @@ static int snd_trident_trigger(struct snd_pcm_substream *substream,
outb(trident->spdif_pcm_ctrl, TRID_REG(trident, NX_SPCTRL_SPCSO + 3));
} else {
outl(trident->spdif_pcm_bits, TRID_REG(trident, SI_SPDIF_CS));
- val = inl(TRID_REG(trident, SI_SERIAL_INTF_CTRL)) | SPDIF_EN;
+ val = inl(TRID_REG(trident, SI_SERIAL_INTF_CTRL));
+ if (go)
+ val |= SPDIF_EN;
+ else
+ val &= ~SPDIF_EN;
outl(val, TRID_REG(trident, SI_SERIAL_INTF_CTRL));
}
}

2008-06-19 10:06:11

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

2008-06-19 12:39:24

by Jaroslav Kysela

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On Thu, 19 Jun 2008, Takashi Iwai wrote:

> At Wed, 18 Jun 2008 09:24:59 +0200,
> Rene Herman wrote:
> >
> > On 17-06-08 21:55, Pierre Ossman wrote:
> >
> > > Ping
> > >
> > > On Sat, 31 May 2008 11:26:06 +0200
> > > Pierre Ossman <[email protected]> wrote:
> > >
> > >> I have a minor annoyance with the snd_trident driver that I was hoping
> > >> you might have a fix for:
> > >>
> > >> Pausing the output does not seem to take on the S/PDIF output. Analog
> > >> outputs properly stop in their tracks, but the S/PDIF port keeps
> > >> looping the current data buffer, which gives a rather unpleasant end
> > >> result. :)
> >
> > Try the alsa-devel list...
>
> Does the patch below help?
>
>
> Takashi
>
> diff --git a/sound/pci/trident/trident_main.c b/sound/pci/trident/trident_main.c
> index bbcee2c..916d4b8 100644
> --- a/sound/pci/trident/trident_main.c
> +++ b/sound/pci/trident/trident_main.c
> @@ -1593,7 +1593,11 @@ static int snd_trident_trigger(struct snd_pcm_substream *substream,
> outb(trident->spdif_pcm_ctrl, TRID_REG(trident, NX_SPCTRL_SPCSO + 3));
> } else {
> outl(trident->spdif_pcm_bits, TRID_REG(trident, SI_SPDIF_CS));
> - val = inl(TRID_REG(trident, SI_SERIAL_INTF_CTRL)) | SPDIF_EN;
> + val = inl(TRID_REG(trident, SI_SERIAL_INTF_CTRL));
> + if (go)
> + val |= SPDIF_EN;
> + else
> + val &= ~SPDIF_EN;
> outl(val, TRID_REG(trident, SI_SERIAL_INTF_CTRL));
> }
> }

I don't think that this patch is correct. DMA transfers should be disabled
by:

outl(what, TRID_REG(trident, T4D_STOP_B));
outl(val, TRID_REG(trident, T4D_AINTEN_B));

lines. Adding &= ~SPDIF_EN can disable output from AC97 to S/PDIF as well.

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

2008-06-19 12:48:34

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

At Thu, 19 Jun 2008 14:39:05 +0200 (CEST),
Jaroslav Kysela wrote:
>
> On Thu, 19 Jun 2008, Takashi Iwai wrote:
>
> > At Wed, 18 Jun 2008 09:24:59 +0200,
> > Rene Herman wrote:
> > >
> > > On 17-06-08 21:55, Pierre Ossman wrote:
> > >
> > > > Ping
> > > >
> > > > On Sat, 31 May 2008 11:26:06 +0200
> > > > Pierre Ossman <[email protected]> wrote:
> > > >
> > > >> I have a minor annoyance with the snd_trident driver that I was hoping
> > > >> you might have a fix for:
> > > >>
> > > >> Pausing the output does not seem to take on the S/PDIF output. Analog
> > > >> outputs properly stop in their tracks, but the S/PDIF port keeps
> > > >> looping the current data buffer, which gives a rather unpleasant end
> > > >> result. :)
> > >
> > > Try the alsa-devel list...
> >
> > Does the patch below help?
> >
> >
> > Takashi
> >
> > diff --git a/sound/pci/trident/trident_main.c b/sound/pci/trident/trident_main.c
> > index bbcee2c..916d4b8 100644
> > --- a/sound/pci/trident/trident_main.c
> > +++ b/sound/pci/trident/trident_main.c
> > @@ -1593,7 +1593,11 @@ static int snd_trident_trigger(struct snd_pcm_substream *substream,
> > outb(trident->spdif_pcm_ctrl, TRID_REG(trident, NX_SPCTRL_SPCSO + 3));
> > } else {
> > outl(trident->spdif_pcm_bits, TRID_REG(trident, SI_SPDIF_CS));
> > - val = inl(TRID_REG(trident, SI_SERIAL_INTF_CTRL)) | SPDIF_EN;
> > + val = inl(TRID_REG(trident, SI_SERIAL_INTF_CTRL));
> > + if (go)
> > + val |= SPDIF_EN;
> > + else
> > + val &= ~SPDIF_EN;
> > outl(val, TRID_REG(trident, SI_SERIAL_INTF_CTRL));
> > }
> > }
>
> I don't think that this patch is correct. DMA transfers should be disabled
> by:
>
> outl(what, TRID_REG(trident, T4D_STOP_B));
> outl(val, TRID_REG(trident, T4D_AINTEN_B));

They are already in the trigger callback as long as I saw the code
quickly. So the problem should be somewhere else.

> lines. Adding &= ~SPDIF_EN can disable output from AC97 to S/PDIF as well.

That's true.

Anyway, if you are up now, I'll let you hunt further as you are the
author of the driver :)

thanks,

Takashi

2008-06-19 23:02:39

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On Thu, 19 Jun 2008 14:39:05 +0200 (CEST)
Jaroslav Kysela <[email protected]> wrote:

>
> I don't think that this patch is correct. DMA transfers should be disabled
> by:
>
> outl(what, TRID_REG(trident, T4D_STOP_B));
> outl(val, TRID_REG(trident, T4D_AINTEN_B));
>
> lines. Adding &= ~SPDIF_EN can disable output from AC97 to S/PDIF as well.
>

FWIW, the patch had zero effect on the system here.

Rgds
--
-- Pierre Ossman

WARNING: This correspondence is being monitored by the
Swedish government. Use end-to-end encryption where
possible.

Attachments:

signature.asc (197.00 B)

2008-06-20 19:45:35

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On Thu, 19 Jun 2008 14:48:19 +0200
Takashi Iwai <[email protected]> wrote:

> At Thu, 19 Jun 2008 14:39:05 +0200 (CEST),
> Jaroslav Kysela wrote:
> >
> > I don't think that this patch is correct. DMA transfers should be disabled
> > by:
> >
> > outl(what, TRID_REG(trident, T4D_STOP_B));
> > outl(val, TRID_REG(trident, T4D_AINTEN_B));
>
> They are already in the trigger callback as long as I saw the code
> quickly. So the problem should be somewhere else.
>
> > lines. Adding &= ~SPDIF_EN can disable output from AC97 to S/PDIF as well.
>
> That's true.
>
> Anyway, if you are up now, I'll let you hunt further as you are the
> author of the driver :)
>

Are there any specs on this hardware? I could play around a bit myself,
but I can't really find any documentation on the registers.

Rgds
--
-- Pierre Ossman

WARNING: This correspondence is being monitored by the
Swedish government. Use end-to-end encryption where
possible.

Attachments:

signature.asc (197.00 B)

2008-06-20 19:52:32

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On 20-06-08 21:45, Pierre Ossman wrote:

> On Thu, 19 Jun 2008 14:48:19 +0200
> Takashi Iwai <[email protected]> wrote:
>
>> At Thu, 19 Jun 2008 14:39:05 +0200 (CEST),
>> Jaroslav Kysela wrote:
>>> I don't think that this patch is correct. DMA transfers should be disabled
>>> by:
>>>
>>> outl(what, TRID_REG(trident, T4D_STOP_B));
>>> outl(val, TRID_REG(trident, T4D_AINTEN_B));
>> They are already in the trigger callback as long as I saw the code
>> quickly. So the problem should be somewhere else.
>>
>>> lines. Adding &= ~SPDIF_EN can disable output from AC97 to S/PDIF as well.
>> That's true.
>>
>> Anyway, if you are up now, I'll let you hunt further as you are the
>> author of the driver :)
>>
>
> Are there any specs on this hardware? I could play around a bit myself,
> but I can't really find any documentation on the registers.

At:

ftp://ftp.alsa-project.org/pub/manuals/trident/

there's a 4D-Wave DX technical reference manual and .doc files which at
first glance look useful.

Rene.

2008-06-20 22:01:16

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On Fri, 20 Jun 2008 21:52:55 +0200
Rene Herman <[email protected]> wrote:

> On 20-06-08 21:45, Pierre Ossman wrote:
> >
> > Are there any specs on this hardware? I could play around a bit myself,
> > but I can't really find any documentation on the registers.
>
> At:
>
> ftp://ftp.alsa-project.org/pub/manuals/trident/
>
> there's a 4D-Wave DX technical reference manual and .doc files which at
> first glance look useful.
>

Ah, thanks. This is an NX, but hopefully they don't differ that much.

Rgds
--
-- Pierre Ossman

WARNING: This correspondence is being monitored by the
Swedish government. Use end-to-end encryption where
possible.

Attachments:

signature.asc (197.00 B)

2008-06-20 22:23:28

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On 21-06-08 00:00, Pierre Ossman wrote:

> On Fri, 20 Jun 2008 21:52:55 +0200 Rene Herman
> <[email protected]> wrote:
>
>> On 20-06-08 21:45, Pierre Ossman wrote:
>>> Are there any specs on this hardware? I could play around a bit
>>> myself, but I can't really find any documentation on the
>>> registers.
>> At:
>>
>> ftp://ftp.alsa-project.org/pub/manuals/trident/
>>
>> there's a 4D-Wave DX technical reference manual and .doc files
>> which at first glance look useful.
>>
>
> Ah, thanks. This is an NX, but hopefully they don't differ that much.

No idea, but if you come up with something and keep me in CC I'll test
it on a DX. Just acquired one on eBay a few days ago as a coincedence.

Rene.

2008-06-20 23:09:29

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On Sat, 21 Jun 2008 00:23:51 +0200
Rene Herman <[email protected]> wrote:

> On 21-06-08 00:00, Pierre Ossman wrote:
>
> > On Fri, 20 Jun 2008 21:52:55 +0200 Rene Herman
> > <[email protected]> wrote:
> >
> >> On 20-06-08 21:45, Pierre Ossman wrote:
> >>> Are there any specs on this hardware? I could play around a bit
> >>> myself, but I can't really find any documentation on the
> >>> registers.
> >> At:
> >>
> >> ftp://ftp.alsa-project.org/pub/manuals/trident/
> >>
> >> there's a 4D-Wave DX technical reference manual and .doc files
> >> which at first glance look useful.
> >>
> >
> > Ah, thanks. This is an NX, but hopefully they don't differ that much.
>
> No idea, but if you come up with something and keep me in CC I'll test
> it on a DX. Just acquired one on eBay a few days ago as a coincedence.
>

Well aren't things just so much easier with a decent spec. I have this
one solved now. :)

The following patch disables both the S/PDIF output, and the DMA engine
when paused. As for Jaroslav's comment, the documentation seems to
suggest that the DMA disabled by the main code path is just for the
mixer engine, which is not used when feeding the S/PDIF directly. This
is supported by my tests as S/PDIF output is paused when using hw:0.0
(the mixer engine), but not with hw:0.2 (the raw S/PDIF).

Regarding the DX, the docs say that the DX lacks S/PDIF, so it should
be unaffected. For the SiS chip, Takashi's patch might still be needed.

--

trident: pause s/pdif output

Stop the S/PDIF DMA engine and output when the device is told to pause.
It will keep on looping the current buffer contents if this isn't done.

Signed-off-by: Pierre Ossman <[email protected]>
--

diff --git a/sound/pci/trident/trident_main.c b/sound/pci/trident/trident_main.c
index bbcee2c..a69b420 100644
--- a/sound/pci/trident/trident_main.c
+++ b/sound/pci/trident/trident_main.c
@@ -1590,7 +1590,10 @@ static int snd_trident_trigger(struct snd_pcm_substream *substream,
if (spdif_flag) {
if (trident->device != TRIDENT_DEVICE_ID_SI7018) {
outl(trident->spdif_pcm_bits, TRID_REG(trident, NX_SPCSTATUS));
- outb(trident->spdif_pcm_ctrl, TRID_REG(trident, NX_SPCTRL_SPCSO + 3));
+ val = trident->spdif_pcm_ctrl;
+ if (!go)
+ val &= ~(0x28);
+ outb(val, TRID_REG(trident, NX_SPCTRL_SPCSO + 3));
} else {
outl(trident->spdif_pcm_bits, TRID_REG(trident, SI_SPDIF_CS));
val = inl(TRID_REG(trident, SI_SERIAL_INTF_CTRL)) | SPDIF_EN;

--
-- Pierre Ossman

WARNING: This correspondence is being monitored by the
Swedish government. Use end-to-end encryption where
possible.

Attachments:

signature.asc (197.00 B)

2008-06-21 12:14:18

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On 21-06-08 01:09, Pierre Ossman wrote:

> trident: pause s/pdif output
>
> Stop the S/PDIF DMA engine and output when the device is told to pause.
> It will keep on looping the current buffer contents if this isn't done.
>
> Signed-off-by: Pierre Ossman <[email protected]>

Verified to fix the reported pause problem on a 4DWaveNX (and indeed to
not affect a 4DWaveDX). The Trident SPDIF doesn't seem capable of 44100
which for me means it's not all that useful but you no doubt know
yourself that/if it is for you...

Tested-by: Rene Herman <[email protected]>

Rene.

2008-06-30 17:11:52

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On Sat, 21 Jun 2008 14:14:38 +0200
Rene Herman <[email protected]> wrote:

> On 21-06-08 01:09, Pierre Ossman wrote:
>
> > trident: pause s/pdif output
> >
> > Stop the S/PDIF DMA engine and output when the device is told to pause.
> > It will keep on looping the current buffer contents if this isn't done.
> >
> > Signed-off-by: Pierre Ossman <[email protected]>
>
> Verified to fix the reported pause problem on a 4DWaveNX (and indeed to
> not affect a 4DWaveDX). The Trident SPDIF doesn't seem capable of 44100
> which for me means it's not all that useful but you no doubt know
> yourself that/if it is for you...
>
> Tested-by: Rene Herman <[email protected]>

Sooo.... ack? nak? limbo?

--
-- Pierre Ossman

WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.

Attachments:

signature.asc (197.00 B)

2008-06-30 18:20:22

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On 30-06-08 19:11, Pierre Ossman wrote:

> On Sat, 21 Jun 2008 14:14:38 +0200
> Rene Herman <[email protected]> wrote:
>
>> On 21-06-08 01:09, Pierre Ossman wrote:
>>
>>> trident: pause s/pdif output
>>>
>>> Stop the S/PDIF DMA engine and output when the device is told to pause.
>>> It will keep on looping the current buffer contents if this isn't done.
>>>
>>> Signed-off-by: Pierre Ossman <[email protected]>
>> Verified to fix the reported pause problem on a 4DWaveNX (and indeed to
>> not affect a 4DWaveDX). The Trident SPDIF doesn't seem capable of 44100
>> which for me means it's not all that useful but you no doubt know
>> yourself that/if it is for you...
>>
>> Tested-by: Rene Herman <[email protected]>
>
>
> Sooo.... ack? nak? limbo?

... come on shake your body baby, do the limbo, I know ...

Your fix is commit f19a62ecba9d0963cee2b673f0ad34917b5e57ad in the
"sound-2.6" git repo (Takashi Iwai's tree), branch "master", at

git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git

also viewable at:

http://git.kernel.org/?p=linux/kernel/git/tiwai/sound-2.6.git;a=commit;h=f19a62ecba9d0963cee2b673f0ad34917b5e57ad

That commit is pre the 1.0.17-rc3 release/tag so it's in. Will find its
way into the kernel during the next merge window no doubt...

Rene.

2008-06-30 19:22:48

[permalink] [raw]

Subject: Re: [alsa-devel] looping S/PDIF data

On Mon, 30 Jun 2008 20:21:09 +0200
Rene Herman <[email protected]> wrote:

>
> Your fix is commit f19a62ecba9d0963cee2b673f0ad34917b5e57ad in the
> "sound-2.6" git repo (Takashi Iwai's tree), branch "master", at
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git
>
> also viewable at:
>
> http://git.kernel.org/?p=linux/kernel/git/tiwai/sound-2.6.git;a=commit;h=f19a62ecba9d0963cee2b673f0ad34917b5e57ad
>
> That commit is pre the 1.0.17-rc3 release/tag so it's in. Will find its
> way into the kernel during the next merge window no doubt...
>

Delightful. :)

Now I just need to poke my vendor of choice to backport it aswell...

Rgds
--
-- Pierre Ossman

WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.

Attachments:

signature.asc (197.00 B)

2008-07-01 10:17:09