diff mbox series

ASoC: Intel: Skylake: Compile when any configuration is selected

Message ID 20210125115441.10383-1-cezary.rojewski@intel.com (mailing list archive)
State Accepted, archived
Commit cfa0faec5fc0544f84b9c599b6cf49cd3cc709f3
Headers show
Series ASoC: Intel: Skylake: Compile when any configuration is selected | expand

Commit Message

Cezary Rojewski Jan. 25, 2021, 11:54 a.m. UTC
Skylake is dependent on SND_SOC_INTEL_SKYLAKE (aka "all SST platforms")
whereas selecting specific configuration such as KBL-only will not
cause driver code to compile. Switch to SND_SOC_INTEL_SKYLAKE_COMMON
dependency so selecting any configuration causes the driver to be built.

Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Suggested-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Fixes: 35bc99aaa1a3 ("ASoC: Intel: Skylake: Add more platform granularity")
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/Makefile         | 2 +-
 sound/soc/intel/skylake/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Kai-Heng Feng Jan. 27, 2021, 8:03 a.m. UTC | #1
On Mon, Jan 25, 2021 at 7:55 PM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
> Skylake is dependent on SND_SOC_INTEL_SKYLAKE (aka "all SST platforms")
> whereas selecting specific configuration such as KBL-only will not
> cause driver code to compile. Switch to SND_SOC_INTEL_SKYLAKE_COMMON
> dependency so selecting any configuration causes the driver to be built.
>
> Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Suggested-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> Fixes: 35bc99aaa1a3 ("ASoC: Intel: Skylake: Add more platform granularity")
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>

Still not working:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1902457/comments/66

Kai-Heng

> ---
>  sound/soc/intel/Makefile         | 2 +-
>  sound/soc/intel/skylake/Makefile | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/intel/Makefile b/sound/soc/intel/Makefile
> index 4e0248d2accc..7c5038803be7 100644
> --- a/sound/soc/intel/Makefile
> +++ b/sound/soc/intel/Makefile
> @@ -5,7 +5,7 @@ obj-$(CONFIG_SND_SOC) += common/
>  # Platform Support
>  obj-$(CONFIG_SND_SST_ATOM_HIFI2_PLATFORM) += atom/
>  obj-$(CONFIG_SND_SOC_INTEL_CATPT) += catpt/
> -obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE) += skylake/
> +obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE_COMMON) += skylake/
>  obj-$(CONFIG_SND_SOC_INTEL_KEEMBAY) += keembay/
>
>  # Machine support
> diff --git a/sound/soc/intel/skylake/Makefile b/sound/soc/intel/skylake/Makefile
> index dd39149b89b1..1c4649bccec5 100644
> --- a/sound/soc/intel/skylake/Makefile
> +++ b/sound/soc/intel/skylake/Makefile
> @@ -7,7 +7,7 @@ ifdef CONFIG_DEBUG_FS
>    snd-soc-skl-objs += skl-debug.o
>  endif
>
> -obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE) += snd-soc-skl.o
> +obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE_COMMON) += snd-soc-skl.o
>
>  #Skylake Clock device support
>  snd-soc-skl-ssp-clk-objs := skl-ssp-clk.o
> --
> 2.17.1
>
Cezary Rojewski Jan. 27, 2021, 3:22 p.m. UTC | #2
On 2021-01-27 9:03 AM, Kai-Heng Feng wrote:
> On Mon, Jan 25, 2021 at 7:55 PM Cezary Rojewski
> <cezary.rojewski@intel.com> wrote:
>>
>> Skylake is dependent on SND_SOC_INTEL_SKYLAKE (aka "all SST platforms")
>> whereas selecting specific configuration such as KBL-only will not
>> cause driver code to compile. Switch to SND_SOC_INTEL_SKYLAKE_COMMON
>> dependency so selecting any configuration causes the driver to be built.
>>
>> Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> Suggested-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>> Fixes: 35bc99aaa1a3 ("ASoC: Intel: Skylake: Add more platform granularity")
>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> 
> Still not working:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1902457/comments/66
> 

Hello,

