CS4206 and S/PDIF (again)

CS4206 and S/PDIF (again)

[Arun Raghavan]

Hello,
I saw an earlier thread where someone with a MacBook Pro 6.2 couldn't
get S/PDIF working. I'm facing the same problem on my MacBook Pro 5.5. I
know this used to work at some point because the LED from the digital
out used to glow (but my last recollection of this is from ca. Jan 2010,
so not too helpful).

I tried jiggling some controls in hda-analyzer, but I don't yet know
enough about this to be sure what I was doing made any sense.

I've uploaded alsa-info [1] and codecgraph [2] outout, and would really
appreciate help figuring out how I can go about debugging this problem.

[1] http://people.collabora.co.uk/~arun/alsa-info.txt
[2] http://people.collabora.co.uk/~arun/cs4206-mbp55.svg

Cheers,
Arun

p.s.: fwiw, I'm testing with a: aplay -v -D iec958:CARD=NVidia,DEV=0
SURROUNDTEST_011212.wav

Re: CS4206 and S/PDIF (again)

[Arun Raghavan]

On Tue, 2011-03-01 at 10:36 +0530, Arun Raghavan wrote:
> Hello,
> I saw an earlier thread where someone with a MacBook Pro 6.2 couldn't
> get S/PDIF working. I'm facing the same problem on my MacBook Pro 5.5. I
> know this used to work at some point because the LED from the digital
> out used to glow (but my last recollection of this is from ca. Jan 2010,
> so not too helpful).

Another thing that's unlikely to be helpful. Occasionally, jiggling the
headphone jack causes the light to blink once.

-- Arun

Re: CS4206 and S/PDIF (again)

[Arun Raghavan]

On Tue, 2011-03-01 at 11:13 +0530, Arun Raghavan wrote:
> On Tue, 2011-03-01 at 10:36 +0530, Arun Raghavan wrote:
> > Hello,
> > I saw an earlier thread where someone with a MacBook Pro 6.2 couldn't
> > get S/PDIF working. I'm facing the same problem on my MacBook Pro 5.5. I
> > know this used to work at some point because the LED from the digital
> > out used to glow (but my last recollection of this is from ca. Jan 2010,
> > so not too helpful).
> 
> Another thing that's unlikely to be helpful. Occasionally, jiggling the
> headphone jack causes the light to blink once.

I meant headphone mixer control, sorry.

-- Arun

CS4206 and S/PDIF (again)

[Arun Raghavan]

I bisected this down to the erratum fix added in commit a769cbcf. From
what I can tell, the erratum only applies to CS4207, so I'm attaching a
patch to make the fix conditional. CC'ing the author of that commit to
verify that this is correct.

Cheers,
Arun

[PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Arun Raghavan]

From: Arun Raghavan <arun@accosted.net>

The commit a769cbcf60cee51f4431c0938acd39e7e5b76b8d
    ALSA: hda - Add errata initverb sequence for CS42xx codecs
applies a fix for a chip erratum that is specific to CS4207
unconditionally. On CS4206, this causes digital output to never be
activated.
---
 sound/pci/hda/patch_cirrus.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index a07b031..9804b09 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -1065,8 +1065,9 @@ static int cs_init(struct hda_codec *codec)
 {
 	struct cs_spec *spec = codec->spec;
 
-	/* init_verb sequence for C0/C1/C2 errata*/
-	snd_hda_sequence_write(codec, cs_errata_init_verbs);
+	/* init_verb sequence for CS4207 C0/C1/C2 errata */
+	if (codec->vendor_id == 0x10134207)
+		snd_hda_sequence_write(codec, cs_errata_init_verbs);
 
 	snd_hda_sequence_write(codec, cs_coef_init_verbs);
 
-- 
1.7.4.1

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Austin, Brian]

That should not happen. This sequence was tested on all version of cs420x

Is there some output of the failure you can report?

On 3/2/11 3:48 PM, "Arun Raghavan" <arun.raghavan@collabora.co.uk> wrote:

