[alsa-devel] [PATCH] ALSA: hda - Remove temporary buffer for ELD update
With the introduce of pcm_lock and per_pin->lock, this temporary buffer is not needed anymore.
Signed-off-by: Hyungwon Hwang hyungwon.hwang7@gmail.com --- sound/pci/hda/patch_hdmi.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 5af372d..a24d5b4 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1395,8 +1395,7 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, struct hda_jack_tbl *jack; 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; + struct hdmi_eld *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)
On Mon, 11 Apr 2016 19:10:51 +0200, Hyungwon Hwang wrote:
With the introduce of pcm_lock and per_pin->lock, this temporary buffer is not needed anymore.
The use of a temporary buffer is not only to avoid the race. It's also for avoiding the corruption due to an erroneous read. That is, pin_get_eld() might overwrite some bogus data while it returns an error. Since the driver retries upon the error, we need to keep the old good data as much as possible.
thanks,
Takashi
Signed-off-by: Hyungwon Hwang hyungwon.hwang7@gmail.com
sound/pci/hda/patch_hdmi.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 5af372d..a24d5b4 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1395,8 +1395,7 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, struct hda_jack_tbl *jack; 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;
- struct hdmi_eld *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)
-- 2.5.0
Dear Takashi,
2016년 04월 12일 18:32에 Takashi Iwai 이(가) 쓴 글:
On Mon, 11 Apr 2016 19:10:51 +0200, Hyungwon Hwang wrote:
With the introduce of pcm_lock and per_pin->lock, this temporary buffer is not needed anymore.
The use of a temporary buffer is not only to avoid the race. It's also for avoiding the corruption due to an erroneous read. That is, pin_get_eld() might overwrite some bogus data while it returns an error. Since the driver retries upon the error, we need to keep the old good data as much as possible.
I missed that pin_get_eld() can ruin the structure. I agree with you. Then I will send another patch which set eld->monitor_present by the value of pin_eld->monitor_present. So proc can show the correct current status.
Thanks, Hyungwon Hwang
thanks,
Takashi
Signed-off-by: Hyungwon Hwang hyungwon.hwang7@gmail.com
sound/pci/hda/patch_hdmi.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 5af372d..a24d5b4 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1395,8 +1395,7 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, struct hda_jack_tbl *jack; 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;
- struct hdmi_eld *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)
-- 2.5.0
participants (2)
-
Hyungwon Hwang
-
Takashi Iwai