Re: [PATCH] ALSA: hda/hdmi - Don't report Jack event if no need to do that

[Takashi Iwai <tiwai@suse.de>]

On Tue, 30 Apr 2019 10:42:55 +0200,
Hui Wang wrote:
> 
> 
> On 2019/4/30 下午3:35, Takashi Iwai wrote:
> > On Tue, 30 Apr 2019 08:57:11 +0200,
> > Hui Wang wrote:
> >> On the machines with AMD GPU or Nvidia GPU, we often meet this issues:
> >> after s3, there are 4 HDMI/DP audio devices in the gnome-sound-setting
> >> even there is no any monitors plugged.
> >>
> >> When this problem happens, we check the /proc/asound/cardX/eld#N.M, we
> >> will find the monitor_present=1, eld_valid=0.
> >>
> >> The root cause is somehow the pin_sense reports the monitor is present
> >> and eld is valid when there is no monitor plugged.
> >>
> >> The current driver will read the eld data if the pin_sense reports the
> >> eld is valid, because of no monitor is plugged, there is no valid eld
> >> data, then the eld->valid is set to 0.
> >>
> >> If we don't let driver report Jack event when monitor_present=1 while
> >> eld_valid=0, there will be no this issue.
> >>
> >> After this change, the driver only reports Jack event with one of the
> >> below 2 conditons:
> >>   eld->monitor_present=1 and eld->eld_valid=1 (a valid monitor detect)
> >>   eld->monitor_present=0 (a monitor is unplugged)
> >>
> >> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> > Well, if the eld_valid=1 is mandatory, basically we can use it as the
> > condition of jack=1, like the patch below.  The return value from
> > hdmi_present_sense() indicates only whether we may sync jack state or
> > not, and it's not about the jack state itself.
> >
> >
> > thanks,
> >
> > Takashi
> >
> > ---
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -1625,7 +1625,7 @@ static void sync_eld_via_acomp(struct hda_codec *codec,
> >   	if (jack == NULL)
> >   		goto unlock;
> >   	snd_jack_report(jack,
> > -			eld->monitor_present ? SND_JACK_AVOUT : 0);
> > +			(eld->monitor_present && eld->eld_valid) ? SND_JACK_AVOUT : 0);
> >    unlock:
> >   	mutex_unlock(&per_pin->lock);
> >   }
> 
> All Intel machines which get eld via acomp don't have this issue, so
> no need to change this function.

Yes, but the HDMI audio won't work without the valid ELD no matter
whether it's reported via audio-component or verbs.  So this change
also corrects a corner case of Intel platforms, too.

> For those machines (with nv gpu card or amd gpu card)  which need to
> call hdmi_present_sense_via_verbs(), they have this issue. if
> hdmi_present_sense_via_verbs() returns true, the hdmi_present_sense()
> returns true, then snd_hda_jack_report_sync() will be called. So if we
> want to fix this issue, we need to let hdmi_present_sense_via_verbs()
> not return true or we change the read_pin_sense() in the hda_jack.c

The report_sync() doesn't report immediately; it updates the state
then notifies *only if* the state change happened.  That is, the root
cause is the false state change as if it were worth for reporting.

IOW, the state "monitor_detected=1 and eld_valid=0" can happen at any
time, not only at resume.  User may put a polling work, for example.
And, reporting this as the jack=1 causes the trouble, no matter
whether it's resume path or not.  So we rather need to fix this false
state reporting instead.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel