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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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 */ };
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
participants (4)
-
Arun Raghavan -
Austin, Brian -
Mohammad Bahathir Hashim -
Takashi Iwai