Re: mmotm 2010-10-20-15-01 uploaded

Previous thread: [PATCH] Correct comment for ext[23] filesystem "ctime" field by Robert P. J. Day on Wednesday, October 20, 2010 - 1:26 pm. (1 message)

Next thread: [PATCH 1/6] Free up pf flag PF_KSOFTIRQD by Venkatesh Pallipadi on Wednesday, October 20, 2010 - 3:48 pm. (7 messages)
From: akpm
Date: Wednesday, October 20, 2010 - 3:01 pm

The mm-of-the-moment snapshot 2010-10-20-15-01 has been uploaded to

   http://userweb.kernel.org/~akpm/mmotm/

and will soon be available at

   git://zen-kernel.org/kernel/mmotm.git

It contains the following patches against ...
From: Li Zefan
Date: Wednesday, October 20, 2010 - 7:54 pm

Got this:

arch/x86/pci/xen.c: In function 'pci_xen_init':
arch/x86/pci/xen.c:138: error: 'isapnp_disable' undeclared (first use in this function)
arch/x86/pci/xen.c:138: error: (Each undeclared identifier is reported only once
arch/x86/pci/xen.c:138: error: for each function it appears in.)

# quilt patches arch/x86/pci/xen.c
linux-next.patch
--

From: Li Zefan
Date: Wednesday, October 20, 2010 - 8:15 pm

I just commented out isapnp_disable, and after boot I saw this warning:

------------[ cut here ]------------
WARNING: at arch/x86/mm/highmem_32.c:82 __kunmap_atomic+0x80/0xd4()
Hardware name: ASPIRE AG1720
Modules linked in:
Pid: 1, comm: swapper Not tainted 2.6.36-rc8-mm1+ #1
Call Trace:
 [<c0440982>] warn_slowpath_common+0x6a/0x7f
 [<c042e6c5>] ? __kunmap_atomic+0x80/0xd4
 [<c04409ab>] warn_slowpath_null+0x14/0x18
 [<c042e6c5>] __kunmap_atomic+0x80/0xd4
 [<c04f2d1f>] zero_user_segments+0x5e/0x64
 [<c04f2e58>] simple_write_begin+0x5e/0x6c
 [<c04a6518>] generic_file_buffered_write+0xc0/0x1c5
 [<c0444f41>] ? current_fs_time+0x1b/0x1e
 [<c04a7745>] __generic_file_aio_write+0x25e/0x28f
 [<c04a77d8>] generic_file_aio_write+0x62/0xaa
 [<c04db6df>] do_sync_write+0x8f/0xca
 [<c075fc8f>] ? _raw_write_unlock_irqrestore+0x41/0x4d
 [<c074aedd>] ? kmemleak_alloc+0x59/0x78
 [<c09874f3>] ? do_name+0x10e/0x26b
 [<c04db650>] ? do_sync_write+0x0/0xca
 [<c04dbc44>] vfs_write+0x85/0xe3
 [<c04dbd40>] sys_write+0x40/0x62
 [<c098722b>] do_copy+0x24/0xbe
 [<c09870d4>] flush_buffer+0x6a/0x8a
 [<c09a84b1>] gunzip+0x267/0x2e6
 [<c0592458>] ? nofill+0x0/0x8
 [<c09a824a>] ? gunzip+0x0/0x2e6
 [<c0987b7f>] unpack_to_rootfs+0x1e0/0x2ee
 [<c098706a>] ? flush_buffer+0x0/0x8a
 [<c0986e93>] ? error+0x0/0x13
 [<c0987cd9>] populate_rootfs+0x4c/0x200
 [<c05a944e>] ? pci_get_subsys+0x4d/0x59
 [<c075da14>] ? printk+0x14/0x18
 [<c09aa16f>] ? pci_apply_final_quirks+0xc9/0xe6
 [<c0403080>] do_one_initcall+0x76/0x126
 [<c0987c8d>] ? populate_rootfs+0x0/0x200
 [<c098539e>] kernel_init+0x126/0x1a2
 [<c0985278>] ? kernel_init+0x0/0x1a2
 [<c0409942>] kernel_thread_helper+0x6/0x10
---[ end trace e93713a9d40cd06c ]---
--

From: Konrad Rzeszutek Wilk
Date: Wednesday, October 20, 2010 - 8:38 pm

From: Li Zefan
Date: Wednesday, October 20, 2010 - 8:44 pm

It just showed up during boot.

From: Konrad Rzeszutek Wilk
Date: Thursday, October 21, 2010 - 7:28 am

I got this patch queued up. You mentioned that commenting it out did fix
the problem, so I stuck a Tested-by tag from you on it.

From 5bba6c56dc99ff88f79a79572e29ecf445710878 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 21 Oct 2010 09:36:07 -0400
Subject: [PATCH 27/27] X86/PCI: Remove the dependency on isapnp_disable.

This looks to be vestigial dependency that had never been used even
in the original code base (2.6.18) from which this driver
was up-ported. Without this fix, with the CONFIG_ISAPNP, we get this
compile failure:

arch/x86/pci/xen.c: In function 'pci_xen_init':
arch/x86/pci/xen.c:138: error: 'isapnp_disable' undeclared (first use in this function)
arch/x86/pci/xen.c:138: error: (Each undeclared identifier is reported only once
arch/x86/pci/xen.c:138: error: for each function it appears in.)

