[Intel-gfx] [PATCH] snd/hda: Balance hda->need_i915_power across runtime_suspend

[Chris Wilson]

Quoting Takashi Iwai (2019-04-10 06:29:07)
> On Wed, 10 Apr 2019 00:53:31 +0200,
> Chris Wilson wrote:
> > 
> > Quoting Takashi Iwai (2019-04-09 22:35:28)
> > > On Tue, 09 Apr 2019 23:27:41 +0200,
> > > Chris Wilson wrote:
> > > > 
> > > > In runtime_resume, we release the local display_power wakeref if we can
> > > > rely on i915 providing a wakeref along the component. On suspend
> > > > therefore, we should only release the display_power if we kept it from
> > > > runtime_resume.
> > > 
> > > Hrm, it shouldn't matter.  After the recent code rewrite, the control
> > > isn't refcount any longer, hence it's fine to call
> > > display_power(false) again even if it were already powered off.
> > 
> > That is the puzzle. Have a look at the glk-dsi results,
> > https://patchwork.freedesktop.org/series/59253/
> > something does appear to go wrong in azx_probe_continue (and seems to be
> > avoided by this patch).
> > 
> > Perhaps something like https://patchwork.freedesktop.org/patch/297656/?series=59257&rev=1
> > if the pm_runtime_autosuspend is occurring from a workqueue at the same
> > time as we call display_power(false).
> 
> Then how about rather a patch like below?
> 
> 
> thanks,
> 
> Takashi
> 
> --- a/sound/hda/hdac_component.c
> +++ b/sound/hda/hdac_component.c
> @@ -78,18 +78,16 @@ void snd_hdac_display_power(struct hdac_bus *bus, unsigned int idx, bool enable)
>                 return;
>  
>         if (bus->display_power_status) {
> -               if (!bus->display_power_active) {
> +               if (!cmpxchg(&bus->display_power_active, false, true)) {

Except that display_power_active is the wakeref cookie returned by
get_power to be passed back to put_power.

It seems that the cmpxchg is happy so we can conclude this is a race
between display_power(false) and pm_runtime_suspend.
-Chris