[Nouveau] vga_switcheroo audio runpm

[Peter Wu]

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 at 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.
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl