[Nouveau] vga_switcheroo audio runpm

[Daniel Vetter]

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.
> 
> 
> > [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 above (including the load ordering restriction, which we resolve using
EPROBE_DEFER) is how we hand-rolled a fix for this between i915 and
snd-hda-intel. Not for vga-switcheroo, but for rpm depencies between gpu
and snd in general. Not sure why exactly vga-switcheroo jumps into this
here ... but sounds like the device links stuff is the way to go.

> 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.

right, expressing the depencies explicitly (and managing it on the audio
side) would also solve this deadlock.

> 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 at vger.kernel.org mailing list is the right place to inquire
> about the series.
> 
> Thanks for raising this important question.

+1 from me on the overall plan.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch