[alsa-devel] [PATCH v1 1/4] ALSA: hda - Simplify hdmi_present_sense_via_verbs()
The jack report block, was added by commit 464837a7bc0a ("ALSA: hda - block HDMI jack reports while repolling"), to avoid race condition with repolling.
Signed-off-by: Nikhil Mahale nmahale@nvidia.com --- sound/pci/hda/patch_hdmi.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 48bddc218829..ee084676f625 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1569,7 +1569,6 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, * the unsolicited response to avoid custom WARs. */ int present; - bool ret; bool do_repoll = false;
present = snd_hda_jack_pin_sense(codec, pin_nid, dev_id); @@ -1603,16 +1602,15 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, else update_eld(codec, per_pin, eld);
- ret = !repoll || !eld->monitor_present || eld->eld_valid; - jack = snd_hda_jack_tbl_get_mst(codec, pin_nid, per_pin->dev_id); if (jack) { - jack->block_report = !ret; + jack->block_report = do_repoll; jack->pin_sense = (eld->monitor_present && eld->eld_valid) ? AC_PINSENSE_PRESENCE : 0; } mutex_unlock(&per_pin->lock); - return ret; + + return !do_repoll; }
static struct snd_jack *pin_idx_to_jack(struct hda_codec *codec,
Move snd_hda_jack_report_sync() call inside hdmi_present_sense_via_verbs(), and add parameter to control it. snd_hda_jack_report_sync() should get called only for !acomp drivers.
Signed-off-by: Nikhil Mahale nmahale@nvidia.com --- sound/pci/hda/patch_hdmi.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ee084676f625..de45f5e5c724 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -753,7 +753,9 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, * Unsolicited events */
-static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll); +static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, + int repoll, + bool jack_report_sync);
static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid, int dev_id) @@ -764,8 +766,8 @@ static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid, if (pin_idx < 0) return; mutex_lock(&spec->pcm_lock); - if (hdmi_present_sense(get_pin(spec, pin_idx), 1)) - snd_hda_jack_report_sync(codec); + hdmi_present_sense(get_pin(spec, pin_idx), + 1, true /* jack_report_sync */); mutex_unlock(&spec->pcm_lock); }
@@ -1551,8 +1553,9 @@ static bool update_eld(struct hda_codec *codec, }
/* update ELD and jack state via HD-audio verbs */ -static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, - int repoll) +static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, + int repoll, + bool jack_report_sync) { struct hda_jack_tbl *jack; struct hda_codec *codec = per_pin->codec; @@ -1608,9 +1611,11 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, jack->pin_sense = (eld->monitor_present && eld->eld_valid) ? AC_PINSENSE_PRESENCE : 0; } - mutex_unlock(&per_pin->lock);
- return !do_repoll; + if (!do_repoll && jack_report_sync) + snd_hda_jack_report_sync(codec); + + mutex_unlock(&per_pin->lock); }
static struct snd_jack *pin_idx_to_jack(struct hda_codec *codec, @@ -1686,26 +1691,25 @@ static void sync_eld_via_acomp(struct hda_codec *codec, mutex_unlock(&per_pin->lock); }
-static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) +static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, + int repoll, + bool jack_report_sync) { struct hda_codec *codec = per_pin->codec; - int ret;
/* no temporary power up/down needed for component notifier */ if (!codec_has_acomp(codec)) { + int ret; + ret = snd_hda_power_up_pm(codec); if (ret < 0 && pm_runtime_suspended(hda_codec_dev(codec))) { snd_hda_power_down_pm(codec); - return false; } - ret = hdmi_present_sense_via_verbs(per_pin, repoll); + hdmi_present_sense_via_verbs(per_pin, repoll, jack_report_sync); snd_hda_power_down_pm(codec); } else { sync_eld_via_acomp(codec, per_pin); - ret = false; /* don't call snd_hda_jack_report_sync() */ } - - return ret; }
static void hdmi_repoll_eld(struct work_struct *work) @@ -1725,8 +1729,9 @@ static void hdmi_repoll_eld(struct work_struct *work) per_pin->repoll_count = 0;
mutex_lock(&spec->pcm_lock); - if (hdmi_present_sense(per_pin, per_pin->repoll_count)) - snd_hda_jack_report_sync(per_pin->codec); + hdmi_present_sense(per_pin, + per_pin->repoll_count, + true /* jack_report_sync */); mutex_unlock(&spec->pcm_lock); }
@@ -2281,7 +2286,7 @@ static int generic_hdmi_build_controls(struct hda_codec *codec) for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
- hdmi_present_sense(per_pin, 0); + hdmi_present_sense(per_pin, 0, false /* jack_report_sync */); }
/* add channel maps */ @@ -2408,7 +2413,7 @@ static int generic_hdmi_resume(struct hda_codec *codec)
for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); - hdmi_present_sense(per_pin, 1); + hdmi_present_sense(per_pin, 1, false /* jack_report_sync */); } return 0; }
snd_jack_report() does nothing is old and new status are same.
Signed-off-by: Nikhil Mahale nmahale@nvidia.com --- sound/pci/hda/patch_hdmi.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index de45f5e5c724..1cf0604020bc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1485,7 +1485,7 @@ static void hdmi_pcm_reset_pin(struct hdmi_spec *spec, /* update per_pin ELD from the given new ELD; * setup info frame and notification accordingly */ -static bool update_eld(struct hda_codec *codec, +static void update_eld(struct hda_codec *codec, struct hdmi_spec_per_pin *per_pin, struct hdmi_eld *eld) { @@ -1549,7 +1549,6 @@ static bool update_eld(struct hda_codec *codec, SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO, &get_hdmi_pcm(spec, pcm_idx)->eld_ctl->id); - return eld_changed; }
/* update ELD and jack state via HD-audio verbs */ @@ -1654,7 +1653,6 @@ static void sync_eld_via_acomp(struct hda_codec *codec, struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld; struct snd_jack *jack = NULL; - bool changed; int size;
mutex_lock(&per_pin->lock); @@ -1681,10 +1679,10 @@ static void sync_eld_via_acomp(struct hda_codec *codec, * disconnected event. Jack must be fetched before update_eld() */ jack = pin_idx_to_jack(codec, per_pin); - changed = update_eld(codec, per_pin, eld); + update_eld(codec, per_pin, eld); if (jack == NULL) jack = pin_idx_to_jack(codec, per_pin); - if (changed && jack) + if (jack) snd_jack_report(jack, (eld->monitor_present && eld->eld_valid) ? SND_JACK_AVOUT : 0);
On Tue, 04 Feb 2020 08:20:16 +0100, Nikhil Mahale wrote:
snd_jack_report() does nothing is old and new status are same.
Actually it does but partially. The jack infrastructure provides two interfaces, the kctl element and the input device. The former reports only when the state is really changed while the latter reports always when snd_jack_report() is called.
thanks,
Takashi
If dyn_pcm_assign is set, different jack objects are being created for pcm and pins.
If dyn_pcm_assign is set, generic_hdmi_build_jack() calls into add_hdmi_jack_kctl() to create and track separate jack object for pcm. Like sync_eld_via_acomp(), hdmi_present_sense_via_verbs() also need to report status change of the pcm jack.
Rename pin_idx_to_jack() to pin_idx_to_pcm_jack(). Update hdmi_present_sense_via_verbs() to report plug state of pcm jack object. Unlike sync_eld_via_acomp(), for !acomp drivers the pcm jack's plug state must be consistent with plug state of pin's jack.
Fixes: 5398e94fb753 ALSA: hda - Add DP-MST support for NVIDIA codecs Signed-off-by: Nikhil Mahale nmahale@nvidia.com --- sound/pci/hda/patch_hdmi.c | 80 +++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 32 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 1cf0604020bc..6da87dad03ff 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1551,6 +1551,34 @@ static void update_eld(struct hda_codec *codec, &get_hdmi_pcm(spec, pcm_idx)->eld_ctl->id); }
+static struct snd_jack *pin_idx_to_pcm_jack(struct hda_codec *codec, + struct hdmi_spec_per_pin *per_pin) +{ + struct hdmi_spec *spec = codec->spec; + struct snd_jack *jack = NULL; + struct hda_jack_tbl *jack_tbl; + + /* if !dyn_pcm_assign, get jack from hda_jack_tbl + * in !dyn_pcm_assign case, spec->pcm_rec[].jack is not + * NULL even after snd_hda_jack_tbl_clear() is called to + * free snd_jack. This may cause access invalid memory + * when calling snd_jack_report + */ + if (per_pin->pcm_idx >= 0 && spec->dyn_pcm_assign) { + jack = spec->pcm_rec[per_pin->pcm_idx].jack; + } else if (!spec->dyn_pcm_assign) { + /* + * jack tbl doesn't support DP MST + * DP MST will use dyn_pcm_assign, + * so DP MST will never come here + */ + jack_tbl = snd_hda_jack_tbl_get_mst(codec, per_pin->pin_nid, + per_pin->dev_id); + if (jack_tbl) + jack = jack_tbl->jack; + } + return jack; +} /* update ELD and jack state via HD-audio verbs */ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, int repoll, @@ -1572,6 +1600,7 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, */ int present; bool do_repoll = false; + struct snd_jack *pcm_jack = NULL;
present = snd_hda_jack_pin_sense(codec, pin_nid, dev_id);
@@ -1599,10 +1628,17 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, do_repoll = true; }
- if (do_repoll) + if (do_repoll) { schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300)); - else + } else { + /* pcm_idx >=0 before update_eld() means it is in monitor + * disconnected event. Jack must be fetched before update_eld() + */ + pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); update_eld(codec, per_pin, eld); + if (!pcm_jack) + pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); + }
jack = snd_hda_jack_tbl_get_mst(codec, pin_nid, per_pin->dev_id); if (jack) { @@ -1614,36 +1650,16 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, if (!do_repoll && jack_report_sync) snd_hda_jack_report_sync(codec);
- mutex_unlock(&per_pin->lock); -} - -static struct snd_jack *pin_idx_to_jack(struct hda_codec *codec, - struct hdmi_spec_per_pin *per_pin) -{ - struct hdmi_spec *spec = codec->spec; - struct snd_jack *jack = NULL; - struct hda_jack_tbl *jack_tbl; + /* snd_hda_jack_report_sync() updates jack->pin_sense */ + if (spec->dyn_pcm_assign && pcm_jack && !do_repoll) { + int state = 0;
- /* if !dyn_pcm_assign, get jack from hda_jack_tbl - * in !dyn_pcm_assign case, spec->pcm_rec[].jack is not - * NULL even after snd_hda_jack_tbl_clear() is called to - * free snd_jack. This may cause access invalid memory - * when calling snd_jack_report - */ - if (per_pin->pcm_idx >= 0 && spec->dyn_pcm_assign) - jack = spec->pcm_rec[per_pin->pcm_idx].jack; - else if (!spec->dyn_pcm_assign) { - /* - * jack tbl doesn't support DP MST - * DP MST will use dyn_pcm_assign, - * so DP MST will never come here - */ - jack_tbl = snd_hda_jack_tbl_get_mst(codec, per_pin->pin_nid, - per_pin->dev_id); - if (jack_tbl) - jack = jack_tbl->jack; + if (jack && !!(jack->pin_sense & AC_PINSENSE_PRESENCE)) + state = SND_JACK_AVOUT; + snd_jack_report(pcm_jack, state); } - return jack; + + mutex_unlock(&per_pin->lock); }
/* update ELD and jack state via audio component */ @@ -1678,10 +1694,10 @@ static void sync_eld_via_acomp(struct hda_codec *codec, /* pcm_idx >=0 before update_eld() means it is in monitor * disconnected event. Jack must be fetched before update_eld() */ - jack = pin_idx_to_jack(codec, per_pin); + jack = pin_idx_to_pcm_jack(codec, per_pin); update_eld(codec, per_pin, eld); if (jack == NULL) - jack = pin_idx_to_jack(codec, per_pin); + jack = pin_idx_to_pcm_jack(codec, per_pin); if (jack) snd_jack_report(jack, (eld->monitor_present && eld->eld_valid) ?
On Tue, 04 Feb 2020 08:20:17 +0100, Nikhil Mahale wrote:
If dyn_pcm_assign is set, different jack objects are being created for pcm and pins.
If dyn_pcm_assign is set, generic_hdmi_build_jack() calls into add_hdmi_jack_kctl() to create and track separate jack object for pcm. Like sync_eld_via_acomp(), hdmi_present_sense_via_verbs() also need to report status change of the pcm jack.
Rename pin_idx_to_jack() to pin_idx_to_pcm_jack(). Update hdmi_present_sense_via_verbs() to report plug state of pcm jack object. Unlike sync_eld_via_acomp(), for !acomp drivers the pcm jack's plug state must be consistent with plug state of pin's jack.
Fixes: 5398e94fb753 ALSA: hda - Add DP-MST support for NVIDIA codecs
It's a wrong format. It should be Fixes: xxxx ("blah blah")
Signed-off-by: Nikhil Mahale nmahale@nvidia.com
As mentioned in the previous post, the fix must be applicable to 5.5 kernel as it's a clear regression. Get it fixed at first, then go for cleanup the rest.
thanks,
Takashi
On 2/4/20 1:18 PM, Takashi Iwai wrote:
External email: Use caution opening links or attachments
On Tue, 04 Feb 2020 08:20:17 +0100, Nikhil Mahale wrote:
If dyn_pcm_assign is set, different jack objects are being created for pcm and pins.
If dyn_pcm_assign is set, generic_hdmi_build_jack() calls into add_hdmi_jack_kctl() to create and track separate jack object for pcm. Like sync_eld_via_acomp(), hdmi_present_sense_via_verbs() also need to report status change of the pcm jack.
Rename pin_idx_to_jack() to pin_idx_to_pcm_jack(). Update hdmi_present_sense_via_verbs() to report plug state of pcm jack object. Unlike sync_eld_via_acomp(), for !acomp drivers the pcm jack's plug state must be consistent with plug state of pin's jack.
Fixes: 5398e94fb753 ALSA: hda - Add DP-MST support for NVIDIA codecs
It's a wrong format. It should be Fixes: xxxx ("blah blah")
Signed-off-by: Nikhil Mahale nmahale@nvidia.com
As mentioned in the previous post, the fix must be applicable to 5.5 kernel as it's a clear regression. Get it fixed at first, then go for cleanup the rest.
Okey, first I send patch to fix the issue. But are you agree with logic/direction of this patch? Here, I simply keep plug status of pcm jack consistent with that of pin jack.
Thanks, Nikhil Mahale
thanks,
Takashi
----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
On Tue, 04 Feb 2020 08:20:14 +0100, Nikhil Mahale wrote:
The jack report block, was added by commit 464837a7bc0a ("ALSA: hda
- block HDMI jack reports while repolling"), to avoid race condition
with repolling.
This text doesn't explain what and why you're changing. Describe that it's nothing but a cleanup, and no behavior change is intended here.
thanks,
Takashi
Signed-off-by: Nikhil Mahale nmahale@nvidia.com
sound/pci/hda/patch_hdmi.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 48bddc218829..ee084676f625 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1569,7 +1569,6 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, * the unsolicited response to avoid custom WARs. */ int present;
bool ret; bool do_repoll = false;
present = snd_hda_jack_pin_sense(codec, pin_nid, dev_id);
@@ -1603,16 +1602,15 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, else update_eld(codec, per_pin, eld);
- ret = !repoll || !eld->monitor_present || eld->eld_valid;
- jack = snd_hda_jack_tbl_get_mst(codec, pin_nid, per_pin->dev_id); if (jack) {
jack->block_report = !ret;
jack->block_report = do_repoll;
- return ret;
- return !do_repoll;
}
static struct snd_jack *pin_idx_to_jack(struct hda_codec *codec,
2.16.4
participants (2)
-
Nikhil Mahale -
Takashi Iwai