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

[Takashi Iwai]

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