Thanks for your reply Kai-Heng.
I believe you're relating to completely different issue than the one 
this very patch is targeting.

 From the logs you've provided one can see that snd_soc_skl did attempt 
to probe() so the code compiled just fine. Again, compilation issue is 
the one I've addressed here. While we're here, I'd appreciate kconfig 
being provided along dmesg file. Logs alone do not show the entire 
picture, unfortunately.

In regard to missing sound, (looks like we're talking about HDA dsp + 
DMIC configuration) kconfig mentioned above will be required. Pretty 
sure HDAudio support for skylake-driver is not enabled on your machine 
and thus driver exists probe() early without registering any sound card.

Regards,
Czarek
Kai-Heng Feng Feb. 2, 2021, 5:52 a.m. UTC | #3
On Wed, Jan 27, 2021 at 11:22 PM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
> On 2021-01-27 9:03 AM, Kai-Heng Feng wrote:
> > On Mon, Jan 25, 2021 at 7:55 PM Cezary Rojewski
> > <cezary.rojewski@intel.com> wrote:
> >>
> >> Skylake is dependent on SND_SOC_INTEL_SKYLAKE (aka "all SST platforms")
> >> whereas selecting specific configuration such as KBL-only will not
> >> cause driver code to compile. Switch to SND_SOC_INTEL_SKYLAKE_COMMON
> >> dependency so selecting any configuration causes the driver to be built.
> >>
> >> Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> >> Suggested-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> >> Fixes: 35bc99aaa1a3 ("ASoC: Intel: Skylake: Add more platform granularity")
> >> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> >
> > Still not working:
> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1902457/comments/66
> >
>
> Hello,
>
> Thanks for your reply Kai-Heng.
> I believe you're relating to completely different issue than the one
> this very patch is targeting.

Understood.


>
>  From the logs you've provided one can see that snd_soc_skl did attempt
> to probe() so the code compiled just fine. Again, compilation issue is
> the one I've addressed here. While we're here, I'd appreciate kconfig
> being provided along dmesg file. Logs alone do not show the entire
> picture, unfortunately.

Config file here:
https://pastebin.ubuntu.com/p/kGBv6XgHms/

>
> In regard to missing sound, (looks like we're talking about HDA dsp +
> DMIC configuration) kconfig mentioned above will be required. Pretty
> sure HDAudio support for skylake-driver is not enabled on your machine
> and thus driver exists probe() early without registering any sound card.

Do you mean CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC needs to be enabled?

Kai-Heng

>
> Regards,
> Czarek
Cezary Rojewski Feb. 2, 2021, 10:12 a.m. UTC | #4
On 2021-02-02 6:52 AM, Kai-Heng Feng wrote:
> On Wed, Jan 27, 2021 at 11:22 PM Cezary Rojewski
> <cezary.rojewski@intel.com> wrote:
>>

...

>>
>>   From the logs you've provided one can see that snd_soc_skl did attempt
>> to probe() so the code compiled just fine. Again, compilation issue is
>> the one I've addressed here. While we're here, I'd appreciate kconfig
>> being provided along dmesg file. Logs alone do not show the entire
>> picture, unfortunately.
> 
> Config file here:
> https://pastebin.ubuntu.com/p/kGBv6XgHms/

Thank you for the config, Yang.

>>
>> In regard to missing sound, (looks like we're talking about HDA dsp +
>> DMIC configuration) kconfig mentioned above will be required. Pretty
>> sure HDAudio support for skylake-driver is not enabled on your machine
>> and thus driver exists probe() early without registering any sound card.
> 
> Do you mean CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC needs to be enabled?

Indeed.

Below are the options required to enable HDA-dsp + DMIC configuration:
(first navigate to:)
-> Device Drivers -> Sound card support -> ALSA

-> HD-Audio
--> HD Audio PCI
--> Build Realtek HD-audio codec support
--> Build HDMI/DisplayPort HD-audio codec support

-> ASoC
--> Intel ASoC SST drivers
--> Skylake Platforms
--> Kabylake Platforms
--> Broxton/ApolloLake Platforms
--> HDAudio codec support

