From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755849Ab2EHO3o (ORCPT ); Tue, 8 May 2012 10:29:44 -0400 Received: from cantor2.suse.de ([195.135.220.15]:51603 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755671Ab2EHO3k (ORCPT ); Tue, 8 May 2012 10:29:40 -0400 Date: Tue, 08 May 2012 16:29:38 +0200 Message-ID: From: Takashi Iwai To: Konstantin Khlebnikov Cc: "alsa-devel@alsa-project.org" , "linux-kernel@vger.kernel.org" , David Henningsson , Jaroslav Kysela Subject: Re: [PATCH bugfix v3.4] ALSA: hda - fix "silence after S3" regression In-Reply-To: <4FA921E5.4080305@openvz.org> References: <4FA91A27.3030801@openvz.org> <20120508130810.17396.27499.stgit@zurg> <4FA921E5.4080305@openvz.org> User-Agent: Wanderlust/2.15.6 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.7 Emacs/23.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Tue, 08 May 2012 17:38:45 +0400, Konstantin Khlebnikov wrote: > > Takashi Iwai wrote: > > At Tue, 08 May 2012 17:08:10 +0400, > > Konstantin Khlebnikov wrote: > >> > >> This fixes regression in shd-hda-codec after 785f857d1cb0856b612b46a0545b74aa2 > >> ("ALSA: hda - Set codec to D3 forcibly even if not used") > >> > >> hda_set_power_state() call must be explicitly protected against recursive > >> snd_hda_power_up/down() calls, otherwise it accidentally switches device into > >> D0 right before suspend. As result after resume sound device stays in D3 state > >> and does not works because driver believes that the device is on already. > >> > >> Signed-off-by: Konstantin Khlebnikov > > > > Hrm, does this really fix your problem? The path is called only > > during the suspend. Then, in the resume path, the codec is all > > powered up anyway. > > This fixes my problem: during suspend hda_set_power_state() calls snd_hda_power_up() > it set codec->power_on = 1 and switches device into D0. At S3 resume codec->power_on == 1 > but all devices actually in D3 state. As result on resume path device stays in D3, > but after one power-save cycle all works again. Ah, thanks, that explains a lot. Then it'd be actually better to revert the commit instead of another hack to paper over the nasty reincarnation. The power-state change there shouldn't be involved with the whole power-up sequence. It should have been a simple "go-to-D3" sequence. But this would need the assurance that the controller side is ready, etc, and not too trivial to write with the current code base. So, the right fix would be either implementing a straight hda_set_power_state() code without any power-up sequence but only the sanity check of the controller side, or fixing the original problem of ALC269 differently, e.g. by always resuming immediately at the resume callback but immediately performing the power-saving work. In either way, it's too late for 3.4 kernel. And the original problem is no critical issue in comparison with this regression. So, a revert would be the best thing we can do for now, IMO. > > If the race between the leftover power-save work is really a culprit, > > the fix would be just calling cancel_delayed_work_sync() before > > that point... > > Race was always there, my problem was caused by commit which mentioned above. > > Whole this code is very racy, someone needs to fix it. Yeah, I know. I'll cook it up for 3.5 kernel. thanks, Takashi