Takashi Iwai wrote:
Sent: Thursday, August 05, 2010 6:28 AM
At Thu, 5 Aug 2010 14:17:10 +0300, Anssi Hannula wrote:
Takashi Iwai kirjoitti torstai, 5. elokuuta 2010 14:02:27:
At Thu, 5 Aug 2010 13:57:18 +0300,
Anssi Hannula wrote:
Takashi Iwai kirjoitti torstai, 5. elokuuta 2010 13:46:39:
At Thu, 5 Aug 2010 12:49:07 +0300,
Anssi Hannula wrote:
Anssi Hannula kirjoitti tiistai, 3. elokuuta 2010 20:39:20: > Stephen Warren kirjoitti tiistai, 3. elokuuta 2010 19:20:24: > > I don't see anything in patch_intelhdmi.c that relates to sample > > rate support at all. However, I see that patch_nvhdmi.c contains > > e.g.: > > > > static struct hda_pcm_stream nvhdmi_pcm_digital_playback_8ch_89 = > > { > > > > .substreams = 1, > > .channels_min = 2, > > .rates = SUPPORTED_RATES, > > > > Should .rates not be initialized here, > > When .rates is not initialized, they are automatically read from > the codec, i.e. exactly the values you referenced above from > "/proc/asound/card1/codec#1". > > The values seem to differ (32000 vs 22050 is reported as > supported), though. If the codec reported values are wrong, the > .rates should be kept. Otherwise the rates set in .rates should be > fixed or the override just removed.
As AFAICS HDMI specification allows 32000Hz but not 22050Hz, I'd say it is likely that the override is wrong and the codec reported rates are right.
Wei,
Looking at our Windows driver, we should remove 22050 from the supported rates list and add 32000 instead. Can you confirm this? Thanks.
(my receiver doesn't support 32000Hz so I can't confirm).
So, the codec reports correct values based on EDID?
No. The codec reports all the values it supports, regardless if the attached HDMI sink supports them or not.
OK. Then setting a static value was still needed.
How so (for _89 I mean)?
For _89, yeah, that's not needed if it gives really all.
IIRC, the static values there were needed for old Nvidia codecs that don't give the actual values. Maybe newer chips work correctly. If so, rates, maxbps and formats in *_89 (also *_7x, too?) can be removed, indeed.
Can any confirm?
I have an older MCP7A codec as well and can confirm that it reports wrong values (only 48000/88200, while I can confirm in reality 192000 works with it). Though I guess the nonsensical 22050 should probably be 32000 (but I can't test since my receiver doesn't support 32000).
What actually show in eld#* proc file?
The values that the connected HDMI sink supports.
If this shows the correct values, we can add the code to take over rates & formats from ELD at hotplug.
That's exactly what should be done (the intersection of supported formats of the codec and the sink).
OK, what about the patch below (totally untested)?
This updates the PCM info only at PCM open time. Apps usually check parameters only at open (due to hw_params stuff), so it doesn't make much sense to change dynamically at hotplug in the end.
Doesn't that patch only use the ELD to set up the stream's supported parameters? I think the code needs to determine the intersection of what's supported by the codec (from HDA node parsing if that's the correct terminology) *and* the display device (from the ELD), not just one of them.
Takashi
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index d8da18a..c41ba10 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -596,4 +596,42 @@ void snd_hda_eld_proc_free(struct hda_codec *codec, struct hdmi_eld *eld) } EXPORT_SYMBOL_HDA(snd_hda_eld_proc_free);
+/* update PCM info based on ELD */ +void hdmi_eld_update_pcm_info(struct hdmi_eld *eld, struct hda_pcm_stream *pcm) +{
- int i;
- pcm->rates = 0;
- pcm->formats = 0;
- pcm->maxbps = 0;
- pcm->channels_min = -1;
- pcm->channels_max = 0;
- for (i = 0; i < eld->sad_count; i++) {
struct cea_sad *a = &eld->sad[i];pcm->rates |= a->rates;if (a->channels < pcm->channels_min)pcm->channels_min = a->channels;if (a->channels > pcm->channels_max)pcm->channels_max = a->channels;if (a->format == AUDIO_CODING_TYPE_LPCM) {if (a->sample_bits & AC_SUPPCM_BITS_16) {pcm->formats |= SNDRV_PCM_FMTBIT_S16_LE;if (pcm->maxbps < 16)pcm->maxbps = 16;}if (a->sample_bits & AC_SUPPCM_BITS_20) {pcm->formats |= SNDRV_PCM_FMTBIT_S32_LE;if (pcm->maxbps < 20)pcm->maxbps = 20;}if (a->sample_bits & AC_SUPPCM_BITS_24) {pcm->formats |= SNDRV_PCM_FMTBIT_S32_LE;if (pcm->maxbps < 24)pcm->maxbps = 24;}}- }
+} +EXPORT_SYMBOL_HDA(hdmi_eld_update_pcm_info);
#endif /* CONFIG_PROC_FS */ diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index 7a97f12..af2699e 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -604,6 +604,7 @@ struct hdmi_eld { int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid); int snd_hdmi_get_eld(struct hdmi_eld *, struct hda_codec *, hda_nid_t); void snd_hdmi_show_eld(struct hdmi_eld *eld); +void hdmi_eld_update_pcm_info(struct hdmi_eld *eld, struct hda_pcm_stream *pcm);
#ifdef CONFIG_PROC_FS int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld, diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 522e074..4176e6c 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -766,6 +766,29 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t nid, }
/*
- HDA PCM callbacks
- */
+static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
struct hda_codec *codec,struct snd_pcm_substream *substream)+{
- struct hdmi_spec *spec = codec->spec;
- struct hdmi_eld *eld;
- unsigned int idx;
- for (idx = 0; idx < spec->num_cvts; idx++)
if (hinfo->nid == spec->cvt[idx])break;- if (snd_BUG_ON(idx >= spec->num_cvts) ||
snd_BUG_ON(idx >= spec->num_pins))return -EINVAL;- eld = &spec->sink_eld[idx];
- if (eld->sad_count > 0)
hdmi_eld_update_pcm_info(eld, hinfo);- return 0;
+}
+/*
- HDA/HDMI auto parsing
*/
diff --git a/sound/pci/hda/patch_intelhdmi.c b/sound/pci/hda/patch_intelhdmi.c index 5972d5e..d382d3c 100644 --- a/sound/pci/hda/patch_intelhdmi.c +++ b/sound/pci/hda/patch_intelhdmi.c @@ -80,6 +80,7 @@ static struct hda_pcm_stream intel_hdmi_pcm_playback = { .substreams = 1, .channels_min = 2, .ops = {
.open = hdmi_pcm_open,diff --git a/sound/pci/hda/patch_nvhdmi.c b/sound/pci/hda/patch_nvhdmi.c index a281836..004d1c6 100644 --- a/sound/pci/hda/patch_nvhdmi.c +++ b/sound/pci/hda/patch_nvhdmi.c @@ -351,6 +351,7 @@ static struct hda_pcm_stream nvhdmi_pcm_digital_playback_8ch_89 = { .maxbps = SUPPORTED_MAXBPS, .formats = SUPPORTED_FORMATS, .ops = {
.open = hdmi_pcm_open,