Ticket #1735 (closed defect: fixed)

Opened 7 years ago

Last modified 5 years ago

Audio clicking on resume.

Reported by: jg Owned by: wad
Priority: blocker Milestone: Trial-2
Component: kernel Version: Build 406.14
Keywords: power Cc: jayakumar, cjb, wad, wmb@…
Action Needed: Verified: no
Deployments affected: Blocked By:
Blocking:

Description (last modified by jg) (diff)

When resuming, I'm still getting a minor click from the speakers.

The device driver is still leaving the mic light on at this time too.

We need to ensure the hardware is working correctly; this is a B3, and I thought that the hardware click was fixed.

We have this system set up to cause the power button to suspend/resume, rather than shutdown/power up for ease of testing. You can hear a click on resume a half second or so after pressing the power button to resume, and cjb has seen the same when using manual suspend/resume operations.

Attachments

patch.txt (502 bytes) - added by jayakumar 7 years ago.
Add scap for power save.

Change History

  Changed 7 years ago by jg

  • description modified (diff)

  Changed 7 years ago by wmb@…

The fact that the click occurs about a half second after the button is pressed suggests that the problem is not the same hardware issue that we solved before. It is likely to be a driver issue.

On my B3, I can auto-suspend/resume (using RTC to wakeup) with no sound from the speakers.

However, when I turn on the audio driver in preparation for playing test sounds, there is a slight turn-on click. In my experience with other computer audio systems, eliminating such clicks often requires some careful software work to ensure that the DAC output is at 0V prior to turning on the amplifier.

  Changed 7 years ago by wmb@…

In the preceding comment, the "no sound" suspend/resume test was done from OFW, so the Linux driver was not involved. The audio system was turned off at the time.

It may not be possible to do silent suspend/resume if the audio system is active at the time, because the analog voltage levels would generally not be 0 at the instant of the suspend.

follow-up: ↓ 5   Changed 7 years ago by cjb

The clicking doesn't happen in OFW, so this indeed looks like a driver bug.

in reply to: ↑ 4   Changed 7 years ago by jayakumar

Two quick questions. Does the build mentioned above have SND_AC97_POWER_SAVE turned on and does it boot with kernel arguments power_save=1 or options snd-ac97-codec power_save=1 ? Thanks.

follow-up: ↓ 7   Changed 7 years ago by dilinger

FYI, I inverted the power_save default in the kernel; so, we don't need to boot with power_save=1, as it is already the default for olpc-2.6

Jaya, is there any reason why this isn't the case upstream?

in reply to: ↑ 6 ; follow-up: ↓ 8   Changed 7 years ago by jayakumar

Replying to dilinger:

FYI, I inverted the power_save default in the kernel; so, we don't need to boot with power_save=1, as it is already the default for olpc-2.6 Jaya, is there any reason why this isn't the case upstream?

Ah, I see. I believe that if power_save was enabled by default in mainline, then platforms that have hardware induced pop-click issues would then pop-click each time the workqueue kicked in to suspend/resume the ac97 codec.

in reply to: ↑ 7 ; follow-up: ↓ 9   Changed 7 years ago by cjb

Hi Jaya,

So, we're still seeing several problems with the driver on the current hardware:

* we get a pop on resume, as above, even when power_save is =1. * the mic is being powered even when the device hasn't been opened by anything, as in #1420

Any ideas?

Changed 7 years ago by jayakumar

Add scap for power save.

in reply to: ↑ 8 ; follow-up: ↓ 10   Changed 7 years ago by jayakumar

Hi cjb,

I think I see the cause, 2.6.2x added a need for AC97_SCAP_POWER_SAVE. The attached patch should fix the mic LED staying on.

For the pop/click on resume, I guess we will need to take a look at the ac97 resume sequence in alsa ac97 core. Feel free to give it a go.

Thanks, jaya

in reply to: ↑ 9 ; follow-up: ↓ 17   Changed 7 years ago by cjb

Replying to jayakumar:

Hi cjb, I think I see the cause, 2.6.2x added a need for AC97_SCAP_POWER_SAVE. The attached patch should fix the mic LED staying on.

We're getting closer -- with your patch the mic light stays off until the device is opened, but:

* boot: no lights are on
* start video activity: mic and camera lights go on
* close video activity: camera light goes off, mic light stays on

