Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[Kuninori Morimoto]

Hi ojab

> > Please take a look, I want my sound back ._.
> 
> Forgot to mention, it's reproducible on linux master git.

Hmm...
This patch 
	1) removes unneeded if check
	2) adds missing error check.

1) shouldn't have effect to your PC.
Thus, maybe your PC failed by 2).
It had been worked by accident before ?
I guess your kernel is telling some message like this ?

	ASoC: no dapm match for %s --> %s --> %s

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[Pierre-Louis Bossart]

>>> Please take a look, I want my sound back ._.
>>
>> Forgot to mention, it's reproducible on linux master git.
> 
> Hmm...
> This patch
> 	1) removes unneeded if check
> 	2) adds missing error check.
> 
> 1) shouldn't have effect to your PC.
> Thus, maybe your PC failed by 2).
> It had been worked by accident before ?
> I guess your kernel is telling some message like this ?
> 
> 	ASoC: no dapm match for %s --> %s --> %s

It's likely that the topology file does not contain all the widgets/dais 
required by the hard-coded routes the machine driver, and by becoming 
stricter warnings became errors which in turn prevent the card from 
being created. It may be that we have to define a backwards compatible 
behavior that's enabled only for Intel legacy cases.

Please post the results of alsa-info.sh somewhere so that we can take a 
look.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[ojab //]

On Wed, Jan 15, 2020 at 4:41 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
> Please post the results of alsa-info.sh somewhere so that we can take a
> look.

5.3.0: http://alsa-project.org/db/?f=d82b3ef237a5050dfb73231ba114e45a6a4420ef
5.4.0: http://alsa-project.org/db/?f=d922b77306a1287eae3624d241f3d46d347c8098

//wbr ojab
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[ojab //]

On Wed, Jan 15, 2020 at 3:23 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> I guess your kernel is telling some message like this ?
>
>         ASoC: no dapm match for %s --> %s --> %s

No such entries on both 5.3 & 5.4 kernels, but I hope needed dmesg
excerpts can be found in alsa-info.sh output.

//wbr ojab
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[Pierre-Louis Bossart]

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]



On 1/15/20 5:57 AM, ojab // wrote:
> On Wed, Jan 15, 2020 at 4:41 AM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>> Please post the results of alsa-info.sh somewhere so that we can take a
>> look.

Thanks, I think that confirms my theory.

> 5.3.0: http://alsa-project.org/db/?f=d82b3ef237a5050dfb73231ba114e45a6a4420ef

In this case, there's a warning on the routes:

[   22.364318] skl_n88l25_s4567 skl_n88l25_s4567: ASoC: Failed to add 
route iDisp3_out -> direct -> iDisp3 Tx

> 5.4.0: http://alsa-project.org/db/?f=d922b77306a1287eae3624d241f3d46d347c8098

And we know that in 5.4 errors on routes caused the card registration to 
stop. I *think* the patch was  daa480bde6b3a9 ("ASoC: soc-core: tidyup 
for snd_soc_dapm_add_routes()")

This patch won't revert cleanly, can you try the following hack on v5.4 
to see if this improves the card registration? Thanks!



[-- Attachment #2: routes.diff --]
[-- Type: text/x-patch, Size: 1107 bytes --]

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 88978a3036c4..796d14feab4d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1076,8 +1076,8 @@ static int soc_probe_component(struct snd_soc_card *card,
 	ret = snd_soc_dapm_add_routes(dapm,
 				      component->driver->dapm_routes,
 				      component->driver->num_dapm_routes);
-	if (ret < 0)
-		goto err_probe;
+	//if (ret < 0)
+	//	goto err_probe;
 
 	/* see for_each_card_components */
 	list_add(&component->card_list, &card->component_dev_list);
@@ -2065,13 +2065,13 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 
 	ret = snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes,
 				      card->num_dapm_routes);
-	if (ret < 0)
-		goto probe_end;
+	//if (ret < 0)
+	//	goto probe_end;
 
 	ret = snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes,
-				      card->num_of_dapm_routes);
-	if (ret < 0)
-		goto probe_end;
+					      card->num_of_dapm_routes);
+	//if (ret < 0)
+	//	goto probe_end;
 
 	/* try to set some sane longname if DMI is available */
 	snd_soc_set_dmi_name(card, NULL);

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[ojab //]

On Wed, Jan 15, 2020 at 6:07 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
> This patch won't revert cleanly, can you try the following hack on v5.4
> to see if this improves the card registration? Thanks!


Yep, it works with the patch.

//wbr ojab
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[Pierre-Louis Bossart]

>> This patch won't revert cleanly, can you try the following hack on v5.4
>> to see if this improves the card registration? Thanks!
> 
> 
> Yep, it works with the patch.

ok, thanks for testing! So that leaves us with two options:

a) remove the error handling after soc_dapm_add_routes() to be backwards 
compatible with Intel problematic machine drivers. Not really nice for 
everyone else.

b) remove this error conditionally so that legacy Intel solutions still 
work but new ones deal with errors upfront.

