From patchwork Tue Feb 13 08:14:00 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 10215285 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2C45F6055C for ; Tue, 13 Feb 2018 08:14:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1CB1A28E67 for ; Tue, 13 Feb 2018 08:14:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 10B7528E69; Tue, 13 Feb 2018 08:14:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9103C28E67 for ; Tue, 13 Feb 2018 08:14:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 35FB16E129; Tue, 13 Feb 2018 08:14:03 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id B2FCA6E108 for ; Tue, 13 Feb 2018 08:14:01 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 2ABFFAAC8; Tue, 13 Feb 2018 08:14:00 +0000 (UTC) Date: Tue, 13 Feb 2018 09:14:00 +0100 Message-ID: From: Takashi Iwai To: "Saarinen, Jani" In-Reply-To: <43D4F724E12AB6478FC1572B3FBE89D075E034DB@IRSMSX106.ger.corp.intel.com> References: <20180212172953.3958-1-chris@chris-wilson.co.uk> <43D4F724E12AB6478FC1572B3FBE89D075E034DB@IRSMSX106.ger.corp.intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Subject: Re: [Intel-gfx] [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core codec functions to sync power state" X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "intel-gfx@lists.freedesktop.org" , "Kumar, Abhijeet" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 13 Feb 2018 08:34:40 +0100, Saarinen, Jani wrote: > > HI, > > -----Original Message----- > > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of > > Takashi Iwai > > Sent: tiistai 13. helmikuuta 2018 7.11 > > To: Chris Wilson > > Cc: intel-gfx@lists.freedesktop.org; Kumar, Abhijeet > > > > Subject: Re: [Intel-gfx] [PATCH] AWOOGA: Revert "ALSA: hda: Make use of core > > codec functions to sync power state" > > > > On Mon, 12 Feb 2018 18:29:53 +0100, > > Chris Wilson wrote: > > > > > > This reverts commit 3b5b899ca67db07a4c4825911072221f99e157e2. > > > > > > Fixes: 3b5b899ca67d ("ALSA: hda: Make use of core codec functions to > > > sync power state") > > > Cc: Abhijeet Kumar > > > Cc: Takashi Iwai > > > > Did the patch break anything? > > I don't understand it without any real context... > Yes. See resutls from link what is fixed by reverting: > https://patchwork.freedesktop.org/series/38097/ > Test pm_rpm: > Subgroup basic-pci-d3-state: > fail -> PASS (fi-hsw-4770) > fail -> PASS (fi-bdw-5557u) > Subgroup basic-rte: > fail -> PASS (fi-hsw-4770) > fail -> PASS (fi-bdw-5557u) Could you investigate why does the revert fix? Two functions are almost identical. The difference is that the new one has a counter and quit at 500 iteration, and it has a msleep(200) at the error exit. But both shouldn't matter for the normal operation... FWIW, below is the patch to make the new function identical with the original function. Does it change the behavior? thanks, Takashi --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -1079,15 +1079,13 @@ unsigned int snd_hdac_sync_power_state(struct hdac_device *codec, hda_nid_t nid, unsigned int power_state) { unsigned long end_time = jiffies + msecs_to_jiffies(500); - unsigned int state, actual_state, count; + unsigned int state, actual_state; - for (count = 0; count < 500; count++) { + for (;;) { state = snd_hdac_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0); - if (state & AC_PWRST_ERROR) { - msleep(20); + if (state & AC_PWRST_ERROR) break; - } actual_state = (state >> 4) & 0x0f; if (actual_state == power_state) break;