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

[Takashi Iwai]

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)) {
 			if (acomp->ops->get_power)
 				acomp->ops->get_power(acomp->dev);
 			snd_hdac_set_codec_wakeup(bus, true);
 			snd_hdac_set_codec_wakeup(bus, false);
-			bus->display_power_active = true;
 		}
 	} else {
-		if (bus->display_power_active) {
+		if (cmpxchg(&bus->display_power_active, true, false)) {
 			if (acomp->ops->put_power)
 				acomp->ops->put_power(acomp->dev);
-			bus->display_power_active = false;
 		}
 	}
 }