[alsa-devel] [PATCH] ALSA: hda_intel: disable corb rirb when single_cmd active
Poulsbo(US15W) cannot have any corb registers initialized when using single_cmd mode. When send_cmd timeout occur, note error.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com --- sound/pci/hda/hda_intel.c | 16 +++++++++++----- 1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index c9ad182..c2ff585 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -524,6 +524,16 @@ static void azx_init_cmd_io(struct azx *chip) /* CORB set up */ chip->corb.addr = chip->rb.addr; chip->corb.buf = (u32 *)chip->rb.area; + + /* RIRB set up */ + chip->rirb.addr = chip->rb.addr + 2048; + chip->rirb.buf = (u32 *)(chip->rb.area + 2048); + chip->rirb.wp = chip->rirb.rp = 0; + memset(chip->rirb.cmds, 0, sizeof(chip->rirb.cmds)); + if (chip->single_cmd) { + spin_unlock_irq(&chip->reg_lock); + return; + } azx_writel(chip, CORBLBASE, (u32)chip->corb.addr); azx_writel(chip, CORBUBASE, upper_32_bits(chip->corb.addr));
@@ -536,11 +546,6 @@ static void azx_init_cmd_io(struct azx *chip) /* enable corb dma */ azx_writeb(chip, CORBCTL, ICH6_CORBCTL_RUN);
- /* RIRB set up */ - chip->rirb.addr = chip->rb.addr + 2048; - chip->rirb.buf = (u32 *)(chip->rb.area + 2048); - chip->rirb.wp = chip->rirb.rp = 0; - memset(chip->rirb.cmds, 0, sizeof(chip->rirb.cmds)); azx_writel(chip, RIRBLBASE, (u32)chip->rirb.addr); azx_writel(chip, RIRBUBASE, upper_32_bits(chip->rirb.addr));
@@ -783,6 +788,7 @@ static int azx_single_send_cmd(struct hda_bus *bus, u32 val) if (printk_ratelimit()) snd_printd(SFX "send_cmd timeout: IRS=0x%x, val=0x%x\n", azx_readw(chip, IRS), val); + chip->rirb.res[addr] = -1; return -EIO; }
At Tue, 3 Nov 2009 12:22:37 -0700, Troy Kisky wrote:
Poulsbo(US15W) cannot have any corb registers initialized when using single_cmd mode. When send_cmd timeout occur, note error.
Could you be more specific? What errors do you get?
And, how it goes to single_cmd mode? The single_cmd mode is very last resort, and reaching there means already a serious problem.
thanks,
Takashi
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
sound/pci/hda/hda_intel.c | 16 +++++++++++----- 1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index c9ad182..c2ff585 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -524,6 +524,16 @@ static void azx_init_cmd_io(struct azx *chip) /* CORB set up */ chip->corb.addr = chip->rb.addr; chip->corb.buf = (u32 *)chip->rb.area;
- /* RIRB set up */
- chip->rirb.addr = chip->rb.addr + 2048;
- chip->rirb.buf = (u32 *)(chip->rb.area + 2048);
- chip->rirb.wp = chip->rirb.rp = 0;
- memset(chip->rirb.cmds, 0, sizeof(chip->rirb.cmds));
- if (chip->single_cmd) {
spin_unlock_irq(&chip->reg_lock);return;- } azx_writel(chip, CORBLBASE, (u32)chip->corb.addr); azx_writel(chip, CORBUBASE, upper_32_bits(chip->corb.addr));
@@ -536,11 +546,6 @@ static void azx_init_cmd_io(struct azx *chip) /* enable corb dma */ azx_writeb(chip, CORBCTL, ICH6_CORBCTL_RUN);
- /* RIRB set up */
- chip->rirb.addr = chip->rb.addr + 2048;
- chip->rirb.buf = (u32 *)(chip->rb.area + 2048);
- chip->rirb.wp = chip->rirb.rp = 0;
- memset(chip->rirb.cmds, 0, sizeof(chip->rirb.cmds)); azx_writel(chip, RIRBLBASE, (u32)chip->rirb.addr); azx_writel(chip, RIRBUBASE, upper_32_bits(chip->rirb.addr));
@@ -783,6 +788,7 @@ static int azx_single_send_cmd(struct hda_bus *bus, u32 val) if (printk_ratelimit()) snd_printd(SFX "send_cmd timeout: IRS=0x%x, val=0x%x\n", azx_readw(chip, IRS), val);
- chip->rirb.res[addr] = -1; return -EIO;
}
-- 1.5.6.3
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Takashi Iwai wrote:
At Tue, 3 Nov 2009 12:22:37 -0700, Troy Kisky wrote:
Poulsbo(US15W) cannot have any corb registers initialized when using single_cmd mode. When send_cmd timeout occur, note error.
Could you be more specific? What errors do you get?
And, how it goes to single_cmd mode? The single_cmd mode is very last resort, and reaching there means already a serious problem.
thanks,
Takashi
No error messages, but the response read is always 0. For testing, I passed single_cmd=1 as a modules option.
HDAudio_03.pdf says, "If implemented, these registers must not be used at the same time as the CORB and RIRB command/response mechanisms, as the operations will conflict."
Plus, if the RIRB irq is enabled, the interrupt routine will print out a spurious interrupt message.
That said, my hardware is switching to single_cmd eventually, even if not passed as a module option. But at least now, when that happens my audio isn't dead.
Troy
At Wed, 04 Nov 2009 12:45:20 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Tue, 3 Nov 2009 12:22:37 -0700, Troy Kisky wrote:
Poulsbo(US15W) cannot have any corb registers initialized when using single_cmd mode. When send_cmd timeout occur, note error.
Could you be more specific? What errors do you get?
And, how it goes to single_cmd mode? The single_cmd mode is very last resort, and reaching there means already a serious problem.
thanks,
Takashi
No error messages, but the response read is always 0.
This is odd. Something is already wrong, then.
For testing, I passed single_cmd=1 as a modules option.
HDAudio_03.pdf says, "If implemented, these registers must not be used at the same time as the CORB and RIRB command/response mechanisms, as the operations will conflict."
I know this, but actually this never be a problem in the real hardware. So, it's left there.
Plus, if the RIRB irq is enabled, the interrupt routine will print out a spurious interrupt message.
That said, my hardware is switching to single_cmd eventually, even if not passed as a module option. But at least now, when that happens my audio isn't dead.
It's not dead but living-dead :) The single_cmd mode is really only for debugging. It's never for any running system.
thanks,
Takashi
Takashi Iwai wrote:
At Wed, 04 Nov 2009 12:45:20 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Tue, 3 Nov 2009 12:22:37 -0700, Troy Kisky wrote:
Poulsbo(US15W) cannot have any corb registers initialized when using single_cmd mode. When send_cmd timeout occur, note error.
Could you be more specific? What errors do you get?
And, how it goes to single_cmd mode? The single_cmd mode is very last resort, and reaching there means already a serious problem.
thanks,
Takashi
No error messages, but the response read is always 0.
This is odd. Something is already wrong, then.
For testing, I passed single_cmd=1 as a modules option.
HDAudio_03.pdf says, "If implemented, these registers must not be used at the same time as the CORB and RIRB command/response mechanisms, as the operations will conflict."
I know this, but actually this never be a problem in the real hardware. So, it's left there.
My hardware is real.
Plus, if the RIRB irq is enabled, the interrupt routine will print out a spurious interrupt message.
I meant "spurious response"
That said, my hardware is switching to single_cmd eventually, even if not passed as a module option. But at least now, when that happens my audio isn't dead.
It's not dead but living-dead :) The single_cmd mode is really only for debugging. It's never for any running system.
thanks,
Takashi
I'm all in favor of totally yanking out all the single_cmd related code, but if you're leaving it in, you should apply this patch.
If you want someone else to verify the bug, can you ask someone with a US15W to add the module parameter single_cmd=1 ???
I really do not understand your reluctance to change code that is "really only for debugging." That should make changes less dangerous, not more. Especially as the change seems like an obvious fix to a minor oversight.
My only hesitation would be that unsolicited responses are not handled when in single_cmd mode. Should they be disabled as well?
Troy
At Thu, 05 Nov 2009 12:42:03 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Wed, 04 Nov 2009 12:45:20 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Tue, 3 Nov 2009 12:22:37 -0700, Troy Kisky wrote:
Poulsbo(US15W) cannot have any corb registers initialized when using single_cmd mode. When send_cmd timeout occur, note error.
Could you be more specific? What errors do you get?
And, how it goes to single_cmd mode? The single_cmd mode is very last resort, and reaching there means already a serious problem.
thanks,
Takashi
No error messages, but the response read is always 0.
This is odd. Something is already wrong, then.
For testing, I passed single_cmd=1 as a modules option.
HDAudio_03.pdf says, "If implemented, these registers must not be used at the same time as the CORB and RIRB command/response mechanisms, as the operations will conflict."
I know this, but actually this never be a problem in the real hardware. So, it's left there.
My hardware is real.
Yeah, the first hit it seems.
Plus, if the RIRB irq is enabled, the interrupt routine will print out a spurious interrupt message.
I meant "spurious response"
So your patch alone isn't enough?
That said, my hardware is switching to single_cmd eventually, even if not passed as a module option. But at least now, when that happens my audio isn't dead.
It's not dead but living-dead :) The single_cmd mode is really only for debugging. It's never for any running system.
thanks,
Takashi
I'm all in favor of totally yanking out all the single_cmd related code, but if you're leaving it in, you should apply this patch.
Well, it's no right fix in a strict manner. The right fix would be to remove the reinitialization or the first call of azx_init_cmd_io() (and disable the unsolicited events, too).
If you want someone else to verify the bug, can you ask someone with a US15W to add the module parameter single_cmd=1 ???
I really do not understand your reluctance to change code that is "really only for debugging." That should make changes less dangerous, not more. Especially as the change seems like an obvious fix to a minor oversight.
This was no oversight but a sort of intention. The point is that the single_cmd mode could be used as the fallback when the serious communication error occurs. But the unsolicited handling can't be disabled easily from the codec side (and with a hope that RIRB still works), so I left the CORB/RIRB code as is.
My only hesitation would be that unsolicited responses are not handled when in single_cmd mode. Should they be disabled as well?
Yes. Strictly speaking, it can't be handled with single_cmd mode. Maybe resetting GCTL_UNSOL register bit might be enough, but I'm not sure.
In anyway, your patch is no real fix for your hardware. Fixing the behavior of single_cmd mode is nice, but this is no urgent issue. The fact that the driver falls into this mode means something very wrong elsewhere, and this is more important thing to fix right now. That's why I wrote it's living-dead. You don't see it's dead but it is :)
So... this obviously isn't a topic that we focus on and waste time.
thanks,
Takashi
At Fri, 06 Nov 2009 08:19:08 +0100, I wrote:
At Thu, 05 Nov 2009 12:42:03 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Wed, 04 Nov 2009 12:45:20 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Tue, 3 Nov 2009 12:22:37 -0700, Troy Kisky wrote:
Poulsbo(US15W) cannot have any corb registers initialized when using single_cmd mode. When send_cmd timeout occur, note error.
Could you be more specific? What errors do you get?
And, how it goes to single_cmd mode? The single_cmd mode is very last resort, and reaching there means already a serious problem.
thanks,
Takashi
No error messages, but the response read is always 0.
This is odd. Something is already wrong, then.
For testing, I passed single_cmd=1 as a modules option.
HDAudio_03.pdf says, "If implemented, these registers must not be used at the same time as the CORB and RIRB command/response mechanisms, as the operations will conflict."
I know this, but actually this never be a problem in the real hardware. So, it's left there.
My hardware is real.
Yeah, the first hit it seems.
Plus, if the RIRB irq is enabled, the interrupt routine will print out a spurious interrupt message.
I meant "spurious response"
So your patch alone isn't enough?
That said, my hardware is switching to single_cmd eventually, even if not passed as a module option. But at least now, when that happens my audio isn't dead.
It's not dead but living-dead :) The single_cmd mode is really only for debugging. It's never for any running system.
thanks,
Takashi
I'm all in favor of totally yanking out all the single_cmd related code, but if you're leaving it in, you should apply this patch.
Well, it's no right fix in a strict manner. The right fix would be to remove the reinitialization or the first call of azx_init_cmd_io() (and disable the unsolicited events, too).
... like the patch below (untested!)
Takashi
--- diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 55c7da3..cce837a 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -722,9 +722,10 @@ static unsigned int azx_rirb_get_response(struct hda_bus *bus, chip->last_cmd[addr]); chip->single_cmd = 1; bus->response_reset = 0; - /* re-initialize CORB/RIRB */ + /* release CORB/RIRB */ azx_free_cmd_io(chip); - azx_init_cmd_io(chip); + /* disable unsolicited responses */ + azx_writel(chip, GCTL, azx_readl(chip, GCTL) & ~ICH6_GCTL_UNSOL); return -1; }
@@ -865,7 +866,9 @@ static int azx_reset(struct azx *chip) }
/* Accept unsolicited responses */ - azx_writel(chip, GCTL, azx_readl(chip, GCTL) | ICH6_GCTL_UNSOL); + if (!chip->single_cmd) + azx_writel(chip, GCTL, azx_readl(chip, GCTL) | + ICH6_GCTL_UNSOL);
/* detect codecs */ if (!chip->codec_mask) { @@ -980,7 +983,8 @@ static void azx_init_chip(struct azx *chip) azx_int_enable(chip);
/* initialize the codec command I/O */ - azx_init_cmd_io(chip); + if (!chip->single_cmd) + azx_init_cmd_io(chip);
/* program the position buffer */ azx_writel(chip, DPLBASE, (u32)chip->posbuf.addr);
Takashi Iwai wrote:
At Fri, 06 Nov 2009 08:19:08 +0100, I wrote:
At Thu, 05 Nov 2009 12:42:03 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Wed, 04 Nov 2009 12:45:20 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Tue, 3 Nov 2009 12:22:37 -0700, Troy Kisky wrote: > Poulsbo(US15W) cannot have any corb registers initialized > when using single_cmd mode. > When send_cmd timeout occur, note error. Could you be more specific? What errors do you get?
And, how it goes to single_cmd mode? The single_cmd mode is very last resort, and reaching there means already a serious problem.
thanks,
Takashi
No error messages, but the response read is always 0.
This is odd. Something is already wrong, then.
For testing, I passed single_cmd=1 as a modules option.
HDAudio_03.pdf says, "If implemented, these registers must not be used at the same time as the CORB and RIRB command/response mechanisms, as the operations will conflict."
I know this, but actually this never be a problem in the real hardware. So, it's left there.
My hardware is real.
Yeah, the first hit it seems.
Plus, if the RIRB irq is enabled, the interrupt routine will print out a spurious interrupt message.
I meant "spurious response"
So your patch alone isn't enough?
That said, my hardware is switching to single_cmd eventually, even if not passed as a module option. But at least now, when that happens my audio isn't dead.
It's not dead but living-dead :) The single_cmd mode is really only for debugging. It's never for any running system.
thanks,
Takashi
I'm all in favor of totally yanking out all the single_cmd related code, but if you're leaving it in, you should apply this patch.
Well, it's no right fix in a strict manner. The right fix would be to remove the reinitialization or the first call of azx_init_cmd_io() (and disable the unsolicited events, too).
... like the patch below (untested!)
Takashi
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 55c7da3..cce837a 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -722,9 +722,10 @@ static unsigned int azx_rirb_get_response(struct hda_bus *bus, chip->last_cmd[addr]); chip->single_cmd = 1; bus->response_reset = 0;
- /* re-initialize CORB/RIRB */
- /* release CORB/RIRB */ azx_free_cmd_io(chip);
- azx_init_cmd_io(chip);
- /* disable unsolicited responses */
- azx_writel(chip, GCTL, azx_readl(chip, GCTL) & ~ICH6_GCTL_UNSOL); return -1;
}
@@ -865,7 +866,9 @@ static int azx_reset(struct azx *chip) }
/* Accept unsolicited responses */
- azx_writel(chip, GCTL, azx_readl(chip, GCTL) | ICH6_GCTL_UNSOL);
if (!chip->single_cmd)
azx_writel(chip, GCTL, azx_readl(chip, GCTL) |ICH6_GCTL_UNSOL);/* detect codecs */ if (!chip->codec_mask) {
@@ -980,7 +983,8 @@ static void azx_init_chip(struct azx *chip) azx_int_enable(chip);
/* initialize the codec command I/O */
- azx_init_cmd_io(chip);
if (!chip->single_cmd)
azx_init_cmd_io(chip);/* program the position buffer */ azx_writel(chip, DPLBASE, (u32)chip->posbuf.addr);
This patch works fine for me when passing single_cmd=1.
Thanks for spending your time on something you'd rather not.
Troy
At Fri, 06 Nov 2009 18:01:27 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Fri, 06 Nov 2009 08:19:08 +0100, I wrote:
At Thu, 05 Nov 2009 12:42:03 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Wed, 04 Nov 2009 12:45:20 -0700, Troy Kisky wrote:
Takashi Iwai wrote: > At Tue, 3 Nov 2009 12:22:37 -0700, > Troy Kisky wrote: >> Poulsbo(US15W) cannot have any corb registers initialized >> when using single_cmd mode. >> When send_cmd timeout occur, note error. > Could you be more specific? What errors do you get? > > And, how it goes to single_cmd mode? The single_cmd mode is very last > resort, and reaching there means already a serious problem. > > > thanks, > > Takashi > No error messages, but the response read is always 0.
This is odd. Something is already wrong, then.
For testing, I passed single_cmd=1 as a modules option.
HDAudio_03.pdf says, "If implemented, these registers must not be used at the same time as the CORB and RIRB command/response mechanisms, as the operations will conflict."
I know this, but actually this never be a problem in the real hardware. So, it's left there.
My hardware is real.
Yeah, the first hit it seems.
Plus, if the RIRB irq is enabled, the interrupt routine will print out a spurious interrupt message.
I meant "spurious response"
So your patch alone isn't enough?
That said, my hardware is switching to single_cmd eventually, even if not passed as a module option. But at least now, when that happens my audio isn't dead.
It's not dead but living-dead :) The single_cmd mode is really only for debugging. It's never for any running system.
thanks,
Takashi
I'm all in favor of totally yanking out all the single_cmd related code, but if you're leaving it in, you should apply this patch.
Well, it's no right fix in a strict manner. The right fix would be to remove the reinitialization or the first call of azx_init_cmd_io() (and disable the unsolicited events, too).
... like the patch below (untested!)
Takashi
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 55c7da3..cce837a 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -722,9 +722,10 @@ static unsigned int azx_rirb_get_response(struct hda_bus *bus, chip->last_cmd[addr]); chip->single_cmd = 1; bus->response_reset = 0;
- /* re-initialize CORB/RIRB */
- /* release CORB/RIRB */ azx_free_cmd_io(chip);
- azx_init_cmd_io(chip);
- /* disable unsolicited responses */
- azx_writel(chip, GCTL, azx_readl(chip, GCTL) & ~ICH6_GCTL_UNSOL); return -1;
}
@@ -865,7 +866,9 @@ static int azx_reset(struct azx *chip) }
/* Accept unsolicited responses */
- azx_writel(chip, GCTL, azx_readl(chip, GCTL) | ICH6_GCTL_UNSOL);
if (!chip->single_cmd)
azx_writel(chip, GCTL, azx_readl(chip, GCTL) |ICH6_GCTL_UNSOL);/* detect codecs */ if (!chip->codec_mask) {
@@ -980,7 +983,8 @@ static void azx_init_chip(struct azx *chip) azx_int_enable(chip);
/* initialize the codec command I/O */
- azx_init_cmd_io(chip);
if (!chip->single_cmd)
azx_init_cmd_io(chip);/* program the position buffer */ azx_writel(chip, DPLBASE, (u32)chip->posbuf.addr);
This patch works fine for me when passing single_cmd=1.
Thanks for a quick test! I'll merge it soon.
Thanks for spending your time on something you'd rather not.
Well, I'm willing to fix any bugs, of course ;) So I really appreciated your report and patch.
But, more important bug still remains -- why single_cmd mode is activated. Let's track it down.
thanks,
Takashi
Takashi Iwai wrote:
Thanks for a quick test! I'll merge it soon.
Thanks for spending your time on something you'd rather not.
Well, I'm willing to fix any bugs, of course ;) So I really appreciated your report and patch.
But, more important bug still remains -- why single_cmd mode is activated. Let's track it down.
I have a little more information on that. The RIRB engine is fine but the CORB engine is stopping at seemingly random times. Usually CORB has about 7 commands queued when it switches to single_cmd mode. After the engine dies, even a rmmod/insmod sequence won't revive it. It seems to require a power down/up. I have noticed a couple of things that I thought might be related, but testing didn't show much difference.
1. The Poulsbo manual says that CORB READ Pointer Reset must be cleared back to 0 and read back as 0 to verify that the clear completed correctly.
2. azx_corb_send_cmd doesn't compare CORBWP with CORBRP to see if adding an entry will result in an empty queue.
Do you have a suggestion on a more thorough reset? I hate having to reboot all the time.
Thanks Troy
At Mon, 09 Nov 2009 12:36:31 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
Thanks for a quick test! I'll merge it soon.
Thanks for spending your time on something you'd rather not.
Well, I'm willing to fix any bugs, of course ;) So I really appreciated your report and patch.
But, more important bug still remains -- why single_cmd mode is activated. Let's track it down.
I have a little more information on that. The RIRB engine is fine but the CORB engine is stopping at seemingly random times. Usually CORB has about 7 commands queued when it switches to single_cmd mode. After the engine dies, even a rmmod/insmod sequence won't revive it. It seems to require a power down/up. I have noticed a couple of things that I thought might be related, but testing didn't show much difference.
- The Poulsbo manual says that CORB READ Pointer Reset must be cleared
back to 0 and read back as 0 to verify that the clear completed correctly.
At which timing?
- azx_corb_send_cmd doesn't compare CORBWP with CORBRP to see if adding
an entry will result in an empty queue.
It's because azx_corb_send_cmd is asynchronous. It doesn't wait. That's the purpose of CORB being a ring buffer...
Do you have a suggestion on a more thorough reset? I hate having to reboot all the time.
Try to set codec->bus->sync_write = 1 somewhere in the initialization. This will make the driver to wait and synchronize for each verb response.
thanks,
Takashi
Takashi Iwai wrote:
At Mon, 09 Nov 2009 12:36:31 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
Thanks for a quick test! I'll merge it soon.
Thanks for spending your time on something you'd rather not.
Well, I'm willing to fix any bugs, of course ;) So I really appreciated your report and patch.
But, more important bug still remains -- why single_cmd mode is activated. Let's track it down.
I have a little more information on that. The RIRB engine is fine but the CORB engine is stopping at seemingly random times. Usually CORB has about 7 commands queued when it switches to single_cmd mode. After the engine dies, even a rmmod/insmod sequence won't revive it. It seems to require a power down/up. I have noticed a couple of things that I thought might be related, but testing didn't show much difference.
- The Poulsbo manual says that CORB READ Pointer Reset must be cleared
back to 0 and read back as 0 to verify that the clear completed correctly.
At which timing?
The manual was talking about the CORB read pointer reset sequence. It says to set the bit, verify set, clear the bit, verify clear. Whereas the normal hda spec says to just set it and forget it, which is what the code does.
- azx_corb_send_cmd doesn't compare CORBWP with CORBRP to see if adding
an entry will result in an empty queue.
It's because azx_corb_send_cmd is asynchronous. It doesn't wait. That's the purpose of CORB being a ring buffer...
Still, the circular buffer is a fixed size (256 entries in this case). The code does not detect if you try to stuff 257 entries into the queue. Actually, 255 entries is the max you should stuff. The next entry being stuffed will make the read pointer equal the write pointer and the queue will be empty. I'm not saying this is likely to ever happen. But if it did happen, you would end up in single_cmd mode.
Do you have a suggestion on a more thorough reset? I hate having to reboot all the time.
Try to set codec->bus->sync_write = 1 somewhere in the initialization. This will make the driver to wait and synchronize for each verb response.
OK. I'll give a try.
Thanks Troy
At Tue, 10 Nov 2009 14:15:27 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
At Mon, 09 Nov 2009 12:36:31 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
Thanks for a quick test! I'll merge it soon.
Thanks for spending your time on something you'd rather not.
Well, I'm willing to fix any bugs, of course ;) So I really appreciated your report and patch.
But, more important bug still remains -- why single_cmd mode is activated. Let's track it down.
I have a little more information on that. The RIRB engine is fine but the CORB engine is stopping at seemingly random times. Usually CORB has about 7 commands queued when it switches to single_cmd mode. After the engine dies, even a rmmod/insmod sequence won't revive it. It seems to require a power down/up. I have noticed a couple of things that I thought might be related, but testing didn't show much difference.
- The Poulsbo manual says that CORB READ Pointer Reset must be cleared
back to 0 and read back as 0 to verify that the clear completed correctly.
At which timing?
The manual was talking about the CORB read pointer reset sequence. It says to set the bit, verify set, clear the bit, verify clear. Whereas the normal hda spec says to just set it and forget it, which is what the code does.
Hm, CORB read pointer might play actually something for your device, then.
- azx_corb_send_cmd doesn't compare CORBWP with CORBRP to see if adding
an entry will result in an empty queue.
It's because azx_corb_send_cmd is asynchronous. It doesn't wait. That's the purpose of CORB being a ring buffer...
Still, the circular buffer is a fixed size (256 entries in this case). The code does not detect if you try to stuff 257 entries into the queue. Actually, 255 entries is the max you should stuff. The next entry being stuffed will make the read pointer equal the write pointer and the queue will be empty. I'm not saying this is likely to ever happen. But if it did happen, you would end up in single_cmd mode.
Yes, in theory. But this doesn't happen actually because you usually read the response often. The read sequence synchronizes.
thanks,
Takashi
Takashi Iwai wrote:
Yes, in theory. But this doesn't happen actually because you usually read the response often. The read sequence synchronizes.
Can resume type processing send a lot of verbs without waiting for a response?
Speaking of synchronizing, if 2 different processes simultaneously send different command verbs to the same codec, won't they both read the response of the last verb sent? Or is there locking somewhere to prevent this?
Troy
At Wed, 11 Nov 2009 12:05:39 -0700, Troy Kisky wrote:
Takashi Iwai wrote:
Yes, in theory. But this doesn't happen actually because you usually read the response often. The read sequence synchronizes.
Can resume type processing send a lot of verbs without waiting for a response?
Usually not many like 256, I guess. In doubt, give some printks and count by yourself...
Speaking of synchronizing, if 2 different processes simultaneously send different command verbs to the same codec, won't they both read the response of the last verb sent? Or is there locking somewhere to prevent this?
A mutex protection around the send/response pair.
Takashi
participants (2)
-
Takashi Iwai -
Troy Kisky