On Sun, Jul 10, 2016 at 03:20:13PM +0200, Lukas Wunner wrote:
Hi Peter,
[12:42] Lekensteyn: Should the video card always be powered up when a register is read from the HDMI audio controller? Or would it be better to leave the video card suspended and just fail the HDA register reads?
It should probably be powered up.
Seems sensible, a video signal is apparently also required if you want to play audio.
[12:43] Lekensteyn: I'm trying to figure out how vga_switcheroo should
handle this, maybe l1k has an idea?[12:48] Lekensteyn: The current implemented policy is that the video card dictates the power state and that the HDMI audio device has to adapt (even if it is seemingly in use)
This current architecture seems to have come about historically (Dave Airlie first implemented vga_switcheroo with manual power control, then added runtime pm in a second step).
It may also be motivated by the fact that the driver core is currently not supporting dependencies between devices beyond the hierarchical parent/child relationship.
Rafael Wysocki (cc:) posted a series in January addressing the latter problem with so-called "device links". The series was reposted recently by Marek Szyprowski: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1170031.html
The vga_switcheroo audio handling should probably be reworked to use such "device links". The result would be that the GPU won't runtime suspend as long as a ref is held for the audio card. The audio card needs to then make sure that it releases any refs if it has nothing to do.
The "device links" series seems to impose more restrictions than we actually need here, it seems to require that the "supplier" is bound to a driver before the "consumer" may probe. IOW nouveau needs to be bound before snd_hda_audio can probe. I'm not sure if that additional (unnecessary) restriction is a problem.
The device links feature looks promising. My initial guess that it is OK to wait for nouveau to become available (as supplier), the audio port (as consumer) probably does not work anyway without a video signal.
I've recently tried to add runtime pm for muxed laptops to vga_switcheroo, this is something that Pierre Moreau (cc:) has expressed an interest in for his MacBook Pro. I came across a major roadblock in the form of vga_switcheroo_set_dynamic_switch(). That function is called from the DRM driver when the GPU runtime suspends and resumes. It takes the vgasr_mutex. The problem is, if the user initiates a switch of the mux, that mutex is already taken in vga_switcheroo_debugfs_write(). If the GPU we're switching to is asleep, it'll wake up for the switch and we'll get a deadlock when the DRM driver subsequently calls vga_switcheroo_set_dynamic_switch().
I would like to get rid of vga_switcheroo_set_dynamic_switch() to solve this. The function has two purposes: Number one, vga_switcheroo updates its internal power state representation for the GPU. That is actually pointless for driver power control because we can always query the driver core for this information (e.g. pm_runtime_suspended()). The second purpose is to switch the audio client on and off. If we would use a "device link" to express the dependency between the audio device and the GPU, we wouldn't need this. So moving to "device links" is a prerequisite to make runtime pm work for muxed laptops.
This internal state is likely a historical artifact due to the manual control (ON / OFF) that was needed in the past. I have recently tried to draw the dependencies on paper and the suspend/resume to dynamic switch flow is not the prettiest ;) Using runtime pm would probably make the dependencies clearer in a logical way.
If you want to take a stab at using "device links" for vga_switcheroo then please go ahead as I'm swamped with other work. (I've reverse- engineered retrieval of Apple device properties from EFI this week, which is needed for Thunderbolt.) Let me know if you have any questions or need stuff reviewed or whatever. Since the "device links" series hasn't landed yet, it's still possible I think to ask for feature requests should it not work for this particular use case. The linux-pm@vger.kernel.org mailing list is the right place to inquire about the series.
Thanks for raising this important question.
I'll give this a go after finishing the PR3 nouveau patches and fixing some locking issues.