--> Intel Machine drivers
---> DMIC machine board driver
---> SKL/KBL/BXT/APL with HDA codecs

Let me know how the situation looks with these set.

Regards,
Czarek
Cezary Rojewski Feb. 2, 2021, 10:56 a.m. UTC | #5
On 2021-02-02 11:12 AM, Cezary Rojewski wrote:
> 
> On 2021-02-02 6:52 AM, Kai-Heng Feng wrote:
>>
>> Config file here:
>> https://pastebin.ubuntu.com/p/kGBv6XgHms/
> 
> Thank you for the config, Yang.
> 

Sorry for the typo, meant to say: Kai-Heng. Was writing several emails 
in short succession.

Regards,
Czarek
Kai-Heng Feng Feb. 2, 2021, 12:41 p.m. UTC | #6
On Tue, Feb 2, 2021 at 6:56 PM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
> On 2021-02-02 11:12 AM, Cezary Rojewski wrote:
> >
> > On 2021-02-02 6:52 AM, Kai-Heng Feng wrote:
> >>
> >> Config file here:
> >> https://pastebin.ubuntu.com/p/kGBv6XgHms/
> >
> > Thank you for the config, Yang.
> >
>
> Sorry for the typo, meant to say: Kai-Heng. Was writing several emails
> in short succession.

It's okay :)

