| Message ID | 20200204102746.1356-1-nmahale@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: hda - Fix DP-MST support for NVIDIA codecs | expand |
On Tue, 04 Feb 2020 11:27:46 +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. Thanks, the new patch looks better. > Fixes: 5398e94fb753 ("ALSA: hda - Add DP-MST support for NVIDIA codecs") > Signed-off-by: Nikhil Mahale <nmahale@nvidia.com> We need Cc to stable here. I'll add it when applying. Also, it deserves Reported-by from Martin. Martin, could you retest with this patch? I'll queue the patch once after confirmation. Just one minor nitpick: > + if (spec->dyn_pcm_assign && pcm_jack && !do_repoll) { > + int state = 0; > + > + if (!!(jack->pin_sense & AC_PINSENSE_PRESENCE)) > + state = SND_JACK_AVOUT; The "!!" is superfluous. I'll drop it. Takashi
Hi, On Tue, 4 Feb 2020, Nikhil Mahale wrote: > If dyn_pcm_assign is set, different jack objects are being created > for pcm and pins. seems good. With the one change Takashi spotted: Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Br, Kai
Applied the new patch. The device is detected correctly by pulseaudio. Thanks for your efforts. On 04.02.20 12:18, Takashi Iwai wrote: > On Tue, 04 Feb 2020 11:27:46 +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. > Thanks, the new patch looks better. > >> Fixes: 5398e94fb753 ("ALSA: hda - Add DP-MST support for NVIDIA codecs") >> Signed-off-by: Nikhil Mahale <nmahale@nvidia.com> > We need Cc to stable here. I'll add it when applying. > > Also, it deserves Reported-by from Martin. > Martin, could you retest with this patch? I'll queue the patch once > after confirmation. > > Just one minor nitpick: > >> + if (spec->dyn_pcm_assign && pcm_jack && !do_repoll) { >> + int state = 0; >> + >> + if (!!(jack->pin_sense & AC_PINSENSE_PRESENCE)) >> + state = SND_JACK_AVOUT; > The "!!" is superfluous. I'll drop it. > > > Takashi
On Tue, 04 Feb 2020 17:10:45 +0100, Martin Regner wrote: > > Applied the new patch. The device is detected correctly by pulseaudio. > Thanks for your efforts. Great, thanks for quick testing. Now I applied and pushed out. Takashi > > On 04.02.20 12:18, Takashi Iwai wrote: > > On Tue, 04 Feb 2020 11:27:46 +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. > > Thanks, the new patch looks better. > > > >> Fixes: 5398e94fb753 ("ALSA: hda - Add DP-MST support for NVIDIA codecs") > >> Signed-off-by: Nikhil Mahale <nmahale@nvidia.com> > > We need Cc to stable here. I'll add it when applying. > > > > Also, it deserves Reported-by from Martin. > > Martin, could you retest with this patch? I'll queue the patch once > > after confirmation. > > > > Just one minor nitpick: > > > >> + if (spec->dyn_pcm_assign && pcm_jack && !do_repoll) { > >> + int state = 0; > >> + > >> + if (!!(jack->pin_sense & AC_PINSENSE_PRESENCE)) > >> + state = SND_JACK_AVOUT; > > The "!!" is superfluous. I'll drop it. > > > > > > Takashi > >
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 48bddc218829..c1d3ce423142 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1550,6 +1550,34 @@ static bool update_eld(struct hda_codec *codec, return eld_changed; } +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 bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, int repoll) @@ -1571,6 +1599,7 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, int present; bool ret; bool do_repoll = false; + struct snd_jack *pcm_jack = NULL; present = snd_hda_jack_pin_sense(codec, pin_nid, dev_id); @@ -1598,10 +1627,19 @@ static bool 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); + } ret = !repoll || !eld->monitor_present || eld->eld_valid; @@ -1610,38 +1648,32 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, jack->block_report = !ret; jack->pin_sense = (eld->monitor_present && eld->eld_valid) ? AC_PINSENSE_PRESENCE : 0; - } - mutex_unlock(&per_pin->lock); - 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 (spec->dyn_pcm_assign && pcm_jack && !do_repoll) { + int state = 0; + + if (!!(jack->pin_sense & AC_PINSENSE_PRESENCE)) + state = SND_JACK_AVOUT; + snd_jack_report(pcm_jack, state); + } - /* 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 + * snd_hda_jack_pin_sense() call at the beginning of this + * function, updates jack->pins_sense and clears + * jack->jack_dirty, therefore snd_hda_jack_report_sync() will + * not override the jack->pin_sense. + * + * snd_hda_jack_report_sync() is superfluous for dyn_pcm_assign + * case. The jack->pin_sense update was already performed, and + * hda_jack->jack is NULL for dyn_pcm_assign. + * + * Don't call snd_hda_jack_report_sync() for + * dyn_pcm_assign. */ - jack_tbl = snd_hda_jack_tbl_get_mst(codec, per_pin->pin_nid, - per_pin->dev_id); - if (jack_tbl) - jack = jack_tbl->jack; + ret = ret && !spec->dyn_pcm_assign; } - return jack; + mutex_unlock(&per_pin->lock); + return ret; } /* update ELD and jack state via audio component */ @@ -1677,10 +1709,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); changed = 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 (changed && jack) snd_jack_report(jack, (eld->monitor_present && eld->eld_valid) ?
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 | 94 +++++++++++++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 31 deletions(-)