At Mon, 07 Jan 2008 23:23:02 -0800,
Tobin Davis wrote:
>
> On Tue, 2008-01-08 at 08:11 +0100, Takashi Iwai wrote:
>
> At Mon, 07 Jan 2008 10:00:20 -0800,
> Tobin Davis wrote:
> >
> > On Mon, 2008-01-07 at 11:59 +0100, Takashi Iwai wrote:
> >
> > At Thu, 03 Jan 2008 15:34:05 -0800,
> > Tobin Davis wrote:
> > >
> > > Summary: hda_intel.c use gcap register to determine number of streams
> > > supported by southbridge.
> > >
> > > This patch removes hardcoded values for the number of streams supported
> > > by the southbridge in most chipsets, and reads these values from the
> > > chipset directly. Most systems are hardwired for 4 streams in each
> > > direction, but newer chipsets change that capability.
> > >
> > >
> > > Signed off by Tobin Davis <tdavis@xxxxxxxxxxxx>
> > > [2 hda-gcap.patch <text/x-patch; UTF-8 (7bit)>]
> > > diff -r 1227a1c12325 pci/hda/hda_intel.c
> > > --- a/pci/hda/hda_intel.c Mon Dec 24 14:40:56 2007 +0100
> > > +++ b/pci/hda/hda_intel.c Thu Jan 03 15:27:10 2008 -0800
> > > @@ -1709,12 +1709,13 @@ static int __devinit azx_create(struct s
> > > {
> > > struct azx *chip;
> > > int err;
> > > + unsigned short gcap;
> > > static struct snd_device_ops ops = {
> > > .dev_free = azx_dev_free,
> > > };
> > >
> > > *rchip = NULL;
> > > -
> > > +
> > > err = pci_enable_device(pci);
> > > if (err < 0)
> > > return err;
> > > @@ -1790,10 +1791,19 @@ static int __devinit azx_create(struct s
> > > chip->capture_index_offset = ATIHDMI_CAPTURE_INDEX;
> > > break;
> > > default:
> > > - chip->playback_streams = ICH6_NUM_PLAYBACK;
> > > - chip->capture_streams = ICH6_NUM_CAPTURE;
> > > - chip->playback_index_offset = ICH6_PLAYBACK_INDEX;
> > > - chip->capture_index_offset = ICH6_CAPTURE_INDEX;
> > > + /* read number of streams from GCAP register instead of using
> > > + * hardcoded value
> > > + */
> > > + gcap = azx_readw(chip, GCAP);
> > > + if(!gcap) {
> > > + snd_printk(KERN_ERR "Device has no streams \n");
> > > + goto errout;
> > > + };
> >
> > I think it's safer to assign old ICH6_* values in this case (just to
> > be sure) after a warning message.
> >
> > Actually, this should never have been hardcoded in the first place. The HDA
> > PRM (April 2005) states that the value returned is hardwired to 4 streams in
> > the ICH6/7 southbridge of the Intel chipset. By hardcoding the values in the
> > driver, we don't allow changes to future (hint - real soon) chipsets. This
> > just cleans up this bit of code that has been around since this code was
> > created. If I had the other systems in the case statement to test, I'm sure
> > we could do away with the case statement entirely, just by reading the values
> > instead of second guessing them. If the chipsets follow the spec, this method
> > should work all around.
>
> Well, I wasn't clear about the comment above. I meant the constant
> values can be used as fallbacks in case these values can't be read,
> instead of returning error and quit. This would avoid regressions, at
> least.
>
> > > + chip->playback_streams = (gcap&(0xF<<12))>>12;
> > > + chip->capture_streams = (gcap&(0xF<<8))>>8;
> > > + chip->playback_index_offset = (gcap&(0xF<<12))>>12;
> > > + chip->capture_index_offset = 0;
> >
> > I'm not entirely convinced of this method for the index_offsets, but it works
> > on tested systems here. I'll review the PRM for more guidance.
>
> It should be fine as is. A stream can be assigned freely regardless
> the I/O direction on ICH (and others too). So, basically it's always:
>
> capture_index_offset = 0;
> playback_index_offset = capture_streams;
>
> This isn't entirely true, at least based on the ATIHDMI code in the previous
> case statement.
ATIHDMI has no capture. Thus the above is OK.
> It would be interesting to see what GCAP returns for the two
> cases (ATIHDMI, and ULI). If they could be used by this, we could do away
> with the switch entirely (except maybe as a fallback).
Yes, but it's not so important at this stage unless they release new
(incompatible) hardwares.
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