I am not sure if there's a 'clean' way to implement b), maybe with a 
Kconfig selected by some machine drivers? Morimito-san, any suggestions 
now that we've root caused the problem (copied again below for reference)?


diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 88978a3036c4..796d14feab4d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1076,8 +1076,8 @@ static int soc_probe_component(struct snd_soc_card 
*card,
         ret = snd_soc_dapm_add_routes(dapm,
                                       component->driver->dapm_routes,
                                       component->driver->num_dapm_routes);
-       if (ret < 0)
-               goto err_probe;
+       //if (ret < 0)
+       //      goto err_probe;

         /* see for_each_card_components */
         list_add(&component->card_list, &card->component_dev_list);
@@ -2065,13 +2065,13 @@ static int snd_soc_instantiate_card(struct 
snd_soc_card *card)

         ret = snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes,
                                       card->num_dapm_routes);
-       if (ret < 0)
-               goto probe_end;
+       //if (ret < 0)
+       //      goto probe_end;

         ret = snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes,
-                                     card->num_of_dapm_routes);
-       if (ret < 0)
-               goto probe_end;
+                                             card->num_of_dapm_routes);
+       //if (ret < 0)
+       //      goto probe_end;

         /* try to set some sane longname if DMI is available */
         snd_soc_set_dmi_name(card, NULL);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[Kuninori Morimoto]

Hi Pierre-Louis
Cc Mark

> a) remove the error handling after soc_dapm_add_routes() to be
> backwards compatible with Intel problematic machine drivers. Not
> really nice for everyone else.
> 
> b) remove this error conditionally so that legacy Intel solutions
> still work but new ones deal with errors upfront.
> 
> I am not sure if there's a 'clean' way to implement b), maybe with a
> Kconfig selected by some machine drivers? Morimito-san, any
> suggestions now that we've root caused the problem (copied again below
> for reference)?

Is it possible to *real* fixup in the future, some day ?
We need correct code, but backward compatibility is also needed.
Not only Intel but some machine might have similar issue,
so, how about to indicate WARNING, and comment ?

Like this

         ret = snd_soc_dapm_add_routes(dapm,
                                       component->driver->dapm_routes,
                                       component->driver->num_dapm_routes);
        if (ret < 0)
 -               goto err_probe;
 +               /*
 +                * This is error, but legacy Intel want
 +                * compatibility until xxxx.
 +                * call "goto err_probe" after that
 +                */
 +               dev_warn(dev, "DAPM route add failed.")

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[Pierre-Louis Bossart]

>> a) remove the error handling after soc_dapm_add_routes() to be
>> backwards compatible with Intel problematic machine drivers. Not
>> really nice for everyone else.
>>
>> b) remove this error conditionally so that legacy Intel solutions
>> still work but new ones deal with errors upfront.
>>
>> I am not sure if there's a 'clean' way to implement b), maybe with a
>> Kconfig selected by some machine drivers? Morimito-san, any
>> suggestions now that we've root caused the problem (copied again below
>> for reference)?
> 
> Is it possible to *real* fixup in the future, some day ?

