[PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Mateusz Gorski]

[ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]

Add alternative topology binary file name based on used machine driver
and fallback to use this name after failed attempt to load topology file
with name based on NHLT.
This change addresses multiple issues with current mechanism, for
example - there are devices without NHLT table, and that currently
results in tplg_name being empty.

Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200427132727.24942-2-mateusz.gorski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---

This functionality is merged on upstream kernel and widely used. Merging
it to LTS kernel would improve the user experience and resolve some of the
problems regarding topology naming that the users are facing.

 sound/soc/intel/skylake/skl-topology.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 69cd7a81bf2a..4b114ece58c6 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -14,6 +14,7 @@
 #include <linux/uuid.h>
 #include <sound/intel-nhlt.h>
 #include <sound/soc.h>
+#include <sound/soc-acpi.h>
 #include <sound/soc-topology.h>
 #include <uapi/sound/snd_sst_tokens.h>
 #include <uapi/sound/skl-tplg-interface.h>
@@ -3565,8 +3566,20 @@ int skl_tplg_init(struct snd_soc_component *component, struct hdac_bus *bus)
 
 	ret = request_firmware(&fw, skl->tplg_name, bus->dev);
 	if (ret < 0) {
-		dev_info(bus->dev, "tplg fw %s load failed with %d, falling back to dfw_sst.bin",
-				skl->tplg_name, ret);
+		char alt_tplg_name[64];
+
+		snprintf(alt_tplg_name, sizeof(alt_tplg_name), "%s-tplg.bin",
+			 skl->mach->drv_name);
+		dev_info(bus->dev, "tplg fw %s load failed with %d, trying alternative tplg name %s",
+			 skl->tplg_name, ret, alt_tplg_name);
+
+		ret = request_firmware(&fw, alt_tplg_name, bus->dev);
+		if (!ret)
+			goto component_load;
+
+		dev_info(bus->dev, "tplg %s failed with %d, falling back to dfw_sst.bin",
+			 alt_tplg_name, ret);
+
 		ret = request_firmware(&fw, "dfw_sst.bin", bus->dev);
 		if (ret < 0) {
 			dev_err(bus->dev, "Fallback tplg fw %s load failed with %d\n",
@@ -3575,6 +3588,8 @@ int skl_tplg_init(struct snd_soc_component *component, struct hdac_bus *bus)
 		}
 	}
 
+component_load:
+
 	/*
 	 * The complete tplg for SKL is loaded as index 0, we don't use
 	 * any other index
-- 
2.17.1

[PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Mateusz Gorski]

[ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]

Add alternative topology binary file name based on used machine driver
and fallback to use this name after failed attempt to load topology file
with name based on NHLT.
This change addresses multiple issues with current mechanism, for
example - there are devices without NHLT table, and that currently
results in tplg_name being empty.

Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200427132727.24942-2-mateusz.gorski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---

This functionality is merged on upstream kernel and widely used. Merging
it to LTS kernel would improve the user experience and resolve some of the
problems regarding topology naming that the users are facing.

 sound/soc/intel/skylake/skl-topology.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 69cd7a81bf2a..4b114ece58c6 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -14,6 +14,7 @@
 #include <linux/uuid.h>
 #include <sound/intel-nhlt.h>
 #include <sound/soc.h>
+#include <sound/soc-acpi.h>
 #include <sound/soc-topology.h>
 #include <uapi/sound/snd_sst_tokens.h>
 #include <uapi/sound/skl-tplg-interface.h>
@@ -3565,8 +3566,20 @@ int skl_tplg_init(struct snd_soc_component *component, struct hdac_bus *bus)
 
 	ret = request_firmware(&fw, skl->tplg_name, bus->dev);
 	if (ret < 0) {
-		dev_info(bus->dev, "tplg fw %s load failed with %d, falling back to dfw_sst.bin",
-				skl->tplg_name, ret);
+		char alt_tplg_name[64];
+
+		snprintf(alt_tplg_name, sizeof(alt_tplg_name), "%s-tplg.bin",
+			 skl->mach->drv_name);
+		dev_info(bus->dev, "tplg fw %s load failed with %d, trying alternative tplg name %s",
+			 skl->tplg_name, ret, alt_tplg_name);
+
+		ret = request_firmware(&fw, alt_tplg_name, bus->dev);
+		if (!ret)
+			goto component_load;
+
+		dev_info(bus->dev, "tplg %s failed with %d, falling back to dfw_sst.bin",
+			 alt_tplg_name, ret);
+
 		ret = request_firmware(&fw, "dfw_sst.bin", bus->dev);
 		if (ret < 0) {
 			dev_err(bus->dev, "Fallback tplg fw %s load failed with %d\n",
@@ -3575,6 +3588,8 @@ int skl_tplg_init(struct snd_soc_component *component, struct hdac_bus *bus)
 		}
 	}
 
+component_load:
+
 	/*
 	 * The complete tplg for SKL is loaded as index 0, we don't use
 	 * any other index
-- 
2.17.1

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Greg KH]

On Tue, Nov 03, 2020 at 03:10:47PM +0100, Mateusz Gorski wrote:
> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
> 
> Add alternative topology binary file name based on used machine driver
> and fallback to use this name after failed attempt to load topology file
> with name based on NHLT.
> This change addresses multiple issues with current mechanism, for
> example - there are devices without NHLT table, and that currently
> results in tplg_name being empty.
> 
> Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Link: https://lore.kernel.org/r/20200427132727.24942-2-mateusz.gorski@linux.intel.com
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> 
> This functionality is merged on upstream kernel and widely used. Merging
> it to LTS kernel would improve the user experience and resolve some of the
> problems regarding topology naming that the users are facing.

What problems are people facing, and what kernel(s) are you asking for
this to be ported to, and why can't people just use 5.8 or newer if they
have this new hardware?

thanks,

greg k-h

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Greg KH]

On Tue, Nov 03, 2020 at 03:10:47PM +0100, Mateusz Gorski wrote:
> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
> 
> Add alternative topology binary file name based on used machine driver
> and fallback to use this name after failed attempt to load topology file
> with name based on NHLT.
> This change addresses multiple issues with current mechanism, for
> example - there are devices without NHLT table, and that currently
> results in tplg_name being empty.
> 
> Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Link: https://lore.kernel.org/r/20200427132727.24942-2-mateusz.gorski@linux.intel.com
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> 
> This functionality is merged on upstream kernel and widely used. Merging
> it to LTS kernel would improve the user experience and resolve some of the
> problems regarding topology naming that the users are facing.

What problems are people facing, and what kernel(s) are you asking for
this to be ported to, and why can't people just use 5.8 or newer if they
have this new hardware?

thanks,

greg k-h

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Gorski, Mateusz]

>> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
>>
>> Add alternative topology binary file name based on used machine driver
>> and fallback to use this name after failed attempt to load topology file
>> with name based on NHLT.
>> This change addresses multiple issues with current mechanism, for
>> example - there are devices without NHLT table, and that currently
>> results in tplg_name being empty.
>>
>> Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
>> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
>> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> Link: https://lore.kernel.org/r/20200427132727.24942-2-mateusz.gorski@linux.intel.com
>> Signed-off-by: Mark Brown <broonie@kernel.org>
>> ---
>>
>> This functionality is merged on upstream kernel and widely used. Merging
>> it to LTS kernel would improve the user experience and resolve some of the
>> problems regarding topology naming that the users are facing.
> What problems are people facing, and what kernel(s) are you asking for
> this to be ported to, and why can't people just use 5.8 or newer if they
> have this new hardware?
>
> thanks,
>
> greg k-h

I forgot to add - I wanted this change to be merged to stable 5.4 
kernel. Please let me know if I should resend this patch with this 
information included.

As for the user issues - topology binary file name is currently created 
according to information from NHLT. The problem is, that some laptops 
(for example Dell XPS 13) do not have NHLT at all. This results in 
topology binary name being empty (" ").
This patch adds alternative name based on loaded machine driver.

It applies not only to new hardware, please note that the mentioned Dell 
XPS 13 is based on Kabylake. This issue existed on upstream from the 
beginning of Skylake driver and was only recently addressed.

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Gorski, Mateusz]

>> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
>>
>> Add alternative topology binary file name based on used machine driver
>> and fallback to use this name after failed attempt to load topology file
>> with name based on NHLT.
>> This change addresses multiple issues with current mechanism, for
>> example - there are devices without NHLT table, and that currently
>> results in tplg_name being empty.
>>
>> Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
>> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
>> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> Link: https://lore.kernel.org/r/20200427132727.24942-2-mateusz.gorski@linux.intel.com
>> Signed-off-by: Mark Brown <broonie@kernel.org>
>> ---
>>
>> This functionality is merged on upstream kernel and widely used. Merging
>> it to LTS kernel would improve the user experience and resolve some of the
>> problems regarding topology naming that the users are facing.
> What problems are people facing, and what kernel(s) are you asking for
> this to be ported to, and why can't people just use 5.8 or newer if they
> have this new hardware?
>
> thanks,
>
> greg k-h

I forgot to add - I wanted this change to be merged to stable 5.4 
kernel. Please let me know if I should resend this patch with this 
information included.

As for the user issues - topology binary file name is currently created 
according to information from NHLT. The problem is, that some laptops 
(for example Dell XPS 13) do not have NHLT at all. This results in 
topology binary name being empty (" ").
This patch adds alternative name based on loaded machine driver.

It applies not only to new hardware, please note that the mentioned Dell 
XPS 13 is based on Kabylake. This issue existed on upstream from the 
beginning of Skylake driver and was only recently addressed.

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Greg KH]

On Wed, Nov 04, 2020 at 12:46:36PM +0100, Gorski, Mateusz wrote:
> 
> > > [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
> > > 
> > > Add alternative topology binary file name based on used machine driver
> > > and fallback to use this name after failed attempt to load topology file
> > > with name based on NHLT.
> > > This change addresses multiple issues with current mechanism, for
> > > example - there are devices without NHLT table, and that currently
> > > results in tplg_name being empty.
> > > 
> > > Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
> > > Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
> > > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > Link: https://lore.kernel.org/r/20200427132727.24942-2-mateusz.gorski@linux.intel.com
> > > Signed-off-by: Mark Brown <broonie@kernel.org>
> > > ---
> > > 
> > > This functionality is merged on upstream kernel and widely used. Merging
> > > it to LTS kernel would improve the user experience and resolve some of the
> > > problems regarding topology naming that the users are facing.
> > What problems are people facing, and what kernel(s) are you asking for
> > this to be ported to, and why can't people just use 5.8 or newer if they
> > have this new hardware?
> > 
> > thanks,
> > 
> > greg k-h
> 
> I forgot to add - I wanted this change to be merged to stable 5.4 kernel.
> Please let me know if I should resend this patch with this information
> included.
> 
> As for the user issues - topology binary file name is currently created
> according to information from NHLT. The problem is, that some laptops (for
> example Dell XPS 13) do not have NHLT at all. This results in topology
> binary name being empty (" ").
> This patch adds alternative name based on loaded machine driver.
> 
> It applies not only to new hardware, please note that the mentioned Dell XPS
> 13 is based on Kabylake. This issue existed on upstream from the beginning
> of Skylake driver and was only recently addressed.

When was that laptop released and is this the only change that is needed
in order for the 5.4.y kernel to work properly on it?

thanks,

greg k-h

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Greg KH]

On Wed, Nov 04, 2020 at 12:46:36PM +0100, Gorski, Mateusz wrote:
> 
> > > [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
> > > 
> > > Add alternative topology binary file name based on used machine driver
> > > and fallback to use this name after failed attempt to load topology file
> > > with name based on NHLT.
> > > This change addresses multiple issues with current mechanism, for
> > > example - there are devices without NHLT table, and that currently
> > > results in tplg_name being empty.
> > > 
> > > Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
> > > Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
> > > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > Link: https://lore.kernel.org/r/20200427132727.24942-2-mateusz.gorski@linux.intel.com
> > > Signed-off-by: Mark Brown <broonie@kernel.org>
> > > ---
> > > 
> > > This functionality is merged on upstream kernel and widely used. Merging
> > > it to LTS kernel would improve the user experience and resolve some of the
> > > problems regarding topology naming that the users are facing.
> > What problems are people facing, and what kernel(s) are you asking for
> > this to be ported to, and why can't people just use 5.8 or newer if they
> > have this new hardware?
> > 
> > thanks,
> > 
> > greg k-h
> 
> I forgot to add - I wanted this change to be merged to stable 5.4 kernel.
> Please let me know if I should resend this patch with this information
> included.
> 
> As for the user issues - topology binary file name is currently created
> according to information from NHLT. The problem is, that some laptops (for
> example Dell XPS 13) do not have NHLT at all. This results in topology
> binary name being empty (" ").
> This patch adds alternative name based on loaded machine driver.
> 
> It applies not only to new hardware, please note that the mentioned Dell XPS
> 13 is based on Kabylake. This issue existed on upstream from the beginning
> of Skylake driver and was only recently addressed.

When was that laptop released and is this the only change that is needed
in order for the 5.4.y kernel to work properly on it?

thanks,

greg k-h

RE: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Rojewski, Cezary]

On 2020-11-04 12:58 PM, Greg KH wrote:
> On Wed, Nov 04, 2020 at 12:46:36PM +0100, Gorski, Mateusz wrote:
>>
>>>> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
>>>>
>>>> Add alternative topology binary file name based on used machine driver
>>>> and fallback to use this name after failed attempt to load topology file
>>>> with name based on NHLT.
>>>> This change addresses multiple issues with current mechanism, for
>>>> example - there are devices without NHLT table, and that currently
>>>> results in tplg_name being empty.
...

>>> What problems are people facing, and what kernel(s) are you asking for
>>> this to be ported to, and why can't people just use 5.8 or newer if they
>>> have this new hardware?
>>>
>>
>> I forgot to add - I wanted this change to be merged to stable 5.4 kernel.
>> Please let me know if I should resend this patch with this information
>> included.
>>
>> As for the user issues - topology binary file name is currently created
>> according to information from NHLT. The problem is, that some laptops (for
>> example Dell XPS 13) do not have NHLT at all. This results in topology
>> binary name being empty (" ").
>> This patch adds alternative name based on loaded machine driver.
>>
>> It applies not only to new hardware, please note that the mentioned Dell XPS
>> 13 is based on Kabylake. This issue existed on upstream from the beginning
>> of Skylake driver and was only recently addressed.
> 
> When was that laptop released and is this the only change that is needed
> in order for the 5.4.y kernel to work properly on it?
> 

Sorry for the late answer, Greg. To address your concerns and questions
let me elaborate:

Indeed, this change is not the only one required to enable DMIC + HDA
configuration for customers. The following series is essential:

[PATCH 0/7] ASoC: Intel: Skylake: Fix HDaudio and Dmic
https://lore.kernel.org/alsa-devel/20200305145314.32579-1-cezary.rojewski@intel.com/

as it's not just enabling said configuration, it is fixing several
issues along the road with /skylake and HDA. And the issues are
certainly not limited to just Dell XPS 13, these trouble many
SKL/KBL/AML laptop models available on the market - those where AudioDSP
configuration is enabled by default.

Backport presented in this very patch provided by Mateusz touches on yet
another subject. More often than not, incomplete -or- incorrect topology
binary is the cause of configuration issues and updating it usually
resolves them. Unfortunately, user needed to be very precise when naming
the new topology file. Name is quite complicated as it is based on data
coming from NHLT's (ACPI table) header. Asking user to either provide
their NHLT or decode the header themselves is inconvenient to say
the least. And thus alternative-topology-name patch came to life.

I'll check what's missing on v5.4.y and get to porting mentioned series.
Should happen early next week.

Thanks,
Czarek

RE: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Rojewski, Cezary]

On 2020-11-04 12:58 PM, Greg KH wrote:
> On Wed, Nov 04, 2020 at 12:46:36PM +0100, Gorski, Mateusz wrote:
>>
>>>> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
>>>>
>>>> Add alternative topology binary file name based on used machine driver
>>>> and fallback to use this name after failed attempt to load topology file
>>>> with name based on NHLT.
>>>> This change addresses multiple issues with current mechanism, for
>>>> example - there are devices without NHLT table, and that currently
>>>> results in tplg_name being empty.
...

>>> What problems are people facing, and what kernel(s) are you asking for
>>> this to be ported to, and why can't people just use 5.8 or newer if they
>>> have this new hardware?
>>>
>>
>> I forgot to add - I wanted this change to be merged to stable 5.4 kernel.
>> Please let me know if I should resend this patch with this information
>> included.
>>
>> As for the user issues - topology binary file name is currently created
>> according to information from NHLT. The problem is, that some laptops (for
>> example Dell XPS 13) do not have NHLT at all. This results in topology
>> binary name being empty (" ").
>> This patch adds alternative name based on loaded machine driver.
>>
>> It applies not only to new hardware, please note that the mentioned Dell XPS
>> 13 is based on Kabylake. This issue existed on upstream from the beginning
>> of Skylake driver and was only recently addressed.
> 
> When was that laptop released and is this the only change that is needed
> in order for the 5.4.y kernel to work properly on it?
> 

Sorry for the late answer, Greg. To address your concerns and questions
let me elaborate:

Indeed, this change is not the only one required to enable DMIC + HDA
configuration for customers. The following series is essential:

[PATCH 0/7] ASoC: Intel: Skylake: Fix HDaudio and Dmic
https://lore.kernel.org/alsa-devel/20200305145314.32579-1-cezary.rojewski@intel.com/

as it's not just enabling said configuration, it is fixing several
issues along the road with /skylake and HDA. And the issues are
certainly not limited to just Dell XPS 13, these trouble many
SKL/KBL/AML laptop models available on the market - those where AudioDSP
configuration is enabled by default.

Backport presented in this very patch provided by Mateusz touches on yet
another subject. More often than not, incomplete -or- incorrect topology
binary is the cause of configuration issues and updating it usually
resolves them. Unfortunately, user needed to be very precise when naming
the new topology file. Name is quite complicated as it is based on data
coming from NHLT's (ACPI table) header. Asking user to either provide
their NHLT or decode the header themselves is inconvenient to say
the least. And thus alternative-topology-name patch came to life.

I'll check what's missing on v5.4.y and get to porting mentioned series.
Should happen early next week.

Thanks,
Czarek

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Greg KH]

On Sun, Nov 08, 2020 at 04:17:16PM +0000, Rojewski, Cezary wrote:
> On 2020-11-04 12:58 PM, Greg KH wrote:
> > On Wed, Nov 04, 2020 at 12:46:36PM +0100, Gorski, Mateusz wrote:
> >>
> >>>> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
> >>>>
> >>>> Add alternative topology binary file name based on used machine driver
> >>>> and fallback to use this name after failed attempt to load topology file
> >>>> with name based on NHLT.
> >>>> This change addresses multiple issues with current mechanism, for
> >>>> example - there are devices without NHLT table, and that currently
> >>>> results in tplg_name being empty.
> ...
> 
> >>> What problems are people facing, and what kernel(s) are you asking for
> >>> this to be ported to, and why can't people just use 5.8 or newer if they
> >>> have this new hardware?
> >>>
> >>
> >> I forgot to add - I wanted this change to be merged to stable 5.4 kernel.
> >> Please let me know if I should resend this patch with this information
> >> included.
> >>
> >> As for the user issues - topology binary file name is currently created
> >> according to information from NHLT. The problem is, that some laptops (for
> >> example Dell XPS 13) do not have NHLT at all. This results in topology
> >> binary name being empty (" ").
> >> This patch adds alternative name based on loaded machine driver.
> >>
> >> It applies not only to new hardware, please note that the mentioned Dell XPS
> >> 13 is based on Kabylake. This issue existed on upstream from the beginning
> >> of Skylake driver and was only recently addressed.
> > 
> > When was that laptop released and is this the only change that is needed
> > in order for the 5.4.y kernel to work properly on it?
> > 
> 
> Sorry for the late answer, Greg. To address your concerns and questions
> let me elaborate:
> 
> Indeed, this change is not the only one required to enable DMIC + HDA
> configuration for customers. The following series is essential:
> 
> [PATCH 0/7] ASoC: Intel: Skylake: Fix HDaudio and Dmic
> https://lore.kernel.org/alsa-devel/20200305145314.32579-1-cezary.rojewski@intel.com/

Great, then they should just use a newer kernel version.  It's crazy to
think that you can go back in time and get older kernels working for
newer hardware :)

thanks,

greg k-h

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Greg KH]

On Sun, Nov 08, 2020 at 04:17:16PM +0000, Rojewski, Cezary wrote:
> On 2020-11-04 12:58 PM, Greg KH wrote:
> > On Wed, Nov 04, 2020 at 12:46:36PM +0100, Gorski, Mateusz wrote:
> >>
> >>>> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
> >>>>
> >>>> Add alternative topology binary file name based on used machine driver
> >>>> and fallback to use this name after failed attempt to load topology file
> >>>> with name based on NHLT.
> >>>> This change addresses multiple issues with current mechanism, for
> >>>> example - there are devices without NHLT table, and that currently
> >>>> results in tplg_name being empty.
> ...
> 
> >>> What problems are people facing, and what kernel(s) are you asking for
> >>> this to be ported to, and why can't people just use 5.8 or newer if they
> >>> have this new hardware?
> >>>
> >>
> >> I forgot to add - I wanted this change to be merged to stable 5.4 kernel.
> >> Please let me know if I should resend this patch with this information
> >> included.
> >>
> >> As for the user issues - topology binary file name is currently created
> >> according to information from NHLT. The problem is, that some laptops (for
> >> example Dell XPS 13) do not have NHLT at all. This results in topology
> >> binary name being empty (" ").
> >> This patch adds alternative name based on loaded machine driver.
> >>
> >> It applies not only to new hardware, please note that the mentioned Dell XPS
> >> 13 is based on Kabylake. This issue existed on upstream from the beginning
> >> of Skylake driver and was only recently addressed.
> > 
> > When was that laptop released and is this the only change that is needed
> > in order for the 5.4.y kernel to work properly on it?
> > 
> 
> Sorry for the late answer, Greg. To address your concerns and questions
> let me elaborate:
> 
> Indeed, this change is not the only one required to enable DMIC + HDA
> configuration for customers. The following series is essential:
> 
> [PATCH 0/7] ASoC: Intel: Skylake: Fix HDaudio and Dmic
> https://lore.kernel.org/alsa-devel/20200305145314.32579-1-cezary.rojewski@intel.com/

Great, then they should just use a newer kernel version.  It's crazy to
think that you can go back in time and get older kernels working for
newer hardware :)

thanks,

greg k-h

RE: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Rojewski, Cezary]

On 2020-11-08 6:00 PM, Greg KH wrote:
> On Sun, Nov 08, 2020 at 04:17:16PM +0000, Rojewski, Cezary wrote:
>> On 2020-11-04 12:58 PM, Greg KH wrote:
>>> On Wed, Nov 04, 2020 at 12:46:36PM +0100, Gorski, Mateusz wrote:
>>>>
>>>>>> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
>>>>>>
>>>>>> Add alternative topology binary file name based on used machine driver
>>>>>> and fallback to use this name after failed attempt to load topology file
>>>>>> with name based on NHLT.
>>>>>> This change addresses multiple issues with current mechanism, for
>>>>>> example - there are devices without NHLT table, and that currently
>>>>>> results in tplg_name being empty.
...

>>>>> What problems are people facing, and what kernel(s) are you asking for
>>>>> this to be ported to, and why can't people just use 5.8 or newer if they
>>>>> have this new hardware?
>>>>>
>>>>
>>>> I forgot to add - I wanted this change to be merged to stable 5.4 kernel.
>>>> Please let me know if I should resend this patch with this information
>>>> included.
>>>>
>>>> As for the user issues - topology binary file name is currently created
>>>> according to information from NHLT. The problem is, that some laptops (for
>>>> example Dell XPS 13) do not have NHLT at all. This results in topology
>>>> binary name being empty (" ").
>>>> This patch adds alternative name based on loaded machine driver.
>>>>
>>>> It applies not only to new hardware, please note that the mentioned Dell XPS
>>>> 13 is based on Kabylake. This issue existed on upstream from the beginning
>>>> of Skylake driver and was only recently addressed.
>>>
>>> When was that laptop released and is this the only change that is needed
>>> in order for the 5.4.y kernel to work properly on it?
>>>
>>
>> Sorry for the late answer, Greg. To address your concerns and questions
>> let me elaborate:
>>
>> Indeed, this change is not the only one required to enable DMIC + HDA
>> configuration for customers. The following series is essential:
>>
>> [PATCH 0/7] ASoC: Intel: Skylake: Fix HDaudio and Dmic
>> https://lore.kernel.org/alsa-devel/20200305145314.32579-1-cezary.rojewski@intel.com/
> 
> Great, then they should just use a newer kernel version.  It's crazy to
> think that you can go back in time and get older kernels working for
> newer hardware :)

Skylake and Kabylake-based platforms are few years old already, that's
not exactly a "newer hardware". Icelake and such are and these are not
part of /skylake driver. Fact is, this should all be part of 4.19 or
earlier since DMIC + HDA configuration are available even there. And
receiving laptop with such kernel and no patches fails miserably ; (

Unfortunately, kernels 4.19 and below need many more changes than "just"
6 fixes as HDA-generic soundcard isn't available there. That, however,
can easily be called "a new feature" so stopping at 5.4 is a fair call.

Thanks,
Czarek

RE: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Rojewski, Cezary]

On 2020-11-08 6:00 PM, Greg KH wrote:
> On Sun, Nov 08, 2020 at 04:17:16PM +0000, Rojewski, Cezary wrote:
>> On 2020-11-04 12:58 PM, Greg KH wrote:
>>> On Wed, Nov 04, 2020 at 12:46:36PM +0100, Gorski, Mateusz wrote:
>>>>
>>>>>> [ Upstream commit 1b290ef023b3eeb4f4688b582fecb773915ef937 ]
>>>>>>
>>>>>> Add alternative topology binary file name based on used machine driver
>>>>>> and fallback to use this name after failed attempt to load topology file
>>>>>> with name based on NHLT.
>>>>>> This change addresses multiple issues with current mechanism, for
>>>>>> example - there are devices without NHLT table, and that currently
>>>>>> results in tplg_name being empty.
...

>>>>> What problems are people facing, and what kernel(s) are you asking for
>>>>> this to be ported to, and why can't people just use 5.8 or newer if they
>>>>> have this new hardware?
>>>>>
>>>>
>>>> I forgot to add - I wanted this change to be merged to stable 5.4 kernel.
>>>> Please let me know if I should resend this patch with this information
>>>> included.
>>>>
>>>> As for the user issues - topology binary file name is currently created
>>>> according to information from NHLT. The problem is, that some laptops (for
>>>> example Dell XPS 13) do not have NHLT at all. This results in topology
>>>> binary name being empty (" ").
>>>> This patch adds alternative name based on loaded machine driver.
>>>>
>>>> It applies not only to new hardware, please note that the mentioned Dell XPS
>>>> 13 is based on Kabylake. This issue existed on upstream from the beginning
>>>> of Skylake driver and was only recently addressed.
>>>
>>> When was that laptop released and is this the only change that is needed
>>> in order for the 5.4.y kernel to work properly on it?
>>>
>>
>> Sorry for the late answer, Greg. To address your concerns and questions
>> let me elaborate:
>>
>> Indeed, this change is not the only one required to enable DMIC + HDA
>> configuration for customers. The following series is essential:
>>
>> [PATCH 0/7] ASoC: Intel: Skylake: Fix HDaudio and Dmic
>> https://lore.kernel.org/alsa-devel/20200305145314.32579-1-cezary.rojewski@intel.com/
> 
> Great, then they should just use a newer kernel version.  It's crazy to
> think that you can go back in time and get older kernels working for
> newer hardware :)

Skylake and Kabylake-based platforms are few years old already, that's
not exactly a "newer hardware". Icelake and such are and these are not
part of /skylake driver. Fact is, this should all be part of 4.19 or
earlier since DMIC + HDA configuration are available even there. And
receiving laptop with such kernel and no patches fails miserably ; (

Unfortunately, kernels 4.19 and below need many more changes than "just"
6 fixes as HDA-generic soundcard isn't available there. That, however,
can easily be called "a new feature" so stopping at 5.4 is a fair call.

Thanks,
Czarek

[PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Mateusz Gorski]

This commit adds alternative topology binary file name based on used
machine driver and fallback to use this name after failed attempt to
load topology file with name based on NHLT.

Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
---
 sound/soc/intel/skylake/skl-topology.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 69cd7a81bf2a..344b06df0e15 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -3565,8 +3565,20 @@ int skl_tplg_init(struct snd_soc_component *component, struct hdac_bus *bus)
 
 	ret = request_firmware(&fw, skl->tplg_name, bus->dev);
 	if (ret < 0) {
-		dev_info(bus->dev, "tplg fw %s load failed with %d, falling back to dfw_sst.bin",
-				skl->tplg_name, ret);
+		char alt_tplg_name[64];
+
+		snprintf(alt_tplg_name, sizeof(alt_tplg_name), "%s-tplg.bin",
+				skl->i2s_dev->name);
+		dev_info(bus->dev, "tplg fw %s load failed with %d, trying alternative tplg name %s",
+				skl->tplg_name, ret, alt_tplg_name);
+
+		ret = request_firmware(&fw, alt_tplg_name, bus->dev);
+		if (!ret)
+			goto component_load;
+
+		dev_info(bus->dev, "tplg %s failed with %d, falling back to dfw_sst.bin",
+				alt_tplg_name, ret);
+
 		ret = request_firmware(&fw, "dfw_sst.bin", bus->dev);
 		if (ret < 0) {
 			dev_err(bus->dev, "Fallback tplg fw %s load failed with %d\n",
@@ -3575,6 +3587,9 @@ int skl_tplg_init(struct snd_soc_component *component, struct hdac_bus *bus)
 		}
 	}
 
+component_load:
+
+
 	/*
 	 * The complete tplg for SKL is loaded as index 0, we don't use
 	 * any other index
-- 
2.17.1

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Pierre-Louis Bossart]

On 3/25/20 7:22 AM, Mateusz Gorski wrote:
> This commit adds alternative topology binary file name based on used
> machine driver and fallback to use this name after failed attempt to
> load topology file with name based on NHLT.
> 
> Signed-off-by: Mateusz Gorski <mateusz.gorski@linux.intel.com>
> ---
>   sound/soc/intel/skylake/skl-topology.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
> index 69cd7a81bf2a..344b06df0e15 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -3565,8 +3565,20 @@ int skl_tplg_init(struct snd_soc_component *component, struct hdac_bus *bus)
>   
>   	ret = request_firmware(&fw, skl->tplg_name, bus->dev);
>   	if (ret < 0) {
> -		dev_info(bus->dev, "tplg fw %s load failed with %d, falling back to dfw_sst.bin",
> -				skl->tplg_name, ret);
> +		char alt_tplg_name[64];
> +
> +		snprintf(alt_tplg_name, sizeof(alt_tplg_name), "%s-tplg.bin",
> +				skl->i2s_dev->name);

That's progress but is this complete?

skl->i2s_dev->name is the name of the machine driver, I don't see the 
part where this is modified to deal with the number of dmics?

In your topology patches, the names are: hda_dsp_noDMIC hda_dsp_DMIC_2ch 
hda_dsp_DMIC_4ch

How would the relevant file be found based on the number of DMICs on the 
platform? I must be missing something here?

> +		dev_info(bus->dev, "tplg fw %s load failed with %d, trying alternative tplg name %s",
> +				skl->tplg_name, ret, alt_tplg_name);
> +
> +		ret = request_firmware(&fw, alt_tplg_name, bus->dev);
> +		if (!ret)
> +			goto component_load;
> +
> +		dev_info(bus->dev, "tplg %s failed with %d, falling back to dfw_sst.bin",
> +				alt_tplg_name, ret);
> +
>   		ret = request_firmware(&fw, "dfw_sst.bin", bus->dev);
>   		if (ret < 0) {
>   			dev_err(bus->dev, "Fallback tplg fw %s load failed with %d\n",
> @@ -3575,6 +3587,9 @@ int skl_tplg_init(struct snd_soc_component *component, struct hdac_bus *bus)
>   		}
>   	}
>   
> +component_load:
> +
> +
>   	/*
>   	 * The complete tplg for SKL is loaded as index 0, we don't use
>   	 * any other index
> 

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Gorski, Mateusz]

> That's progress but is this complete?
>
> skl->i2s_dev->name is the name of the machine driver, I don't see the 
> part where this is modified to deal with the number of dmics?
>
> In your topology patches, the names are: hda_dsp_noDMIC 
> hda_dsp_DMIC_2ch hda_dsp_DMIC_4ch
>
> How would the relevant file be found based on the number of DMICs on 
> the platform? I must be missing something here?
>

The intention of this patch was not to deal with the different DMIC 
configurations problem. It only simplifies the topology binary 
selection. As you mentioned in one of previous mails, currently there 
are two options:

- name based on NHLT, which is pretty complicated, especially for end 
user, and is also depending on things like OEM name so the same topology 
will need to be renamed multiple times different devices (additionally, 
there are laptops on the market that do not have NHLT table at all)

- dfw_sst.bin, which is only a fallback binary name and should not be 
actually used because it could be misleading for users/distro intergrators

This change adds the third option, which is, in my opinion, the right 
way to deal with this problem. This name is simpler, does not depend on  
existence of NHLT, and makes life easier for users/distros.


And as for the mentioned DMIC confguration problem - I am doing a 
research to find the simplest way to deal with this.


Mateusz

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Pierre-Louis Bossart]

On 3/26/20 10:00 AM, Gorski, Mateusz wrote:
> 
>> That's progress but is this complete?
>>
>> skl->i2s_dev->name is the name of the machine driver, I don't see the 
>> part where this is modified to deal with the number of dmics?
>>
>> In your topology patches, the names are: hda_dsp_noDMIC 
>> hda_dsp_DMIC_2ch hda_dsp_DMIC_4ch
>>
>> How would the relevant file be found based on the number of DMICs on 
>> the platform? I must be missing something here?
>>
> 
> The intention of this patch was not to deal with the different DMIC 
> configurations problem. It only simplifies the topology binary 
> selection. As you mentioned in one of previous mails, currently there 
> are two options:
> 
> - name based on NHLT, which is pretty complicated, especially for end 
> user, and is also depending on things like OEM name so the same topology 
> will need to be renamed multiple times different devices (additionally, 
> there are laptops on the market that do not have NHLT table at all)
> 
> - dfw_sst.bin, which is only a fallback binary name and should not be 
> actually used because it could be misleading for users/distro intergrators
> 
> This change adds the third option, which is, in my opinion, the right 
> way to deal with this problem. This name is simpler, does not depend on 
> existence of NHLT, and makes life easier for users/distros.

Right, and that's fine to avoid the NHLT-name and dfw_sst.bin, no issue 
here. The point is to go one step further and require ZERO configuration 
from users.

> And as for the mentioned DMIC confguration problem - I am doing a 
> research to find the simplest way to deal with this.

Just append the number of mics detected to the topology file name? 
Asking users to copy/symlink hda_dsp_DMIC_2ch.tplg as hda-dsp.tplg 
doesn't really help, you can make things simpler.

Re: [PATCH] ASoC: Intel: Skylake: Add alternative topology binary name

[Gorski, Mateusz]

>>
>> The intention of this patch was not to deal with the different DMIC 
>> configurations problem. It only simplifies the topology binary 
>> selection. As you mentioned in one of previous mails, currently there 
>> are two options:
>>
>> - name based on NHLT, which is pretty complicated, especially for end 
>> user, and is also depending on things like OEM name so the same 
>> topology will need to be renamed multiple times different devices 
>> (additionally, there are laptops on the market that do not have NHLT 
>> table at all)
>>
>> - dfw_sst.bin, which is only a fallback binary name and should not be 
>> actually used because it could be misleading for users/distro 
>> intergrators
>>
>> This change adds the third option, which is, in my opinion, the right 
>> way to deal with this problem. This name is simpler, does not depend 
>> on existence of NHLT, and makes life easier for users/distros.
>
> Right, and that's fine to avoid the NHLT-name and dfw_sst.bin, no 
> issue here. The point is to go one step further and require ZERO 
> configuration from users.
>
>> And as for the mentioned DMIC confguration problem - I am doing a 
>> research to find the simplest way to deal with this.
>
> Just append the number of mics detected to the topology file name? 
> Asking users to copy/symlink hda_dsp_DMIC_2ch.tplg as hda-dsp.tplg 
> doesn't really help, you can make things simpler.


Sent another patch to add path multiconfiguration feature to the driver. 
This way we will have one topology binary for multiple DMIC 
configurations, so we do not need to select different file according to 
its name. Also new version of topology conf was provided, the one with 
multiple configs.


Thanks,
Mateusz