Commit cc2d025a81a9 ("ASoC: Intel: Skylake: Update description for
HDaudio kconfig") removed "DEPRECATED" from the
SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC, does that mean the option is safe
and should be enabled in downstream distros?

Kai-Heng

>
> Regards,
> Czarek
Cezary Rojewski Feb. 7, 2021, 2:49 p.m. UTC | #7
On 2021-02-02 1:41 PM, Kai-Heng Feng wrote:
> On Tue, Feb 2, 2021 at 6:56 PM Cezary Rojewski
> <cezary.rojewski@intel.com> wrote:

...

> Commit cc2d025a81a9 ("ASoC: Intel: Skylake: Update description for
> HDaudio kconfig") removed "DEPRECATED" from the
> SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC, does that mean the option is safe
> and should be enabled in downstream distros?

Skylake driver - sound/soc/intel/skylake - is your only option if you 
want to enable HDA (dsp) + DMIC configuration on SPT platforms 
(skl/kbl/kbl-r/aml/cml-s).

Several problems that had been troubling it have been address early 2020 
[1]. Later, fixes were ported to v5.4 [2] so LTS users can enjoy working 
hda+dmic configuration. Please note: topology binary is required to make 
this work, kernel patches alone won't cut it. ASoC topologies are stored 
in a separate repo (git.alsa-project.org/alsa-topology-conf). These 
topologies should end up in /lib/firmware/intel, eventually - once 
converted via alsatplg to their binary form.

Regards,
Czarek


[1]: 
https://lore.kernel.org/alsa-devel/20200305145314.32579-1-cezary.rojewski@intel.com/
[2]: https://www.spinics.net/lists/alsa-devel/msg119230.html
Cezary Rojewski Feb. 15, 2021, 3:43 p.m. UTC | #8
On 2021-02-07 3:49 PM, Cezary Rojewski wrote:
> 
> On 2021-02-02 1:41 PM, Kai-Heng Feng wrote:
>> On Tue, Feb 2, 2021 at 6:56 PM Cezary Rojewski
>> <cezary.rojewski@intel.com> wrote:

...

>> Commit cc2d025a81a9 ("ASoC: Intel: Skylake: Update description for
>> HDaudio kconfig") removed "DEPRECATED" from the
>> SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC, does that mean the option is safe
>> and should be enabled in downstream distros?
> 
> Skylake driver - sound/soc/intel/skylake - is your only option if you 
> want to enable HDA (dsp) + DMIC configuration on SPT platforms 
> (skl/kbl/kbl-r/aml/cml-s).

Hello Kai-Heng,

I'd like to close the compilation issue which this patch is addressing. 
Could you confirm that the presented change fixes the issue on your end?

Regards,
Czarek
Kai-Heng Feng Feb. 22, 2021, 3:04 p.m. UTC | #9
Hi Cezary,

On Mon, Feb 15, 2021 at 11:43 PM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
> On 2021-02-07 3:49 PM, Cezary Rojewski wrote:
> >
> > On 2021-02-02 1:41 PM, Kai-Heng Feng wrote:
> >> On Tue, Feb 2, 2021 at 6:56 PM Cezary Rojewski
> >> <cezary.rojewski@intel.com> wrote:
>
> ...
>
> >> Commit cc2d025a81a9 ("ASoC: Intel: Skylake: Update description for
> >> HDaudio kconfig") removed "DEPRECATED" from the
> >> SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC, does that mean the option is safe
> >> and should be enabled in downstream distros?
> >
> > Skylake driver - sound/soc/intel/skylake - is your only option if you
> > want to enable HDA (dsp) + DMIC configuration on SPT platforms
> > (skl/kbl/kbl-r/aml/cml-s).
>
> Hello Kai-Heng,
>
> I'd like to close the compilation issue which this patch is addressing.
> Could you confirm that the presented change fixes the issue on your end?

No, the SST regression is not fixed.
However, it's not the scope of this patch, which is to fix a different issue.

So please proceed to merge the patch. We can discuss the SST
regression in other thread.

Kai-Heng

>
> Regards,
> Czarek
Cezary Rojewski March 1, 2021, 12:37 p.m. UTC | #10
On 2021-02-22 4:04 PM, Kai-Heng Feng wrote:
> Hi Cezary,
> 

...

>>
>> I'd like to close the compilation issue which this patch is addressing.
>> Could you confirm that the presented change fixes the issue on your end?
> 
> No, the SST regression is not fixed.
> However, it's not the scope of this patch, which is to fix a different issue.
> 
> So please proceed to merge the patch. We can discuss the SST
> regression in other thread.
> 

Thanks for the reply Kai-Heng.
Could you elaborate on the SST regression subject though?

Mark, do you want me to re-send the patch?

Regards,
Czarek
Mark Brown March 1, 2021, 11:34 p.m. UTC | #11
On Mon, 25 Jan 2021 12:54:41 +0100, Cezary Rojewski wrote:
> Skylake is dependent on SND_SOC_INTEL_SKYLAKE (aka "all SST platforms")
> whereas selecting specific configuration such as KBL-only will not
> cause driver code to compile. Switch to SND_SOC_INTEL_SKYLAKE_COMMON
> dependency so selecting any configuration causes the driver to be built.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: Intel: Skylake: Compile when any configuration is selected
      commit: cfa0faec5fc0544f84b9c599b6cf49cd3cc709f3

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
diff mbox series

Patch

diff --git a/sound/soc/intel/Makefile b/sound/soc/intel/Makefile
index 4e0248d2accc..7c5038803be7 100644
--- a/sound/soc/intel/Makefile
+++ b/sound/soc/intel/Makefile
@@ -5,7 +5,7 @@  obj-$(CONFIG_SND_SOC) += common/
 # Platform Support
 obj-$(CONFIG_SND_SST_ATOM_HIFI2_PLATFORM) += atom/
 obj-$(CONFIG_SND_SOC_INTEL_CATPT) += catpt/
-obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE) += skylake/
+obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE_COMMON) += skylake/
 obj-$(CONFIG_SND_SOC_INTEL_KEEMBAY) += keembay/
 
 # Machine support
diff --git a/sound/soc/intel/skylake/Makefile b/sound/soc/intel/skylake/Makefile
index dd39149b89b1..1c4649bccec5 100644
--- a/sound/soc/intel/skylake/Makefile
+++ b/sound/soc/intel/skylake/Makefile
@@ -7,7 +7,7 @@  ifdef CONFIG_DEBUG_FS
   snd-soc-skl-objs += skl-debug.o
 endif
 
-obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE) += snd-soc-skl.o
+obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE_COMMON) += snd-soc-skl.o
 
 #Skylake Clock device support
 snd-soc-skl-ssp-clk-objs := skl-ssp-clk.o