Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 05 Jan 2023 09:55:03 +0100
Message-ID: <877cy1mkd4.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Ben Carter <craterrender@gmail.com>
Cc:     Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
        Cezary Rojewski <cezary.rojewski@intel.com>,
        Amadeusz =?ISO-8859-2?Q?S=B3awi=F1s?= =?ISO-8859-2?Q?ki?= 
        <amadeuszx.slawinski@linux.intel.com>,
        Mark Brown <broonie@kernel.org>,
        Divya Prakash <divya1.prakash@intel.com>,
        alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sound: hda: increase timeouts to stop crash on resume with ALC3204 and others
In-Reply-To: <20230104221024.23524-1-craterrender@gmail.com>
References: <20230104221024.23524-1-craterrender@gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Wed, 04 Jan 2023 23:10:24 +0100,
Ben Carter wrote:
> 
> Upon waking after system suspend, ALSA often crashed with:
>   snd_hda_intel 0000:00:1f.3: CORB reset timeout#2, CORBRP = 65535
>   snd_hda_codec_hdmi hdaudioC1D2: Unable to sync register 0x2f8100. -5
>   snd_hda_codec_realtek hdaudioC1D0: Unable to sync register 0x2b8000. -5
> A temporary fix was established by reloading snd_hda_intel, but increasing
> the rather strict timeout of 1ms to 100ms has remedied the issue on my
> device. Although this is a much larger delay, most hardware took less than
> 1ms anyway and it's preferable to the whole audio system crashing.

100ms is way too long for an atomic context.  How long it took in
reality?

If we extend to 100ms, the loop should be outside the spinlock.
Maybe it's worth to rewrite with the standard iopoll helper, too.


thanks,

Takashi

> 
> Signed-off-by: Ben Carter <craterrender@gmail.com>
> ---
>  sound/hda/hdac_controller.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
> index 3c7af6558249..1ab573248452 100644
> --- a/sound/hda/hdac_controller.c
> +++ b/sound/hda/hdac_controller.c
> @@ -16,7 +16,7 @@ static void azx_clear_corbrp(struct hdac_bus *bus)
>  {
>  	int timeout;
>  
> -	for (timeout = 1000; timeout > 0; timeout--) {
> +	for (timeout = 100000; timeout > 0; timeout--) {
>  		if (snd_hdac_chip_readw(bus, CORBRP) & AZX_CORBRP_RST)
>  			break;
>  		udelay(1);
> @@ -26,7 +26,7 @@ static void azx_clear_corbrp(struct hdac_bus *bus)
>  			snd_hdac_chip_readw(bus, CORBRP));
>  
>  	snd_hdac_chip_writew(bus, CORBRP, 0);
> -	for (timeout = 1000; timeout > 0; timeout--) {
> +	for (timeout = 100000; timeout > 0; timeout--) {
>  		if (snd_hdac_chip_readw(bus, CORBRP) == 0)
>  			break;
>  		udelay(1);
> -- 
> 2.39.0
>