>From: Arun Raghavan <arun@accosted.net>
>
>The commit a769cbcf60cee51f4431c0938acd39e7e5b76b8d
>    ALSA: hda - Add errata initverb sequence for CS42xx codecs
>applies a fix for a chip erratum that is specific to CS4207
>unconditionally. On CS4206, this causes digital output to never be
>activated.
>---
> sound/pci/hda/patch_cirrus.c |    5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
>index a07b031..9804b09 100644
>--- a/sound/pci/hda/patch_cirrus.c
>+++ b/sound/pci/hda/patch_cirrus.c
>@@ -1065,8 +1065,9 @@ static int cs_init(struct hda_codec *codec)
> {
>     struct cs_spec *spec = codec->spec;
> 
>-    /* init_verb sequence for C0/C1/C2 errata*/
>-    snd_hda_sequence_write(codec, cs_errata_init_verbs);
>+    /* init_verb sequence for CS4207 C0/C1/C2 errata */
>+    if (codec->vendor_id == 0x10134207)
>+        snd_hda_sequence_write(codec, cs_errata_init_verbs);
> 
>     snd_hda_sequence_write(codec, cs_coef_init_verbs);
> 
>-- 
>1.7.4.1
>
>

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Mohammad Bahathir Hashim]

On 2011-03-02, Austin, Brian <Brian.Austin@cirrus.com> wrote:
> That should not happen. This sequence was tested on all version of cs420x
>
> Is there some output of the failure you can report?
>

Yes, no SPDIF out from my MBP 6,2 if the sequence is enabled. Then,
MBP  might be using special version of CS4206. :) If this is true,
then, exclude MacBookPro6,2 from loading the errata code.

Thank you.

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Arun Raghavan]

On Wed, 2011-03-02 at 22:03 +0000, Austin, Brian wrote:
> That should not happen. This sequence was tested on all version of cs420x
> 
> Is there some output of the failure you can report?

I haven't seen any output on failure (other than the fact that the
digital out never comes on if this sequence is used). My hardware is a
MacBook Pro 5.5 (and another person sees this line causing the same
issue for them on the MBP .62 as well).

-- Arun

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Takashi Iwai]

At Thu,  3 Mar 2011 03:18:18 +0530,
Arun Raghavan wrote:
> 
> From: Arun Raghavan <arun@accosted.net>
> 
> The commit a769cbcf60cee51f4431c0938acd39e7e5b76b8d
>     ALSA: hda - Add errata initverb sequence for CS42xx codecs
> applies a fix for a chip erratum that is specific to CS4207
> unconditionally. On CS4206, this causes digital output to never be
> activated.

It might be the power-state the verb table sets?
Try to comment out only the line setting SPDIF Tx to D3,
  {0x08, AC_VERB_SET_POWER_STATE, 0x03}
instead of disabling the whole verbs, just for testing.


thanks,

Takashi

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Arun Raghavan]

On Thu, 2011-03-03 at 13:01 +0100, Takashi Iwai wrote:
> At Thu,  3 Mar 2011 03:18:18 +0530,
> Arun Raghavan wrote:
> > 
> > From: Arun Raghavan <arun@accosted.net>
> > 
> > The commit a769cbcf60cee51f4431c0938acd39e7e5b76b8d
> >     ALSA: hda - Add errata initverb sequence for CS42xx codecs
> > applies a fix for a chip erratum that is specific to CS4207
> > unconditionally. On CS4206, this causes digital output to never be
> > activated.
> 
> It might be the power-state the verb table sets?
> Try to comment out only the line setting SPDIF Tx to D3,
>   {0x08, AC_VERB_SET_POWER_STATE, 0x03}
> instead of disabling the whole verbs, just for testing.

Indeed, this also works.

-- Arun

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Takashi Iwai]

