[alsa-devel] [PATCH 0/3] Random set of small HDA HDMI fixes
Hi,
Here's some random small fixes to the HDA HDMI code. First affects both generic and ATI/AMD codecs and the last two concern ATI/AMD only.
At least 2 and 3 should go to 3.13.
No idea what caused the codec reads to fail in the first place regarding the first patch... This was reported at: http://forum.xbmc.org/showthread.php?tid=174854&pid=1546159#pid1546159
sound/pci/hda/hda_eld.c | 30 ++++++++++++++++++++++++------ sound/pci/hda/patch_hdmi.c | 5 ++++- 2 files changed, 28 insertions(+), 7 deletions(-)
Anssi Hannula (3): ALSA: hda - hdmi: Add error-checking to some codec reads ALSA: hda - hdmi: Skip out-of-range latency values in AMD ELD generator ALSA: hda - hdmi: Fix wrong baseline length in ATI/AMD generated ELD
Add error checks to HBR status reads (both generic and ATI/AMD) and ATI/AMD codec reads for ELD generation.
Unchecked errors in these just caused more errors later on (invalid codec writes for the HBR ones and ELD parsing errors for the ATI/AMD ELD ones), but it is better to catch them earlier.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/hda_eld.c | 5 ++++- sound/pci/hda/patch_hdmi.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index 32d3e3855a6e..e8c55f5a34ff 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -680,7 +680,7 @@ int snd_hdmi_get_eld_ati(struct hda_codec *codec, hda_nid_t nid,
spkalloc = snd_hda_codec_read(codec, nid, 0, ATI_VERB_GET_SPEAKER_ALLOCATION, 0);
- if (!spkalloc) { + if (spkalloc <= 0) { snd_printd(KERN_INFO "HDMI ATI/AMD: no speaker allocation for ELD\n"); return -EINVAL; } @@ -742,6 +742,9 @@ int snd_hdmi_get_eld_ati(struct hda_codec *codec, hda_nid_t nid, snd_hda_codec_write(codec, nid, 0, ATI_VERB_SET_AUDIO_DESCRIPTOR, i << 3); ati_sad = snd_hda_codec_read(codec, nid, 0, ATI_VERB_GET_AUDIO_DESCRIPTOR, 0);
+ if (ati_sad <= 0) + continue; + if (ati_sad & ATI_AUDIODESC_RATES) { /* format is supported, copy SAD as-is */ buf[pos++] = (ati_sad & 0x0000ff) >> 0; diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index a96403a828af..fe3b150f97d9 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1247,6 +1247,9 @@ static int hdmi_pin_hbr_setup(struct hda_codec *codec, hda_nid_t pin_nid, pinctl = snd_hda_codec_read(codec, pin_nid, 0, AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
+ if (pinctl < 0) + return hbr ? -EINVAL : 0; + new_pinctl = pinctl & ~AC_PINCTL_EPT; if (hbr) new_pinctl |= AC_PINCTL_EPT_HBR; @@ -3094,7 +3097,7 @@ static int atihdmi_pin_hbr_setup(struct hda_codec *codec, hda_nid_t pin_nid, int hbr_ctl, hbr_ctl_new;
hbr_ctl = snd_hda_codec_read(codec, pin_nid, 0, ATI_VERB_GET_HBR_CONTROL, 0); - if (hbr_ctl & ATI_HBR_CAPABLE) { + if (hbr_ctl >= 0 && (hbr_ctl & ATI_HBR_CAPABLE)) { if (hbr) hbr_ctl_new = hbr_ctl | ATI_HBR_ENABLE; else
The ATI/AMD video/audio latencies are specified in apparent HDMI VSDB format. In this format values above 251 are not valid (or stream component is not supported - 255), but no checking is performed since this was not mentioned in the AMD HDA verbs specification.
Check that the latencies are valid before using them, and add a comment describing the formats.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/hda_eld.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index e8c55f5a34ff..9b697a28cf53 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -768,13 +768,28 @@ int snd_hdmi_get_eld_ati(struct hda_codec *codec, hda_nid_t nid, return -EINVAL; }
+ /* + * HDMI VSDB latency format: + * separately for both audio and video: + * 0 field not valid or unknown latency + * [1..251] msecs = (x-1)*2 (max 500ms with x = 251 = 0xfb) + * 255 audio/video not supported + * + * HDA latency format: + * single value indicating video latency relative to audio: + * 0 unknown or 0ms + * [1..250] msecs = x*2 (max 500ms with x = 250 = 0xfa) + * [251..255] reserved + */ aud_synch = snd_hda_codec_read(codec, nid, 0, ATI_VERB_GET_AUDIO_VIDEO_DELAY, 0); if ((aud_synch & ATI_DELAY_VIDEO_LATENCY) && (aud_synch & ATI_DELAY_AUDIO_LATENCY)) { - int video_latency = (aud_synch & ATI_DELAY_VIDEO_LATENCY) - 1; - int audio_latency = ((aud_synch & ATI_DELAY_AUDIO_LATENCY) >> 8) - 1; + int video_latency_hdmi = (aud_synch & ATI_DELAY_VIDEO_LATENCY); + int audio_latency_hdmi = (aud_synch & ATI_DELAY_AUDIO_LATENCY) >> 8;
- if (video_latency > audio_latency) - buf[6] = min(video_latency - audio_latency, 0xfa); + if (video_latency_hdmi <= 0xfb && audio_latency_hdmi <= 0xfb && + video_latency_hdmi > audio_latency_hdmi) + buf[6] = video_latency_hdmi - audio_latency_hdmi; + /* else unknown/invalid or 0ms or video ahead of audio, so use zero */ }
/* Baseline length */
According to the HDA specification the baseline ELD length is counted in DW of 4 bytes instead of in bytes.
Fix the code accordingly.
Baseline length is not used by the kernel so only the ELD exported to userspace was affected. No issues have been reported.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/hda_eld.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index 9b697a28cf53..cc814d02a0e4 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -793,7 +793,7 @@ int snd_hdmi_get_eld_ati(struct hda_codec *codec, hda_nid_t nid, }
/* Baseline length */ - buf[2] = pos - 4; + buf[2] = DIV_ROUND_UP(pos - 4, 4);
/* SAD count */ buf[5] |= ((pos - ELD_FIXED_BYTES - sink_desc_len) / 3) << 4;
According to the HDA specification the baseline ELD length is counted in DW of 4 bytes instead of in bytes.
Fix the code accordingly.
Baseline length is not used by the kernel so only the ELD exported to userspace was affected. No issues have been reported.
v2: Fixed so that eld_size is adjusted upwards accordingly as well.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/hda_eld.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index 9b697a28cf53..79ca80f6c77a 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -792,12 +792,15 @@ int snd_hdmi_get_eld_ati(struct hda_codec *codec, hda_nid_t nid, /* else unknown/invalid or 0ms or video ahead of audio, so use zero */ }
- /* Baseline length */ - buf[2] = pos - 4; - /* SAD count */ buf[5] |= ((pos - ELD_FIXED_BYTES - sink_desc_len) / 3) << 4;
+ /* Baseline ELD block length is 4-byte aligned */ + pos = round_up(pos, 4); + + /* Baseline ELD length (4-byte header is not counted in) */ + buf[2] = (pos - 4) / 4; + *eld_size = pos;
return 0;
At Sun, 10 Nov 2013 20:56:09 +0200, Anssi Hannula wrote:
Hi,
Here's some random small fixes to the HDA HDMI code. First affects both generic and ATI/AMD codecs and the last two concern ATI/AMD only.
At least 2 and 3 should go to 3.13.
No idea what caused the codec reads to fail in the first place regarding the first patch... This was reported at: http://forum.xbmc.org/showthread.php?tid=174854&pid=1546159#pid1546159
sound/pci/hda/hda_eld.c | 30 ++++++++++++++++++++++++------ sound/pci/hda/patch_hdmi.c | 5 ++++- 2 files changed, 28 insertions(+), 7 deletions(-)
Anssi Hannula (3): ALSA: hda - hdmi: Add error-checking to some codec reads ALSA: hda - hdmi: Skip out-of-range latency values in AMD ELD generator ALSA: hda - hdmi: Fix wrong baseline length in ATI/AMD generated ELD
Applied all three patches (3 is v2).
Thanks.
Takashi
participants (2)
-
Anssi Hannula -
Takashi Iwai