[v1,4/4] ALSA: hda - Fix DP-MST support for NVIDIA codecs
diff mbox series

Message ID 20200204072017.9554-4-nmahale@nvidia.com
State New
Headers show
Series
  • [v1,1/4] ALSA: hda - Simplify hdmi_present_sense_via_verbs()
Related show

Commit Message

Nikhil Mahale Feb. 4, 2020, 7:20 a.m. UTC
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(-)

Comments

Takashi Iwai Feb. 4, 2020, 7:48 a.m. UTC | #1
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
Nikhil Mahale Feb. 4, 2020, 8:18 a.m. UTC | #2
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.
-----------------------------------------------------------------------------------

Patch
diff mbox series

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) ?