At Thu, 03 Mar 2011 17:50:22 +0530,
Arun Raghavan wrote:
> 
> On Thu, 2011-03-03 at 13:01 +0100, Takashi Iwai wrote:
> > At Thu,  3 Mar 2011 03:18:18 +0530,
> > Arun Raghavan wrote:
> > > 
> > > From: Arun Raghavan <arun@accosted.net>
> > > 
> > > The commit a769cbcf60cee51f4431c0938acd39e7e5b76b8d
> > >     ALSA: hda - Add errata initverb sequence for CS42xx codecs
> > > applies a fix for a chip erratum that is specific to CS4207
> > > unconditionally. On CS4206, this causes digital output to never be
> > > activated.
> > 
> > It might be the power-state the verb table sets?
> > Try to comment out only the line setting SPDIF Tx to D3,
> >   {0x08, AC_VERB_SET_POWER_STATE, 0x03}
> > instead of disabling the whole verbs, just for testing.
> 
> Indeed, this also works.

OK, to be sure, try the patch below.


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Date: Thu, 3 Mar 2011 14:54:19 +0100
Subject: [PATCH] ALSA: hda - Don't set to D3 in Cirrus errata init verbs

The errata init verbs for CS42xx codecs contain the verbs to set
the power-state of SPDIF nodes to D3, which seem to break the SPDIF
output on some MacBooks.  Since this is executed during the power-up
initialization, we shouldn't turn them down there.

Reported-by: Arun Raghavan <arun.raghavan@collabora.co.uk>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_cirrus.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index a07b031..067982f 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -1039,9 +1039,11 @@ static struct hda_verb cs_errata_init_verbs[] = {
 	{0x11, AC_VERB_SET_PROC_COEF, 0x0008},
 	{0x11, AC_VERB_SET_PROC_STATE, 0x00},
 
+#if 0 /* Don't to set to D3 as we are in power-up sequence */
 	{0x07, AC_VERB_SET_POWER_STATE, 0x03}, /* S/PDIF Rx: D3 */
 	{0x08, AC_VERB_SET_POWER_STATE, 0x03}, /* S/PDIF Tx: D3 */
 	/*{0x01, AC_VERB_SET_POWER_STATE, 0x03},*/ /* AFG: D3 This is already handled */
+#endif
 
 	{} /* terminator */
 };
-- 
1.7.3.4

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Arun Raghavan]

On Thu, 2011-03-03 at 14:58 +0100, Takashi Iwai wrote:
> At Thu, 03 Mar 2011 17:50:22 +0530,
> Arun Raghavan wrote:
> > 
> > On Thu, 2011-03-03 at 13:01 +0100, Takashi Iwai wrote:
> > > At Thu,  3 Mar 2011 03:18:18 +0530,
> > > Arun Raghavan wrote:
> > > > 
> > > > From: Arun Raghavan <arun@accosted.net>
> > > > 
> > > > The commit a769cbcf60cee51f4431c0938acd39e7e5b76b8d
> > > >     ALSA: hda - Add errata initverb sequence for CS42xx codecs
> > > > applies a fix for a chip erratum that is specific to CS4207
> > > > unconditionally. On CS4206, this causes digital output to never be
> > > > activated.
> > > 
> > > It might be the power-state the verb table sets?
> > > Try to comment out only the line setting SPDIF Tx to D3,
> > >   {0x08, AC_VERB_SET_POWER_STATE, 0x03}
> > > instead of disabling the whole verbs, just for testing.
> > 
> > Indeed, this also works.
> 
> OK, to be sure, try the patch below.

Yep, works just fine.

Thanks,
Arun

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Takashi Iwai]

