Hi,

On 15-12-17 14:49, Jaroslav Kysela wrote:

Dne 15.12.2017 v 13:35 Hans de Goede napsal(a):

...

Hi All,

For Fedora 28 we plan to ship the kernel with snd_hda_intel.power_save defaulting to 1:

https://fedoraproject.org/wiki/Changes/ImprovedLaptopBatteryLife

We've had an update for F27 in updates-testing for a while which made the same change for F27, but we've received several bug reports about plops / click sounds on start / stop of playing audio:

https://bugzilla.redhat.com/show_bug.cgi?id=1525104

I know others have probably already tried fixing this, but I'm wondering if there are any ideas how to fix this / things to try?

Ideas of my own:

1. Mute the speakers / headphone output in the mixer before

activating power-saving. Question how hard would it be to make this happen?

2. Put a delay between various steps involved in turning

things off / on.

Hi,

I am a bit skeptic about those proposals. I think that the drivers should do this job, because some hardware variants have different requirements for the proper power-off/on sequence.

Ok, this were just some quick ideas.

The problem appears like an improper voltage change and the HDA driver has already a lot of code which is trying to eliminate these pops (but as you can see, it is not sufficient).

Honestly, I don't know if it's good to keep this "hidden" to users or enable the power save feature by default and let users to report this, so we can look into this problem more when the affected hardware is identified.

I'm trying to have power_save=1 as default for the upcoming Fedora 28, because the power-savings of this are quite significant.

So I agree with you that we should enable the feature by default and get users to file bugs (and make sure there is 1 bug per model, not a lot of me too comments) for machines where there still are issues.

But getting these issues fixed is going to take time, so I think we will also need a blacklist (with a comment pointing to the machine specific bug per entry) to at least temporarily disable the snd_hda_intel powersaving feature on affected machines.

My proposal is to use a new /lib/udev/hwdb.d/60-snd_hda_intel.hwdb file for this blacklist (I believe it is better to keep this out of the kernel), combined with a udev rule modifying the power_save setting based on hwdb blacklist entries.

The udev rule will need some way to modify the power_save option, AFAIK modifying module parameters from udev is not really easily doable, unless we invoke a shell as helper to do an actual echo which seems ugly (please correct me if I'm wrong).

My proposal for the udev role problem is to add a power_save sysfs attribute to the snd_hda_intel PCI device (or maybe to the alsa card sound class dev?), so e.g. have a /sys/bus/pci/devices/0000:00:1f.3/power_save file. This sysfs attribute will be a 2-way mirror of the module option, so that tools like TLP and power-top which write the module-option will keep working. This does mean that in the theoretical case of having 2 intel_hda devices changing the option on one will also change it on the other.

I plan to start working on implementing this sometime next week, or the week after that, depending on how my current project goes. In the mean time any feedback on my proposal how to handle this would be much appreciated.

Regards,

Hans