[alsa-devel] [PATCH] ALSA: hda_intel: disable corb rirb when single_cmd active

[Takashi Iwai]

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);