[PATCH 1/2] ASoC: Intel: Skylake: fix error message of NHLT blob selection
The query to select NHLT blob had been changed to use parameters of the pipeline instead of PCM, however the error message returned when it fails still uses PCM properties.
Fix this inconsistency.
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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index 9bdf020a2b64..19994ec8bba1 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -1864,7 +1864,8 @@ static int skl_tplg_be_fill_pipe_params(struct snd_soc_dai *dai, } else { dev_err(dai->dev, "Blob NULL for id:%d type:%d dirn:%d ch:%d, freq:%d, fmt:%d\n", mconfig->vbus_id, link_type, params->stream, - params->ch, params->s_freq, params->s_fmt); + pipe_fmt->channels, pipe_fmt->freq, + pipe_fmt->bps); 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(+)
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; }
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);
在 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-07 7:26 PM, Icenowy Zheng wrote:
在 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).
...
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.
Thanks for the NHLT dump. Total of five endpoints are part of the description:
id 0, DMIC capture, formats: [0] 2/16/16/48000 [1] 2/32/32/48000 id 1, I2S SSP0 playback, formats: [0] 2/24/32/48000 id 2, I2S SSP0 capture, formats: [0] 4/32/32/48000 id 3, I2S SSP1 playback, formats: [0] 2/24/32/48000 id 4, I2S SSP1 capture, formats: [0] 2/24/32/48000
I know not what "speaker-test" means. Could you specify which endpoint you are speaking of? Providing either alsa info or at least output of 'lsmod | grep snd' would help. I'd like to be aware of which machine board are we talking about.
What could be guessed with the current info (and some google query), is that we are dealing with SKL-Y m5-6Y57, codenamed 'Chell' and the speaker-test is playing through an I2S codec connected to SSP0 port. It's probably max98357a. The skylake-driver version you have gets confused with 24/32 format (userspace is probably attempting 32/32). That or topology file is incorrect..
..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.
Regards, Czarek
On Tue, Aug 16, 2022 at 09:08:31PM +0200, Cezary Rojewski wrote:
I know not what "speaker-test" means. Could you specify which endpoint you are speaking of? Providing either alsa info or at least output of 'lsmod | grep snd' would help. I'd like to be aware of which machine board are we talking about.
speaker-test is one of the standard alsa-utils apps, it plays generated audio (sine waves, various varieties of noise, whatever) with command line specified control of the audio parameters.
在 2022-08-16星期二的 21:08 +0200，Cezary Rojewski写道：
On 2022-08-07 7:26 PM, Icenowy Zheng wrote:
在 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).
...
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.
Thanks for the NHLT dump. Total of five endpoints are part of the description:
id 0, DMIC capture, formats: [0] 2/16/16/48000 [1] 2/32/32/48000 id 1, I2S SSP0 playback, formats: [0] 2/24/32/48000 id 2, I2S SSP0 capture, formats: [0] 4/32/32/48000 id 3, I2S SSP1 playback, formats: [0] 2/24/32/48000 id 4, I2S SSP1 capture, formats: [0] 2/24/32/48000
I know not what "speaker-test" means. Could you specify which endpoint
Well to be honest I know nearly nothing about Intel ASoC. (In fact I have only a little experience dealing with ASoC on DT-based platforms with asoc-simple-card)
I assume it uses the first playback stream, because I didn't explicitly choose any streams; and speaker-test is only a playback test program.
you are speaking of? Providing either alsa info or at least output of 'lsmod | grep snd' would help. I'd like to be aware of which machine board are we talking about.
What could be guessed with the current info (and some google query), is that we are dealing with SKL-Y m5-6Y57, codenamed 'Chell' and the speaker-test is playing through an I2S codec connected to SSP0 port. It's probably max98357a. The skylake-driver version you have gets confused with 24/32 format (userspace is probably attempting 32/32). That or topology file is incorrect..
Yes, it's Chell.
BTW do you need other ACPI tables?
..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?
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).
Regards, Czarek
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
在 2022-08-17星期三的 15:19 +0200，Cezary Rojewski写道：
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 how could I fix the topology?
I now use topology files from GalliumOS (which, I assume, is extracted from ChromeOS).
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
participants (3)
-
Cezary Rojewski -
Icenowy Zheng -
Mark Brown