Quite unlikely I am afraid. This is a mismatch between topology and 
machine driver, and there are no planned updates of those topologies 
(which were never released for upstream use).

In this case I guess the topology file was taken from a ChromeOS 
distribution (based on v4.4 IIRC), so there's really no guarantee that a 
fix would ever reach the user without a convoluted set of manual updates.

> We need correct code, but backward compatibility is also needed.
> Not only Intel but some machine might have similar issue,
> so, how about to indicate WARNING, and comment ?
> 
> Like this
> 
>           ret = snd_soc_dapm_add_routes(dapm,
>                                         component->driver->dapm_routes,
>                                         component->driver->num_dapm_routes);
>          if (ret < 0)
>   -               goto err_probe;
>   +               /*
>   +                * This is error, but legacy Intel want
>   +                * compatibility until xxxx.
>   +                * call "goto err_probe" after that
>   +                */
>   +               dev_warn(dev, "DAPM route add failed.")

we already had a warning before:

[   22.364318] skl_n88l25_s4567 skl_n88l25_s4567: ASoC: Failed to add 
route iDisp3_out -> direct -> iDisp3 Tx

and it was ignored since there was no functional impact....

Maybe we could have some sort of boolean flag in the component->driver 
definition and explicitly request a backwards-compatible behavior (e.g. 
for all SKL/KBL machine drivers) when that driver is known to be flaky. 
There's already things like 'fully_routed', maybe we can add something 
such as 'disable_route_check'?
		
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[Kuninori Morimoto]

Hi Pierre-Louis, Mark

> > Is it possible to *real* fixup in the future, some day ?
> 
> Quite unlikely I am afraid. This is a mismatch between topology and
> machine driver, and there are no planned updates of those topologies
> (which were never released for upstream use).
> 
> In this case I guess the topology file was taken from a ChromeOS
> distribution (based on v4.4 IIRC), so there's really no guarantee that
> a fix would ever reach the user without a convoluted set of manual
> updates.
(snip)
> Maybe we could have some sort of boolean flag in the component->driver
> definition and explicitly request a backwards-compatible behavior
> (e.g. for all SKL/KBL machine drivers) when that driver is known to be
> flaky. There's already things like 'fully_routed', maybe we can add
> something such as 'disable_route_check'?

Hmm...
I have no idea, but have no objection about this.


Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 482 bytes --]

On Wed, Jan 15, 2020 at 07:04:48PM -0600, Pierre-Louis Bossart wrote:

> Maybe we could have some sort of boolean flag in the component->driver
> definition and explicitly request a backwards-compatible behavior (e.g. for
> all SKL/KBL machine drivers) when that driver is known to be flaky. There's
> already things like 'fully_routed', maybe we can add something such as
> 'disable_route_check'?

