[alsa-devel] [PATCH v1 1/4] ALSA: hda - Simplify hdmi_present_sense_via_verbs()

[alsa-devel] [PATCH v1 1/4] ALSA: hda - Simplify hdmi_present_sense_via_verbs()

[Nikhil Mahale]

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,
-- 
2.16.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH v1 2/4] ALSA: hda - Move snd_hda_jack_report_sync() call inside hdmi_present_sense_via_verbs()

[Nikhil Mahale]

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;
 }
-- 
2.16.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH v1 3/4] ALSA: hda - Change return type of update_eld() to void

[Nikhil Mahale]

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);
-- 
2.16.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v1 3/4] ALSA: hda - Change return type of update_eld() to void

[Takashi Iwai]

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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH v1 4/4] ALSA: hda - Fix DP-MST support for NVIDIA codecs

[Nikhil Mahale]

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

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v1 4/4] ALSA: hda - Fix DP-MST support for NVIDIA codecs

[Takashi Iwai]

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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v1 4/4] ALSA: hda - Fix DP-MST support for NVIDIA codecs

[Nikhil Mahale]

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.
-----------------------------------------------------------------------------------
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v1 1/4] ALSA: hda - Simplify hdmi_present_sense_via_verbs()

[Takashi Iwai]

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;
>  		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,
> -- 
> 2.16.4
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel