Re: [PATCH 3/4] ALSA: hda: Refactor controller PM to use direct-complete optimization - Kai-Heng Feng

From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: tiwai@suse.com, Jaroslav Kysela <perex@perex.cz>,
	Kai Vehmanen <kai.vehmanen@linux.intel.com>,
	Alex Deucher <alexander.deucher@amd.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	"moderated list:SOUND" <alsa-devel@alsa-project.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/4] ALSA: hda: Refactor controller PM to use direct-complete optimization
Date: Fri, 23 Oct 2020 21:02:35 +0800	[thread overview]
Message-ID: <98CFD5BD-3BA0-4A7A-8C24-D6004F019CDF@canonical.com> (raw)
In-Reply-To: <s5hh7ql898j.wl-tiwai@suse.de>

> On Oct 23, 2020, at 19:36, Takashi Iwai <tiwai@suse.de> wrote:
> 
> On Fri, 23 Oct 2020 12:23:37 +0200,
> Kai-Heng Feng wrote:
>> @@ -1103,10 +1096,8 @@ static int azx_runtime_suspend(struct device *dev)
>> 	chip = card->private_data;
>> 
>> 	/* enable controller wake up event */
>> -	if (snd_power_get_state(card) == SNDRV_CTL_POWER_D0) {
>> -		azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) |
>> -			   STATESTS_INT_MASK);
>> -	}
>> +	azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) |
>> +		   STATESTS_INT_MASK);
> 
> Hrm, this doesn't look safe.  Applying WAKEEN unconditionally means
> that the machine may get woken up from the system suspend, and we
> don't want that.

Yes, WAKEEN should be enabled for runtime suspend and disabled for system suspend.
In principle we should always do runtime-resume -> suspend flow when runtime and system PM requires different wakeup settings.

That also means HDA controllers can't use direct-complete at all.

However, I did some testing on keeping WAKEEN enabled for graphics card's audio controller, and they didn't wake system up.
But yes, in principle they are not safe, I'll change it in v2.

Kai-Heng

> 
> 
> thanks,
> 
> Takashi