[PATCH 0/3] ALSA: hda - Delay HDMI presence reports while waiting for ELD information

[PATCH 0/3] ALSA: hda - Delay HDMI presence reports while waiting for ELD information

[David Henningsson]

Hi Takashi,

I tested the patch you sent me last week on an Intel machine and it seems to
work. I don't have any full set of PulseAudio patches written yet that makes of
use of this, so I tested mainly with printk's to see that the eld was updated
before the jack was sent to userspace.

In addition, I added a workaround for the race condition that could happen
if you try to probe two monitors simultaneously.

For completeness I'm also sending the patch you sent me in this series. I think
it is good enough to go into 3.13.

[PATCH 1/3] ALSA: hda - Delay HDMI presence reports while waiting for ELD information

[David Henningsson]

From: Takashi Iwai <tiwai@suse.de>

---
 sound/pci/hda/patch_hdmi.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index e22323f..497e84d 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1140,7 +1140,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
  * Unsolicited events
  */
 
-static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
+static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
 
 static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 {
@@ -1166,8 +1166,8 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 	if (pin_idx < 0)
 		return;
 
-	hdmi_present_sense(get_pin(spec, pin_idx), 1);
-	snd_hda_jack_report_sync(codec);
+	if (hdmi_present_sense(get_pin(spec, pin_idx), 1))
+		snd_hda_jack_report_sync(codec);
 }
 
 static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
@@ -1475,7 +1475,7 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
 	return 0;
 }
 
-static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
+static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 {
 	struct hda_codec *codec = per_pin->codec;
 	struct hdmi_spec *spec = codec->spec;
@@ -1493,6 +1493,7 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 	int present = snd_hda_pin_sense(codec, pin_nid);
 	bool update_eld = false;
 	bool eld_changed = false;
+	bool ret;
 
 	mutex_lock(&per_pin->lock);
 	pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
@@ -1559,7 +1560,12 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 			       SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO,
 			       &per_pin->eld_ctl->id);
  unlock:
+	if ((codec->vendor_id & 0xffff0000) == 0x10020000)
+		ret = true; /* FIXME: implement fake ELD for AMD */
+	else
+		ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
 	mutex_unlock(&per_pin->lock);
+	return ret;
 }
 
 static void hdmi_repoll_eld(struct work_struct *work)
@@ -1570,7 +1576,8 @@ static void hdmi_repoll_eld(struct work_struct *work)
 	if (per_pin->repoll_count++ > 6)
 		per_pin->repoll_count = 0;
 
-	hdmi_present_sense(per_pin, per_pin->repoll_count);
+	if (hdmi_present_sense(per_pin, per_pin->repoll_count))
+		snd_hda_jack_report_sync(per_pin->codec);
 }
 
 static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
-- 
1.8.4.2

[PATCH 2/3] ALSA: hda - Add a block_report flag to jacks

[David Henningsson]

