[alsa-devel] [PATCH] Show processing coefficients in codec proc file
This patch is helpful for tracking down bugs without having all information about the chip.
At Thu, 19 Aug 2010 20:17:42 +0200, David Henningsson wrote:
This patch is helpful for tracking down bugs without having all information about the chip.
I don't want to change coef index in reading a proc file in general. If any, you should implement a codec-specific hook.
thanks,
Takashi
-- David Henningsson, Canonical Ltd. http://launchpad.net/~diwic [2 0001-Show-processing-coefficients-in-codec-proc-file.patch <text/x-patch (7bit)>]
From 568a3c0e45396cf6cfd8e8dac13f76f756c7e3f3 Mon Sep 17 00:00:00 2001
From: David Henningsson david.henningsson@canonical.com Date: Thu, 19 Aug 2010 18:40:44 +0200 Subject: [PATCH 1/2] Show processing coefficients in codec proc file
Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_proc.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index f025200..88e40f0 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -467,11 +467,23 @@ static void print_unsol_cap(struct snd_info_buffer *buffer, static void print_proc_caps(struct snd_info_buffer *buffer, struct hda_codec *codec, hda_nid_t nid) {
int i, val;- unsigned int ncoeff = (proc_caps & AC_PCAP_NUM_COEF) >>
AC_PCAP_NUM_COEF_SHIFT;- snd_iprintf(buffer, " Processing caps: benign=%d, ncoeff=%d\n",
proc_caps & AC_PCAP_BENIGN,(proc_caps & AC_PCAP_NUM_COEF) >> AC_PCAP_NUM_COEF_SHIFT);
proc_caps & AC_PCAP_BENIGN, ncoeff);- for (i = 0; i < ncoeff; i++) {
snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_COEF_INDEX,i);val = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_PROC_COEF, 0);snd_iprintf(buffer, " Processing coef %d: 0x%x\n", i,val);}}
static void print_conn_list(struct snd_info_buffer *buffer,
1.7.0.4
2010-08-19 20:28, Takashi Iwai skrev:
At Thu, 19 Aug 2010 20:17:42 +0200, David Henningsson wrote:
This patch is helpful for tracking down bugs without having all information about the chip.
I don't want to change coef index in reading a proc file in general.
Good point. But I think the solution to that problem would be to - read current index - do the loop - restore current index
...and perhaps protect that with an appropriate mutex (which one)?
If any, you should implement a codec-specific hook.
I'm sorry, but I don't really see how that would help...?
Another option would be to have a generic snd_hda_codec_write_coef(nid, index, value) and snd_hda_codec_read_coef(nid, index) in hda_codec.c (and perhaps protect it with a spin lock?), then remove all other code that directly sets the coefficient index.
At Thu, 19 Aug 2010 20:48:56 +0200, David Henningsson wrote:
2010-08-19 20:28, Takashi Iwai skrev:
At Thu, 19 Aug 2010 20:17:42 +0200, David Henningsson wrote:
This patch is helpful for tracking down bugs without having all information about the chip.
I don't want to change coef index in reading a proc file in general.
Good point. But I think the solution to that problem would be to
- read current index
- do the loop
- restore current index
...and perhaps protect that with an appropriate mutex (which one)?
If any, you should implement a codec-specific hook.
I'm sorry, but I don't really see how that would help...?
If it's specific to a codec, then you know whether reading the coef in that way can be harmful or not. It's not only about the race via proc file but also the influence of coef on the actual codec behavior.
Takashi
2010-08-19 22:24, Takashi Iwai skrev:
At Thu, 19 Aug 2010 20:48:56 +0200, David Henningsson wrote:
2010-08-19 20:28, Takashi Iwai skrev:
At Thu, 19 Aug 2010 20:17:42 +0200, David Henningsson wrote:
This patch is helpful for tracking down bugs without having all information about the chip.
I don't want to change coef index in reading a proc file in general.
Good point. But I think the solution to that problem would be to
- read current index
- do the loop
- restore current index
...and perhaps protect that with an appropriate mutex (which one)?
If any, you should implement a codec-specific hook.
I'm sorry, but I don't really see how that would help...?
If it's specific to a codec, then you know whether reading the coef in that way can be harmful or not. It's not only about the race via proc file but also the influence of coef on the actual codec behavior.
Oh, so there are chips that are *that* broken...
Yet it could be a lifesaver so I wouldn't want to give up on the idea just yet...
Do you know off the top of your head approximately what codec chips this is about, where reading a coef (or setting its index) can cause unwanted side-effects?
At Fri, 20 Aug 2010 16:46:25 +0200, David Henningsson wrote:
2010-08-19 22:24, Takashi Iwai skrev:
At Thu, 19 Aug 2010 20:48:56 +0200, David Henningsson wrote:
2010-08-19 20:28, Takashi Iwai skrev:
At Thu, 19 Aug 2010 20:17:42 +0200, David Henningsson wrote:
This patch is helpful for tracking down bugs without having all information about the chip.
I don't want to change coef index in reading a proc file in general.
Good point. But I think the solution to that problem would be to
- read current index
- do the loop
- restore current index
...and perhaps protect that with an appropriate mutex (which one)?
If any, you should implement a codec-specific hook.
I'm sorry, but I don't really see how that would help...?
If it's specific to a codec, then you know whether reading the coef in that way can be harmful or not. It's not only about the race via proc file but also the influence of coef on the actual codec behavior.
Oh, so there are chips that are *that* broken...
Yet it could be a lifesaver so I wouldn't want to give up on the idea just yet...
As mentioned, I don't mind to add it but as a beginning, let's add it conditionally for known-working codecs. Eventually we can put it to the generic part, with a blacklisting if needed.
Do you know off the top of your head approximately what codec chips this is about, where reading a coef (or setting its index) can cause unwanted side-effects?
I don't remember exactly which one. I thought either an old Realtek or AD codec.
Takashi
participants (2)
-
David Henningsson -
Takashi Iwai