[alsa-devel] HDA: Enable chipset gcap usage

[Tobin Davis]

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 at dsl-only.net>
> > [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.


> > +		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.


> > +
> >  		break;
> >  	}
> >  	chip->num_streams = chip->playback_streams + chip->capture_streams;
> 
> The patch has some coding style issues, for example, no space around
> operators, etc.  Try to run $LINUX/scripts/checkpatch.pl, fix the
> errors and the issue above, and please repost the patch again.

This must be a new script in the kernel.  My development system (2.6.17)
doesn't have it, but it is on my laptop (2.6.22).  I ran it, ant it
reported one error " if(!gcap) " which I fixed.  I also added some
spacing around the bit manipulation code for streams and offsets.  

This code actually was part of a larger patch to enable audio on a new
chipset due out later this year.  I only took the code that can be
mainstreamed (the rest were prerelease specific fixes that are not
relevant to production silicon - and none of it would have passed coding
style tests).  I'll have another patch in a few months with the new
device info, once it clears legal.


Signed off by Tobin Davis <tdavis at dsl-only.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hda-gcap.patch
Type: text/x-patch
Size: 1286 bytes
Desc: not available
Url : http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20080107/6811a142/attachment.patch