Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 30 Apr 2020 19:49:03 +0200
Message-ID: <s5h1ro4kgog.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
Cc:     Alex Deucher <alexdeucher@gmail.com>,
        "Zhou, David(ChunMing)" <David1.Zhou@amd.com>,
        "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
        Takashi Iwai <tiwai@suse.com>, Lukas Wunner <lukas@wunner.de>,
        "Deucher, Alexander" <Alexander.Deucher@amd.com>,
        "Koenig, Christian" <Christian.Koenig@amd.com>
Subject: Re: [PATCH 0/1] Fiji GPU audio register timeout when in BACO state
In-Reply-To: <PSXP216MB04381A30909F66867E6B6BCC80AA0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM>
References: <s5hv9lkm49n.wl-tiwai@suse.de>
        <PSXP216MB043899DC52E6C6BF728D77CD80AC0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM>
        <s5ha72ulp2y.wl-tiwai@suse.de>
        <PSXP216MB043822350CDE9E7EEA37730880AD0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM>
        <CADnq5_MCQ7xHY=yhNtRW=ze0LRPzxuu-Mm7pD4kFa5R52UrGSw@mail.gmail.com>
        <s5h1ro6jn0v.wl-tiwai@suse.de>
        <CADnq5_Mjb_FnNOzjUfJZ7GSDzi-+Cfc1ZTuqm7UWCWVvY6DU_w@mail.gmail.com>
        <s5hwo5xj98v.wl-tiwai@suse.de>
        <PSXP216MB0438FE3E1CA577805BEC23C880AA0@PSXP216MB0438.KORP216.PROD.OUTLOOK.COM>
        <s5hh7x0kiwb.wl-tiwai@suse.de>
        <PSXP216MB04381A30909F66867E6B6BCC80AA0@PSXP216MB0438.KORP216.PROD.OUTLOOK.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")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 30 Apr 2020 19:38:16 +0200,
Nicholas Johnson wrote:
> 
> On Thu, Apr 30, 2020 at 07:01:08PM +0200, Takashi Iwai wrote:
> > On Thu, 30 Apr 2020 18:52:20 +0200,
> > Nicholas Johnson wrote:
> > > 
> > > On Thu, Apr 30, 2020 at 05:14:56PM +0200, Takashi Iwai wrote:
> > > > On Wed, 29 Apr 2020 18:19:57 +0200,
> > > > Alex Deucher wrote:
> > > > > 
> > > > > On Wed, Apr 29, 2020 at 12:05 PM Takashi Iwai <tiwai@suse.de> wrote:
> > > > > > Well, but the code path there is the runtime PM resume of the audio
> > > > > > device and it means that GPU must have been runtime-resumed again
> > > > > > beforehand via the device link.  So, it should have worked from the
> > > > > > beginning but in reality not -- that is, apparently some inconsistency
> > > > > > is found in the initial attempt of the runtime resume...
> > > > > 
> > > > > Yeah, it should be covered, but I wonder if there is something in the
> > > > > ELD update sequence that needs to call pm_runtime_get_sync()?  The ELD
> > > > > sequence on AMD GPUs doesn't work the same as on other vendors.  The
> > > > > GPU driver has a backdoor into the HDA device's verbs to set update
> > > > > the audio state rather than doing it via an ELD buffer update.  We
> > > > > still update the ELD buffer for consistency.  Maybe when the GPU
> > > > > driver sets the audio state at monitor detection time that triggers an
> > > > > interrupt or something on the HDA side which races with the CPU and
> > > > > the power down of the GPU.  That still seems unlikely though since the
> > > > > runtime pm on the GPU side defaults to a 5 second suspend timer.
> > > > 
> > > > I'm not sure whether it's the race between runtime suspend of GPU vs
> > > > runtime resume of audio.  My wild guess is rather that it's the timing
> > > > GPU notifies to the audio; then the audio driver notifies to
> > > > user-space and user-space opens the stream, which in turn invokes the
> > > > runtime resume of GPU. But in GPU side, it's still under processing,
> > > > so it proceeds before the GPU finishes its initialization job.
> > > > 
> > > > Nicholas, could you try the patch below and see whether the problem
> > > > still appears?  The patch artificially delays the notification and ELD
> > > > update for 300msec.  If this works, it means the timing problem.
> > > The bug still occurred after applying the patch.
> > > 
> > > But you were absolutely correct - it just needed to be increased to 
> > > 3000ms - then the bug stopped.
> > 
> > Interesting.  3 seconds are too long, but I guess 1 second would work
> > as well?
> 1000ms indeed worked as well.
> 
> > 
> > In anyway, the success with a long delay means that the sound setup
> > after the full runtime resume of GPU seems working.
> > 
> > > Now the question is, what do we do now that we know this?
> > > 
> > > Also, are you still interested in the contents of the ELD# files? I can 
> > > dump them all into a file at some specific moment in time which you 
> > > request, if needed.
> > 
> > Yes, please take the snapshot before plugging, right after plugging
> > and right after enabling.  I'm not sure whether your monitor supports
> > the audio, and ELD contents should show that, at least.
> The monitor supports the audio. There is 3.5mm audio out jack. No 
> inbuilt speakers, although Samsung did sell a sound bar to suit it. The 
> sound bar, which I do not own, presumably attaches via 3.5mm jack.
> 
> I am not sure if by plugging, you mean hot-adding Thunderbolt GPU or 
> plugging the monitor to the GPU, so I have covered extra cases to be 
> sure. I have taken the eld# files with the 1000ms patch applied, so the 
> error is not triggered.

OK, thanks.  If I understand correctly...

> ####
> Before hot-adding the Thunderbolt GPU:
> /proc/asound/card1 not present
> ####
> ####
> After hot-adding the GPU with no monitor attached:
> 
> /proc/asound/card1 contains:
> eld#0.0  eld#0.1  eld#0.2  eld#0.3  eld#0.4  eld#0.5
> 
> All of the above have the same contents:
> 
> monitor_present         0
> eld_valid               0
> ####
> ####
> Monitor attached to Fiji GPU but not enabled:
> 
> Same as above
> ####
> ####
> Monitor enabled:

... the error is triggered at this moment, right?


> All files with same contents except for eld#0.1 which looks like:
> 
> monitor_present         1
> eld_valid               1
> monitor_name            U32E850
> connection_type         DisplayPort
> eld_version             [0x2] CEA-861D or below
> edid_version            [0x3] CEA-861-B, C or D
> manufacture_id          0x2d4c
> product_id              0xce3
> port_id                 0x0
> support_hdcp            0
> support_ai              0
> audio_sync_delay        0
> speakers                [0x1] FL/FR
> sad_count               1
> sad0_coding_type        [0x1] LPCM
> sad0_channels           2
> sad0_rates              [0xe0] 32000 44100 48000
> sad0_bits               [0xe0000] 16 20 24

So your monitor supports the audio :)


Takashi