[alsa-devel] [PATCH 0/3] ALSA: hda - Delay HDMI presence reports while waiting for ELD information
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.
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,
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,
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)
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 */
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; }
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
participants (3)
-
Anssi Hannula -
David Henningsson -
Takashi Iwai