At Thu, 03 Mar 2011 19:37:52 +0530,
Arun Raghavan wrote:
> 
> On Thu, 2011-03-03 at 14:58 +0100, Takashi Iwai wrote:
> > At Thu, 03 Mar 2011 17:50:22 +0530,
> > Arun Raghavan wrote:
> > > 
> > > On Thu, 2011-03-03 at 13:01 +0100, Takashi Iwai wrote:
> > > > At Thu,  3 Mar 2011 03:18:18 +0530,
> > > > Arun Raghavan wrote:
> > > > > 
> > > > > From: Arun Raghavan <arun@accosted.net>
> > > > > 
> > > > > The commit a769cbcf60cee51f4431c0938acd39e7e5b76b8d
> > > > >     ALSA: hda - Add errata initverb sequence for CS42xx codecs
> > > > > applies a fix for a chip erratum that is specific to CS4207
> > > > > unconditionally. On CS4206, this causes digital output to never be
> > > > > activated.
> > > > 
> > > > It might be the power-state the verb table sets?
> > > > Try to comment out only the line setting SPDIF Tx to D3,
> > > >   {0x08, AC_VERB_SET_POWER_STATE, 0x03}
> > > > instead of disabling the whole verbs, just for testing.
> > > 
> > > Indeed, this also works.
> > 
> > OK, to be sure, try the patch below.
> 
> Yep, works just fine.

Thanks for checking.  Merged now to sound git tree.


Takashi

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Austin, Brian]

So the verb sequence that this refers to needs to be done _before_ the
controls are built.  If the controls are built, then this sequence can
clobber some outputs and inputs.  It's my fault, I didn¹t put this code in
the correct place. This sequence should go into the beginning of
cs_build_controls() so that it is run before build_output and build_input
are called.

I can make the changes to fix this.

Thanks again for the help
Brian

On 3/3/11 8:20 AM, "Takashi Iwai" <tiwai@suse.de> wrote:

>At Thu, 03 Mar 2011 19:37:52 +0530,
>Arun Raghavan wrote:
>> 
>> On Thu, 2011-03-03 at 14:58 +0100, Takashi Iwai wrote:
>> > At Thu, 03 Mar 2011 17:50:22 +0530,
>> > Arun Raghavan wrote:
>> > > 
>> > > On Thu, 2011-03-03 at 13:01 +0100, Takashi Iwai wrote:
>> > > > At Thu,  3 Mar 2011 03:18:18 +0530,
>> > > > Arun Raghavan wrote:
>> > > > > 
>> > > > > From: Arun Raghavan <arun@accosted.net>
>> > > > > 
>> > > > > The commit a769cbcf60cee51f4431c0938acd39e7e5b76b8d
>> > > > >     ALSA: hda - Add errata initverb sequence for CS42xx codecs
>> > > > > applies a fix for a chip erratum that is specific to CS4207
>> > > > > unconditionally. On CS4206, this causes digital output to never
>>be
>> > > > > activated.
>> > > > 
>> > > > It might be the power-state the verb table sets?
>> > > > Try to comment out only the line setting SPDIF Tx to D3,
>> > > >   {0x08, AC_VERB_SET_POWER_STATE, 0x03}
>> > > > instead of disabling the whole verbs, just for testing.
>> > > 
>> > > Indeed, this also works.
>> > 
>> > OK, to be sure, try the patch below.
>> 
>> Yep, works just fine.
>
>Thanks for checking.  Merged now to sound git tree.
>
>
>Takashi
>

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Takashi Iwai]

At Thu, 3 Mar 2011 18:49:42 +0000,
Austin, Brian wrote:
> 
> So the verb sequence that this refers to needs to be done _before_ the
> controls are built.  If the controls are built, then this sequence can
> clobber some outputs and inputs.  It's my fault, I didn¹t put this code in
> the correct place. This sequence should go into the beginning of
> cs_build_controls() so that it is run before build_output and build_input
> are called.

The init is actually called before build_controls.  So this should be
fine as is.

The problem was that this verb is a part of initialization after the
power-status change.  The power-status change is done before calling
the init (otherwise the tree-parsing might fail), and the power-status
isn't changed in the init itself.


thanks,

Takashi