So we're not powering down when the device is closed.

  Changed 7 years ago by kimquirk

  • milestone changed from Trial-2 to BTest-4

Would like to fix this on the B4 boards.

  Changed 7 years ago by cjb

I tested whether adding AC97_SCAP_POWER_SAVE stops the clicking noise on resume -- it doesn't, because the mic is powered up briefly (the LED flashes on and then off again) at resume.

  Changed 7 years ago by wmb@…

a) When is the mic powered up "at resume"? Immediately after the resume signal, or a fraction of a second thereafter?

b) What rev board? B3?

c) Can you isolate the software component that is running when the MIC is powered on, perhaps by hacking different parts of the resume code so that they hang or print a message and wait a second, or something?

  Changed 7 years ago by jg

a) A half second later, maybe, there is a small "click" or "pop". The microphone light is a different issue. b) B3 c) whether the microphone power being on when it shouldn't is related to the clicking is unclear.

follow-up: ↓ 18   Changed 7 years ago by jg

  • milestone changed from BTest-4 to BTest-2

  Changed 7 years ago by jg

  • milestone changed from BTest-2 to Trial-2

in reply to: ↑ 10   Changed 7 years ago by jayakumar

Replying to cjb:

We're getting closer -- with your patch the mic light stays off until the device is opened, but: {{{ * boot: no lights are on * start video activity: mic and camera lights go on * close video activity: camera light goes off, mic light stays on }}} So we're not powering down when the device is closed.

I see. There is a shared workqueue that is used by ac97 to suspend the codec after the pcm is closed. It may not be getting kicked off for some reason after the video activity closes. If you have a chance, could you try with aplay test.wav instead of the video activity. I'm traveling till the 25th, so feel free to hack on the code.

in reply to: ↑ 15   Changed 7 years ago by jayakumar

Replying to jg:

whether the microphone power being on when it shouldn't is related to the clicking is unclear.

Yes, you are correct. There is no direct dependency between the two. Basically, the AC97 power saving method is to fully turn off the codec whenever the device is idle for x seconds, and then turn it back on when any process accesses audio. That means that that the mic LED which is tied to VRef also goes on and off each time that happens, as does the pop/click that you hear which is coming from ac97 resume. I recognize 2 bugs here. 1 is the pop/click on resume which is a bug in our ac97 resume code. 2 is that we currently default to V_Ref bias being enabled whereas on OLPC it ought to default to being off until we detect a jack insertion.

follow-up: ↓ 20   Changed 7 years ago by wmb@…

The MIC bias should not be tied to jack insertion because the built-in internal MIC is present when nothing is inserted in the jack.

The MIC bias should be on when audio input is active.

in reply to: ↑ 19   Changed 7 years ago by jayakumar

Replying to wmb@firmworks.com:

The MIC bias should not be tied to jack insertion because the built-in internal MIC is present when nothing is inserted in the jack. The MIC bias should be on when audio input is active.

Oops. Yes, you're right. I forgot about the internal mic. Ok, so the one definite bug is pop/click from ac97 resume. The other is the observation cjb made about the led staying on after the video activity is closed. I'm hoping someone can test with aplay on a build with the above patch and see if aplay also causes the same behavior.

  Changed 7 years ago by cjb

Hi Jaya,

Yes, the LED/power is staying on after aplay finishes as well.

  Changed 7 years ago by cjb

Mitch suggested tracking down the popping sound by instrumenting the driver with printk() and sleeps, which I did. The pop happens exactly at:

snd_ac97_tune_hardware(cs5535au->ac97, ac97_quirks, ac97_quirk);

  Changed 7 years ago by jayakumar

Interesting. ac97_quirks is just null assuming no module options like ac97_quirk=inv_eapd are applied. So in theory that should hit if (!quirk) return -EINVAL so it's odd that there's a pop at that time.

  Changed 7 years ago by cjb

Sorry, my mistake; the pop is at mixer setup. The line in mixer setup that causes the pop seems to be:

/* nothing should be in powerdown mode */ snd_ac97_write_cache(ac97, AC97_GENERAL_PURPOSE, 0);

Does that make sense?

  Changed 7 years ago by cjb

