alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ALSA: hda - Remove temporary buffer for ELD update
@ 2016-04-11 17:10 Hyungwon Hwang
  2016-04-12  9:32 ` Takashi Iwai
  0 siblings, 1 reply; 3+ messages in thread
From: Hyungwon Hwang @ 2016-04-11 17:10 UTC (permalink / raw)
  To: perex, tiwai, alsa-devel; +Cc: Hyungwon Hwang

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] ALSA: hda - Remove temporary buffer for ELD update
  2016-04-11 17:10 [PATCH] ALSA: hda - Remove temporary buffer for ELD update Hyungwon Hwang
@ 2016-04-12  9:32 ` Takashi Iwai
  2016-04-13  0:26   ` Hyungwon Hwang
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2016-04-12  9:32 UTC (permalink / raw)
  To: Hyungwon Hwang; +Cc: alsa-devel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] ALSA: hda - Remove temporary buffer for ELD update
  2016-04-12  9:32 ` Takashi Iwai
@ 2016-04-13  0:26   ` Hyungwon Hwang
  0 siblings, 0 replies; 3+ messages in thread
From: Hyungwon Hwang @ 2016-04-13  0:26 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-04-13  0:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-11 17:10 [PATCH] ALSA: hda - Remove temporary buffer for ELD update Hyungwon Hwang
2016-04-12  9:32 ` Takashi Iwai
2016-04-13  0:26   ` Hyungwon Hwang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).