Reported-by: Li Zefan <lizf@cn.fujitsu.com>
Tested-by: Li Zefan <lizf@cn.fujitsu.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/pci/xen.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index b19c873..4e37106 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -133,11 +133,6 @@ int __init pci_xen_init(void)
 	acpi_noirq = 1;
 #endif
 
-#ifdef CONFIG_ISAPNP
-	/* Stop isapnp from probing */
-	isapnp_disable = 1;
-#endif
-
 #ifdef CONFIG_PCI_MSI
 	x86_msi.setup_msi_irqs = xen_setup_msi_irqs;
 	x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
-- 
1.7.1

--

From: Andrew Morton
Date: Wednesday, October 20, 2010 - 8:46 pm

WARN_ON_ONCE(idx !=

It would have been clearer to do

	WARN_ON_ONCE(vaddr != __fix_to_virt(FIX_KMAP_BEGIN + idx))

but I don't see what's gone wrong here.  Peter?


--

From: Peter Zijlstra
Date: Thursday, October 21, 2010 - 12:17 am

Right, so that's the warning that the unmapped address isn't actually
the top-of-stack one.

Your version is much nicer, although I haven't looked too hard at it, I
probably got my head in a twist.

But like you, I cannot directly see what's going wrong here,
zero_user_segments() seems a simple enough function without any
kmap_atomic nesting, so I'm not at all seeing how that's going wrong.

/me goes find where you hide your mmotm patch-queue and stare at the
actual code.

--

From: Andrew Morton
Date: Thursday, October 21, 2010 - 12:30 am

hm, OK.  The x86 guys have been mucking with the fixmap code lately but
I don't see anything which could cause this.

btw, it's a bit sad to use KM_TYPE_NR*NR_CPUS pages of virtual address
space for kmap_atomic().  I'd expect that distros set NR_CPUS quite
large (Fedora has 256).  That's 20MB of virtual address space consumed,
I think.

And we consume it on non-highmem kernels, too...
--

From: Peter Zijlstra
Date: Thursday, October 21, 2010 - 2:52 am

OK, so found it, and its really rather embarrassing.. I really blotched
those x86 WARN_ON_ONCE()s, for some reason I got my brain in a twist and
messed those up big-time. All other architectures have the form you
suggested earlier.

The problem is that fixmaps are top-down, so the original warning ended
up trying to compare x with (-x) >> PAGE_SHIFT, which clearly won't
work.

The sad part is that I apparently only compile tested the debug code :/

With the warning fixed (and CONFIG_DEBUG_HIGHMEM=y) an otherwise
i386-defconfig seems to fully complete boot without warnings on a qemu
with 2048 MB of memory.

---
Subject: mm, x86: Fix stack based kmap_atomic debug warnings
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Thu Oct 21 11:45:08 CEST 2010

Due to a massive brainfart I got the x86 kunmap_atomic debug code that
is supposed to avoid stack violations wrong.

Use the form all other architectures already use (which was also
independently suggested by Andrew).

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 arch/x86/mm/highmem_32.c |    3 +--
 arch/x86/mm/iomap_32.c   |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/x86/mm/highmem_32.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/highmem_32.c
+++ linux-2.6/arch/x86/mm/highmem_32.c
@@ -78,8 +78,7 @@ void __kunmap_atomic(void *kvaddr)
 		idx = type + KM_TYPE_NR * smp_processor_id();
 
 #ifdef CONFIG_DEBUG_HIGHMEM
-		WARN_ON_ONCE(idx !=
-			((vaddr - __fix_to_virt(FIX_KMAP_BEGIN)) >> PAGE_SHIFT));
+		WARN_ON_ONCE(vaddr != __fix_to_virt(FIX_KMAP_BEGIN + idx));
 #endif
 		/*
 		 * Force other mappings to Oops if they'll try to access this
Index: linux-2.6/arch/x86/mm/iomap_32.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/iomap_32.c
+++ linux-2.6/arch/x86/mm/iomap_32.c
@@ -102,8 +102,7 @@ iounmap_atomic(void __iomem *kvaddr)
 		idx = type + KM_TYPE_NR * ...
From: Peter Zijlstra
Date: Thursday, October 21, 2010 - 2:54 am

I guess its something to look at, luckily i386 kernels tend to have
small NR_CPUS and x86_64 kernels have gobs of vaddr space so its not
really a problem, but yeah would be nice to fix.



--

From: Jiri Slaby
Date: Thursday, October 21, 2010 - 12:41 am

Hi, I've lost sound with my intel hda (sigmatel) in this release
(regression against mmotm 2010-10-13-17-13).

All outputs were silented at level 0 after reboot, but even after
setting them non-zero in alsamixer -D hw, no sound can be heard by:
mplayer -ao alsa:device=hw /usr/share/sounds/alsa/Front_Center.wav

I don't knwo if it's related, but aplay complains like (even if I pass
-c 2):
aplay -D hw /usr/share/sounds/alsa/Front_Center.wav
Playing WAVE '/usr/share/sounds/alsa/Front_Center.wav' : Signed 16 bit
Little Endian, Rate 48000 Hz, Mono
aplay: set_params:1065: Channels count non available


alsa-info:
http://www.alsa-project.org/db/?f=a7a09703bcc8c43386c87a984e513ce35fc91ca4

regards,
-- 
js
--

From: Takashi Iwai
Date: Thursday, October 21, 2010 - 12:49 am

At Thu, 21 Oct 2010 09:41:08 +0200,

This is irrelevant.  The hardware supports only stereo streams while

I see the Front volume is set to zero.  Try to raise it.


Takashi
--

From: Jiri Slaby
Date: Thursday, October 21, 2010 - 12:57 am

I set all the values to ~ 70 and then tried with pure alsa. It basically
works.

BUT, when I run pulse and it suspends the device (or whatever), all the
levels get down to 0 back again. When I raise it and run mplayer, it
gets to 0 immediately. If I raise it gets to 0 when mplayer finishes and
pulse writes 'protocol-native.c: Connection died.'. It never raises
automatically. And if I raise it during playback, nothing plays at all.

regards,
-- 
js
--

From: Takashi Iwai
Date: Thursday, October 21, 2010 - 1:05 am

At Thu, 21 Oct 2010 09:57:16 +0200,

Hrm, I don't remember any so critical changes done recently in the
sound tree.  The only possibly affecting commits are:

  commit 1cc9e8f4c45999e6069f41521d9d391eeeccc3b3
    ALSA: hda - Fix codec muted after rebooting from Windows

  commit de8c85f7840e5e29629de95f5af24297fb325e0b
    ALSA: HDA: Sigmatel: work around incorrect master muting
    
Could you try to revert them?    


thanks,

Takashi
--

From: Jiri Slaby
Date: Thursday, October 21, 2010 - 11:27 am

Reverting this one helps.

thanks,
-- 
js
--

From: Takashi Iwai
Date: Thursday, October 21, 2010 - 12:20 pm

At Thu, 21 Oct 2010 20:27:01 +0200,

The latter one?


Takashi
--

From: Jiri Slaby
Date: Thursday, October 21, 2010 - 12:21 pm

Yeah. Otherwise I would write it to the former :).

-- 
js
--

From: Takashi Iwai
Date: Thursday, October 21, 2010 - 12:22 pm

At Thu, 21 Oct 2010 21:21:32 +0200,

Yeah, I'm just be careful now ;)


Takashi
--

From: Takashi Iwai
Date: Thursday, October 21, 2010 - 12:40 pm

At Thu, 21 Oct 2010 21:21:32 +0200,

This might be a user-space issue, then.

Could you try once the latest alsa-lib?  If you are using SUSE packages,
the snapshot rpms are available on OBS multimedia:audio:snapshot/alsa
repo.  (The version number of the package is still 1.0.22, but don't
care; I just forgot to change the spec file.)


thanks,

Takashi
--

From: Valdis.Kletnieks
Date: Thursday, October 21, 2010 - 1:29 pm

Confirming - reverting that one commit fixes sound on my Dell laptop as

That would be icky, if we have to lockstep a 2.6.37 kernel upgrade to
an alsa-lib upgrade.  Stuff like udev and perf having to lockstep is

Hmm... on my laptop, Fedora Rawhide is showing:

% rpm -qi alsa-lib
Name        : alsa-lib                     Relocations: (not relocatable)
Version     : 1.0.23                            Vendor: Fedora Project
Release     : 1.fc14                        Build Date: Fri 16 Apr 2010 07:48:17 AM EDT
Install Date: Sun 18 Apr 2010 03:22:04 PM EDT      Build Host: x86-02.phx2.fedoraproject.org
Group       : System Environment/Libraries   Source RPM: alsa-lib-1.0.23-1.fc14.src.rpm
Size        : 1188301                          License: LGPLv2+
Signature   : (none)
Packager    : Fedora Project
URL         : http://www.alsa-project.org/
Summary     : The Advanced Linux Sound Architecture (ALSA) library
Description :
The Advanced Linux Sound Architecture (ALSA) provides audio and MIDI
functionality to the Linux operating system.

Is that "recent enough", or do I need to find something newer?


From: Takashi Iwai
Date: Thursday, October 21, 2010 - 1:48 pm

[Added Colin to Cc]

At Thu, 21 Oct 2010 16:29:17 -0400,

Yeah, if this is really the culprit, we should think of reverting the

Well, this doesn't tell much...
The fix patch for alsa-lib is pretty recent.  Attached below.


thanks,

Takashi

---
commit 2f6206da0c1ff88235e6eca0077343f22a4b43ee
Author: Clemens Ladisch <clemens@ladisch.de>
Date:   Fri Oct 15 10:33:20 2010 +0200

    tlv: fix returned dB information for min-is-mute controls
    
    For TLV information that indicates that the minimum value is actually
    muted, the returned range used the wrong minimum dB value, and
    converting dB values to raw control values did not round up correctly
    near the minimum.
    
    Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
    Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/src/control/tlv.c b/src/control/tlv.c
index ba52752..f7c9976 100644
--- a/src/control/tlv.c
+++ b/src/control/tlv.c
@@ -167,17 +167,23 @@ int snd_tlv_get_dB_range(unsigned int *tlv, long rangemin, long rangemax,
 	}
 	case SND_CTL_TLVT_DB_SCALE: {
 		int step;
-		*min = (int)tlv[2];
+		if (tlv[3] & 0x10000)
+			*min = SND_CTL_TLV_DB_GAIN_MUTE;
+		else
+			*min = (int)tlv[2];
 		step = (tlv[3] & 0xffff);
-		*max = *min + (long)(step * (rangemax - rangemin));
+		*max = (int)tlv[2] + step * (rangemax - rangemin);
 		return 0;
 	}
 	case SND_CTL_TLVT_DB_MINMAX:
-	case SND_CTL_TLVT_DB_MINMAX_MUTE:
 	case SND_CTL_TLVT_DB_LINEAR:
 		*min = (int)tlv[2];
 		*max = (int)tlv[3];
 		return 0;
+	case SND_CTL_TLVT_DB_MINMAX_MUTE:
+		*min = SND_CTL_TLV_DB_GAIN_MUTE;
+		*max = (int)tlv[3];
+		return 0;
 	}
 	return -EINVAL;
 }
@@ -217,7 +223,7 @@ int snd_tlv_convert_to_dB(unsigned int *tlv, long rangemin, long rangemax,
 		min = tlv[2];
 		step = (tlv[3] & 0xffff);
 		mute = (tlv[3] >> 16) & 1;
-		if (mute && volume == rangemin)
+		if (mute && volume <= rangemin)
 			*db_gain = SND_CTL_TLV_DB_GAIN_MUTE;
 		else
 			*db_gain = (volume - rangemin) * step + min;
@@ ...
From: Valdis.Kletnieks
Date: Thursday, October 21, 2010 - 2:24 pm

That looks like a pretty self-contained patch, I should be able to build a
local test alsa-lib with that one added and see if it works with both old and
new kernels.  If I don't get a chance to do it tonight, won't be till Monday
I suspect, am booked solid with other stuff till then...

From: Takashi Iwai
Date: Thursday, October 21, 2010 - 2:31 pm

At Thu, 21 Oct 2010 17:24:24 -0400,

Well, it's not confirmed whether it just triggered an alsa-lib bug
or not.  A revert is an easy option, and we can do it at any time.

OK.  Maybe someone else can check it meanwhile.


thanks,

Takashi
--

From: Jiri Slaby
Date: Thursday, October 21, 2010 - 2:42 pm

Someone else rebooted the 13th time today and checked :):
$ rpm -q `rpmqpack |egrep 'alsa|asound'|sort`
alsa-1.0.22.git20101018-1.1.x86_64
alsa-firmware-1.0.23-3.7.noarch
alsa-oss-1.0.17-31.4.x86_64
alsa-plugins-1.0.23-4.5.x86_64
alsa-plugins-pulse-1.0.23-4.5.x86_64
alsa-plugins-pulse-32bit-1.0.23-4.5.x86_64
alsa-plugins-32bit-1.0.23-4.5.x86_64
alsa-utils-1.0.23-4.5.x86_64
libasound2-1.0.22.git20101018-1.1.x86_64
libasound2-32bit-1.0.23-6.5.x86_64

No success. I still need the revert.

regards,
-- 
js
--

From: Colin Guthrie
Date: Friday, October 22, 2010 - 1:46 am

Just tested the latest kernel patch in a "proper" (i.e. patched in
kernel) build (as opposed to my previous out-of-tree builds).

It's working great for me (stac9200)

I tried without the userspace patch and things still work fine for me
under this setup - it's just that PAs flat volumes do not properly
control the Master+PCM pipeline (they both go to mute when master hits 0).

But it doesn't seem to cause any other problems for me.

I'm no expert at the kernel side, but can't see much in the code that
would cause this.

However, from Jiri's alsa-info, this looks a bit suspicious:

Simple mixer control 'Master',0
  Capabilities: pvolume pvolume-joined pswitch pswitch-joined penum
  Playback channels: Mono
  Limits: Playback 0 - 64
  Mono: Playback 40 [62%] [1620.40dB] [on]

1620.40dB?? Really?

Is perhaps the TLV fix for min-is-mute conflicting with a db range fix?

Col


-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]
--

From: Takashi Iwai
Date: Friday, October 22, 2010 - 2:02 am

At Fri, 22 Oct 2010 09:46:06 +0100,

So, you mean it works without alsa-lib patch on your system?

The symptom was somehow related with the suspend or initialization.
In the early post (sorry, this wasn't cited in the mail you added to
Cc), Jiri mentioned:

  BUT, when I run pulse and it suspends the device (or whatever), all
  the levels get down to 0 back again. When I raise it and run
  mplayer, it gets to 0 immediately. If I raise it gets to 0 when
  mplayer finishes and pulse writes 'protocol-native.c: Connection
  died.'. It never raises automatically. And if I raise it during

Well, it doesn't conflict but the old alsa-lib just takes the value
as is -- i.e. the mute high bit is evaluated as a normal value.  So,
obviously it passes a wrong value for the old alsa-lib.

The above result is likely because alsa-info was generated with the
unpatched alsa-lib.  Jiri, could you regenerate alsa-info output with
the fixed alsa-lib packages?  With the fixed alsa-lib, the dB value
should be given correctly.

Anyway, judging from the fact above (giving a really wrong value), we
will have to revert the commit...


thanks,

Takashi
--

From: Jiri Slaby
Date: Friday, October 22, 2010 - 2:18 am

Sure, but I don't know which version you want, so with the patch reverted:
http://www.alsa-project.org/db/?f=4efd8e3b74a51adbd014a85052a83b4a3c5bd9fc

with the patch NOT reverted:
http://www.alsa-project.org/db/?f=e619bba74bb9ea007c2e5314761411a658ff677b

regards,
-- 
js
--

From: Takashi Iwai
Date: Friday, October 22, 2010 - 2:40 am

At Fri, 22 Oct 2010 11:18:37 +0200,

Hrm, is it with alsa.rpm and libasound2.rpm from OBS
multimedia:audio:snapshot repo?

If yes, something wrong in the alsa-lib packaging...


Takashi
--

From: Jiri Slaby
Date: Friday, October 22, 2010 - 2:45 am

From: Takashi Iwai
Date: Friday, October 22, 2010 - 3:26 am

At Fri, 22 Oct 2010 11:45:36 +0200,

It's still showing a strange value.  Weird.

I updated the version number and triggered rebuilds on OBS
multimedia:audio:snapshot packages.  To be sure, install alsa.rpm,
libasound2.rpm and alsa-utils.rpm later from the project.
Now they should have version 1.0.23.git*.

Also, you don't need to check the reverted kernel.  The only
interesting thing is to see whether the dB range brokeness is fixed
by alsa-lib update.


thanks,

Takashi
--

From: Jiri Slaby
Date: Friday, October 22, 2010 - 4:10 am

alsa-1.0.23.git20101018-1.1.x86_64
alsa-firmware-1.0.23-3.7.noarch
alsa-oss-1.0.17-31.4.x86_64
alsa-plugins-1.0.23-4.5.x86_64
alsa-plugins-pulse-1.0.23-4.5.x86_64
alsa-plugins-pulse-32bit-1.0.23-4.5.x86_64
alsa-plugins-32bit-1.0.23-4.5.x86_64
alsa-utils-1.0.23.git20101022-3.1.x86_64
libasound2-1.0.23.git20101018-1.1.x86_64

With patch not reverted (i.e. no sound):
http://www.alsa-project.org/db/?f=378efb40f8c5b1d6204e93221512506e7bb7b061

regards,
-- 
js
--

From: Colin Guthrie
Date: Friday, October 22, 2010 - 2:41 am

Yeah everything still works OK, but the infamous "mute at 16%" issue is
obviously unresolved.

The difference with the kernel patch applied vs. not applied is that
alsamixer reports the device volume as "mute" when Master is at 0 with

Yeah, I did read that via the lklm archive. It's very odd that the
volumes change when pulse suspends the device... I could understand if
it changed when it opens them again (it will look at the product of the
mixers in the pipeline - e.g. Master+PCM - then work out the over all dB
then rearrange the sliders so that the same dB is ultimately obtained,
but perhaps via a different combination of mixer positions (e.g. it will
use all of Master first and only then adjust PCM). Overall the volume

Right. This actually probably explains things thinking about it...


Because the range is incorrectly reflected without the alsa-lib patch.
PA tries to set the volume to a "sensible" dB value.

This value would likely cause Master to be set to 0 (seeing as 1620.40dB
is 40, $sensible_dB is likely mapped to 0). When the device hits 0, the
kernel patch causes this to be properly reflected as "mute" (or -infdB?)
and thus PA notices this and sets both Master and PCM to 0 for that reason.

That might not be exactly what's going on, but it's probably some


I'm surprised that the updated alsa-lib did not fix this problem. We're
now pushing out both the kernel and userspace fix, so I'll see if anyone
is affected by this in the same way as Jiri.

Jiri, when you tested the userspace updated package did you remember to
restart PA? If not then the test will not have worked as expected (the
fact that both your subsequent alsa-info's have sensible dB values for
Master suggests that the userspace fix is installed). Also when Takashi
asked for updated alsa-info script above, he meant to do it with both

Yes, sadly that seems so. Any bright idea on how to fix this in a less
invasive (i.e. backwards compatible) way?

Cheers

Col


-- 

Colin ...
From: Clemens Ladisch
Date: Friday, October 22, 2010 - 4:31 am

This one introduced even more wrong volume information on some codecs.
Pleasy try this patch on top:

--- a/sound/pci/hda/patch_sigmatel.c
+++ b/sound/pci/hda/patch_sigmatel.c
@@ -32,6 +32,7 @@
 #include <sound/core.h>
 #include <sound/asoundef.h>
 #include <sound/jack.h>
+#include <sound/tlv.h>
 #include "hda_codec.h"
 #include "hda_local.h"
 #include "hda_beep.h"
@@ -1145,7 +1146,7 @@ static int stac92xx_build_controls(struct hda_codec *codec)
 		/* correct volume offset */
 		vmaster_tlv[2] += vmaster_tlv[3] * spec->volume_offset;
 		/* minimum value is actually mute */
-		vmaster_tlv[3] |= 0x1000;
+		vmaster_tlv[3] |= TLV_DB_SCALE_MUTE;
 		err = snd_hda_add_vmaster(codec, "Master Playback Volume",
 					  vmaster_tlv, slave_vols);
 		if (err < 0)

--

From: Jiri Slaby
Date: Friday, October 22, 2010 - 4:51 am

thanks,
-- 
js
--

From: Takashi Iwai
Date: Friday, October 22, 2010 - 7:03 am

At Fri, 22 Oct 2010 13:31:53 +0200,

Argh!  There is always a reason why we should define a constant.
I applied the fix now.

This explains why the old patch worked on Colin's machine.
His codec chip is different (STAC9200?), and it doesn't go in this
code path but has a static Master volume creation.


thanks,

--

From: Colin Guthrie
Date: Friday, October 22, 2010 - 12:53 pm

Yup I have a STAC9200 so that explains it... silly me as I did see this
line and figured "the value will be right" rather than questioning it.

Anyway, glad this has been found and there is now (presumably) no need
for a revert :)

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]
--

