| Message ID | 1533653765-26080-1-git-send-email-yong.zhi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: Intel: hdac_hdmi: Add sampling frequencies constraint | expand |
Hi, Takashi, Thanks for the review. > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Tuesday, August 7, 2018 10:23 AM > To: Zhi, Yong <yong.zhi@intel.com> > Cc: broonie@kernel.org; pierre-louis.bossart@linux.intel.com; alsa- > devel@alsa-project.org; N, Harshapriya <harshapriya.n@intel.com>; > vkoul@kernel.org; M, Naveen <naveen.m@intel.com>; Kale, Sanyog R > <sanyog.r.kale@intel.com> > Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: hdac_hdmi: Add sampling > frequencies constraint > > On Tue, 07 Aug 2018 16:56:05 +0200, > Yong Zhi wrote: > > > > Playback of 44.1Khz contents with HDMI plugged returns "Invalid pipe > > config". > > Why? Is it a limitation of i915 graphics side? > If it's a generic issue, we'd need to fix also in the legacy HDMI driver, too. > The HDMI paths in DFW topology are configured for 48Khz operation. > (snip) > .... > > + > > + sad_rates = sad_sample_rates_lpcm(sad); > > + /* Filter out 44.1, 88.2 and 176.4Khz */ > > + for (j = 0; j < 7; j += 2) > > + if (sad_rates & BIT(j)) > > + rates |= cea_sampling_freqs[j]; > > + > > + snd_pcm_hw_constraint_mask64(runtime, > > + > SNDRV_PCM_HW_PARAM_RATE, > > + rates); > > The whole changes are too complex. You don't have to reduce the list > dynamically, but just need to tell the all possible rates with a static array. > > Or, even simpler, just filter the rates bits in > hdac_hdmi_create_dais() from the beginning like below. > > > thanks, > > Takashi > Ack, will make changes in hdac_hdmi_create_dais() thanks!! > --- a/sound/soc/codecs/hdac_hdmi.c > +++ b/sound/soc/codecs/hdac_hdmi.c > @@ -1410,6 +1410,12 @@ static int hdac_hdmi_create_dais(struct > hdac_device *hdev, > if (ret) > return ret; > > + /* Filter out 44.1, 88.2 and 176.4Khz */ > + rates &= ~(SNDRV_PCM_RATE_44100 | > SNDRV_PCM_RATE_88200 | > + SNDRV_PCM_RATE_176400); > + if (!rates) > + return -EINVAL; > + > sprintf(dai_name, "intel-hdmi-hifi%d", i+1); > hdmi_dais[i].name = devm_kstrdup(&hdev->dev, > dai_name, GFP_KERNEL);
On Tue, 07 Aug 2018 17:36:12 +0200, Zhi, Yong wrote: > > Hi, Takashi, > > Thanks for the review. > > > -----Original Message----- > > From: Takashi Iwai [mailto:tiwai@suse.de] > > Sent: Tuesday, August 7, 2018 10:23 AM > > To: Zhi, Yong <yong.zhi@intel.com> > > Cc: broonie@kernel.org; pierre-louis.bossart@linux.intel.com; alsa- > > devel@alsa-project.org; N, Harshapriya <harshapriya.n@intel.com>; > > vkoul@kernel.org; M, Naveen <naveen.m@intel.com>; Kale, Sanyog R > > <sanyog.r.kale@intel.com> > > Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: hdac_hdmi: Add sampling > > frequencies constraint > > > > On Tue, 07 Aug 2018 16:56:05 +0200, > > Yong Zhi wrote: > > > > > > Playback of 44.1Khz contents with HDMI plugged returns "Invalid pipe > > > config". > > > > Why? Is it a limitation of i915 graphics side? > > If it's a generic issue, we'd need to fix also in the legacy HDMI driver, too. > > > > The HDMI paths in DFW topology are configured for 48Khz operation. OK, so it's specific to this driver, not for the legacy configuration, so far. What if the legacy HDMI codec driver is used on top of Intel SST (as the recent patchset "Enable HDA Codec support on Intel Platforms")? The legacy HDMI codec driver isn't enabled there, but in theory it can be. I suppose that a similar workaround is required, right? thanks, Takashi
Hi, Takashi, > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Tuesday, August 7, 2018 10:41 AM > To: Zhi, Yong <yong.zhi@intel.com> > Cc: broonie@kernel.org; pierre-louis.bossart@linux.intel.com; alsa- > devel@alsa-project.org; N, Harshapriya <harshapriya.n@intel.com>; > vkoul@kernel.org; M, Naveen <naveen.m@intel.com>; Kale, Sanyog R > <sanyog.r.kale@intel.com> > Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: hdac_hdmi: Add sampling > frequencies constraint > > On Tue, 07 Aug 2018 17:36:12 +0200, > Zhi, Yong wrote: > > > > Hi, Takashi, > > > > Thanks for the review. > > > > > -----Original Message----- > > > From: Takashi Iwai [mailto:tiwai@suse.de] > > > Sent: Tuesday, August 7, 2018 10:23 AM > > > To: Zhi, Yong <yong.zhi@intel.com> > > > Cc: broonie@kernel.org; pierre-louis.bossart@linux.intel.com; alsa- > > > devel@alsa-project.org; N, Harshapriya <harshapriya.n@intel.com>; > > > vkoul@kernel.org; M, Naveen <naveen.m@intel.com>; Kale, Sanyog R > > > <sanyog.r.kale@intel.com> > > > Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: hdac_hdmi: Add > > > sampling frequencies constraint > > > > > > On Tue, 07 Aug 2018 16:56:05 +0200, > > > Yong Zhi wrote: > > > > > > > > Playback of 44.1Khz contents with HDMI plugged returns "Invalid > > > > pipe config". > > > > > > Why? Is it a limitation of i915 graphics side? > > > If it's a generic issue, we'd need to fix also in the legacy HDMI driver, too. > > > > > > > The HDMI paths in DFW topology are configured for 48Khz operation. > > OK, so it's specific to this driver, not for the legacy configuration, so far. > > What if the legacy HDMI codec driver is used on top of Intel SST (as the > recent patchset "Enable HDA Codec support on Intel Platforms")? > The legacy HDMI codec driver isn't enabled there, but in theory it can be. I > suppose that a similar workaround is required, right? > > Think so if that's the case, but I am far from expert on this topic :) > thanks, > > Takashi
On 08/07/2018 11:01 AM, Zhi, Yong wrote: > Hi, Takashi, > >> -----Original Message----- >> From: Takashi Iwai [mailto:tiwai@suse.de] >> Sent: Tuesday, August 7, 2018 10:41 AM >> To: Zhi, Yong <yong.zhi@intel.com> >> Cc: broonie@kernel.org; pierre-louis.bossart@linux.intel.com; alsa- >> devel@alsa-project.org; N, Harshapriya <harshapriya.n@intel.com>; >> vkoul@kernel.org; M, Naveen <naveen.m@intel.com>; Kale, Sanyog R >> <sanyog.r.kale@intel.com> >> Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: hdac_hdmi: Add sampling >> frequencies constraint >> >> On Tue, 07 Aug 2018 17:36:12 +0200, >> Zhi, Yong wrote: >>> Hi, Takashi, >>> >>> Thanks for the review. >>> >>>> -----Original Message----- >>>> From: Takashi Iwai [mailto:tiwai@suse.de] >>>> Sent: Tuesday, August 7, 2018 10:23 AM >>>> To: Zhi, Yong <yong.zhi@intel.com> >>>> Cc: broonie@kernel.org; pierre-louis.bossart@linux.intel.com; alsa- >>>> devel@alsa-project.org; N, Harshapriya <harshapriya.n@intel.com>; >>>> vkoul@kernel.org; M, Naveen <naveen.m@intel.com>; Kale, Sanyog R >>>> <sanyog.r.kale@intel.com> >>>> Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: hdac_hdmi: Add >>>> sampling frequencies constraint >>>> >>>> On Tue, 07 Aug 2018 16:56:05 +0200, >>>> Yong Zhi wrote: >>>>> Playback of 44.1Khz contents with HDMI plugged returns "Invalid >>>>> pipe config". >>>> Why? Is it a limitation of i915 graphics side? >>>> If it's a generic issue, we'd need to fix also in the legacy HDMI driver, too. >>>> >>> The HDMI paths in DFW topology are configured for 48Khz operation. >> OK, so it's specific to this driver, not for the legacy configuration, so far. >> >> What if the legacy HDMI codec driver is used on top of Intel SST (as the >> recent patchset "Enable HDA Codec support on Intel Platforms")? >> The legacy HDMI codec driver isn't enabled there, but in theory it can be. I >> suppose that a similar workaround is required, right? >> >> > Think so if that's the case, but I am far from expert on this topic :) There are two issues here. 1. the firmware relies on a timer which isn't aligned with any 44.1 frequency. In general we don't support 44.1kHz in master mode (we've relied on 48kHz frequencies since 2013 and no one cared or bothered to complain). If you look at all machine drivers using hdmi_hdac they are all based on 48kHz already. I suggested to Yong to do this filtering in the codec itself rather than in the front-ends since the DSP *could* do some resampling. I was also planning to remove all these front-end restrictions which prevent folks from using more that 48kHz/stereo/16 bits over HDMI. 2. it's not clear how the link itself would be configured (need the 12-11-11- pattern), it's not clear to me it's supported in decoupled mode (and I don't know how to test it in the first place due to issue #1) For the case with the HDA codec path, we are also using that same hdac_hdmi codec in the machine driver, along with the new hdac_hda pseudo codec, so the same limitations will be enforced. As to your question on whether this applies to the legacy driver, I don't think so. I just tried on my Skylake NUC, all HDMI devices seem to support 44.1kHz without errors. The limitation that Yong added is just a pragmatic view that 44.1 support is broken when going through the DSP and there isn't any real plan to fix this for now since this isn't a required feature and doesn't break compliance (HDMI sources may select one of 32,44.1,48kHz, HDMI sinks must support all of these frequencies). > >> thanks, >> >> Takashi
diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c index 7b8533abf637..b222a2e91463 100644 --- a/sound/soc/codecs/hdac_hdmi.c +++ b/sound/soc/codecs/hdac_hdmi.c @@ -288,6 +288,38 @@ static unsigned int sad_sample_bits_lpcm(const u8 *sad) return (sad[2] & 7); } +/** + * sad_sample_rates_lpcm - Find supported sampling frequencies + * + * @sad: pointer to CEA_SADs entry byte 2 which details sampling frequencies + * supported according to CEA-861 EDID V3. In little endian byte order. + * + * bit 7: Reserved (0) + * bit 6: 192kHz + * bit 5: 176kHz + * bit 4: 96kHz + * bit 3: 88kHz + * bit 2: 48kHz + * bit 1: 44kHz + * bit 0: 32kHz + * + * Return: bitmask of supported sampling frequencies. + */ +static u8 sad_sample_rates_lpcm(const u8 *sad) +{ + return (sad[1] & 0x7F); +} + +static const unsigned int cea_sampling_freqs[7] = { + SNDRV_PCM_RATE_32000, /* 0: 32000Hz */ + SNDRV_PCM_RATE_44100, /* 1: 44100Hz */ + SNDRV_PCM_RATE_48000, /* 2: 48000Hz */ + SNDRV_PCM_RATE_88200, /* 3: 88200Hz */ + SNDRV_PCM_RATE_96000, /* 4: 96000Hz */ + SNDRV_PCM_RATE_176400, /* 5: 176400Hz */ + SNDRV_PCM_RATE_192000, /* 6: 192000Hz */ +}; + static int hdac_hdmi_eld_limit_formats(struct snd_pcm_runtime *runtime, void *eld) { @@ -301,6 +333,8 @@ static int hdac_hdmi_eld_limit_formats(struct snd_pcm_runtime *runtime, for (i = drm_eld_sad_count(eld_buf); i > 0; i--, sad += 3) { if (sad_format(sad) == 1) { /* AUDIO_CODING_TYPE_LPCM */ + unsigned int rates = 0; + u8 sad_rates, j; /* * the controller support 20 and 24 bits in 32 bit @@ -308,6 +342,16 @@ static int hdac_hdmi_eld_limit_formats(struct snd_pcm_runtime *runtime, */ if (sad_sample_bits_lpcm(sad) & 0x6) formats |= SNDRV_PCM_FMTBIT_S32; + + sad_rates = sad_sample_rates_lpcm(sad); + /* Filter out 44.1, 88.2 and 176.4Khz */ + for (j = 0; j < 7; j += 2) + if (sad_rates & BIT(j)) + rates |= cea_sampling_freqs[j]; + + snd_pcm_hw_constraint_mask64(runtime, + SNDRV_PCM_HW_PARAM_RATE, + rates); } }
Playback of 44.1Khz contents with HDMI plugged returns "Invalid pipe config". This patch adds rate constraint to allow user space SRC to do the converting. Signed-off-by: Yong Zhi <yong.zhi@intel.com> --- sound/soc/codecs/hdac_hdmi.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)