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@xxxxxxx> 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. 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. Thanks. Regards, Nicholas > > > thanks, > > Takashi > > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -767,6 +767,7 @@ static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid, > if (pin_idx < 0) > return; > mutex_lock(&spec->pcm_lock); > + get_pin(spec, pin_idx)->repoll_count = 1; > hdmi_present_sense(get_pin(spec, pin_idx), 1); > mutex_unlock(&spec->pcm_lock); > } > @@ -1647,7 +1648,10 @@ static void sync_eld_via_acomp(struct hda_codec *codec, > per_pin->dev_id, &eld->monitor_present, > eld->eld_buffer, ELD_MAX_SIZE); > eld->eld_valid = (eld->eld_size > 0); > - update_eld(codec, per_pin, eld, 0); > + if (per_pin->repoll_count) > + schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300)); > + else > + update_eld(codec, per_pin, eld, 0); > mutex_unlock(&per_pin->lock); > } > > @@ -1669,6 +1673,11 @@ static void hdmi_repoll_eld(struct work_struct *work) > struct hdmi_spec *spec = codec->spec; > struct hda_jack_tbl *jack; > > + if (codec_has_acomp(codec)) { > + per_pin->repoll_count = 0; > + goto check; > + } > + > jack = snd_hda_jack_tbl_get_mst(codec, per_pin->pin_nid, > per_pin->dev_id); > if (jack) > @@ -1677,6 +1686,7 @@ static void hdmi_repoll_eld(struct work_struct *work) > if (per_pin->repoll_count++ > 6) > per_pin->repoll_count = 0; > > + check: > mutex_lock(&spec->pcm_lock); > hdmi_present_sense(per_pin, per_pin->repoll_count); > mutex_unlock(&spec->pcm_lock);