On Mon, 03 Feb 2020 11:06: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(). The code to report status change of pcm jack, move it to update_eld() which is common for acomp and !acomp code paths.
Thanks, that's the cause of the regression. However, this needs yet more careful handling, I'm afraid.
- hdmi_present_sense_via_verbs() may return true, and its callers (both check_presence_and_report() and hdmi_repoll_eld()) do call snd_hda_jack_report_sync() again.
- For non-dyn_pcm_assign case, we shouldn't call jack report there, but rather simply return true for calling report sync.
- There is another workaround to block the jack report in hdmi_present_sense_via_verbs() which is applied after update_eld(), and this will be ignored.
So, I guess that we need the conditional application of the individual snd_jack_report() only for spec->dyn_pcm_assign==true, and assure that hdmi_present() returns false.
The last item (the jack report block) is still unclear to me; it's a workaround that was needed for Nvidia drivers in the past due to instability. If this is still needed for DP-MST case, we have to reconsider how to deal with it. Otherwise, this can be applied only for non-dyn_pcm_assign case.
BTW, the condition for jack->block_report and return value in hdmi_present_sense_via_verbs() looks currently complicated, but it could have been simplified like:
-- 8< -- --- 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,14 @@ 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, -- 8< --
Takashi
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 | 87 ++++++++++++++++++++++++---------------------- 1 file changed, 45 insertions(+), 42 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 48bddc218829..7b5360037217 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1480,6 +1480,35 @@ static void hdmi_pcm_reset_pin(struct hdmi_spec *spec, per_pin->channels = 0; }
+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 per_pin ELD from the given new ELD;
- setup info frame and notification accordingly
*/ @@ -1490,9 +1519,15 @@ static bool update_eld(struct hda_codec *codec, struct hdmi_eld *pin_eld = &per_pin->sink_eld; struct hdmi_spec *spec = codec->spec; bool old_eld_valid = pin_eld->eld_valid;
struct snd_jack *pcm_jack; bool eld_changed; int pcm_idx;
/* 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);
/* for monitor disconnection, save pcm_idx firstly */ pcm_idx = per_pin->pcm_idx; if (spec->dyn_pcm_assign) {
@@ -1547,6 +1582,14 @@ 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);
- if (!pcm_jack)
pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);- if (eld_changed && pcm_jack)
snd_jack_report(pcm_jack,(eld->monitor_present && eld->eld_valid) ?SND_JACK_AVOUT : 0);- return eld_changed;
}
@@ -1615,43 +1658,12 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, return ret; }
-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;
- /* 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 audio component */ static void sync_eld_via_acomp(struct hda_codec *codec, struct hdmi_spec_per_pin *per_pin) { 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);
@@ -1674,17 +1686,8 @@ static void sync_eld_via_acomp(struct hda_codec *codec, eld->eld_size = 0; }
- /* 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);
- changed = update_eld(codec, per_pin, eld);
- if (jack == NULL)
jack = pin_idx_to_jack(codec, per_pin);- if (changed && jack)
snd_jack_report(jack,(eld->monitor_present && eld->eld_valid) ?SND_JACK_AVOUT : 0);
- update_eld(codec, per_pin, eld);
- mutex_unlock(&per_pin->lock);
}
-- 2.16.4