If the jack should not be reported to userspace (e g, because it is
in some transitional state), one can set this flag.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/hda_jack.c | 2 +-
 sound/pci/hda/hda_jack.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 05b3e3e..afe5944 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -286,7 +286,7 @@ void snd_hda_jack_report_sync(struct hda_codec *codec)
 	jack = codec->jacktbl.list;
 	for (i = 0; i < codec->jacktbl.used; i++, jack++)
 		if (jack->nid) {
-			if (!jack->kctl)
+			if (!jack->kctl || jack->block_report)
 				continue;
 			state = get_jack_plug_state(jack->pin_sense);
 			snd_kctl_jack_report(codec->bus->card, jack->kctl, state);
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index 379420c..46e1ea8 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -28,6 +28,7 @@ struct hda_jack_tbl {
 	unsigned int jack_detect:1;	/* capable of jack-detection? */
 	unsigned int jack_dirty:1;	/* needs to update? */
 	unsigned int phantom_jack:1;    /* a fixed, always present port? */
+	unsigned int block_report:1;    /* in a transitional state - do not report to userspace */
 	hda_nid_t gating_jack;		/* valid when gating jack plugged */
 	hda_nid_t gated_jack;		/* gated is dependent on this jack */
 	struct snd_kcontrol *kctl;	/* assigned kctl for jack-detection */
-- 
1.8.4.2

[PATCH 3/3] ALSA: hda - block HDMI jack reports while repolling

[David Henningsson]

This fixes a race condition in case several monitors are being
repolled in parallel.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_hdmi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 497e84d..29ca8de 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1477,6 +1477,7 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
 
 static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 {
+	struct hda_jack_tbl *jack;
 	struct hda_codec *codec = per_pin->codec;
 	struct hdmi_spec *spec = codec->spec;
 	struct hdmi_eld *eld = &spec->temp_eld;
@@ -1564,6 +1565,11 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 		ret = true; /* FIXME: implement fake ELD for AMD */
 	else
 		ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
+
+	jack = snd_hda_jack_tbl_get(codec, pin_nid);
+	if (jack)
+		jack->block_report = !ret;
+
 	mutex_unlock(&per_pin->lock);
 	return ret;
 }
-- 
1.8.4.2

Re: [PATCH 0/3] ALSA: hda - Delay HDMI presence reports while waiting for ELD information

[Takashi Iwai]

At Thu,  7 Nov 2013 13:38:22 +0100,
David Henningsson wrote:
> 
> Hi Takashi,
> 
> I tested the patch you sent me last week on an Intel machine and it seems to
> work. I don't have any full set of PulseAudio patches written yet that makes of
> use of this, so I tested mainly with printk's to see that the eld was updated
> before the jack was sent to userspace.
> 
> In addition, I added a workaround for the race condition that could happen
> if you try to probe two monitors simultaneously.
> 
> For completeness I'm also sending the patch you sent me in this series. I think
> it is good enough to go into 3.13.

OK, now merged these patches (the first one was with some comments).


thanks,

Takashi

Re: [PATCH 1/3] ALSA: hda - Delay HDMI presence reports while waiting for ELD information

[Anssi Hannula]

07.11.2013 14:38, David Henningsson kirjoitti:
> From: Takashi Iwai <tiwai@suse.de>
> 
> ---
>  sound/pci/hda/patch_hdmi.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index e22323f..497e84d 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1140,7 +1140,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
>   * Unsolicited events
>   */
>  
> -static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
> +static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
>  
>  static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
>  {
> @@ -1166,8 +1166,8 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
>  	if (pin_idx < 0)
>  		return;
>  
> -	hdmi_present_sense(get_pin(spec, pin_idx), 1);
> -	snd_hda_jack_report_sync(codec);
> +	if (hdmi_present_sense(get_pin(spec, pin_idx), 1))
> +		snd_hda_jack_report_sync(codec);
>  }
>  
>  static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
> @@ -1475,7 +1475,7 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
>  	return 0;
>  }
>  
> -static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> +static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
>  {
>  	struct hda_codec *codec = per_pin->codec;
>  	struct hdmi_spec *spec = codec->spec;
> @@ -1493,6 +1493,7 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
>  	int present = snd_hda_pin_sense(codec, pin_nid);
>  	bool update_eld = false;
>  	bool eld_changed = false;
> +	bool ret;
>  
>  	mutex_lock(&per_pin->lock);
>  	pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
> @@ -1559,7 +1560,12 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
>  			       SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO,
>  			       &per_pin->eld_ctl->id);
>   unlock:
> +	if ((codec->vendor_id & 0xffff0000) == 0x10020000)
> +		ret = true; /* FIXME: implement fake ELD for AMD */
> +	else
> +		ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;

Hmm, what is the reason for this special AMD handling?

AFAICS the AMD behavior should be the same as generic codecs from the
perspective of this code, i.e. the repolling mechanism, monitor_present
and eld_valid work the same.


>  	mutex_unlock(&per_pin->lock);
> +	return ret;
>  }
>  
>  static void hdmi_repoll_eld(struct work_struct *work)
> @@ -1570,7 +1576,8 @@ static void hdmi_repoll_eld(struct work_struct *work)
>  	if (per_pin->repoll_count++ > 6)
>  		per_pin->repoll_count = 0;
>  
> -	hdmi_present_sense(per_pin, per_pin->repoll_count);
> +	if (hdmi_present_sense(per_pin, per_pin->repoll_count))
> +		snd_hda_jack_report_sync(per_pin->codec);
>  }
>  
>  static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
> 


