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

[Takashi Iwai]

On Wed, 10 Apr 2019 09:59:19 +0200,
Chris Wilson wrote:
> 
> 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.

My patch is for 5.1.

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

I believe it worth to fix this in 5.1 at first.  Let me submit and
queue the fix for 5.1-rc5.


thanks,

Takashi