From: Valdis.Kletnieks
Date: Thursday, October 21, 2010 - 8:22 am

Seeing the same thing on a Dell Latitude E6500, so Jiri isn't hallucinating.

Didn't we have some flustercluck a while back that gave pulseaudio
similar indigestion?  Oh, here it is, it was an fsnotify botch, of all things.

commit 2069601b3f0ea38170d4b509b89f3ca0a373bdc1
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu Aug 12 14:23:04 2010 -0700

    Revert "fsnotify: store struct file not struct path"
    
    This reverts commit 3bcf3860a4ff9bbc522820b4b765e65e4deceb3e (and the
    accompanying commit c1e5c954020e "vfs/fsnotify: fsnotify_close can delay
    the final work in fput" that was a horribly ugly hack to make it work at
    all).


Did we manage to revert the revert, or re-break this code? I haven't looked at
this at all in detail.
From: Jiri Slaby
Date: Thursday, October 21, 2010 - 10:59 am

Yes, I was attacked by that in the past, but I doubt that's it now.
Pulse was unable to start -- the second open of the snd device failed
which doesn't seem to happen now.

Going to switch hda intel from Y to M and play with that (revert the
patches at first).

regards,
-- 
js
--

From: Zimny Lech
Date: Thursday, October 21, 2010 - 4:10 am

