[alsa-devel] [PATCH v2] ALSA: hda - Enable runtime PM only for discrete GPU

[Takashi Iwai]

On Thu, 13 Sep 2018 15:58:13 +0200,
Lukas Wunner wrote:
> 
> On Thu, Sep 13, 2018 at 03:35:36PM +0200, Takashi Iwai wrote:
> > On Thu, 13 Sep 2018 15:27:16 +0200, Jian-Hong Pan wrote:
> > > 2018-09-13 14:56 GMT+08:00 Takashi Iwai <tiwai at suse.de>:
> > > > The recent change of vga_switcheroo allowed the runtime PM for
> > > > HD-audio on AMD GPUs, but this also resulted in a regression.  When
> > > > the HD-audio controller driver gets runtime-suspended, HD-audio link
> > > > is turned off, and the hotplug notification is ignored.  This leads to
> > > > the inconsistent audio state (the connection isn't notified and ELD is
> > > > ignored).
> > > >
> > > > The best fix would be to implement the proper ELD notification via the
> > > > audio component, but it's still not ready.  As a quick workaround,
> > > > this patch adds the check of runtime_idle and allows the runtime
> > > > suspend only when the vga_switcheroo is bound with discrete GPU.
> > > > That is, a system with a single GPU and APU would be again without
> > > > runtime PM to keep the HD-audio link for the hotplug notification and
> > > > ELD read out.
> > > >
> > > > Also, the codec->auto_runtime_pm flag is set only for the discrete GPU
> > > > at the time GPU gets bound via vga_switcheroo (i.e. only dGPU is
> > > > forcibly runtime-PM enabled), so that APU can still get the ELD
> > > > notification.
> > > >
> > > > For identifying which GPU is bound, a new vga_switcheroo client
> > > > callback, gpu_bound, is implemented.  The vga_switcheroo simply calls
> > > > this when GPU is bound, and tells whether it's dGPU or APU.
> > > 
> > > Tested-by: Jian-Hong Pan <jian-hong at endlessm.com>
> > 
> > Thanks.  I should put you also as the original reporter.
> > 
> > Lukas, could you check the vga_switcheroo change whether it's
> > acceptable?
> 
> I looked over it this morning and didn't get every little detail,
> e.g. why chip->running = 0 in azx_free() (seems like an unrelated
> change, but I'm not sure).

It's to avoid the possible call of set_default_power_save() in
azx_vs_gpu_bound() call, i.e. when a GPU is bound during the audio
driver is being deregistered.  It matters just theoretically.

> But I do understand the idea underlying
> the patch and it LGTM.  It's somewhat regrettable that we have to
> make things more complicated again to properly differentiate between
> integrated and discrete GPUs.  I wish we could later on come up with
> a scheme that is lightweight and easy to follow.  Obviously, such a
> scheme will likely not be eligible as a fix for 4.19.  Anyway,
> 
> Acked-by: Lukas Wunner <lukas at wunner.de>
> 
> and thanks a lot for looking into it.  Please cc dri-devel for
> vga_switcheroo-related patches in the future (now added, + Alex,
> links for context:
> https://patchwork.kernel.org/patch/10598735/
> https://bugzilla.kernel.org/show_bug.cgi?id=200945 ).

Oh right, I seem to have forgotten Cc, sorry for that.


thanks,

Takashi