A quirk for old stuff that can't be fixed sounds like a sensible
solution to this.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[ojab //]

On Thu, Jan 16, 2020 at 5:57 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Jan 15, 2020 at 07:04:48PM -0600, Pierre-Louis Bossart wrote:
>
> > Maybe we could have some sort of boolean flag in the component->driver
> > definition and explicitly request a backwards-compatible behavior (e.g. for
> > all SKL/KBL machine drivers) when that driver is known to be flaky. There's
> > already things like 'fully_routed', maybe we can add something such as
> > 'disable_route_check'?
>
> A quirk for old stuff that can't be fixed sounds like a sensible
> solution to this.

Any update on this?

//wbr ojab
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[Pierre-Louis Bossart]

On 2/17/20 9:10 AM, ojab // wrote:
> On Thu, Jan 16, 2020 at 5:57 PM Mark Brown <broonie@kernel.org> wrote:
>>
>> On Wed, Jan 15, 2020 at 07:04:48PM -0600, Pierre-Louis Bossart wrote:
>>
>>> Maybe we could have some sort of boolean flag in the component->driver
>>> definition and explicitly request a backwards-compatible behavior (e.g. for
>>> all SKL/KBL machine drivers) when that driver is known to be flaky. There's
>>> already things like 'fully_routed', maybe we can add something such as
>>> 'disable_route_check'?
>>
>> A quirk for old stuff that can't be fixed sounds like a sensible
>> solution to this.
> 
> Any update on this?

Sorry, this is what I had in mind (not even compile-tested). I don't 
know if the checks need to be disabled twice.

diff --git a/include/sound/soc.h b/include/sound/soc.h
index f0e4f36f83bf..7a4643d87e39 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1058,6 +1058,7 @@ struct snd_soc_card {
         const struct snd_soc_dapm_route *of_dapm_routes;
         int num_of_dapm_routes;
         bool fully_routed;
+       bool disable_route_checks;

         /* lists of probed devices belonging to this card */
         struct list_head component_dev_list;
diff --git a/sound/soc/intel/boards/skl_nau88l25_max98357a.c 
b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
index e6de3b28d840..b57f55731390 100644
--- a/sound/soc/intel/boards/skl_nau88l25_max98357a.c
+++ b/sound/soc/intel/boards/skl_nau88l25_max98357a.c
@@ -644,6 +644,7 @@ static struct snd_soc_card skylake_audio_card = {
         .num_dapm_routes = ARRAY_SIZE(skylake_map),
         .fully_routed = true,
         .late_probe = skylake_card_late_probe,
+       .disable_route_checks = true,
  };

  static int skylake_audio_probe(struct platform_device *pdev)
diff --git a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c 
b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
index c99c8b23e509..8f3b724fec27 100644
--- a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
+++ b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c
@@ -687,6 +687,7 @@ static struct snd_soc_card skylake_audio_card = {
         .num_configs = ARRAY_SIZE(ssm4567_codec_conf),
         .fully_routed = true,
         .late_probe = skylake_card_late_probe,
+       .disable_route_checks = true,
  };

  static int skylake_audio_probe(struct platform_device *pdev)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 068d809c349a..d56cb655d89c 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1257,7 +1257,10 @@ static int soc_probe_component(struct 
snd_soc_card *card,
                                       component->driver->dapm_routes,
                                       component->driver->num_dapm_routes);
         if (ret < 0)
-               goto err_probe;
+               if (card->disable_route_checks)
+                       dev_info(component->dev, "Ignoring errors on 
snd_soc_dapm_add_routes\n");
+               else
+                       goto err_probe;

         /* see for_each_card_components */
         list_add(&component->card_list, &card->component_dev_list);
@@ -1939,7 +1942,10 @@ static int snd_soc_bind_card(struct snd_soc_card 
*card)
         ret = snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes,
                                       card->num_dapm_routes);
         if (ret < 0)
-               goto probe_end;
+               if (card->disable_route_checks)
+                       dev_info(component->dev, "Ignoring errors on 
snd_soc_dapm_add_routes\n");
+               else
+                       goto probe_end;

         ret = snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes,
                                       card->num_of_dapm_routes);

Re: [alsa-devel] No sound since 5.4 on skl_n88l25_s4567

[ojab //]

On Tue, Feb 18, 2020 at 8:24 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
>
> On 2/17/20 9:10 AM, ojab // wrote:
> > On Thu, Jan 16, 2020 at 5:57 PM Mark Brown <broonie@kernel.org> wrote:
> >>
> >> On Wed, Jan 15, 2020 at 07:04:48PM -0600, Pierre-Louis Bossart wrote:
> >>
> >>> Maybe we could have some sort of boolean flag in the component->driver
> >>> definition and explicitly request a backwards-compatible behavior (e.g. for
> >>> all SKL/KBL machine drivers) when that driver is known to be flaky. There's
> >>> already things like 'fully_routed', maybe we can add something such as
> >>> 'disable_route_check'?
> >>
> >> A quirk for old stuff that can't be fixed sounds like a sensible
> >> solution to this.
> >
> > Any update on this?
>
> Sorry, this is what I had in mind (not even compile-tested). I don't
> know if the checks need to be disabled twice.

Unfortunately no sound with this patch on top of 5.6-rc2
http://alsa-project.org/db/?f=0f5049d8f7dc906bd13f354056282a48d2e626cc

//wbr ojab