[alsa-devel] [PATCH 1/3] ALSA: hda - show ICT/KAE control bits
Enable two debug options for S/PDIF Converter Control. KAE: Keep Alive Enable; ICT: IEC Coding Type.
Signed-off-by: Wang Xingchao xingchao.wang@intel.com --- sound/pci/hda/hda_codec.h | 4 ++++ sound/pci/hda/hda_proc.c | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index c422d33..141b3b8 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -386,6 +386,10 @@ enum { /* DIGITAL2 bits */ #define AC_DIG2_CC (0x7f<<0)
+/* DIGITAL3 bits */ +#define AC_DIG3_ICT (0xf<<0) +#define AC_DIG3_KAE (1<<23) + /* Pin widget control - 8bit */ #define AC_PINCTL_EPT (0x3<<0) #define AC_PINCTL_EPT_NATIVE 0 diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index 7e46258..a222aae 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -419,9 +419,13 @@ static void print_digital_conv(struct snd_info_buffer *buffer, snd_iprintf(buffer, " Pro"); if (digi1 & AC_DIG1_LEVEL) snd_iprintf(buffer, " GenLevel"); + if (digi1 & AC_DIG3_KAE) + snd_iprintf(buffer, " KAE"); snd_iprintf(buffer, "\n"); snd_iprintf(buffer, " Digital category: 0x%x\n", (digi1 >> 8) & AC_DIG2_CC); + snd_iprintf(buffer, " IEC Coding Type: 0x%x\n", + (digi1 >> 16) & AC_DIG3_ICT); }
static const char *get_pwr_state(u32 state)
As spec said, 1 indicates no copyright is asserted.
Signed-off-by: Wang Xingchao xingchao.wang@intel.com --- sound/pci/hda/hda_proc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index a222aae..bb9e8f9 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -412,7 +412,7 @@ static void print_digital_conv(struct snd_info_buffer *buffer, if (digi1 & AC_DIG1_EMPHASIS) snd_iprintf(buffer, " Preemphasis"); if (digi1 & AC_DIG1_COPYRIGHT) - snd_iprintf(buffer, " Copyright"); + snd_iprintf(buffer, " Non-Copyright"); if (digi1 & AC_DIG1_NONAUDIO) snd_iprintf(buffer, " Non-Audio"); if (digi1 & AC_DIG1_PROFESSIONAL)
At Mon, 13 Aug 2012 14:11:10 +0800, Wang Xingchao wrote:
As spec said, 1 indicates no copyright is asserted.
Signed-off-by: Wang Xingchao xingchao.wang@intel.com
Applied to for-linus branch (to be merge to 3.6 kernel). I also added Cc to stable, as this is an obvious bug to be fixed.
thanks,
Takashi
sound/pci/hda/hda_proc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index a222aae..bb9e8f9 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -412,7 +412,7 @@ static void print_digital_conv(struct snd_info_buffer *buffer, if (digi1 & AC_DIG1_EMPHASIS) snd_iprintf(buffer, " Preemphasis"); if (digi1 & AC_DIG1_COPYRIGHT)
snd_iprintf(buffer, " Copyright");
snd_iprintf(buffer, " Non-Copyright");-- 1.7.9.5
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Follow the protection way in nvhdmi_8ch_7x_pcm_prepare().
Signed-off-by: Wang Xingchao xingchao.wang@intel.com --- sound/pci/hda/patch_hdmi.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 641408d..aa414b9 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -831,6 +831,7 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, { int pinctl; int new_pinctl = 0; + struct hda_spdif_out *spdif = snd_hda_spdif_out_of_nid(codec, cvt_nid);
if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR) { pinctl = snd_hda_codec_read(codec, pin_nid, 0, @@ -859,7 +860,21 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, return -EINVAL; }
+ /* turn off SPDIF once; otherwise the IEC958 bits won't be updated */ + if (codec->spdif_status_reset && (spdif->ctls & AC_DIG1_ENABLE)) + snd_hda_codec_write(codec, + cvt_nid, + 0, + AC_VERB_SET_DIGI_CONVERT_1, + spdif->ctls & ~AC_DIG1_ENABLE & 0xff); snd_hda_codec_setup_stream(codec, cvt_nid, stream_tag, 0, format); + /* turn on again (if needed) */ + if (codec->spdif_status_reset && (spdif->ctls & AC_DIG1_ENABLE)) + snd_hda_codec_write(codec, + cvt_nid, + 0, + AC_VERB_SET_DIGI_CONVERT_1, + spdif->ctls & 0xff); return 0; }
At Mon, 13 Aug 2012 14:11:11 +0800, Wang Xingchao wrote:
Follow the protection way in nvhdmi_8ch_7x_pcm_prepare().
Could you clarify why this is needed?
I don't mean an objection, but just need to know if there is any other devices that need this fix. If there really is, better to put a bit more details in changelog or comment.
thanks,
Takashi
Signed-off-by: Wang Xingchao xingchao.wang@intel.com
sound/pci/hda/patch_hdmi.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 641408d..aa414b9 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -831,6 +831,7 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, { int pinctl; int new_pinctl = 0;
struct hda_spdif_out *spdif = snd_hda_spdif_out_of_nid(codec, cvt_nid);
if (snd_hda_query_pin_caps(codec, pin_nid) & AC_PINCAP_HBR) { pinctl = snd_hda_codec_read(codec, pin_nid, 0,
@@ -859,7 +860,21 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, return -EINVAL; }
- /* turn off SPDIF once; otherwise the IEC958 bits won't be updated */
- if (codec->spdif_status_reset && (spdif->ctls & AC_DIG1_ENABLE))
snd_hda_codec_write(codec,cvt_nid,0,AC_VERB_SET_DIGI_CONVERT_1,spdif->ctls & ~AC_DIG1_ENABLE & 0xff);- /* turn on again (if needed) */
- if (codec->spdif_status_reset && (spdif->ctls & AC_DIG1_ENABLE))
snd_hda_codec_write(codec,cvt_nid,0,AC_VERB_SET_DIGI_CONVERT_1,spdif->ctls & 0xff);}
-- 1.7.9.5
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, August 13, 2012 3:32 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH 3/3] ALSA: hda - Disable DigEn bit before stream-id change
At Mon, 13 Aug 2012 14:11:11 +0800, Wang Xingchao wrote:
Follow the protection way in nvhdmi_8ch_7x_pcm_prepare().
Could you clarify why this is needed?
I don't mean an objection, but just need to know if there is any other devices that need this fix. If there really is, better to put a bit more details in changelog or comment.
No, it's not really a fix for the issue. I just found the change when to investigate HDMI HBR issue and it does NOT help fix the issue. However I thought it's a fix to some other bug(seen from comments), so I think it's also needed as a common solution. Please correct me if I am wrong.
Thanks --xingchao
At Mon, 13 Aug 2012 07:37:12 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, August 13, 2012 3:32 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH 3/3] ALSA: hda - Disable DigEn bit before stream-id change
At Mon, 13 Aug 2012 14:11:11 +0800, Wang Xingchao wrote:
Follow the protection way in nvhdmi_8ch_7x_pcm_prepare().
Could you clarify why this is needed?
I don't mean an objection, but just need to know if there is any other devices that need this fix. If there really is, better to put a bit more details in changelog or comment.
No, it's not really a fix for the issue. I just found the change when to investigate HDMI HBR issue and it does NOT help fix the issue. However I thought it's a fix to some other bug(seen from comments), so I think it's also needed as a common solution. Please correct me if I am wrong.
I guess this was copied from setup_dig_out_stream() in hda_codec.c. This was a workaround for SPDIF output on some old non-HDMI codecs. But most of recent codecs shouldn't need it. You see that it's protected by codec->spdif_status_reset check.
Takashi
At Mon, 13 Aug 2012 09:51:33 +0200, Takashi Iwai wrote:
At Mon, 13 Aug 2012 07:37:12 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, August 13, 2012 3:32 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH 3/3] ALSA: hda - Disable DigEn bit before stream-id change
At Mon, 13 Aug 2012 14:11:11 +0800, Wang Xingchao wrote:
Follow the protection way in nvhdmi_8ch_7x_pcm_prepare().
Could you clarify why this is needed?
I don't mean an objection, but just need to know if there is any other devices that need this fix. If there really is, better to put a bit more details in changelog or comment.
No, it's not really a fix for the issue. I just found the change when to investigate HDMI HBR issue and it does NOT help fix the issue. However I thought it's a fix to some other bug(seen from comments), so I think it's also needed as a common solution. Please correct me if I am wrong.
I guess this was copied from setup_dig_out_stream() in hda_codec.c. This was a workaround for SPDIF output on some old non-HDMI codecs. But most of recent codecs shouldn't need it. You see that it's protected by codec->spdif_status_reset check.
BTW, you might have forgotten to set codec->spdif_status_reset = 1 during your test? Otherwise the code there isn't activated.
Though, this doesn't explain why it worked for nvidia and not for Intel. The IEC958 status temporary toggle code isn't active even for Nvidia, too.
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, August 13, 2012 4:05 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH 3/3] ALSA: hda - Disable DigEn bit before stream-id change
At Mon, 13 Aug 2012 09:51:33 +0200, Takashi Iwai wrote:
At Mon, 13 Aug 2012 07:37:12 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, August 13, 2012 3:32 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org Subject: Re: [PATCH 3/3] ALSA: hda - Disable DigEn bit before stream-id change
At Mon, 13 Aug 2012 14:11:11 +0800, Wang Xingchao wrote:
Follow the protection way in nvhdmi_8ch_7x_pcm_prepare().
Could you clarify why this is needed?
I don't mean an objection, but just need to know if there is any other devices that need this fix. If there really is, better to put a bit more details in changelog or comment.
No, it's not really a fix for the issue. I just found the change when to investigate HDMI HBR issue and it does
NOT help fix the issue.
However I thought it's a fix to some other bug(seen from comments), so I think it's also needed as a common solution. Please correct me if I am
wrong.
I guess this was copied from setup_dig_out_stream() in hda_codec.c. This was a workaround for SPDIF output on some old non-HDMI codecs. But most of recent codecs shouldn't need it. You see that it's protected by codec->spdif_status_reset check.
BTW, you might have forgotten to set codec->spdif_status_reset = 1 during your test? Otherwise the code there isn't activated.
Oh, thanks for clarification, will test that again. :)
Though, this doesn't explain why it worked for nvidia and not for Intel. The IEC958 status temporary toggle code isn't active even for Nvidia, too.
Thanks --xingchao
At Mon, 13 Aug 2012 14:11:09 +0800, Wang Xingchao wrote:
Enable two debug options for S/PDIF Converter Control. KAE: Keep Alive Enable; ICT: IEC Coding Type.
Signed-off-by: Wang Xingchao xingchao.wang@intel.com
sound/pci/hda/hda_codec.h | 4 ++++ sound/pci/hda/hda_proc.c | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index c422d33..141b3b8 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -386,6 +386,10 @@ enum { /* DIGITAL2 bits */ #define AC_DIG2_CC (0x7f<<0)
+/* DIGITAL3 bits */ +#define AC_DIG3_ICT (0xf<<0) +#define AC_DIG3_KAE (1<<23)
For DIG3, it should be 1<<7. (dig3 begins from bit 16.)
/* Pin widget control - 8bit */ #define AC_PINCTL_EPT (0x3<<0) #define AC_PINCTL_EPT_NATIVE 0 diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index 7e46258..a222aae 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -419,9 +419,13 @@ static void print_digital_conv(struct snd_info_buffer *buffer, snd_iprintf(buffer, " Pro"); if (digi1 & AC_DIG1_LEVEL) snd_iprintf(buffer, " GenLevel");
Comparing dig1 and dig3 bitmask looks strange. For clarity, better to have unsigned char dig3; ... dig3 = dig1 >> 16; if (dig3 & AC_DIG3_KAE) snd_iprintf(buffer, " KAE"); ... snd_iprintf(buffer, " IEC Coding Type: 0x%x\n", dig3 & AC_DIG3_ICT);
Takashi
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Takashi Iwai Sent: Monday, August 13, 2012 3:38 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 1/3] ALSA: hda - show ICT/KAE control bits
At Mon, 13 Aug 2012 14:11:09 +0800, Wang Xingchao wrote:
Enable two debug options for S/PDIF Converter Control. KAE: Keep Alive Enable; ICT: IEC Coding Type.
Signed-off-by: Wang Xingchao xingchao.wang@intel.com
sound/pci/hda/hda_codec.h | 4 ++++ sound/pci/hda/hda_proc.c | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index c422d33..141b3b8 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -386,6 +386,10 @@ enum { /* DIGITAL2 bits */ #define AC_DIG2_CC (0x7f<<0)
+/* DIGITAL3 bits */ +#define AC_DIG3_ICT (0xf<<0) +#define AC_DIG3_KAE (1<<23)
For DIG3, it should be 1<<7. (dig3 begins from bit 16.)
Okay.
/* Pin widget control - 8bit */ #define AC_PINCTL_EPT (0x3<<0) #define AC_PINCTL_EPT_NATIVE 0 diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index 7e46258..a222aae 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -419,9 +419,13 @@ static void print_digital_conv(struct snd_info_buffer
*buffer,
snd_iprintf(buffer, " Pro");if (digi1 & AC_DIG1_LEVEL) snd_iprintf(buffer, " GenLevel");
Comparing dig1 and dig3 bitmask looks strange.
What about also add digi2 to specify AC_DIG1_LEVEL and CC? They are bit 7:14.
For clarity, better to have unsigned char dig3; ... dig3 = dig1 >> 16; if (dig3 & AC_DIG3_KAE) snd_iprintf(buffer, " KAE"); ... snd_iprintf(buffer, " IEC Coding Type: 0x%x\n", dig3 & AC_DIG3_ICT);
Okay, I will fix that in next version. Thanks your review, Takashi. :)
thanks --xingchao
At Mon, 13 Aug 2012 07:49:40 +0000, Wang, Xingchao wrote:
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Takashi Iwai Sent: Monday, August 13, 2012 3:38 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 1/3] ALSA: hda - show ICT/KAE control bits
At Mon, 13 Aug 2012 14:11:09 +0800, Wang Xingchao wrote:
Enable two debug options for S/PDIF Converter Control. KAE: Keep Alive Enable; ICT: IEC Coding Type.
Signed-off-by: Wang Xingchao xingchao.wang@intel.com
sound/pci/hda/hda_codec.h | 4 ++++ sound/pci/hda/hda_proc.c | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index c422d33..141b3b8 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -386,6 +386,10 @@ enum { /* DIGITAL2 bits */ #define AC_DIG2_CC (0x7f<<0)
+/* DIGITAL3 bits */ +#define AC_DIG3_ICT (0xf<<0) +#define AC_DIG3_KAE (1<<23)
For DIG3, it should be 1<<7. (dig3 begins from bit 16.)
Okay.
/* Pin widget control - 8bit */ #define AC_PINCTL_EPT (0x3<<0) #define AC_PINCTL_EPT_NATIVE 0 diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index 7e46258..a222aae 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -419,9 +419,13 @@ static void print_digital_conv(struct snd_info_buffer
*buffer,
snd_iprintf(buffer, " Pro");if (digi1 & AC_DIG1_LEVEL) snd_iprintf(buffer, " GenLevel");
Comparing dig1 and dig3 bitmask looks strange.
What about also add digi2 to specify AC_DIG1_LEVEL and CC? They are bit 7:14.
These are DIG1_LEVEL is dig1. DIG2_CC isn't _compared_ with dig1. But it would be clearer if it's deduced from dig2, yes.
For clarity, better to have unsigned char dig3; ... dig3 = dig1 >> 16; if (dig3 & AC_DIG3_KAE) snd_iprintf(buffer, " KAE"); ... snd_iprintf(buffer, " IEC Coding Type: 0x%x\n", dig3 & AC_DIG3_ICT);
Okay, I will fix that in next version. Thanks your review, Takashi. :)
thanks,
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, August 13, 2012 3:54 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 1/3] ALSA: hda - show ICT/KAE control bits
At Mon, 13 Aug 2012 07:49:40 +0000, Wang, Xingchao wrote:
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Takashi Iwai Sent: Monday, August 13, 2012 3:38 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 1/3] ALSA: hda - show ICT/KAE control bits
At Mon, 13 Aug 2012 14:11:09 +0800, Wang Xingchao wrote:
Enable two debug options for S/PDIF Converter Control. KAE: Keep Alive Enable; ICT: IEC Coding Type.
Signed-off-by: Wang Xingchao xingchao.wang@intel.com
sound/pci/hda/hda_codec.h | 4 ++++ sound/pci/hda/hda_proc.c | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index c422d33..141b3b8 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -386,6 +386,10 @@ enum { /* DIGITAL2 bits */ #define AC_DIG2_CC (0x7f<<0)
+/* DIGITAL3 bits */ +#define AC_DIG3_ICT (0xf<<0) +#define AC_DIG3_KAE (1<<23)
For DIG3, it should be 1<<7. (dig3 begins from bit 16.)
Okay.
/* Pin widget control - 8bit */ #define AC_PINCTL_EPT (0x3<<0) #define AC_PINCTL_EPT_NATIVE 0 diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index 7e46258..a222aae 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -419,9 +419,13 @@ static void print_digital_conv(struct snd_info_buffer
*buffer,
snd_iprintf(buffer, " Pro");if (digi1 & AC_DIG1_LEVEL) snd_iprintf(buffer, " GenLevel");
Comparing dig1 and dig3 bitmask looks strange.
What about also add digi2 to specify AC_DIG1_LEVEL and CC? They are bit
7:14.
These are DIG1_LEVEL is dig1. DIG2_CC isn't _compared_ with dig1. But it would be clearer if it's deduced from dig2, yes.
Yeah, I made the v2 change.
Thanks --xingchao
participants (3)
-
Takashi Iwai -
Wang Xingchao -
Wang, Xingchao