-- 
Anssi Hannula

Re: [PATCH 1/3] ALSA: hda - Delay HDMI presence reports while waiting for ELD information

[Takashi Iwai]

At Thu, 07 Nov 2013 17:25:35 +0200,
Anssi Hannula wrote:
> 
> 07.11.2013 14:38, David Henningsson kirjoitti:
> > From: Takashi Iwai <tiwai@suse.de>
> > 
> > ---
> >  sound/pci/hda/patch_hdmi.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> > index e22323f..497e84d 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -1140,7 +1140,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
> >   * Unsolicited events
> >   */
> >  
> > -static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
> > +static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
> >  
> >  static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
> >  {
> > @@ -1166,8 +1166,8 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
> >  	if (pin_idx < 0)
> >  		return;
> >  
> > -	hdmi_present_sense(get_pin(spec, pin_idx), 1);
> > -	snd_hda_jack_report_sync(codec);
> > +	if (hdmi_present_sense(get_pin(spec, pin_idx), 1))
> > +		snd_hda_jack_report_sync(codec);
> >  }
> >  
> >  static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
> > @@ -1475,7 +1475,7 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
> >  	return 0;
> >  }
> >  
> > -static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> > +static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> >  {
> >  	struct hda_codec *codec = per_pin->codec;
> >  	struct hdmi_spec *spec = codec->spec;
> > @@ -1493,6 +1493,7 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> >  	int present = snd_hda_pin_sense(codec, pin_nid);
> >  	bool update_eld = false;
> >  	bool eld_changed = false;
> > +	bool ret;
> >  
> >  	mutex_lock(&per_pin->lock);
> >  	pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
> > @@ -1559,7 +1560,12 @@ static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> >  			       SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO,
> >  			       &per_pin->eld_ctl->id);
> >   unlock:
> > +	if ((codec->vendor_id & 0xffff0000) == 0x10020000)
> > +		ret = true; /* FIXME: implement fake ELD for AMD */
> > +	else
> > +		ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
> 
> Hmm, what is the reason for this special AMD handling?
> 
> AFAICS the AMD behavior should be the same as generic codecs from the
> perspective of this code, i.e. the repolling mechanism, monitor_present
> and eld_valid work the same.

The patch was based on patch_hdmi.c before your patch, and eld_valid
hasn't been set correctly at that time.  But, looking back at the
latest code again, it seems that we can remove this check since
snd_hdmi_get_eld_ati() should return the correct ELD.

I'm going to apply the below.


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: hda - Get rid of AMD HDMI exception in hdmi_present_sense()

Since the recent fake ELD patches, we can remove the check for AMD
HDMI in hdmi_present_sense() and decide the return value from
eld_valid value.

Suggested by Anssi Hannula.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_hdmi.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index a96403a828af..e68792311bb2 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1561,10 +1561,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 			       SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO,
 			       &per_pin->eld_ctl->id);
  unlock:
-	if ((codec->vendor_id & 0xffff0000) == 0x10020000)
-		ret = true; /* AMD codecs create ELD by itself */
-	else
-		ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
+	ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
 
 	jack = snd_hda_jack_tbl_get(codec, pin_nid);
 	if (jack)
-- 
1.8.4.2