[alsa-devel] HDMI audio: TV vs. codec supported sample rates
Takashi Iwai
tiwai at suse.de
Thu Aug 5 20:22:49 CEST 2010
At Thu, 5 Aug 2010 07:23:26 -0700,
Stephen Warren wrote:
>
> 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.
Good point. A revised patch is below.
Can anyone test it?
thanks,
Takashi
---
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c
index d8da18a..803b298 100644
--- a/sound/pci/hda/hda_eld.c
+++ b/sound/pci/hda/hda_eld.c
@@ -596,4 +596,53 @@ 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,
+ struct hda_pcm_stream *codec_pars)
+{
+ 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;
+ }
+ }
+ }
+
+ if (!codec_pars)
+ return;
+
+ /* restrict the parameters by the values the codec provides */
+ pcm->rates &= codec_pars->rates;
+ pcm->formats &= codec_pars->formats;
+ pcm->channels_min = max(pcm->channels_min, codec_pars->channels_min);
+ pcm->channels_max = min(pcm->channels_max, codec_pars->channels_max);
+ pcm->maxbps = min(pcm->maxbps, codec_pars->maxbps);
+}
+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..28ab4ae 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -604,6 +604,8 @@ 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,
+ struct hda_pcm_stream *codec_pars);
#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..2bc0f07 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -46,6 +46,7 @@ struct hdmi_spec {
* export one pcm per pipe
*/
struct hda_pcm pcm_rec[MAX_HDMI_CVTS];
+ struct hda_pcm_stream codec_pcm_pars[MAX_HDMI_CVTS];
/*
* nvhdmi specific
@@ -766,6 +767,47 @@ 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;
+ struct hda_pcm_stream *codec_pars;
+ 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;
+
+ /* save the PCM info the codec provides */
+ codec_pars = &spec->codec_pcm_pars[idx];
+ if (!codec_pars->rates)
+ *codec_pars = *hinfo;
+
+ eld = &spec->sink_eld[idx];
+ if (eld->sad_count > 0) {
+ hdmi_eld_update_pcm_info(eld, hinfo, codec_pars);
+ if (hinfo->channels_min > hinfo->channels_max ||
+ !hinfo->rates || !hinfo->formats)
+ return -ENODEV;
+ } else {
+ /* fallback to the codec default */
+ hinfo->channels_min = codec_pars->channels_min;
+ hinfo->channels_max = codec_pars->channels_max;
+ hinfo->rates = codec_pars->rates;
+ hinfo->formats = codec_pars->formats;
+ hinfo->maxbps = codec_pars->maxbps;
+ }
+ 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,
.prepare = intel_hdmi_playback_pcm_prepare,
.cleanup = intel_hdmi_playback_pcm_cleanup,
},
diff --git a/sound/pci/hda/patch_nvhdmi.c b/sound/pci/hda/patch_nvhdmi.c
index a281836..55d5248 100644
--- a/sound/pci/hda/patch_nvhdmi.c
+++ b/sound/pci/hda/patch_nvhdmi.c
@@ -347,10 +347,8 @@ static int nvhdmi_dig_playback_pcm_prepare_2ch(struct hda_pcm_stream *hinfo,
static struct hda_pcm_stream nvhdmi_pcm_digital_playback_8ch_89 = {
.substreams = 1,
.channels_min = 2,
- .rates = SUPPORTED_RATES,
- .maxbps = SUPPORTED_MAXBPS,
- .formats = SUPPORTED_FORMATS,
.ops = {
+ .open = hdmi_pcm_open,
.prepare = nvhdmi_dig_playback_pcm_prepare_8ch_89,
.cleanup = nvhdmi_playback_pcm_cleanup,
},
More information about the Alsa-devel
mailing list