[PATCH] hda_intel: Add Oaktrail identifiers

[PATCH] hda_intel: Add Oaktrail identifiers

[Alan Cox]

From: Li Peng <pengi.li@intel.com>

Oaktrail has 0x8086, 0x080a - AZX_DRIVER_SCH

Taken from the Meego patches for Oaktrail

Signed-off-by: Li Peng <peng.li@intel.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 sound/pci/hda/hda_intel.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)


diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index c2f79e6..b339eb5 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2972,6 +2972,9 @@ static DEFINE_PCI_DEVICE_TABLE(azx_ids) = {
 	{ PCI_DEVICE(0x8086, 0x811b),
 	  .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP |
 	  AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_LPIB }, /* Poulsbo */
+	{ PCI_DEVICE(0x8086, 0x080a),
+	  .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP |
+	  AZX_DCAPS_BUFSIZE},
 	/* ICH */
 	{ PCI_DEVICE(0x8086, 0x2668),
 	  .driver_data = AZX_DRIVER_ICH | AZX_DCAPS_OLD_SSYNC |

Re: [PATCH] hda_intel: Add Oaktrail identifiers

[Takashi Iwai]

At Wed, 28 Dec 2011 15:17:26 +0000,
Alan Cox wrote:
> 
> From: Li Peng <pengi.li@intel.com>
> 
> Oaktrail has 0x8086, 0x080a - AZX_DRIVER_SCH
> 
> Taken from the Meego patches for Oaktrail
> 
> Signed-off-by: Li Peng <peng.li@intel.com>
> Signed-off-by: Alan Cox <alan@linux.intel.com>

Thanks, applied.

Meanwhile, could you double-check whether AZX_DCAPS_POSFIX_LPIB isn't
needed?  We have enabled this bit for Poulsbo recently for working
around problems regarding the playback positions.


Takashi

> ---
> 
>  sound/pci/hda/hda_intel.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index c2f79e6..b339eb5 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -2972,6 +2972,9 @@ static DEFINE_PCI_DEVICE_TABLE(azx_ids) = {
>  	{ PCI_DEVICE(0x8086, 0x811b),
>  	  .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP |
>  	  AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_LPIB }, /* Poulsbo */
> +	{ PCI_DEVICE(0x8086, 0x080a),
> +	  .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP |
> +	  AZX_DCAPS_BUFSIZE},
>  	/* ICH */
>  	{ PCI_DEVICE(0x8086, 0x2668),
>  	  .driver_data = AZX_DRIVER_ICH | AZX_DCAPS_OLD_SSYNC |
> 

Re: [PATCH] hda_intel: Add Oaktrail identifiers

[Alan Cox]

> Thanks, applied.
> 
> Meanwhile, could you double-check whether AZX_DCAPS_POSFIX_LPIB isn't
> needed?  We have enabled this bit for Poulsbo recently for working
> around problems regarding the playback positions.

Is there a good way to test this and I'll run the test and see.

Alan

Re: [PATCH] hda_intel: Add Oaktrail identifiers

[Takashi Iwai]

At Sat, 31 Dec 2011 17:00:01 +0000,
Alan Cox wrote:
> 
> > Thanks, applied.
> > 
> > Meanwhile, could you double-check whether AZX_DCAPS_POSFIX_LPIB isn't
> > needed?  We have enabled this bit for Poulsbo recently for working
> > around problems regarding the playback positions.
> 
> Is there a good way to test this and I'll run the test and see.

Well, we also don't know how exactly the problem can be triggered.
Usually it can be triggered more easily by switching the different
sound backends (PulseAudio, the direct ALSA dmix, etc).  People may
seem to find it via mplayer, e.g. the sound often skips or becomes
noisy, stuttering.

If you haven't seen any problems so far, then it's probably no need
for the workaround.


thanks,

Takashi

Re: [PATCH] hda_intel: Add Oaktrail identifiers

[David Henningsson]

On 12/31/2011 06:00 PM, Alan Cox wrote:
>> Thanks, applied.
>>
>> Meanwhile, could you double-check whether AZX_DCAPS_POSFIX_LPIB isn't
>> needed?  We have enabled this bit for Poulsbo recently for working
>> around problems regarding the playback positions.

That was my thought as well, but Takashi was quicker, as usual :-)

> Is there a good way to test this and I'll run the test and see.

I wouldn't say we have a really good way to test it; but PulseAudio is 
relying heavily on this to be correct. So in short - test both playback 
and recording through PulseAudio, and also test making volume changes 
while playing back (or recording). If one position fix method is broken, 
it usually manifests itself by glitches, audio breaking up, or maybe 
stops completely. (There is actually a test tool called alsa-time-test 
written by Lennart Poettering, but AFAIK it only tests playback, and I 
don't exactly know how to use it.)

A quick way to test different position fix methods is to add this module 
parameter:

# This will read from LPIB
options snd-hda-intel position_fix=1

# This will read from DMA Position buffer (the default as specified in 
your patch)
options snd-hda-intel position_fix=2

Btw, you seem to be employed by Intel these days? [1] Any chance you can 
talk to your firmware developers of this chip? I assume somebody at 
Intel should know what method to prefer. That would be the safest/best 
option IMHO.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

[1] https://secure.wikimedia.org/wikipedia/en/wiki/Alan_Cox

Re: [PATCH] hda_intel: Add Oaktrail identifiers

[Alan Cox]

> Btw, you seem to be employed by Intel these days? [1] Any chance you can 
> talk to your firmware developers of this chip? I assume somebody at 
> Intel should know what method to prefer. That would be the safest/best 
> option IMHO.

Indeed I am and by the time I got around to looking at it other Intel
folsk on the list were already digging. Yes we think it needs the same
settings as Poulsbo.

Alan

Re: [PATCH] hda_intel: Add Oaktrail identifiers

[David Henningsson]

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

On 01/04/2012 12:06 AM, Alan Cox wrote:
>> Btw, you seem to be employed by Intel these days? [1] Any chance you can
>> talk to your firmware developers of this chip? I assume somebody at
>> Intel should know what method to prefer. That would be the safest/best
>> option IMHO.
>
> Indeed I am and by the time I got around to looking at it other Intel
> folsk on the list were already digging. Yes we think it needs the same
> settings as Poulsbo.

Ok, thanks. Let's start there, then. Takashi, would you mind committing 
the attached patch?

Also, we could consider sending both patches to stable as well?

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

[-- Attachment #2: 0001-ALSA-HDA-Use-LPIB-position-fix-for-Oaktrail.patch --]
[-- Type: text/x-patch, Size: 1162 bytes --]

>From a7c7074e3b9fb993a5e551a911dcd7ec6fba5082 Mon Sep 17 00:00:00 2001
From: David Henningsson <david.henningsson@canonical.com>
Date: Wed, 4 Jan 2012 10:12:54 +0100
Subject: [PATCH] ALSA: HDA: Use LPIB position fix for Oaktrail

According to the thread on alsa-devel, the LPIB method is to prefer
for Oaktrail controller chip.

Reference: http://mailman.alsa-project.org/pipermail/alsa-devel/2012-January/047800.html

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/hda_intel.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index bd447b6..8e232ab 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2997,7 +2997,7 @@ static DEFINE_PCI_DEVICE_TABLE(azx_ids) = {
 	  AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_LPIB }, /* Poulsbo */
 	{ PCI_DEVICE(0x8086, 0x080a),
 	  .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP |
-	  AZX_DCAPS_BUFSIZE},
+	  AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_LPIB }, /* Oaktrail */
 	/* ICH */
 	{ PCI_DEVICE(0x8086, 0x2668),
 	  .driver_data = AZX_DRIVER_ICH | AZX_DCAPS_OLD_SSYNC |
-- 
1.7.5.4


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] hda_intel: Add Oaktrail identifiers

[Takashi Iwai]

At Wed, 04 Jan 2012 10:22:45 +0100,
David Henningsson wrote:
> 
> [1  <text/plain; ISO-8859-1 (7bit)>]
> On 01/04/2012 12:06 AM, Alan Cox wrote:
> >> Btw, you seem to be employed by Intel these days? [1] Any chance you can
> >> talk to your firmware developers of this chip? I assume somebody at
> >> Intel should know what method to prefer. That would be the safest/best
> >> option IMHO.
> >
> > Indeed I am and by the time I got around to looking at it other Intel
> > folsk on the list were already digging. Yes we think it needs the same
> > settings as Poulsbo.
> 
> Ok, thanks. Let's start there, then. Takashi, would you mind committing 
> the attached patch?

Applied.

> Also, we could consider sending both patches to stable as well?

Well, this could be, but the patch won't be applicable without
modification to old kernels, so you'd need to resend the patches
manually to stable tree in anyway...


thanks,

Takashi


> 
> -- 
> David Henningsson, Canonical Ltd.
> http://launchpad.net/~diwic
> [2 0001-ALSA-HDA-Use-LPIB-position-fix-for-Oaktrail.patch <text/x-patch (7bit)>]
> >From a7c7074e3b9fb993a5e551a911dcd7ec6fba5082 Mon Sep 17 00:00:00 2001
> From: David Henningsson <david.henningsson@canonical.com>
> Date: Wed, 4 Jan 2012 10:12:54 +0100
> Subject: [PATCH] ALSA: HDA: Use LPIB position fix for Oaktrail
> 
> According to the thread on alsa-devel, the LPIB method is to prefer
> for Oaktrail controller chip.
> 
> Reference: http://mailman.alsa-project.org/pipermail/alsa-devel/2012-January/047800.html
> 
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  sound/pci/hda/hda_intel.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index bd447b6..8e232ab 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -2997,7 +2997,7 @@ static DEFINE_PCI_DEVICE_TABLE(azx_ids) = {
>  	  AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_LPIB }, /* Poulsbo */
>  	{ PCI_DEVICE(0x8086, 0x080a),
>  	  .driver_data = AZX_DRIVER_SCH | AZX_DCAPS_SCH_SNOOP |
> -	  AZX_DCAPS_BUFSIZE},
> +	  AZX_DCAPS_BUFSIZE | AZX_DCAPS_POSFIX_LPIB }, /* Oaktrail */
>  	/* ICH */
>  	{ PCI_DEVICE(0x8086, 0x2668),
>  	  .driver_data = AZX_DRIVER_ICH | AZX_DCAPS_OLD_SSYNC |
> -- 
> 1.7.5.4
> 

Re: [PATCH] hda_intel: Add Oaktrail identifiers

[David Henningsson]

On 01/04/2012 12:06 AM, Alan Cox wrote:
>> Btw, you seem to be employed by Intel these days? [1] Any chance you can
>> talk to your firmware developers of this chip? I assume somebody at
>> Intel should know what method to prefer. That would be the safest/best
>> option IMHO.
>
> Indeed I am and by the time I got around to looking at it other Intel
> folsk on the list were already digging. Yes we think it needs the same
> settings as Poulsbo.

While we're at it, I tried to backtrace the PCI Vendor ID:s for the 
individual machine quirks we have, and came up with the following list:

8086:27d8 (rev 01) (Intel Corporation N10/ICH 7 Family) - appeared twice
8086:284b (rev 02) (Intel Corporation 82801H (ICH8 Family))
8086:293e (rev 03) (Intel Corporation 82801I (ICH9 Family))
8086:3b56 (rev 05) (Intel Corporation 5 Series/3400 Series)

If you don't mind doing some more lurking, do you think any of the above 
chips should be tweaked to use LPIB position fix method for the entire 
chip, and not just the individual machine?

(For the sake of completeness, 1106:3288 (rev 10) (VIA Technologies, 
Inc. VT1708/A [Azalia HDAC]) also appeared three times.)

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic