| Message ID | 20220725111002.143765-2-uwu@icenowy.me |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] ASoC: Intel: Skylake: fix error message of NHLT blob selection | expand |
On 2022-07-25 1:10 PM, Icenowy Zheng wrote: > Switching to use pipeline parameters to get NHLT blob breaks audio on > HP Chromebook 13 G1 (at least with MrChromeBox firmware). > > Fix this by retrying to get NHLT blob with PCM parameters (which is the > old behavior) if pipeline parameters fail. > > Fixes: 87b265260046 ("ASoC: Intel: Skylake: Select proper format for NHLT blob") > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> Hello, Could you share the NHLT file from your platform plus the format used by the cras/userspace tool? Did you try playing over simple aplay tool instead? > sound/soc/intel/skylake/skl-topology.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c > index 19994ec8bba1..3d5a3ee1c82c 100644 > --- a/sound/soc/intel/skylake/skl-topology.c > +++ b/sound/soc/intel/skylake/skl-topology.c > @@ -1858,6 +1858,15 @@ static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai, > pipe_fmt->bps, params->s_cont, > pipe_fmt->channels, pipe_fmt->freq, > pipe->direction, dev_type); > + if (!cfg) { > + /* Retry with PCM parameters, as the old behavior */ Drop the "old behavior" - most of the readers are not aware of what that actually means. > + cfg = intel_nhlt_get_endpoint_blob(dai->dev, skl->nhlt, > + mconfig->vbus_id, link_type, > + params->s_fmt, params->s_cont, > + params->ch, params->s_freq, > + params->stream, dev_type); > + } > + > if (cfg) { > mconfig->formats_config[SKL_PARAM_INIT].caps_size = cfg->size; > mconfig->formats_config[SKL_PARAM_INIT].caps = (u32 *)&cfg->caps; > @@ -1866,6 +1875,8 @@ static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai, > mconfig->vbus_id, link_type, params->stream, > pipe_fmt->channels, pipe_fmt->freq, > pipe_fmt->bps); > + dev_err(dai->dev, "PCM: ch %d, freq %d, fmt %d\n", > + params->ch, params->s_freq, params->s_fmt); > return -EINVAL; > } >
在 2022-08-02星期二的 12:30 +0200，Cezary Rojewski写道： > On 2022-07-25 1:10 PM, Icenowy Zheng wrote: > > Switching to use pipeline parameters to get NHLT blob breaks audio > > on > > HP Chromebook 13 G1 (at least with MrChromeBox firmware). > > > > Fix this by retrying to get NHLT blob with PCM parameters (which is > > the > > old behavior) if pipeline parameters fail. > > > > Fixes: 87b265260046 ("ASoC: Intel: Skylake: Select proper format > > for NHLT blob") > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > Hello, > > > Could you share the NHLT file from your platform plus the format used > by > the cras/userspace tool? Did you try playing over simple aplay tool > instead? I tried 48000Hz 2ch 32bit with speaker-test. Attached is /sys/firmware/acpi/tables/NHLT. > > > > sound/soc/intel/skylake/skl-topology.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/sound/soc/intel/skylake/skl-topology.c > > b/sound/soc/intel/skylake/skl-topology.c > > index 19994ec8bba1..3d5a3ee1c82c 100644 > > --- a/sound/soc/intel/skylake/skl-topology.c > > +++ b/sound/soc/intel/skylake/skl-topology.c > > @@ -1858,6 +1858,15 @@ static int > > skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai, > > pipe_fmt->bps, params- > > >s_cont, > > pipe_fmt->channels, > > pipe_fmt->freq, > > pipe->direction, dev_type); > > + if (!cfg) { > > + /* Retry with PCM parameters, as the old behavior > > */ > > Drop the "old behavior" - most of the readers are not aware of what > that > actually means. > > > + cfg = intel_nhlt_get_endpoint_blob(dai->dev, skl- > > >nhlt, > > + mconfig->vbus_id, > > link_type, > > + params->s_fmt, > > params->s_cont, > > + params->ch, params- > > >s_freq, > > + params->stream, > > dev_type); > > + } > > + > > if (cfg) { > > mconfig->formats_config[SKL_PARAM_INIT].caps_size = > > cfg->size; > > mconfig->formats_config[SKL_PARAM_INIT].caps = (u32 > > *)&cfg->caps; > > @@ -1866,6 +1875,8 @@ static int > > skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai, > > mconfig->vbus_id, link_type, params- > > >stream, > > pipe_fmt->channels, pipe_fmt->freq, > > pipe_fmt->bps); > > + dev_err(dai->dev, "PCM: ch %d, freq %d, fmt %d\n", > > + params->ch, params->s_freq, params->s_fmt); > > return -EINVAL; > > } > >
On 2022-08-17 2:48 PM, Icenowy Zheng wrote: > 在 2022-08-16星期二的 21:08 +0200，Cezary Rojewski写道： ... > > Yes, it's Chell. > > BTW do you need other ACPI tables? At this point, no. Thanks for confirming the platform's name. >> >> ..and at this point I probably know more than enough. We have tested >> basically all of the KBL and AML configurations when fixing >> regressions >> during recent skylake-driver up-revs. But Chell (and Lars for that >> matter) families were not among them as these are based on SKL. I'll >> follow up on this with our partners and come back here. I'm almost >> certain topology files for the two families mentioned were not >> updated >> along the way. > > Could this be an issue of Coreboot, which generates the NHLT table? NHLT was left alone across all the updates. Updating it is one way of fixing problems but I don't believe it's necessary in your case. Topology update is more desirable approach. > BTW I think Google pinned the official OS of this hardware to a much > lower kernel version (but I don't want to use the official OS because > of limited storage of Chell and lack of VM ability of the OS on Chell). That clarifies things out. Guess the kernel version used there is v4.4 (plus a ton of un-upstreamed patches). Again, will propagate the information up the chain. Perhaps one of the solutions for end-users would be providing working UCM files to alsa-topology-conf repo so users are not powerless in situations such as this one. Regards, Czarek
On 2022-08-18 11:25 AM, Icenowy Zheng wrote: > 在 2022-08-17星期三的 15:19 +0200，Cezary Rojewski写道： ... >> NHLT was left alone across all the updates. Updating it is one way of >> fixing problems but I don't believe it's necessary in your case. >> Topology update is more desirable approach. > > BTW how could I fix the topology? > > I now use topology files from GalliumOS (which, I assume, is extracted > from ChromeOS). (save) >> That clarifies things out. Guess the kernel version used there is >> v4.4 >> (plus a ton of un-upstreamed patches). Again, will propagate the >> information up the chain. Perhaps one of the solutions for end-users >> would be providing working UCM files to alsa-topology-conf repo so >> users >> are not powerless in situations such as this one. The message of mine above is the answer to this. Without some ADSP firmware knowledge it's almost impossible to write a topology file from scratch. During v4.4 -> v5.4 transition several problems with endpoints were detected as not all of them were behaving as expected. Long story short, I do not believe pure v4.4 on some of the designs (mainly I2S designs) works as intended. Some external patches are needed for that to happen. The situation on v5.4 is much cleaner - the problem there are the topology files, as these are not updated automatically when you flash new kernel. In fact, these were never shared in alsa-topology-ucm repo. Only HDAudio topology has been fixed and shared. As long as you stick to the original ChromeOS the issues on your machine should cease to exist. I need some approvals before I2S designs can be shared. It's not a process that takes a day or two, unfortunately. Regards, Czarek
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index 19994ec8bba1..3d5a3ee1c82c 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -1858,6 +1858,15 @@ static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai, pipe_fmt->bps, params->s_cont, pipe_fmt->channels, pipe_fmt->freq, pipe->direction, dev_type); + if (!cfg) { + /* Retry with PCM parameters, as the old behavior */ + cfg = intel_nhlt_get_endpoint_blob(dai->dev, skl->nhlt, + mconfig->vbus_id, link_type, + params->s_fmt, params->s_cont, + params->ch, params->s_freq, + params->stream, dev_type); + } + if (cfg) { mconfig->formats_config[SKL_PARAM_INIT].caps_size = cfg->size; mconfig->formats_config[SKL_PARAM_INIT].caps = (u32 *)&cfg->caps; @@ -1866,6 +1875,8 @@ static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai, mconfig->vbus_id, link_type, params->stream, pipe_fmt->channels, pipe_fmt->freq, pipe_fmt->bps); + dev_err(dai->dev, "PCM: ch %d, freq %d, fmt %d\n", + params->ch, params->s_freq, params->s_fmt); return -EINVAL; }
Switching to use pipeline parameters to get NHLT blob breaks audio on HP Chromebook 13 G1 (at least with MrChromeBox firmware). Fix this by retrying to get NHLT blob with PCM parameters (which is the old behavior) if pipeline parameters fail. Fixes: 87b265260046 ("ASoC: Intel: Skylake: Select proper format for NHLT blob") Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- sound/soc/intel/skylake/skl-topology.c | 11 +++++++++++ 1 file changed, 11 insertions(+)