On 20/12/2022 11:00, Charles Keepax wrote:
On Mon, Dec 19, 2022 at 04:20:10PM +0100, Emanuele Ghidoli wrote:
On 19/12/2022 10:58, Charles Keepax wrote:
On Sat, Dec 17, 2022 at 12:47:14AM +0100, Emanuele Ghidoli wrote:
I infer, from datasheet, that volume update is applied in different way based on charge pump dynamic vs register control (CP_DYN_PWR bit in CLASS_W register): "Under Register control, the HPOUTL_VOL, HPOUTR_VOL, LINEOUTL_VOL and LINEOUTR_VOL register settings are used to control the charge pump mode of operation. Under Dynamic control, the audio signal level in the DAC is used to control the charge pump mode of operation."
The second sentence do not explain that volume register is still considered by the component but likely in a different way.
It is important to note that I trace I2C transactions and, without the patch, the CLASS_W register is written JUST after volume update registers (with the patch is written before and after).
At this point I have no doubt that we have to update that register before writing volume.
Hmm... I think my only concern here is this feels a bit counter intuitive, the default value is described as "controlled by volume register settings" and we are saying in that situation the volume registers don't seem to update properly. That is far from impossible but I think we should perhaps poke a little more to make sure we understand the bounds here.
I did some more test and I'm not 100% sure of what is going on anymore.
I see that that CP_DYN_PWR bit is disabled when audio is going through one of the bypass paths. Would you be able to enable one of the bypass paths and see if you can manually adjust the volume on the headphone output, with a bypass path active?
With the previous change, I tested all the possible combination with one channel from the DAC and the other toggling from DAC to Bypass changing the volume and it's always correct.
Would also perhaps be interesting as a test to completely remove the write to CP_DYN_PWR from probe and leave things set to manual and see how the volume behaves then?
When I tried to remove any write to this register my modification stopped working.
I guess the interests here are to find out if the SYSCLK is involved at all.
I tested keep the clock always enabled, removing clk_disable_unprepare when going into SND_SOC_BIAS_OFF and it has zero effects. Or did you mean something else?
Said all of that, I did one last test, forcing a volume update on the charge pump enable callback, cp_event(), with this and only this modification in everything is working fine.
Could it just be as easy as that the volume is applied only when the charge pump is already active?
From the datasheet this seems a good explanation:
The Charge Pump is enabled by setting the CP_ENA bit. When enabled, the charge pump adjusts the output voltages (CPVOUTP and CPVOUTN).
What do you think?
Emanuele