> 
> I can make the changes to fix this.
> 
> Thanks again for the help
> Brian
> 
> On 3/3/11 8:20 AM, "Takashi Iwai" <tiwai@suse.de> wrote:
> 
> >At Thu, 03 Mar 2011 19:37:52 +0530,
> >Arun Raghavan wrote:
> >> 
> >> On Thu, 2011-03-03 at 14:58 +0100, Takashi Iwai wrote:
> >> > At Thu, 03 Mar 2011 17:50:22 +0530,
> >> > Arun Raghavan wrote:
> >> > > 
> >> > > On Thu, 2011-03-03 at 13:01 +0100, Takashi Iwai wrote:
> >> > > > At Thu,  3 Mar 2011 03:18:18 +0530,
> >> > > > Arun Raghavan wrote:
> >> > > > > 
> >> > > > > From: Arun Raghavan <arun@accosted.net>
> >> > > > > 
> >> > > > > The commit a769cbcf60cee51f4431c0938acd39e7e5b76b8d
> >> > > > >     ALSA: hda - Add errata initverb sequence for CS42xx codecs
> >> > > > > applies a fix for a chip erratum that is specific to CS4207
> >> > > > > unconditionally. On CS4206, this causes digital output to never
> >>be
> >> > > > > activated.
> >> > > > 
> >> > > > It might be the power-state the verb table sets?
> >> > > > Try to comment out only the line setting SPDIF Tx to D3,
> >> > > >   {0x08, AC_VERB_SET_POWER_STATE, 0x03}
> >> > > > instead of disabling the whole verbs, just for testing.
> >> > > 
> >> > > Indeed, this also works.
> >> > 
> >> > OK, to be sure, try the patch below.
> >> 
> >> Yep, works just fine.
> >
> >Thanks for checking.  Merged now to sound git tree.
> >
> >
> >Takashi
> >
> 
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ALSA: hda - apply CS420X erratum fix for CS4207 only

[Mohammad Bahathir Hashim]

I am happy to report, that the patch works on my MacBookPro6,2 with
vanilla Linux 2.6.37. 

Arigatou Gozaimasita. :)
Thank you.

On 2011-03-03, Takashi Iwai <tiwai@suse.de> wrote:
> OK, to be sure, try the patch below.
>
>
> thanks,
>
> Takashi
>
> ---
> From: Takashi Iwai <tiwai@suse.de>
> Date: Thu, 3 Mar 2011 14:54:19 +0100
> Subject: [PATCH] ALSA: hda - Don't set to D3 in Cirrus errata init verbs
>
> The errata init verbs for CS42xx codecs contain the verbs to set
> the power-state of SPDIF nodes to D3, which seem to break the SPDIF
> output on some MacBooks.  Since this is executed during the power-up
> initialization, we shouldn't turn them down there.
>
> Reported-by: Arun Raghavan <arun.raghavan@collabora.co.uk>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/pci/hda/patch_cirrus.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
> index a07b031..067982f 100644
> --- a/sound/pci/hda/patch_cirrus.c
> +++ b/sound/pci/hda/patch_cirrus.c
> @@ -1039,9 +1039,11 @@ static struct hda_verb cs_errata_init_verbs[] = {
>  	{0x11, AC_VERB_SET_PROC_COEF, 0x0008},
>  	{0x11, AC_VERB_SET_PROC_STATE, 0x00},
>  
> +#if 0 /* Don't to set to D3 as we are in power-up sequence */
>  	{0x07, AC_VERB_SET_POWER_STATE, 0x03}, /* S/PDIF Rx: D3 */
>  	{0x08, AC_VERB_SET_POWER_STATE, 0x03}, /* S/PDIF Tx: D3 */
>  	/*{0x01, AC_VERB_SET_POWER_STATE, 0x03},*/ /* AFG: D3 This is already handled */
> +#endif
>  
>  	{} /* terminator */
>  };

Re: CS4206 and S/PDIF (again)

[Austin, Brian]

The verb sequence works for all 420x versions and has no impact on which
version it is applied to.

Brian

On 3/2/11 3:48 PM, "Arun Raghavan" <arun.raghavan@collabora.co.uk> wrote:

>I bisected this down to the erratum fix added in commit a769cbcf. From
>what I can tell, the erratum only applies to CS4207, so I'm attaching a
>patch to make the fix conditional. CC'ing the author of that commit to
>verify that this is correct.
>
>Cheers,
>Arun
>
>