[alsa-devel] [PATCH] ALSA: hda - Update HDMI monitor_present from a temporary structure

[Takashi Iwai]

On Wed, 13 Apr 2016 02:27:39 +0200,
Hyungwon Hwang wrote:
> 
> pin_eld is a temporary structure. So before calling update_eld(),
> eld should be updated from pin_eld.
> 
> Signed-off-by: Hyungwon Hwang <hyungwon.hwang7 at gmail.com>

It's not clear what you're trying to solve only by the description
above alone.  It's about the inconsistent "monitor present" appearance
in eld proc file, right?  You need to write "why your patch is
necessary" in your patch description.

Now back to the code change:

> ---
>  sound/pci/hda/patch_hdmi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 5af372d..9de114d 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1414,6 +1414,8 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
>  
>  	mutex_lock(&per_pin->lock);
>  	pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
> +	eld->monitor_present = pin_eld->monitor_present;
> +
>  	if (pin_eld->monitor_present)
>  		eld->eld_valid  = !!(present & AC_PINSENSE_ELDV);
>  	else

OK, this would work, but the fundamental problem is that pin_eld is
modified in this function.  This used to work in the past casually due
to a bug, but I plugged it in the commit bd48128539ab
    ALSA: hda - Fix forgotten HDMI monitor_present update
but overlooked the regression in non-component side.  My bad.

So, the real fix is to replace the all pin_eld with eld in this
function.  The untested patch is below.  Could you check whether this
works for you?

Though, as a stable fix, maybe applying your oneliner patch would be
safer.  It restores to the old behavior, at least.  Then we can apply
the patch below on the top.


thanks,

Takashi

---
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 5af372d01834..c83c1a8d9742 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1396,7 +1396,6 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
 	struct hda_codec *codec = per_pin->codec;
 	struct hdmi_spec *spec = codec->spec;
 	struct hdmi_eld *eld = &spec->temp_eld;
-	struct hdmi_eld *pin_eld = &per_pin->sink_eld;
 	hda_nid_t pin_nid = per_pin->pin_nid;
 	/*
 	 * Always execute a GetPinSense verb here, even when called from
@@ -1413,15 +1412,15 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
 	present = snd_hda_pin_sense(codec, pin_nid);
 
 	mutex_lock(&per_pin->lock);
-	pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
-	if (pin_eld->monitor_present)
+	eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
+	if (eld->monitor_present)
 		eld->eld_valid  = !!(present & AC_PINSENSE_ELDV);
 	else
 		eld->eld_valid = false;
 
 	codec_dbg(codec,
 		"HDMI status: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n",
-		codec->addr, pin_nid, pin_eld->monitor_present, eld->eld_valid);
+		codec->addr, pin_nid, eld->monitor_present, eld->eld_valid);
 
 	if (eld->eld_valid) {
 		if (spec->ops.pin_get_eld(codec, pin_nid, eld->eld_buffer,
@@ -1441,7 +1440,7 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
 	else
 		update_eld(codec, per_pin, eld);
 
-	ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
+	ret = !repoll || !eld->monitor_present || eld->eld_valid;
 
 	jack = snd_hda_jack_tbl_get(codec, pin_nid);
 	if (jack)