Wrong again. Make that:

                end_time = jiffies + (HZ / 10);
                do {
                        if ((snd_ac97_read(ac97, AC97_POWERDOWN) & 0x0f) == 0x0f)
                                goto __ready_ok;
                        schedule_timeout_uninterruptible(1);
                } while (time_after_eq(end_time, jiffies));
                snd_printk(KERN_WARNING "AC'97 %d analog subsections not ready\n", ac97->num);
        }

I'll keep narrowing.

  Changed 7 years ago by cjb

And now I'm chasing somewhere different again. I decided to stay late rather than rush to finish before going home, so I'll take my time and give you a concrete explanation of where in the mixer setup it's happening when I have one. Thanks, Jaya!

  Changed 7 years ago by cjb

Heh, so it was in the obvious place:

                if (ac97->scaps & AC97_SCAP_INV_EAPD)
                        set_inv_eapd(ac97, kctl);

So, the question is: why is

        snd_ac97_update_bits(ac97, AC97_POWERDOWN, (1<<15), (1<<15)); /* EAPD up */

generating a pop, and what can we do about it?

  Changed 7 years ago by cjb

So, to summarize what I learnt tonight:

We turn the amp on at audio driver load time (which is called from udev at boot), when the driver's mixer setup notices that we requested EAPD as a module parameter. If it doesn't happen then, it instead happens when ALSA creates the amp mixer control that's tied to EAPD and loads our preference for it. When the amp comes on we get a pop, even on B3, for reasons unknown.

(If we can't fix the pop, we should at least tie the asserting of EAPD to device open/close rather than mixer load/unload. I think the plan was to just fix it, though..)

  Changed 7 years ago by wmb@…

One way to reduce pops is to send some number of 0 samples to the DAC prior to turning on the amp. In some circuits that isn't all that you have to do, but conversely, if the DAC is putting out a nonzero voltage when you turn on the amp, you almost certainly will get a pop.

  Changed 7 years ago by wad

Mitch, are you suggesting 0 or 0x8000 ? Right now the codec outputs 1.5V when not driven, and is outputting 1.5V when the amp is powered on.

  Changed 7 years ago by cjb

Mitch gave me the beginner's introduction to our amp on IRC, in response to some basic questions: ===

I would be tempted to scope the waveforms on pins 4 and 5 of U44 (the amp) at the instant of amp power-on.

The voltage at the CODEC output is going to be about 1.5V with no audio signal, because the CODEC power supply is from 0 to 3.3V. In order for the audio signal to swing both positive and negative, it has to be biased somewhere near the middle of the power supply range.

In the steady-state case, the DC voltages on either side of the coupling capacitors C362 and C363 will equalize to some nominal value, but when the amp is powered off, the input bias voltage may be 0, so there would be a transient current through the capacitors when the input bias suddenly changes to something nonzero.

If we can determine the amp input bias voltage in the case where the amp is powered off, we might be able to minimize the pop by making the CODEC send out close to that value, then immediately after EAPD on, ramp the CODEC output to match the ramp on the amp input.

It will be complicated by the fact that the amp input is differential, so the ramp on C402 will need to be compensated.

The other fly in the ointment is that I have asked Quanta to reduce the values of those coupling capacitors by a substantial factor, in order to roll off the useless low-frequency signals presented to the speakers.

That will change the time constant of the pop waveform, and may, if we are lucky, reduce the pop energy in and of itself.

  Changed 7 years ago by wad

The amp bias voltage at power-on will be determined the next time I'm near a scope. Given that the amp is powered from +5V in our case, I'm betting on 2.5V or so.

The click did get better in B4 (which use the smaller coupling capacitors), favoring this theory.

  Changed 7 years ago by cjb

  • owner changed from dilinger to wad

Reassigning to wad, since if this is a hardware problem it needs to be fixed very soon.

  Changed 7 years ago by wad

  • status changed from new to closed
  • resolution set to fixed

Thanks to help from Analog Devices, the pop has been terminated.

The solution is to depopulate C510, C511, C512, and C513. These 100pF capacitors were present to prevent EMI, but also caused popping in the speakers. If EMI is a problem, we can replace R436, R437, R438, and R439 with ferrites.

The pop is still heard from the open firmware "test /audio" routine. This is due to a 30 mS delay between the deassertion of shutdown and the amplifier actually turning on. OFW starts playing a tone before this delay completes, causing an initial click. The OFW commands "amplifier-on" and "amplifier-off", as well as alsamixer, click no more.

Note: See TracTickets for help on using tickets.