On Fri, Mar 15, 2024 at 06:36:19PM +0100, Alexandre Mergnat wrote:

On 15/03/2024 16:15, Mark Brown wrote:

...

On Fri, Mar 15, 2024 at 04:05:21PM +0100, Alexandre Mergnat wrote:

...

...

...

In the register. You only need to reset the gain to -40dB at the start of the ramp.

...

...

Sorry but I don't understand your logic, I'm not able to implement it... If I'm at -10dB and doing a ramp to reach -40dB, next time I will read the register the value will be -40dB.

...

After we've done the ramp and turned the amplifier off we can just restore the desired value? The hardware is not going to care what the volume is while it's not enabled.

If you do that, HP will be enabled at the saved gain, and after that you will do the ramp. To avoid pop, the driver should be rewrite to:

So reset the volume to -40dB prior to turning the amplifier on...

Read gain in the reg and save it locally Set -40dB in the reg Enable HP Do ramp

...as you yourself suggest?

...

...

AFAII from the comment in the code, it's done to avoid the "pop noises".

...

Yes, that's the usual reason to ramp gains. Though if you've just copied the code without checking that it's needed it's possible that this is something that's been fixed in current hardware.

I did the test at 24dB with and without the "pop filter". Isn't big but I ear the pop at the start of the record without the "pop filter".

OK, it probably is still doing something then.

To be clear, the algo/behavior of this code is an implementation based on the 6k+ lines downstream code for this specific audio codec. But the shape/style is based on upstreamed drivers like mt6358.c.

The Mediatek code has a bunch of issues, I wouldn't read too much into something being present in the code.