Success story:

-- 
Slawa!
N.P.S.

Les fleurs du mal unfold
Comme les fleurs du mal
Dark demons of my soul
Un amour fatal
--

From: Valdis.Kletnieks
Date: Thursday, October 21, 2010 - 4:14 am

Seen during 'make oldconfig':

      Input subsystem LEDs Trigger (LEDS_TRIGGER_INPUT) [N/m/y/?] (NEW) m
      *
      * iptables trigger is under Netfilter config (LED target)
      *
warning: (NETFILTER_XT_MATCH_REALM && NET && INET && NETFILTER && NETFILTER_XTABLES && NETFILTER_ADVANCED || NET_CLS_ROUTE4 && NET && NET_SCHED) selects NET_CLS_ROUTE which has unmet direct dependencies (NET && NET_SCHED)
#
# configuration written to .config
#



From: Patrick McHardy
Date: Friday, October 29, 2010 - 12:11 pm

Thanks for the report, this is how I intend to fix it:

From: Jiri Slaby
Date: Thursday, October 21, 2010 - 10:51 am

Well, this release happens to be a multimedia failure :(.

I hit another regression against mmotm 2010-10-13-17-13, where
0413:6029 (DVB USB tuner) can't be inited when plugged in. Even lsusb
waits inifitely.

usb 2-6: new high speed USB device using ehci_hcd and address 5
dvb-usb: found a 'Leadtek WinFast DTV Dongle Gold' in cold state, will
try to load a firmware
dvb-usb: downloading firmware from file 'dvb-usb-af9015.fw'
dvb-usb: found a 'Leadtek WinFast DTV Dongle Gold' in warm state.
dvb-usb: will pass the complete MPEG2 transport stream to the software
demuxer.
DVB: registering new adapter (Leadtek WinFast DTV Dongle Gold)
af9013: firmware version:4.95.0.0
DVB: registering adapter 1 frontend 0 (Afatech AF9013 DVB-T)...
tda18271 18-00c0: creating new instance
TDA18271HD/C2 detected @ 18-00c0
af9015: command failed:1
tda18271_write_regs: [18-00c0|M] ERROR: idx = 0x0, len = 39,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [18-00c0|M] ERROR: idx = 0x20, len = 1,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [18-00c0|M] ERROR: idx = 0x20, len = 1,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [18-00c0|M] ERROR: idx = 0x20, len = 1,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [18-00c0|M] ERROR: idx = 0x20, len = 1,
i2c_transfer returned: -1
af9015: recv bulk message failed:-71
af9015: bulk message failed:-71 (8/0)
af9013: I2C read failed reg:d417
tda18271_write_regs: [18-00c0|M] ERROR: idx = 0x5, len = 11,
i2c_transfer returned: -1
af9015: bulk message failed:-71 (8/0)
af9013: I2C read failed reg:d417
af9015: bulk message failed:-71 (9/0)
af9015: bulk message failed:-71 (8/0)
af9013: I2C read failed reg:d417
tda18271_write_regs: [18-00c0|M] ERROR: idx = 0x13, len = 1,
i2c_transfer returned: -71
af9015: bulk message failed:-71 (8/0)
af9013: I2C read failed reg:d417
af9015: bulk message failed:-71 (9/0)
af9015: bulk message failed:-71 (8/0)
af9013: I2C ...
From: Jiri Slaby
Date: Thursday, October 21, 2010 - 3:24 pm

-- 
js
--

From: Jiri Slaby
Date: Friday, October 22, 2010 - 2:21 am

Revert of this commit helps:

commit 99ef217e01780668ac21d600eaf4acd3f7666a02
Author: Mauro Carvalho Chehab <mchehab@redhat.com>
Date:   Sun Sep 26 22:58:28 2010 -0300

    [media] tda18271: allow restricting max out to 4 bytes

    By default, tda18271 tries to optimize I2C bus by updating all registers
    at the same time. Unfortunately, some devices doesn't support it.

    The current logic has a problem when small_i2c is equal to 8, since
there
    are some transfers using 11 + 1 bytes.

    Fix the problem by enforcing the max size at the right place, and allows
    reducing it to max = 3 + 1.

    Acked-by: Michael Krufky <mkrufky@kernellabs.com>
    Acked-by: Sri Deevi <Srinivasa.Deevi@conexant.com>
-- 
js
--

From: Mauro Carvalho Chehab
Date: Friday, October 22, 2010 - 6:56 am

Ah, the problem is here (drivers/media/dvb/dvb-usb/af9015.c):

static struct tda18271_config af9015_tda18271_config = {
	.gate = TDA18271_GATE_DIGITAL,
	.small_i2c = 1,
};

small_i2c is an enum, currently defined as:

enum tda18271_small_i2c {
        TDA18271_39_BYTE_CHUNK_INIT = 0,
	TDA18271_16_BYTE_CHUNK_INIT = 16,
	TDA18271_08_BYTE_CHUNK_INIT = 8,
	TDA18271_03_BYTE_CHUNK_INIT = 3,
};

The value is just a magic number, but having it associated with the max
I2C size makes more sense than a random value.

The value, before the patch was:

enum tda18271_small_i2c {
        TDA18271_39_BYTE_CHUNK_INIT = 0,
	TDA18271_16_BYTE_CHUNK_INIT = 1,
	TDA18271_08_BYTE_CHUNK_INIT = 2,
};

So, it seems that the max I2C size for af9015 is 16 bytes. The proper init
should be:

	.small_i2c = TDA18271_16_BYTE_CHUNK_INIT;

Please check if the enclosed patch fixes the issue.

Thanks,
Mauro

---

[media] af9015: Properly initialize tda18271.small_i2c

As reported by Jiri, a regression at mm series is happening on af9015:

tda18271 18-00c0: creating new instance
TDA18271HD/C2 detected @ 18-00c0
af9015: command failed:1
tda18271_write_regs: [18-00c0|M] ERROR: idx = 0x0, len = 39,
i2c_transfer returned: -1
af9015: command failed:1

The cause is that tda18271.small_i2c is an enum. Drivers should use the value
alias, instead of its number, as the "magic" number may change anytime.

Reported-by: Jiri Slaby <jirislaby@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index 759cbf8..f5e4066 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -1144,7 +1144,7 @@ static struct qt1010_config af9015_qt1010_config = {
 
 static struct tda18271_config af9015_tda18271_config = {
 	.gate = TDA18271_GATE_DIGITAL,
-	.small_i2c = 1,
+	.small_i2c = TDA18271_16_BYTE_CHUNK_INIT;,
 };
 
 static struct mxl5005s_config af9015_mxl5003_config = ...
From: Jiri Slaby
Date: Friday, October 22, 2010 - 7:06 am

No, it doesn't help (note the i2c line which prints the small_i2c entry):
dvb-usb: found a 'Leadtek WinFast DTV Dongle Gold' in cold state, will
try to load a firmware
dvb-usb: downloading firmware from file 'dvb-usb-af9015.fw'
dvb-usb: found a 'Leadtek WinFast DTV Dongle Gold' in warm state.
dvb-usb: will pass the complete MPEG2 transport stream to the software
demuxer.
DVB: registering new adapter (Leadtek WinFast DTV Dongle Gold)
af9013: firmware version:4.95.0.0
DVB: registering adapter 0 frontend 0 (Afatech AF9013 DVB-T)...
af9015_tuner_attach: i2c 16
tda18271 16-00c0: creating new instance
TDA18271HD/C2 detected @ 16-00c0
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x0, len = 39,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x20, len = 1,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x20, len = 1,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x20, len = 1,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x20, len = 1,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x5, len = 11,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x13, len = 1,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x13, len = 1,
i2c_transfer returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x3, len = 1, i2c_transfer
returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x5, len = 7, i2c_transfer
returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x4, len = 1, i2c_transfer
returned: -1
af9015: command failed:1
tda18271_write_regs: [16-00c0|M] ERROR: idx = 0x5, len = 11,
i2c_transfer returned: -1
af9015: command ...
From: Mauro Carvalho Chehab
Date: Wednesday, October 27, 2010 - 9:12 am

Hmm...

int tda18271_write_regs(struct dvb_frontend *fe, int idx, int len)
{
...
        switch (priv->small_i2c) {
        case TDA18271_03_BYTE_CHUNK_INIT:
                max = 3;
                break;
        case TDA18271_08_BYTE_CHUNK_INIT:
                max = 8;
                break;
        case TDA18271_16_BYTE_CHUNK_INIT:
                max = 16;
                break;
        case TDA18271_39_BYTE_CHUNK_INIT:
        default:
                max = 39;
        }


with small_i2c = 1, the driver will not restrict the maximum length size.

It's weird that the patch didn't fix it. Are you sure that reverting this
patch is enough to make the driver work?

Please test this one.

It will properly log the size of the message the driver tried to use, and will
reduce the max number of bytes per I2C transfer to 8.

Cheers,
Mauro

---

Fix tda18271 usage with af9015 

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/common/tuners/tda18271-common.c b/drivers/media/common/tuners/tda18271-common.c
index 195b30e..5466d47 100644
--- a/drivers/media/common/tuners/tda18271-common.c
+++ b/drivers/media/common/tuners/tda18271-common.c
@@ -237,7 +237,7 @@ int tda18271_write_regs(struct dvb_frontend *fe, int idx, int len)
 
 	if (ret != 1)
 		tda_err("ERROR: idx = 0x%x, len = %d, "
-			"i2c_transfer returned: %d\n", idx, len, ret);
+			"i2c_transfer returned: %d\n", idx, max, ret);
 
 	return (ret == 1 ? 0 : ret);
 }
diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index 3ef19a8..e3c9bb5 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -1131,7 +1131,7 @@ static struct qt1010_config af9015_qt1010_config = {
 
 static struct tda18271_config af9015_tda18271_config = {
 	.gate = TDA18271_GATE_DIGITAL,
-	.small_i2c = 1,
+	.small_i2c = TDA18271_08_BYTE_CHUNK_INIT,
 };
 
 static struct mxl5005s_config af9015_mxl5003_config = {

	
--

From: Jiri Slaby
Date: Wednesday, October 27, 2010 - 9:36 am

Yes, this one works. And to be sure that 16 doesn't work I unplugged and
replugged the device (I didn't last time). And boom, it works too. So

thanks,
-- 
js
--

From: Mauro Carvalho Chehab
Date: Wednesday, October 27, 2010 - 10:05 am

Ok, I'm adding the enclosing patch to the tree, adding a proper description
about the regression. 

I'll be adding it to the tree.

Thanks!
Mauro

---

commit 7655e594945289b418af39f6669fea4666a7b520
Author: Mauro Carvalho Chehab <mchehab@redhat.com>
Date:   Wed Oct 27 14:55:34 2010 -0200

    [media] af9015: Fix max I2C message size when used with tda18271
    
    Changeset 1724c8fa7eb33d68898e060a08a8e6a88348b62f added an option to change
    the maximum I2C size to 8 bytes. However, it forgot to replace the previous
    usage at af9015 to use the newly defined macro value
    (TDA18271_16_BYTE_CHUNK_INIT).
    
    A latter changeset (e350d44fed8eb86a7192a579e3687fcd76a4645b) extended the
    possible values for .small_i2c field and, instead of using a random sequence
    of numbers, it used a number that makes more sense (e. g. the actual limit,
    in terms of bytes).
    
    However, as af9015 were using .small_i2c = 1, this become undefined, and the
    restriction of a max size of 16 was gone.
    
    While here, fix the reported msg size at tda18271-common.c.
    
    Reported-by: Jiri Slaby <jirislaby@gmail.com>
    Tested-by: Jiri Slaby <jirislaby@gmail.com>
    Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/common/tuners/tda18271-common.c b/drivers/media/common/tuners/tda18271-common.c
index 195b30e..5466d47 100644
--- a/drivers/media/common/tuners/tda18271-common.c
+++ b/drivers/media/common/tuners/tda18271-common.c
@@ -237,7 +237,7 @@ int tda18271_write_regs(struct dvb_frontend *fe, int idx, int len)
 
 	if (ret != 1)
 		tda_err("ERROR: idx = 0x%x, len = %d, "
-			"i2c_transfer returned: %d\n", idx, len, ret);
+			"i2c_transfer returned: %d\n", idx, max, ret);
 
 	return (ret == 1 ? 0 : ret);
 }
diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index 3ef19a8..31c0a0e 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -1131,7 ...
From: Antti Palosaari
Date: Wednesday, October 27, 2010 - 9:41 am

All in all, this small_i2c was added (Michael Krufky and I) when this 
tuner was taken in use with af9015 and it was 16 bytes initially. I 
think those other chunks are added later.
AF9015 I2C adapter can write 21 bytes at once.
Correct solution is to add option which splits writes as wanted (like 
option .i2c_wr_max) to the TDA18271. I have no HW to test.

Antti
-- 
http://palosaari.fi/
--

From: Mauro Carvalho Chehab
Date: Wednesday, October 27, 2010 - 10:11 am

Changing to 21 won't change anything in practice, as it will still use 3 URB's for transferring data.
Only at setup, it needs to write values on more than 16 registers.

The current way that .small_i2c is defined is like that: it specifies the maximum amount of data
that can be transferred.

I won't object to change it from an enum into an integer, although I would
do it only if we need to touch on that code for another reason.

Cheers,
Mauro

--

Previous thread: [PATCH] Correct comment for ext[23] filesystem "ctime" field by Robert P. J. Day on Wednesday, October 20, 2010 - 1:26 pm. (1 message)

Next thread: [PATCH 1/6] Free up pf flag PF_KSOFTIRQD by Venkatesh Pallipadi on Wednesday, October 20, 2010 - 3:48 pm. (7 messages)