Re: [alsa-devel] [GIT PULL] sound updates for 4.15-rc1
[Adding more people and alsa-devel to Cc]
On Wed, 15 Nov 2017 03:40:09 +0100, Linus Torvalds wrote:
On Tue, Nov 14, 2017 at 6:51 AM, Takashi Iwai tiwai@suse.de wrote:
please pull sound updates for v4.15-rc1 from:
Hmm. Making "oldconfig" on my laptop with this, my SND_SOC_INTEL_SKYLAKE went away.
And the reason seems to be that new SND_SOC_INTEL_SST_TOPLEVEL config option.
Which has no help associated with it.
This is not a friendly thing to do to people. It basically breaks existing setups for no documented reason, and with no explanation.
Please fix the config situation. At the very least, add documentation.
Sorry about that. I saw Vinod already submitted a patch to add the help text to CONFIG_SND_SOC_INTEL_SST_TOPLEVEL, so the least fix should go in soon.
But now looking at these changes, I noticed a few things, too:
- With the introduction of SND_SOC_INTEL_SST_TOPLEVEL, keeping SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH individually doesn't make much sense. They can be dropped and replaced with SND_SOC_INTEL_SST_TOPLEVEL as a further cleanup.
- ... or, make SND_SOC_INTEL_SST_TOPLEVEL=y as default, if this is considered to be a top-level filter config (like the network vendor kconfig items). In that case, the reverse-selection of SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH should be avoided, but they should be selected from the actual drivers instead.
And I believe there are a few more possible cleanups / fixes in the messy Intel ASoC Kconfigs. For example, SND_SOC_INTEL_SST is almost always set. The only exception is via SND_SST_ATOM_HIFI2_PLATFORM. But all machine drivers using Atom Hifi2 do set SND_SST_IPC_ACPI, which also requires SND_SOC_INTEL_SST.
Further looking at this, we see that the only entry that does *not* require SND_SOC_INTEL_SST is the case with SND_MFLD_MACHINE in sound/soc/intel/boards. And now more interesting part -- there is no corresponding entry in Makefile. That is, this kconfig is effectively dead! The source code mfld_machine.c exists, but it's just a place holder now. The code was supposed to be integrated into atom directory by the commit b97169da0699, but it seems forgotten to be updated.
Hmm...
Takashi
On Wed, Nov 15, 2017 at 08:34:09AM +0100, Takashi Iwai wrote:
[Adding more people and alsa-devel to Cc]
On Wed, 15 Nov 2017 03:40:09 +0100, Linus Torvalds wrote:
On Tue, Nov 14, 2017 at 6:51 AM, Takashi Iwai tiwai@suse.de wrote:
please pull sound updates for v4.15-rc1 from:
Hmm. Making "oldconfig" on my laptop with this, my SND_SOC_INTEL_SKYLAKE went away.
And the reason seems to be that new SND_SOC_INTEL_SST_TOPLEVEL config option.
Which has no help associated with it.
This is not a friendly thing to do to people. It basically breaks existing setups for no documented reason, and with no explanation.
Please fix the config situation. At the very least, add documentation.
Sorry about that. I saw Vinod already submitted a patch to add the help text to CONFIG_SND_SOC_INTEL_SST_TOPLEVEL, so the least fix should go in soon.
But now looking at these changes, I noticed a few things, too:
With the introduction of SND_SOC_INTEL_SST_TOPLEVEL, keeping SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH individually doesn't make much sense. They can be dropped and replaced with SND_SOC_INTEL_SST_TOPLEVEL as a further cleanup.
... or, make SND_SOC_INTEL_SST_TOPLEVEL=y as default, if this is considered to be a top-level filter config (like the network vendor kconfig items). In that case, the reverse-selection of SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH should be avoided, but they should be selected from the actual drivers instead.
both make sense to me. Sometimes back I also was thinking of having single user config for a group of machine configs. I dont know if people really care about various types of skylake audio machines so enabling all of them with a single option maynot be a very bad idea... thoughts?
This way we can have a skylake based, legacy BYT, HSW group, Atom BYT-CHT group. Distro anyway enable all and having all drivers selected works fine This way our expanding kconfig can be limited.
And I believe there are a few more possible cleanups / fixes in the messy Intel ASoC Kconfigs. For example, SND_SOC_INTEL_SST is almost always set. The only exception is via SND_SST_ATOM_HIFI2_PLATFORM. But all machine drivers using Atom Hifi2 do set SND_SST_IPC_ACPI, which also requires SND_SOC_INTEL_SST.
Further looking at this, we see that the only entry that does *not* require SND_SOC_INTEL_SST is the case with SND_MFLD_MACHINE in sound/soc/intel/boards. And now more interesting part -- there is no corresponding entry in Makefile. That is, this kconfig is effectively dead! The source code mfld_machine.c exists, but it's just a place holder now. The code was supposed to be integrated into atom directory by the commit b97169da0699, but it seems forgotten to be updated.
Looks like it was missed indeed, I will send this one out too and lets discuss which way all like to solve this and I shall send patches :)
On Wed, Nov 15, 2017 at 08:34:09AM +0100, Takashi Iwai wrote:
And I believe there are a few more possible cleanups / fixes in the messy Intel ASoC Kconfigs. For example, SND_SOC_INTEL_SST is almost always set. The only exception is via SND_SST_ATOM_HIFI2_PLATFORM. But all machine drivers using Atom Hifi2 do set SND_SST_IPC_ACPI, which also requires SND_SOC_INTEL_SST.
I'm not 100% convinced that the recent changes away from just having the only user selectable options be the board ones have really helped that much. Though transitioning back would probably just create more misery. It's a real shame that ACPI doesn't have standards for the machine descriptions here like DT does, it'd cut down on the amount of stuff that needs configuring.
On Wed, 15 Nov 2017 12:37:42 +0100, Mark Brown wrote:
On Wed, Nov 15, 2017 at 08:34:09AM +0100, Takashi Iwai wrote:
And I believe there are a few more possible cleanups / fixes in the messy Intel ASoC Kconfigs. For example, SND_SOC_INTEL_SST is almost always set. The only exception is via SND_SST_ATOM_HIFI2_PLATFORM. But all machine drivers using Atom Hifi2 do set SND_SST_IPC_ACPI, which also requires SND_SOC_INTEL_SST.
I'm not 100% convinced that the recent changes away from just having the only user selectable options be the board ones have really helped that much.
Yeah, now it resulted in a mixture of selects and depends, which is a bad sign. I'm not against this kind of reorganization but we need to sort out more.
Though transitioning back would probably just create more misery. It's a real shame that ACPI doesn't have standards for the machine descriptions here like DT does, it'd cut down on the amount of stuff that needs configuring.
True. Although I don't think ACPI is the center of the mess in this case, but rather too many kconfigs is it.
In soc/intel/*, we have the following entries:
as non-selectable ones: - SND_SST_IPC - SND_SST_IPC_PCI - SND_SST_IPC_ACPI - SND_SOC_INTEL_COMMON - SND_SOC_INTEL_SST - SND_SOC_INTEL_SST_FIRMWARE - SND_SOC_INTEL_SST_ACPI - SND_SOC_ACPI_INTEL_MATCH
and the selectable ones: - SND_SOC_INTEL_SST_TOPLEVEL - SND_SOC_INTEL_HASWELL - SND_SOC_INTEL_BAYTRAIL - SND_SST_ATOM_HIFI2_PLATFORM - SND_SOC_INTEL_SKYLAKE
And yet, there are tons of machine drivers as selectable kconfigs: - SND_SOC_INTEL_MACH - SND_MFLD_MACHINE - SND_SOC_INTEL_HASWELL_MACH - SND_SOC_INTEL_BDW_RT5677_MACH - SND_SOC_INTEL_BROADWELL_MACH - SND_SOC_INTEL_BYT_MAX98090_MACH - SND_SOC_INTEL_BYT_RT5640_MACH - SND_SOC_INTEL_BYTCR_RT5640_MACH - SND_SOC_INTEL_BYTCR_RT5651_MACH - SND_SOC_INTEL_CHT_BSW_RT5672_MACH - SND_SOC_INTEL_CHT_BSW_RT5645_MACH - SND_SOC_INTEL_CHT_BSW_MAX98090_TI_MACH - SND_SOC_INTEL_BYT_CHT_DA7213_MACH - SND_SOC_INTEL_BYT_CHT_ES8316_MACH - SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH - SND_SOC_INTEL_SKL_RT286_MACH - SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH - SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH - SND_SOC_INTEL_BXT_DA7219_MAX98357A_MACH - SND_SOC_INTEL_BXT_RT298_MACH - SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH - SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
We should begin with thinking of which entries (and layer) to be selectable, and which not.
Takashi
On Wed, Nov 15, 2017 at 12:58:40PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
On Wed, Nov 15, 2017 at 08:34:09AM +0100, Takashi Iwai wrote:
Though transitioning back would probably just create more misery. It's a real shame that ACPI doesn't have standards for the machine descriptions here like DT does, it'd cut down on the amount of stuff that needs configuring.
True. Although I don't think ACPI is the center of the mess in this case, but rather too many kconfigs is it.
It's the source of all the individual board Kconfigs - we can't just have an equivalent of the of-graph card - and then the explosion of board configs then pushes to have more of the other options user selectable to let people make the list more manageable.
We should begin with thinking of which entries (and layer) to be selectable, and which not.
I'd say either just all the individual machines like it was or all the SoCs. If it's the SoCs it prevents people making really tiny configs, though I'm not sure who cares. If it's the machines then you get a lot of options but I don't know that this is a problem, it's not like end users are routinely configuring their kernel.
On Wed, 15 Nov 2017 13:16:48 +0100, Mark Brown wrote:
On Wed, Nov 15, 2017 at 12:58:40PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
On Wed, Nov 15, 2017 at 08:34:09AM +0100, Takashi Iwai wrote:
Though transitioning back would probably just create more misery. It's a real shame that ACPI doesn't have standards for the machine descriptions here like DT does, it'd cut down on the amount of stuff that needs configuring.
True. Although I don't think ACPI is the center of the mess in this case, but rather too many kconfigs is it.
It's the source of all the individual board Kconfigs - we can't just have an equivalent of the of-graph card - and then the explosion of board configs then pushes to have more of the other options user selectable to let people make the list more manageable.
OK, point taken.
We should begin with thinking of which entries (and layer) to be selectable, and which not.
I'd say either just all the individual machines like it was or all the SoCs. If it's the SoCs it prevents people making really tiny configs, though I'm not sure who cares. If it's the machines then you get a lot of options but I don't know that this is a problem, it's not like end users are routinely configuring their kernel.
It might sound contradicting to my previous statement, but the number of selections itself isn't a big problem. The problem is rather that multiple options have to be selected for reaching to the point to enable one feature on your machine. So, I agree that these two representations would be suitable, and the usual solution is the firmer, to expose *only* individual machine drivers as selectable. (Or, at most, we can have kconfig entries just filtering in addition.)
Takashi
On Wed, Nov 15, 2017 at 02:16:41PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
I'd say either just all the individual machines like it was or all the SoCs. If it's the SoCs it prevents people making really tiny configs, though I'm not sure who cares. If it's the machines then you get a lot of options but I don't know that this is a problem, it's not like end users are routinely configuring their kernel.
It might sound contradicting to my previous statement, but the number of selections itself isn't a big problem. The problem is rather that multiple options have to be selected for reaching to the point to enable one feature on your machine. So, I agree that these two
Right, just finding what you need to enable to enable a given option.
representations would be suitable, and the usual solution is the firmer, to expose *only* individual machine drivers as selectable.
It's definitely the most conservative thing to do, unless someone has a strong reason to do something else I'd probably go that way.
(Or, at most, we can have kconfig entries just filtering in addition.)
Yes, that'd not be quite so bad but it's still adding to the set of things you have to grind through to find the option you want.
On Wed, 15 Nov 2017 14:33:05 +0100, Mark Brown wrote:
On Wed, Nov 15, 2017 at 02:16:41PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
I'd say either just all the individual machines like it was or all the SoCs. If it's the SoCs it prevents people making really tiny configs, though I'm not sure who cares. If it's the machines then you get a lot of options but I don't know that this is a problem, it's not like end users are routinely configuring their kernel.
It might sound contradicting to my previous statement, but the number of selections itself isn't a big problem. The problem is rather that multiple options have to be selected for reaching to the point to enable one feature on your machine. So, I agree that these two
Right, just finding what you need to enable to enable a given option.
representations would be suitable, and the usual solution is the firmer, to expose *only* individual machine drivers as selectable.
It's definitely the most conservative thing to do, unless someone has a strong reason to do something else I'd probably go that way.
(Or, at most, we can have kconfig entries just filtering in addition.)
Yes, that'd not be quite so bad but it's still adding to the set of things you have to grind through to find the option you want.
Right, but these filters are usually with default=y, so at the first invocation, it can go easy. The filter config is useful basically for someone who feel annoyed by too many options appearing, then they can turn this off explicitly.
Takashi
On Wed, Nov 15, 2017 at 12:58:40PM +0100, Takashi Iwai wrote:
On Wed, 15 Nov 2017 12:37:42 +0100, Mark Brown wrote:
On Wed, Nov 15, 2017 at 08:34:09AM +0100, Takashi Iwai wrote:
And I believe there are a few more possible cleanups / fixes in the messy Intel ASoC Kconfigs. For example, SND_SOC_INTEL_SST is almost always set. The only exception is via SND_SST_ATOM_HIFI2_PLATFORM. But all machine drivers using Atom Hifi2 do set SND_SST_IPC_ACPI, which also requires SND_SOC_INTEL_SST.
I'm not 100% convinced that the recent changes away from just having the only user selectable options be the board ones have really helped that much.
Yeah, now it resulted in a mixture of selects and depends, which is a bad sign. I'm not against this kind of reorganization but we need to sort out more.
Though transitioning back would probably just create more misery. It's a real shame that ACPI doesn't have standards for the machine descriptions here like DT does, it'd cut down on the amount of stuff that needs configuring.
True. Although I don't think ACPI is the center of the mess in this case, but rather too many kconfigs is it.
In soc/intel/*, we have the following entries:
as non-selectable ones:
- SND_SST_IPC
- SND_SST_IPC_PCI
- SND_SST_IPC_ACPI
- SND_SOC_INTEL_COMMON
- SND_SOC_INTEL_SST
- SND_SOC_INTEL_SST_FIRMWARE
- SND_SOC_INTEL_SST_ACPI
- SND_SOC_ACPI_INTEL_MATCH
and the selectable ones:
- SND_SOC_INTEL_SST_TOPLEVEL
- SND_SOC_INTEL_HASWELL
- SND_SOC_INTEL_BAYTRAIL
- SND_SST_ATOM_HIFI2_PLATFORM
- SND_SOC_INTEL_SKYLAKE
And yet, there are tons of machine drivers as selectable kconfigs:
- SND_SOC_INTEL_MACH
this will be top level menu entry no select/default here as Takashi suggested.
- SND_MFLD_MACHINE
this would be a lone ranger
- SND_SOC_INTEL_HASWELL_MACH
- SND_SOC_INTEL_BDW_RT5677_MACH
- SND_SOC_INTEL_BROADWELL_MACH
we can bucket this in SND_SOC_INTEL_HSW_MACHS
- SND_SOC_INTEL_BYT_MAX98090_MACH
- SND_SOC_INTEL_BYT_RT5640_MACH
SND_SOC_INTEL_LEGACY_BYT
- SND_SOC_INTEL_BYTCR_RT5640_MACH
- SND_SOC_INTEL_BYTCR_RT5651_MACH
- SND_SOC_INTEL_CHT_BSW_RT5672_MACH
- SND_SOC_INTEL_CHT_BSW_RT5645_MACH
- SND_SOC_INTEL_CHT_BSW_MAX98090_TI_MACH
- SND_SOC_INTEL_BYT_CHT_DA7213_MACH
- SND_SOC_INTEL_BYT_CHT_ES8316_MACH
- SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH
SND_SOC_INTEL_BYT_MACHS
- SND_SOC_INTEL_SKL_RT286_MACH
- SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH
- SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH
- SND_SOC_INTEL_BXT_DA7219_MAX98357A_MACH
- SND_SOC_INTEL_BXT_RT298_MACH
- SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH
- SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
SND_SOC_INTEL_SKL_MACHS
This set is growing and will keep growing. One machine is already posted and it's cousins should follow soon. I don't see a point in having options for these ad distro's would be enabling all of them and having all of them enabled works fine for me
We should begin with thinking of which entries (and layer) to be selectable, and which not.
Trickier part is non selectable ones, I am going to take a stab at them and see if we can merge few which can help is making lesser options and clean that part a bit, maybe a common SST_IPC symbol which adds IPC, MATCH etc.
Stay tuned ... :)
On Wed, Nov 15, 2017 at 08:54:11PM +0530, Vinod Koul wrote:
This set is growing and will keep growing. One machine is already posted and it's cousins should follow soon. I don't see a point in having options for these ad distro's would be enabling all of them and having all of them enabled works fine for me
Distros enable the overwhelming majority of drivers anyway, they're not really relevant in terms of driver configurability. More relevant are people trying to build a stripped down machine specific configuration.
We should begin with thinking of which entries (and layer) to be selectable, and which not.
Trickier part is non selectable ones, I am going to take a stab at them and see if we can merge few which can help is making lesser options and clean that part a bit, maybe a common SST_IPC symbol which adds IPC, MATCH etc.
Those ones are less important so long as they're selected correctly by the things that need them, it's not like there's not lots of complexity in this driver.
On 11/15/17 1:34 AM, Takashi Iwai wrote:
[Adding more people and alsa-devel to Cc]
On Wed, 15 Nov 2017 03:40:09 +0100, Linus Torvalds wrote:
On Tue, Nov 14, 2017 at 6:51 AM, Takashi Iwai tiwai@suse.de wrote:
please pull sound updates for v4.15-rc1 from:
Hmm. Making "oldconfig" on my laptop with this, my SND_SOC_INTEL_SKYLAKE went away.
And the reason seems to be that new SND_SOC_INTEL_SST_TOPLEVEL config option.
Which has no help associated with it.
This is not a friendly thing to do to people. It basically breaks existing setups for no documented reason, and with no explanation.
Please fix the config situation. At the very least, add documentation.
Sorry about that. I saw Vinod already submitted a patch to add the help text to CONFIG_SND_SOC_INTEL_SST_TOPLEVEL, so the least fix should go in soon.
But now looking at these changes, I noticed a few things, too:
With the introduction of SND_SOC_INTEL_SST_TOPLEVEL, keeping SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH individually doesn't make much sense. They can be dropped and replaced with SND_SOC_INTEL_SST_TOPLEVEL as a further cleanup.
... or, make SND_SOC_INTEL_SST_TOPLEVEL=y as default, if this is considered to be a top-level filter config (like the network vendor kconfig items). In that case, the reverse-selection of SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH should be avoided, but they should be selected from the actual drivers instead.
This level was introduced as a shortcut to help select the platform drivers associated with the closed-source audio firmware. There will be a follow-up set of patches coming soon, and we'll need to have the corresponding top-level selector for the drivers handling the open-source audio firmware. Please don't make too many assumptions on this SND_SOC_INTEL_SST_TOPLEVEL, it is not going to be true for everyone moving forward.
If you want look at the Kconfig setup, the latest code cherry-picked on top of v4.14 is here https://github.com/plbossart/sound/tree/backport/intel-audio-stable-v4.14
the menuconfig options are still in device drivers/sound/ASoC but now you have SST and SOF menus.
And I believe there are a few more possible cleanups / fixes in the messy Intel ASoC Kconfigs. For example, SND_SOC_INTEL_SST is almost always set. The only exception is via SND_SST_ATOM_HIFI2_PLATFORM. But all machine drivers using Atom Hifi2 do set SND_SST_IPC_ACPI, which also requires SND_SOC_INTEL_SST.
Further looking at this, we see that the only entry that does *not* require SND_SOC_INTEL_SST is the case with SND_MFLD_MACHINE in sound/soc/intel/boards. And now more interesting part -- there is no corresponding entry in Makefile. That is, this kconfig is effectively dead! The source code mfld_machine.c exists, but it's just a place holder now. The code was supposed to be integrated into atom directory by the commit b97169da0699, but it seems forgotten to be updated.
Hmm...
Takashi
On Thu, 16 Nov 2017 15:55:35 +0100, Pierre-Louis Bossart wrote:
On 11/15/17 1:34 AM, Takashi Iwai wrote:
[Adding more people and alsa-devel to Cc]
On Wed, 15 Nov 2017 03:40:09 +0100, Linus Torvalds wrote:
On Tue, Nov 14, 2017 at 6:51 AM, Takashi Iwai tiwai@suse.de wrote:
please pull sound updates for v4.15-rc1 from:
Hmm. Making "oldconfig" on my laptop with this, my SND_SOC_INTEL_SKYLAKE went away.
And the reason seems to be that new SND_SOC_INTEL_SST_TOPLEVEL config option.
Which has no help associated with it.
This is not a friendly thing to do to people. It basically breaks existing setups for no documented reason, and with no explanation.
Please fix the config situation. At the very least, add documentation.
Sorry about that. I saw Vinod already submitted a patch to add the help text to CONFIG_SND_SOC_INTEL_SST_TOPLEVEL, so the least fix should go in soon.
But now looking at these changes, I noticed a few things, too:
With the introduction of SND_SOC_INTEL_SST_TOPLEVEL, keeping SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH individually doesn't make much sense. They can be dropped and replaced with SND_SOC_INTEL_SST_TOPLEVEL as a further cleanup.
... or, make SND_SOC_INTEL_SST_TOPLEVEL=y as default, if this is considered to be a top-level filter config (like the network vendor kconfig items). In that case, the reverse-selection of SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH should be avoided, but they should be selected from the actual drivers instead.
This level was introduced as a shortcut to help select the platform drivers associated with the closed-source audio firmware. There will be a follow-up set of patches coming soon, and we'll need to have the corresponding top-level selector for the drivers handling the open-source audio firmware. Please don't make too many assumptions on this SND_SOC_INTEL_SST_TOPLEVEL, it is not going to be true for everyone moving forward.
OK, but *before* moving to that direction, please fix the current mess at first. If CONFIG_SND_SOC_INTEL_SST_TOPLEVEL is the entry to filter the Intel ASoC drivers, it should be the one with default=y and doesn't select / enable others as itself. OTOH, if some explicit selection by user becomes mandatory while it wasn't, it's already a bad step.
If you want look at the Kconfig setup, the latest code cherry-picked on top of v4.14 is here https://github.com/plbossart/sound/tree/backport/intel-audio-stable-v4.14
Thanks. But I see only the very same content in sound/soc/intel/*...?
Takashi
the menuconfig options are still in device drivers/sound/ASoC but now you have SST and SOF menus.
And I believe there are a few more possible cleanups / fixes in the messy Intel ASoC Kconfigs. For example, SND_SOC_INTEL_SST is almost always set. The only exception is via SND_SST_ATOM_HIFI2_PLATFORM. But all machine drivers using Atom Hifi2 do set SND_SST_IPC_ACPI, which also requires SND_SOC_INTEL_SST.
Further looking at this, we see that the only entry that does *not* require SND_SOC_INTEL_SST is the case with SND_MFLD_MACHINE in sound/soc/intel/boards. And now more interesting part -- there is no corresponding entry in Makefile. That is, this kconfig is effectively dead! The source code mfld_machine.c exists, but it's just a place holder now. The code was supposed to be integrated into atom directory by the commit b97169da0699, but it seems forgotten to be updated.
Hmm...
Takashi
On 11/16/2017 09:10 AM, Takashi Iwai wrote:
On Thu, 16 Nov 2017 15:55:35 +0100, Pierre-Louis Bossart wrote:
On 11/15/17 1:34 AM, Takashi Iwai wrote:
[Adding more people and alsa-devel to Cc]
On Wed, 15 Nov 2017 03:40:09 +0100, Linus Torvalds wrote:
On Tue, Nov 14, 2017 at 6:51 AM, Takashi Iwai tiwai@suse.de wrote:
please pull sound updates for v4.15-rc1 from:
Hmm. Making "oldconfig" on my laptop with this, my SND_SOC_INTEL_SKYLAKE went away.
And the reason seems to be that new SND_SOC_INTEL_SST_TOPLEVEL config option.
Which has no help associated with it.
This is not a friendly thing to do to people. It basically breaks existing setups for no documented reason, and with no explanation.
Please fix the config situation. At the very least, add documentation.
Sorry about that. I saw Vinod already submitted a patch to add the help text to CONFIG_SND_SOC_INTEL_SST_TOPLEVEL, so the least fix should go in soon.
But now looking at these changes, I noticed a few things, too:
With the introduction of SND_SOC_INTEL_SST_TOPLEVEL, keeping SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH individually doesn't make much sense. They can be dropped and replaced with SND_SOC_INTEL_SST_TOPLEVEL as a further cleanup.
... or, make SND_SOC_INTEL_SST_TOPLEVEL=y as default, if this is considered to be a top-level filter config (like the network vendor kconfig items). In that case, the reverse-selection of SND_SOC_INTEL_COMMON and SND_SOC_INTEL_MACH should be avoided, but they should be selected from the actual drivers instead.
This level was introduced as a shortcut to help select the platform drivers associated with the closed-source audio firmware. There will be a follow-up set of patches coming soon, and we'll need to have the corresponding top-level selector for the drivers handling the open-source audio firmware. Please don't make too many assumptions on this SND_SOC_INTEL_SST_TOPLEVEL, it is not going to be true for everyone moving forward.
OK, but *before* moving to that direction, please fix the current mess at first. If CONFIG_SND_SOC_INTEL_SST_TOPLEVEL is the entry to filter the Intel ASoC drivers, it should be the one with default=y and doesn't select / enable others as itself. OTOH, if some explicit selection by user becomes mandatory while it wasn't, it's already a bad step.
ok, so i will accept that there was a gap in the validation and that I shouldn't have broken Linus' audio. My bad, working on it. The sound/soc/intel/boards/Kconfig file can indeed be simplified, I tried to avoid changing too many things at the same time, but I can provide a patch if people want a nicer solution now if the if/endif and default m. I'll send a proposal later today. I guess I'll play with defconfig, oldconfig and olddefconfig for a quick sanity check.
That said, it looks like we are not aligned on what the patches tried to do: SND_SOC_INTEL_COMMON contains the matching code used by both SST and SOF drivers and it needs to remain as a separate module SND_SOC_INTEL_MACH is a way to make the machine drivers appear, both from SST and SOF drivers (we want common machine drivers) the two are completely unrelated.
I also don't see why we are talking about a reverse selection, the flow is the same top-down selection in both cases
config SND_SOC_INTEL_SST_TOPLEVEL tristate "Intel ASoC SST drivers" select SND_SOC_INTEL_MACH select SND_SOC_INTEL_COMMON
config SND_SOC_SOF_INTEL tristate "SOF support for Intel audio DSPs" depends on SND_SOC_SOF << probably more dependencies needed with randconfig. select SND_SOC_INTEL_COMMON select SND_SOC_INTEL_MACH
then you select the platform drivers needed and the relevant machine drivers are visible.
I am also not sure if there should be hard-coded 'y' default. If you use make defconfig, there is currently nothing selected at all for any of the ASoC drivers, should we instead use 'default m' for all the sensible drivers (i.e. default n for deprecated stuff) and automagically select them when X86+ASoC is true?
If you want look at the Kconfig setup, the latest code cherry-picked on top of v4.14 is here https://github.com/plbossart/sound/tree/backport/intel-audio-stable-v4.14
Thanks. But I see only the very same content in sound/soc/intel/*...?
sorry, please look here: https://github.com/plbossart/sound/tree/topic/sof-1.0-dev-rc2
Takashi
the menuconfig options are still in device drivers/sound/ASoC but now you have SST and SOF menus.
And I believe there are a few more possible cleanups / fixes in the messy Intel ASoC Kconfigs. For example, SND_SOC_INTEL_SST is almost always set. The only exception is via SND_SST_ATOM_HIFI2_PLATFORM. But all machine drivers using Atom Hifi2 do set SND_SST_IPC_ACPI, which also requires SND_SOC_INTEL_SST.
Further looking at this, we see that the only entry that does *not* require SND_SOC_INTEL_SST is the case with SND_MFLD_MACHINE in sound/soc/intel/boards. And now more interesting part -- there is no corresponding entry in Makefile. That is, this kconfig is effectively dead! The source code mfld_machine.c exists, but it's just a place holder now. The code was supposed to be integrated into atom directory by the commit b97169da0699, but it seems forgotten to be updated.
Hmm...
Takashi
On Thu, Nov 16, 2017 at 02:27:48PM -0600, Pierre-Louis Bossart wrote:
I am also not sure if there should be hard-coded 'y' default. If you use make defconfig, there is currently nothing selected at all for any of the ASoC drivers, should we instead use 'default m' for all the sensible drivers (i.e. default n for deprecated stuff) and automagically select them when X86+ASoC is true?
The hard coded defaults were being talked about in the context of the SoC level options that mask out the machines for a given platform - those should default on so the user gets the full list of machines by default and so that existing users don't get the config options for their boards turned off. Those options shouldn't directly enable any code, they should just function to make the list of boards more manageable.
participants (4)
-
Mark Brown -
Pierre-Louis Bossart -
Takashi Iwai -
Vinod Koul