[alsa-devel] [RFC PATCH 0/6] ASoC:Intel:Skylake: Enable HDaudio legacy fallback
This patchset is provided as an RFC and should not be merged as is (Turkey break in the USA and more validation needed). This is however a good time to gather comments. This work is the result of multiple email discussions to finally provide more flexibility for distributions to chose whether to stick with the legacy HDaudio driver or to enable the SST/Skylake driver for newer platforms (required e.g. for digital microphone support)
The patches add support for CoffeeLake, simplify the probe for the Skylake driver, introduce more compile-time granularity so that platforms can be selected individually and last provide a dynamic fallback mechanism when two drivers are registered for the same PCI ID.
When the SOF driver is released, the same mechanism will be used to enable the SOF-legacy fallback. There will be no plans to enable an SOF->SST falback.
Pierre-Louis Bossart (4): ASoC: Intel: Skylake: stop init/probe if DSP is not present ASoC: Intel: Skylake: remove useless tests on DSP presence ASoC: Intel: Skylake: Add more platform granularity ALSA: hda: add fallback capabilities for SKL+ platforms
Takashi Iwai (2): ASoC: Intel: Skylake: Add CFL-S support ALSA: hda: Allow fallback binding with legacy HD-audio for Intel SKL+
include/sound/hdaudio.h | 6 + sound/pci/hda/Kconfig | 40 +++++++ sound/pci/hda/hda_controller.h | 2 +- sound/pci/hda/hda_intel.c | 51 ++++++-- sound/soc/intel/Kconfig | 86 ++++++++++++-- sound/soc/intel/boards/Kconfig | 16 ++- sound/soc/intel/skylake/skl-messages.c | 8 ++ sound/soc/intel/skylake/skl.c | 154 +++++++++++++++++++------ 8 files changed, 311 insertions(+), 52 deletions(-)
From: Takashi Iwai tiwai@suse.de
It's with CNP, supposed to be equivalent with CNL entry.
Signed-off-by: Takashi Iwai tiwai@suse.de Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/skylake/skl-messages.c | 8 ++++++++ sound/soc/intel/skylake/skl.c | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index 8bfb8b0fa3d5..b0e6fb93eaf8 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -247,6 +247,14 @@ static const struct skl_dsp_ops dsp_ops[] = { .init_fw = cnl_sst_init_fw, .cleanup = cnl_sst_dsp_cleanup }, + { + .id = 0xa348, + .num_cores = 4, + .loader_ops = bxt_get_loader_ops, + .init = cnl_sst_dsp_init, + .init_fw = cnl_sst_init_fw, + .cleanup = cnl_sst_dsp_cleanup + }, };
const struct skl_dsp_ops *skl_get_dsp_ops(int pci_id) diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index 3f0ac1312982..df36b8fe6d5e 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -1121,6 +1121,9 @@ static const struct pci_device_id skl_ids[] = { /* CNL */ { PCI_DEVICE(0x8086, 0x9dc8), .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines}, + /* CFL */ + { PCI_DEVICE(0x8086, 0xa348), + .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines}, { 0, } }; MODULE_DEVICE_TABLE(pci, skl_ids);
On Tue, Nov 20, 2018 at 03:36:39PM -0600, Pierre-Louis Bossart wrote:
From: Takashi Iwai tiwai@suse.de
It's with CNP, supposed to be equivalent with CNL entry.
May you consider to switch to PCI_DEVICE_DATA() first?
Signed-off-by: Takashi Iwai tiwai@suse.de Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/intel/skylake/skl-messages.c | 8 ++++++++ sound/soc/intel/skylake/skl.c | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index 8bfb8b0fa3d5..b0e6fb93eaf8 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -247,6 +247,14 @@ static const struct skl_dsp_ops dsp_ops[] = { .init_fw = cnl_sst_init_fw, .cleanup = cnl_sst_dsp_cleanup },
- {
.id = 0xa348,
.num_cores = 4,
.loader_ops = bxt_get_loader_ops,
.init = cnl_sst_dsp_init,
.init_fw = cnl_sst_init_fw,
.cleanup = cnl_sst_dsp_cleanup
- },
};
const struct skl_dsp_ops *skl_get_dsp_ops(int pci_id) diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index 3f0ac1312982..df36b8fe6d5e 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -1121,6 +1121,9 @@ static const struct pci_device_id skl_ids[] = { /* CNL */ { PCI_DEVICE(0x8086, 0x9dc8), .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
- /* CFL */
- { PCI_DEVICE(0x8086, 0xa348),
{ 0, }.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
}; MODULE_DEVICE_TABLE(pci, skl_ids); -- 2.17.1
On 11/21/18 8:27 AM, Andy Shevchenko wrote:
On Tue, Nov 20, 2018 at 03:36:39PM -0600, Pierre-Louis Bossart wrote:
From: Takashi Iwai tiwai@suse.de
It's with CNP, supposed to be equivalent with CNL entry.
May you consider to switch to PCI_DEVICE_DATA() first?
Is this really the recommended path?
The macro generates PCI_DEVICE_ID_##vend##_##dev, and I don't have a turn key #define PCI_DEVICE_ID_INTEL_AUDIO_CFL 0xa348 I can use. In a number of cases we have multiple variants of the same hardware, and it starts being painful to use a 20-letter macro to differentiate between INTEL_AUDIO_CFL_Y and INTEL_AUDIO_CFL_H. The explicit code and a short comment are more readable really.
git grep PCI_DEVICE_ID_INTEL gives me hundreds of definitions, some global, some local to specific drivers, doesn't seem like there is a well-agreed usage of this macro, is there? I don't mind making the change but I don't sense an strong argument for it?
Signed-off-by: Takashi Iwai tiwai@suse.de Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/intel/skylake/skl-messages.c | 8 ++++++++ sound/soc/intel/skylake/skl.c | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index 8bfb8b0fa3d5..b0e6fb93eaf8 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -247,6 +247,14 @@ static const struct skl_dsp_ops dsp_ops[] = { .init_fw = cnl_sst_init_fw, .cleanup = cnl_sst_dsp_cleanup },
{
.id = 0xa348,
.num_cores = 4,
.loader_ops = bxt_get_loader_ops,
.init = cnl_sst_dsp_init,
.init_fw = cnl_sst_init_fw,
.cleanup = cnl_sst_dsp_cleanup
}, };
const struct skl_dsp_ops *skl_get_dsp_ops(int pci_id)
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index 3f0ac1312982..df36b8fe6d5e 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -1121,6 +1121,9 @@ static const struct pci_device_id skl_ids[] = { /* CNL */ { PCI_DEVICE(0x8086, 0x9dc8), .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
- /* CFL */
- { PCI_DEVICE(0x8086, 0xa348),
{ 0, } }; MODULE_DEVICE_TABLE(pci, skl_ids);.driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
-- 2.17.1
On Wed, Nov 21, 2018 at 11:16:50AM -0600, Pierre-Louis Bossart wrote:
On 11/21/18 8:27 AM, Andy Shevchenko wrote:
May you consider to switch to PCI_DEVICE_DATA() first?
Is this really the recommended path?
The macro generates PCI_DEVICE_ID_##vend##_##dev, and I don't have a turn key #define PCI_DEVICE_ID_INTEL_AUDIO_CFL 0xa348 I can use. In a number of cases we have multiple variants of the same hardware, and it starts being painful to use a 20-letter macro to differentiate between INTEL_AUDIO_CFL_Y and INTEL_AUDIO_CFL_H. The explicit code and a short comment are more readable really.
git grep PCI_DEVICE_ID_INTEL gives me hundreds of definitions, some global, some local to specific drivers, doesn't seem like there is a well-agreed usage of this macro, is there? I don't mind making the change but I don't sense an strong argument for it?
Compare:
/* CFL */ { PCI_DEVICE(0x8086, 0xa348), .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
to something like:
#define PCI_DEVICE_ID_INTEL_AUDIO_CFL 0xa348 ...
{PCI_DEVICE_DATA(INTEL, AUDIO_CFL, &snd_soc_acpi_intel_cnl_machines)},
Macro is recently introduced, that's why not many users of it. At least I'm planning to clean up dwc3-pci.c using it.
On Wed, 21 Nov 2018 18:38:41 +0100, Andy Shevchenko wrote:
On Wed, Nov 21, 2018 at 11:16:50AM -0600, Pierre-Louis Bossart wrote:
On 11/21/18 8:27 AM, Andy Shevchenko wrote:
May you consider to switch to PCI_DEVICE_DATA() first?
Is this really the recommended path?
The macro generates PCI_DEVICE_ID_##vend##_##dev, and I don't have a turn key #define PCI_DEVICE_ID_INTEL_AUDIO_CFL 0xa348 I can use. In a number of cases we have multiple variants of the same hardware, and it starts being painful to use a 20-letter macro to differentiate between INTEL_AUDIO_CFL_Y and INTEL_AUDIO_CFL_H. The explicit code and a short comment are more readable really.
git grep PCI_DEVICE_ID_INTEL gives me hundreds of definitions, some global, some local to specific drivers, doesn't seem like there is a well-agreed usage of this macro, is there? I don't mind making the change but I don't sense an strong argument for it?
Compare:
/* CFL */ { PCI_DEVICE(0x8086, 0xa348), .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
to something like:
#define PCI_DEVICE_ID_INTEL_AUDIO_CFL 0xa348 ...
{PCI_DEVICE_DATA(INTEL, AUDIO_CFL, &snd_soc_acpi_intel_cnl_machines)},
The former gives a better grep-ability, though.
I have no big preference over two, just want to mention that both have merits and demerits.
thanks,
Takashi
On Wed, Nov 21, 2018 at 11:17:41PM +0100, Takashi Iwai wrote:
On Wed, 21 Nov 2018 18:38:41 +0100, Andy Shevchenko wrote:
Compare:
/* CFL */ { PCI_DEVICE(0x8086, 0xa348), .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
to something like:
#define PCI_DEVICE_ID_INTEL_AUDIO_CFL 0xa348 ...
{PCI_DEVICE_DATA(INTEL, AUDIO_CFL, &snd_soc_acpi_intel_cnl_machines)},
The former gives a better grep-ability, though.
I have no big preference over two, just want to mention that both have merits and demerits.
True. So, up to you, guys, what to choose.
On 11/22/18 3:56 AM, Andy Shevchenko wrote:
On Wed, Nov 21, 2018 at 11:17:41PM +0100, Takashi Iwai wrote:
On Wed, 21 Nov 2018 18:38:41 +0100, Andy Shevchenko wrote:
Compare:
/* CFL */ { PCI_DEVICE(0x8086, 0xa348), .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines},
to something like:
#define PCI_DEVICE_ID_INTEL_AUDIO_CFL 0xa348 ...
{PCI_DEVICE_DATA(INTEL, AUDIO_CFL, &snd_soc_acpi_intel_cnl_machines)},
The former gives a better grep-ability, though.
I have no big preference over two, just want to mention that both have merits and demerits.
True. So, up to you, guys, what to choose.
I am leaning to leave the PCI stuff alone for now. If we change the PCI definitions I'd like to do it on the SKL and legacy sides at the same time, to avoid multiple definitions or redundancies.
However I like your suggestion for the macros on the other patch so will change the code.
Thanks for the reviews!
Check immediately if the DSP can be found, bail and avoid doing inits to enable legacy fallback without delay.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/skylake/skl.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index df36b8fe6d5e..1d7146773d19 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -931,6 +931,12 @@ static int skl_first_init(struct hdac_bus *bus)
snd_hdac_bus_parse_capabilities(bus);
+ /* check if dsp is there */ + if (!bus->ppcap) { + dev_err(bus->dev, "bus ppcap not set, DSP not present?\n"); + return -ENODEV; + } + if (skl_acquire_irq(bus, 0) < 0) return -EBUSY;
@@ -940,23 +946,25 @@ static int skl_first_init(struct hdac_bus *bus) gcap = snd_hdac_chip_readw(bus, GCAP); dev_dbg(bus->dev, "chipset global capabilities = 0x%x\n", gcap);
- /* allow 64bit DMA address if supported by H/W */ - if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) { - dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64)); - } else { - dma_set_mask(bus->dev, DMA_BIT_MASK(32)); - dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32)); - } - /* read number of streams from GCAP register */ cp_streams = (gcap >> 8) & 0x0f; pb_streams = (gcap >> 12) & 0x0f;
- if (!pb_streams && !cp_streams) + if (!pb_streams && !cp_streams) { + dev_err(bus->dev, "no streams found in GCAP definitions?\n"); return -EIO; + }
bus->num_streams = cp_streams + pb_streams;
+ /* allow 64bit DMA address if supported by H/W */ + if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) { + dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64)); + } else { + dma_set_mask(bus->dev, DMA_BIT_MASK(32)); + dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32)); + } + /* initialize streams */ snd_hdac_ext_stream_init_all (bus, 0, cp_streams, SNDRV_PCM_STREAM_CAPTURE);
On Tue, Nov 20, 2018 at 03:36:40PM -0600, Pierre-Louis Bossart wrote:
Check immediately if the DSP can be found, bail and avoid doing inits to enable legacy fallback without delay.
It does slightly more than described. Please do either remove unrelated changes, or fill the gap in the commit message. In the latter case it may require to split patch to two.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/intel/skylake/skl.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index df36b8fe6d5e..1d7146773d19 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -931,6 +931,12 @@ static int skl_first_init(struct hdac_bus *bus)
snd_hdac_bus_parse_capabilities(bus);
- /* check if dsp is there */
- if (!bus->ppcap) {
dev_err(bus->dev, "bus ppcap not set, DSP not present?\n");
return -ENODEV;
- }
- if (skl_acquire_irq(bus, 0) < 0) return -EBUSY;
@@ -940,23 +946,25 @@ static int skl_first_init(struct hdac_bus *bus) gcap = snd_hdac_chip_readw(bus, GCAP); dev_dbg(bus->dev, "chipset global capabilities = 0x%x\n", gcap);
/* allow 64bit DMA address if supported by H/W */
if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
} else {
dma_set_mask(bus->dev, DMA_BIT_MASK(32));
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
}
/* read number of streams from GCAP register */ cp_streams = (gcap >> 8) & 0x0f; pb_streams = (gcap >> 12) & 0x0f;
if (!pb_streams && !cp_streams)
if (!pb_streams && !cp_streams) {
dev_err(bus->dev, "no streams found in GCAP definitions?\n");
return -EIO;
}
bus->num_streams = cp_streams + pb_streams;
/* allow 64bit DMA address if supported by H/W */
if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
} else {
dma_set_mask(bus->dev, DMA_BIT_MASK(32));
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
}
/* initialize streams */ snd_hdac_ext_stream_init_all (bus, 0, cp_streams, SNDRV_PCM_STREAM_CAPTURE);
-- 2.17.1
On 11/21/18 8:29 AM, Andy Shevchenko wrote:
On Tue, Nov 20, 2018 at 03:36:40PM -0600, Pierre-Louis Bossart wrote:
Check immediately if the DSP can be found, bail and avoid doing inits to enable legacy fallback without delay.
It does slightly more than described. Please do either remove unrelated changes, or fill the gap in the commit message. In the latter case it may require to split patch to two.
ok, maybe I should change the commit message since there are really two test conditions: DSP can be found and DSP streams found. The code really does just that, nothing else.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/intel/skylake/skl.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index df36b8fe6d5e..1d7146773d19 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -931,6 +931,12 @@ static int skl_first_init(struct hdac_bus *bus)
snd_hdac_bus_parse_capabilities(bus);
- /* check if dsp is there */
- if (!bus->ppcap) {
dev_err(bus->dev, "bus ppcap not set, DSP not present?\n");
return -ENODEV;
- }
- if (skl_acquire_irq(bus, 0) < 0) return -EBUSY;
@@ -940,23 +946,25 @@ static int skl_first_init(struct hdac_bus *bus) gcap = snd_hdac_chip_readw(bus, GCAP); dev_dbg(bus->dev, "chipset global capabilities = 0x%x\n", gcap);
/* allow 64bit DMA address if supported by H/W */
if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
} else {
dma_set_mask(bus->dev, DMA_BIT_MASK(32));
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
}
/* read number of streams from GCAP register */ cp_streams = (gcap >> 8) & 0x0f; pb_streams = (gcap >> 12) & 0x0f;
if (!pb_streams && !cp_streams)
if (!pb_streams && !cp_streams) {
dev_err(bus->dev, "no streams found in GCAP definitions?\n");
return -EIO;
}
bus->num_streams = cp_streams + pb_streams;
/* allow 64bit DMA address if supported by H/W */
if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
} else {
dma_set_mask(bus->dev, DMA_BIT_MASK(32));
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
}
/* initialize streams */ snd_hdac_ext_stream_init_all (bus, 0, cp_streams, SNDRV_PCM_STREAM_CAPTURE);
-- 2.17.1
On Wed, Nov 21, 2018 at 10:48:57AM -0600, Pierre-Louis Bossart wrote:
On 11/21/18 8:29 AM, Andy Shevchenko wrote:
On Tue, Nov 20, 2018 at 03:36:40PM -0600, Pierre-Louis Bossart wrote:
Check immediately if the DSP can be found, bail and avoid doing inits to enable legacy fallback without delay.
It does slightly more than described. Please do either remove unrelated changes, or fill the gap in the commit message. In the latter case it may require to split patch to two.
ok, maybe I should change the commit message since there are really two test conditions: DSP can be found and DSP streams found. The code really does just that, nothing else.
How shuffling DMA mask related to any?
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/intel/skylake/skl.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index df36b8fe6d5e..1d7146773d19 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -931,6 +931,12 @@ static int skl_first_init(struct hdac_bus *bus) snd_hdac_bus_parse_capabilities(bus);
- /* check if dsp is there */
- if (!bus->ppcap) {
dev_err(bus->dev, "bus ppcap not set, DSP not present?\n");
return -ENODEV;
- }
- if (skl_acquire_irq(bus, 0) < 0) return -EBUSY;
@@ -940,23 +946,25 @@ static int skl_first_init(struct hdac_bus *bus) gcap = snd_hdac_chip_readw(bus, GCAP); dev_dbg(bus->dev, "chipset global capabilities = 0x%x\n", gcap);
- /* allow 64bit DMA address if supported by H/W */
- if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
- } else {
dma_set_mask(bus->dev, DMA_BIT_MASK(32));
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
- }
- /* read number of streams from GCAP register */ cp_streams = (gcap >> 8) & 0x0f; pb_streams = (gcap >> 12) & 0x0f;
- if (!pb_streams && !cp_streams)
- if (!pb_streams && !cp_streams) {
return -EIO;dev_err(bus->dev, "no streams found in GCAP definitions?\n");
- } bus->num_streams = cp_streams + pb_streams;
- /* allow 64bit DMA address if supported by H/W */
- if (!dma_set_mask(bus->dev, DMA_BIT_MASK(64))) {
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(64));
- } else {
dma_set_mask(bus->dev, DMA_BIT_MASK(32));
dma_set_coherent_mask(bus->dev, DMA_BIT_MASK(32));
- }
- /* initialize streams */ snd_hdac_ext_stream_init_all (bus, 0, cp_streams, SNDRV_PCM_STREAM_CAPTURE);
-- 2.17.1
bus->ppcap is now tested upfront, there is no need to re-check if the DSP is present. Remove tests and remove indentation.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/skylake/skl.c | 40 ++++++++++++++++------------------- 1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index 1d7146773d19..0bdb1f7fdd4a 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -826,12 +826,10 @@ static void skl_probe_work(struct work_struct *work) return; }
- if (bus->ppcap) { - err = skl_machine_device_register(skl); - if (err < 0) { - dev_err(bus->dev, "machine register failed: %d\n", err); - goto out_err; - } + err = skl_machine_device_register(skl); + if (err < 0) { + dev_err(bus->dev, "machine register failed: %d\n", err); + goto out_err; }
/* @@ -1019,25 +1017,23 @@ static int skl_probe(struct pci_dev *pci,
pci_set_drvdata(skl->pci, bus);
- /* check if dsp is there */ - if (bus->ppcap) { - /* create device for dsp clk */ - err = skl_clock_device_register(skl); - if (err < 0) - goto out_clk_free; + /* create device for dsp clk */ + err = skl_clock_device_register(skl); + if (err < 0) + goto out_clk_free;
- err = skl_find_machine(skl, (void *)pci_id->driver_data); - if (err < 0) - goto out_nhlt_free; + err = skl_find_machine(skl, (void *)pci_id->driver_data); + if (err < 0) + goto out_nhlt_free;
- err = skl_init_dsp(skl); - if (err < 0) { - dev_dbg(bus->dev, "error failed to register dsp\n"); - goto out_nhlt_free; - } - skl->skl_sst->enable_miscbdcge = skl_enable_miscbdcge; - skl->skl_sst->clock_power_gating = skl_clock_power_gating; + err = skl_init_dsp(skl); + if (err < 0) { + dev_dbg(bus->dev, "error failed to register dsp\n"); + goto out_nhlt_free; } + skl->skl_sst->enable_miscbdcge = skl_enable_miscbdcge; + skl->skl_sst->clock_power_gating = skl_clock_power_gating; + if (bus->mlcap) snd_hdac_ext_bus_get_ml_capabilities(bus);
The current SKYLAKE kconfig is a all-you-can-eat selection that will support all known plaforms. This is however not necessarily a good thing: most platforms for SKL and KBL don't support the DSP, but a number of CNL/WHL ones do. Selecting this driver in all cases isn't really smart and will require users to muck with blacklists.
Partition the configs to allow distributions to select on which platform this driver is used. Keep the existing SND_SOC_INTEL_SKYLAKE config to select everything for backwards compatibility. This patch does not provide new functionality, only finer-grained choices in supported platforms.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/intel/Kconfig | 73 ++++++++++++++++++++++++++++++---- sound/soc/intel/boards/Kconfig | 16 +++++++- sound/soc/intel/skylake/skl.c | 12 ++++++ 3 files changed, 92 insertions(+), 9 deletions(-)
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index 18e717703685..99a62ba409df 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -102,15 +102,74 @@ config SND_SST_ATOM_HIFI2_PLATFORM_ACPI recommended option
config SND_SOC_INTEL_SKYLAKE - tristate "SKL/BXT/KBL/GLK/CNL... Platforms" + tristate "All Skylake/SST Platforms" depends on PCI && ACPI - select SND_SOC_INTEL_SKYLAKE_COMMON + select SND_SOC_INTEL_SKL + select SND_SOC_INTEL_APL + select SND_SOC_INTEL_KBL + select SND_SOC_INTEL_GLK + select SND_SOC_INTEL_CNL + select SND_SOC_INTEL_CFL help - If you have a Intel Skylake/Broxton/ApolloLake/KabyLake/ - GeminiLake or CannonLake platform with the DSP enabled in the BIOS - then enable this option by saying Y or m. + This is a backwards-compatible option to select all devices + supported by the Intel SST/Skylake driver. This option is no + longer recommended and will be deprecated when the SOF + driver is introduced. Distributions should explicitly + select which platform uses this driver. + +config SND_SOC_INTEL_SKL + tristate "Skylake Platforms" + depends on PCI && ACPI + select SND_SOC_INTEL_SKYLAKE_FAMILY + help + If you have a Intel Skylake platform with the DSP enabled + in the BIOS then enable this option by saying Y or m. + +config SND_SOC_INTEL_APL + tristate "Broxton/ApolloLake Platforms" + depends on PCI && ACPI + select SND_SOC_INTEL_SKYLAKE_FAMILY + help + If you have a Intel Broxton/ApolloLake platform with the DSP + enabled in the BIOS then enable this option by saying Y or m. + +config SND_SOC_INTEL_KBL + tristate "Kabylake Platforms" + depends on PCI && ACPI + select SND_SOC_INTEL_SKYLAKE_FAMILY + help + If you have a Intel Kabylake platform with the DSP + enabled in the BIOS then enable this option by saying Y or m. + +config SND_SOC_INTEL_GLK + tristate "GeminiLake Platforms" + depends on PCI && ACPI + select SND_SOC_INTEL_SKYLAKE_FAMILY + help + If you have a Intel GeminiLake platform with the DSP + enabled in the BIOS then enable this option by saying Y or m. + +config SND_SOC_INTEL_CNL + tristate "CannonLake/WhiskyLake Platforms" + depends on PCI && ACPI + select SND_SOC_INTEL_SKYLAKE_FAMILY + help + If you have a Intel CNL/WHL platform with the DSP + enabled in the BIOS then enable this option by saying Y or m. + +config SND_SOC_INTEL_CFL + tristate "CoffeeLake Platforms" + depends on PCI && ACPI + select SND_SOC_INTEL_SKYLAKE_FAMILY + help + If you have a Intel CoffeeLake platform with the DSP + enabled in the BIOS then enable this option by saying Y or m. + +config SND_SOC_INTEL_SKYLAKE_FAMILY + tristate + select SND_SOC_INTEL_SKYLAKE_COMMON
-if SND_SOC_INTEL_SKYLAKE +if SND_SOC_INTEL_SKYLAKE_FAMILY
config SND_SOC_INTEL_SKYLAKE_SSP_CLK tristate @@ -135,7 +194,7 @@ config SND_SOC_INTEL_SKYLAKE_COMMON GeminiLake or CannonLake platform with the DSP enabled in the BIOS then enable this option by saying Y or m.
-endif ## SND_SOC_INTEL_SKYLAKE +endif ## SND_SOC_INTEL_SKYLAKE_FAMILY
config SND_SOC_ACPI_INTEL_MATCH tristate diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index b177db2a0dbb..980bccaeb7aa 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -172,7 +172,7 @@ config SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH
endif ## SND_SST_ATOM_HIFI2_PLATFORM
-if SND_SOC_INTEL_SKYLAKE +if SND_SOC_INTEL_SKL
config SND_SOC_INTEL_SKL_RT286_MACH tristate "SKL with RT286 I2S mode" @@ -212,6 +212,10 @@ config SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH Say Y or m if you have such a device. This is a recommended option. If unsure select "N".
+endif ## SND_SOC_INTEL_SKL + +if SND_SOC_INTEL_APL + config SND_SOC_INTEL_BXT_DA7219_MAX98357A_MACH tristate "Broxton with DA7219 and MAX98357A in I2S Mode" depends on MFD_INTEL_LPSS && I2C && ACPI @@ -239,6 +243,10 @@ config SND_SOC_INTEL_BXT_RT298_MACH Say Y or m if you have such a device. This is a recommended option. If unsure select "N".
+endif ## SND_SOC_INTEL_APL + +if SND_SOC_INTEL_KBL + config SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH tristate "KBL with RT5663 and MAX98927 in I2S Mode" depends on MFD_INTEL_LPSS && I2C && ACPI @@ -293,6 +301,10 @@ config SND_SOC_INTEL_KBL_DA7219_MAX98927_MACH Say Y if you have such a device. If unsure select "N".
+endif ## SND_SOC_INTEL_KBL + +if SND_SOC_INTEL_GLK + config SND_SOC_INTEL_GLK_RT5682_MAX98357A_MACH tristate "GLK with RT5682 and MAX98357A in I2S Mode" depends on MFD_INTEL_LPSS && I2C && ACPI @@ -307,7 +319,7 @@ config SND_SOC_INTEL_GLK_RT5682_MAX98357A_MACH Say Y if you have such a device. If unsure select "N".
-endif ## SND_SOC_INTEL_SKYLAKE +endif ## SND_SOC_INTEL_GLK
if SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index 0bdb1f7fdd4a..73f82532770e 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -1110,24 +1110,36 @@ static void skl_remove(struct pci_dev *pci)
/* PCI IDs */ static const struct pci_device_id skl_ids[] = { +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_SKL) /* Sunrise Point-LP */ { PCI_DEVICE(0x8086, 0x9d70), .driver_data = (unsigned long)&snd_soc_acpi_intel_skl_machines}, +#endif +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_APL) /* BXT-P */ { PCI_DEVICE(0x8086, 0x5a98), .driver_data = (unsigned long)&snd_soc_acpi_intel_bxt_machines}, +#endif +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_KBL) /* KBL */ { PCI_DEVICE(0x8086, 0x9D71), .driver_data = (unsigned long)&snd_soc_acpi_intel_kbl_machines}, +#endif +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_GLK) /* GLK */ { PCI_DEVICE(0x8086, 0x3198), .driver_data = (unsigned long)&snd_soc_acpi_intel_glk_machines}, +#endif +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CNL) /* CNL */ { PCI_DEVICE(0x8086, 0x9dc8), .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines}, +#endif +#if IS_ENABLED(CONFIG_SND_SOC_INTEL_CFL) /* CFL */ { PCI_DEVICE(0x8086, 0xa348), .driver_data = (unsigned long)&snd_soc_acpi_intel_cnl_machines}, +#endif { 0, } }; MODULE_DEVICE_TABLE(pci, skl_ids);
From: Takashi Iwai tiwai@suse.de
Previous patches enabled distributions to select for which platform the Skylake driver will be used. This is not enough however since the DSP may or may not enabled by the BIOS, and a probe-time dynamic fallback is highly desired.
This patch introduces a new AZX_DCAPS value, and when this field is set for the relevant PCI ID the HDaudio legacy driver will stop its probe. Conversely the Skylake driver will attempt to probe, but in case of errors (typically DSP not present or no HDAudio streams found) it will fall back to the HDaudio legacy.
The behavior can be influenced by static Kconfig definitions to enable or disable this fallback. A 'legacy' parameter also controls the fallback, allowing testers to prevent the Skylake driver from probing or conversely preventing the fallback from happening.
This patch only introduces the fallback capabilities and handling, the platform definitions are provided in a follow-up patch.
Errors beyond the initial DSP capability parsing (e.g. missing firmware file, firmware authentication issue, missing topology file, bad topology configuration, etc) are considered out-of-scope. It is nearly impossible to handle all possible error cases and most errors are only detected after a timeout which isn't compatible with usual probe expectations. It is expected that additional debug hooks will have to be provided at a later point, e.g. module parameters to override hard-coded firmware name and topology files.
Credits to Takashi for the initial code shared on the alsa-devel mailing list. Testing with base-defconfig, sst-defconfig and hdaudio-defconfigs provided at https://github.com/thesofproject/kconfig
Signed-off-by: Takashi Iwai tiwai@suse.de Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- include/sound/hdaudio.h | 6 +++ sound/pci/hda/hda_controller.h | 2 +- sound/pci/hda/hda_intel.c | 32 ++++++++++++++- sound/soc/intel/Kconfig | 7 ++++ sound/soc/intel/skylake/skl.c | 73 ++++++++++++++++++++++++++++++++-- 5 files changed, 114 insertions(+), 6 deletions(-)
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index cd1773d0e08f..2ed67f315962 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -8,6 +8,7 @@
#include <linux/device.h> #include <linux/interrupt.h> +#include <linux/pci.h> #include <linux/pm_runtime.h> #include <linux/timecounter.h> #include <sound/core.h> @@ -634,4 +635,9 @@ static inline unsigned int snd_array_index(struct snd_array *array, void *ptr) for ((idx) = 0, (ptr) = (array)->list; (idx) < (array)->used; \ (ptr) = snd_array_elem(array, ++(idx)))
+/* shared resource with ASoC and legacy HD-audio drivers */ +#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT +const struct pci_driver *snd_hda_intel_probe(struct pci_dev *pci); +#endif + #endif /* __SOUND_HDAUDIO_H */ diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h index c95097bb5a0c..2351115f922e 100644 --- a/sound/pci/hda/hda_controller.h +++ b/sound/pci/hda/hda_controller.h @@ -37,7 +37,7 @@ #else #define AZX_DCAPS_I915_COMPONENT 0 /* NOP */ #endif -/* 14 unused */ +#define AZX_DCAPS_INTEL_SHARED (1 << 14) /* shared with ASoC */ #define AZX_DCAPS_CTX_WORKAROUND (1 << 15) /* X-Fi workaround */ #define AZX_DCAPS_POSFIX_LPIB (1 << 16) /* Use LPIB as default */ /* 17 unused */ diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index d8eb2b5f51ae..eb00e37c1c27 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -2081,8 +2081,8 @@ static const struct hda_controller_ops pci_hda_ops = { .link_power = azx_intel_link_power, };
-static int azx_probe(struct pci_dev *pci, - const struct pci_device_id *pci_id) +static int __azx_probe(struct pci_dev *pci, const struct pci_device_id *pci_id, + bool skip_shared) { static int dev; struct snd_card *card; @@ -2091,6 +2091,10 @@ static int azx_probe(struct pci_dev *pci, bool schedule_probe; int err;
+ /* skip the entry if it's shared with ASoC */ + if (skip_shared && (pci_id->driver_data & AZX_DCAPS_INTEL_SHARED)) + return -ENODEV; + if (dev >= SNDRV_CARDS) return -ENODEV; if (!enable[dev]) { @@ -2158,6 +2162,12 @@ static int azx_probe(struct pci_dev *pci, return err; }
+static int azx_probe(struct pci_dev *pci, const struct pci_device_id *pci_id) +{ + return __azx_probe(pci, pci_id, + IS_ENABLED(CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT)); +} + #ifdef CONFIG_PM /* On some boards setting power_save to a non 0 value leads to clicking / * popping sounds when ever we enter/leave powersaving mode. Ideally we would @@ -2650,4 +2660,22 @@ static struct pci_driver azx_driver = { }, };
+#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT +const struct pci_driver * +snd_hda_intel_probe(struct pci_dev *pci) +{ + const struct pci_device_id *pci_id; + int ret; + + pci_id = pci_match_id(azx_ids, pci); + if (!pci_id) + return ERR_PTR(-ENODEV); + ret = __azx_probe(pci, pci_id, false); + if (ret < 0) + return ERR_PTR(ret); + return &azx_driver; +} +EXPORT_SYMBOL_GPL(snd_hda_intel_probe); +#endif + module_pci_driver(azx_driver); diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index 99a62ba409df..c02d08d31d0d 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -194,6 +194,13 @@ config SND_SOC_INTEL_SKYLAKE_COMMON GeminiLake or CannonLake platform with the DSP enabled in the BIOS then enable this option by saying Y or m.
+config SND_SOC_INTEL_SKL_LEGACY_SUPPORT + bool "Fallback legacy HD-audio binding" + depends on SND_HDA_INTEL=y || SND_SOC_INTEL_SKYLAKE_FAMILY=SND_HDA_INTEL + help + Fallback binding with the legacy HD-audio driver when no DSP is + found + endif ## SND_SOC_INTEL_SKYLAKE_FAMILY
config SND_SOC_ACPI_INTEL_MATCH diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index 73f82532770e..b67cb54d382c 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -41,6 +41,21 @@ #include "../../../soc/codecs/hdac_hda.h" #endif
+enum { + SKL_BIND_AUTO, /* fallback to legacy driver */ + SKL_BIND_LEGACY, /* bind only with legacy driver */ + SKL_BIND_ASOC /* bind only with ASoC driver */ +}; + +#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT +static const struct pci_driver *skl_fallback_driver; +#define FALLBACK_PM_CALL(dev, method) \ + do { \ + if (skl_fallback_driver && \ + skl_fallback_driver->driver.pm->method) \ + return skl_fallback_driver->driver.pm->method(dev); \ + } while (0) + /* * initialize the PCI registers */ @@ -264,6 +279,9 @@ static int skl_suspend_late(struct device *dev) struct hdac_bus *bus = pci_get_drvdata(pci); struct skl *skl = bus_to_skl(bus);
+ if (skl_fallback_driver) + return 0; + return skl_suspend_late_dsp(skl); }
@@ -313,6 +331,8 @@ static int skl_suspend(struct device *dev) struct skl *skl = bus_to_skl(bus); int ret = 0;
+ FALLBACK_PM_CALL(dev, suspend); + /* * Do not suspend if streams which are marked ignore suspend are * running, we need to save the state for these and continue @@ -351,6 +371,8 @@ static int skl_resume(struct device *dev) struct hdac_ext_link *hlink = NULL; int ret;
+ FALLBACK_PM_CALL(dev, resume); + /* Turned OFF in HDMI codec driver after codec reconfiguration */ if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)) { ret = snd_hdac_display_power(bus, true); @@ -405,6 +427,8 @@ static int skl_runtime_suspend(struct device *dev) struct pci_dev *pci = to_pci_dev(dev); struct hdac_bus *bus = pci_get_drvdata(pci);
+ FALLBACK_PM_CALL(dev, runtime_suspend); + dev_dbg(bus->dev, "in %s\n", __func__);
return _skl_suspend(bus); @@ -415,15 +439,24 @@ static int skl_runtime_resume(struct device *dev) struct pci_dev *pci = to_pci_dev(dev); struct hdac_bus *bus = pci_get_drvdata(pci);
+ FALLBACK_PM_CALL(dev, runtime_resume); + dev_dbg(bus->dev, "in %s\n", __func__);
return _skl_resume(bus); } + +static int skl_runtime_idle(struct device *dev) +{ + FALLBACK_PM_CALL(dev, runtime_idle); + return 0; +} #endif /* CONFIG_PM */
static const struct dev_pm_ops skl_pm = { SET_SYSTEM_SLEEP_PM_OPS(skl_suspend, skl_resume) - SET_RUNTIME_PM_OPS(skl_runtime_suspend, skl_runtime_resume, NULL) + SET_RUNTIME_PM_OPS(skl_runtime_suspend, skl_runtime_resume, + skl_runtime_idle) .suspend_late = skl_suspend_late, };
@@ -980,8 +1013,8 @@ static int skl_first_init(struct hdac_bus *bus) return skl_init_chip(bus, true); }
-static int skl_probe(struct pci_dev *pci, - const struct pci_device_id *pci_id) +static int __skl_probe(struct pci_dev *pci, + const struct pci_device_id *pci_id) { struct skl *skl; struct hdac_bus *bus = NULL; @@ -1060,6 +1093,25 @@ static int skl_probe(struct pci_dev *pci, return err; }
+static int skl_probe(struct pci_dev *pci, + const struct pci_device_id *pci_id) +{ + int ret = -ENODEV; + + if (skl_legacy_binding != SKL_BIND_LEGACY) + ret = __skl_probe(pci, pci_id); + +#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT + if (ret < 0 && skl_legacy_binding != SKL_BIND_ASOC) { + skl_fallback_driver = snd_hda_intel_probe(pci); + ret = PTR_ERR_OR_ZERO(skl_fallback_driver); + if (ret) + skl_fallback_driver = NULL; + } +#endif + return ret; +} + static void skl_shutdown(struct pci_dev *pci) { struct hdac_bus *bus = pci_get_drvdata(pci); @@ -1067,6 +1119,13 @@ static void skl_shutdown(struct pci_dev *pci) struct hdac_ext_stream *stream; struct skl *skl;
+#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT + if (skl_fallback_driver) { + skl_fallback_driver->shutdown(pci); + return; + } +#endif + if (!bus) return;
@@ -1089,6 +1148,14 @@ static void skl_remove(struct pci_dev *pci) struct hdac_bus *bus = pci_get_drvdata(pci); struct skl *skl = bus_to_skl(bus);
+#ifdef CONFIG_SND_SOC_INTEL_SKL_LEGACY_SUPPORT + if (skl_fallback_driver) { + skl_fallback_driver->remove(pci); + skl_fallback_driver = NULL; + return; + } +#endif + release_firmware(skl->tplg);
pm_runtime_get_noresume(&pci->dev);
Enable fallback for select PCI IDs
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/pci/hda/Kconfig | 40 +++++++++++++++++++++++++++++++++++++++ sound/pci/hda/hda_intel.c | 19 +++++++++++++------ sound/soc/intel/Kconfig | 6 ++++++ 3 files changed, 59 insertions(+), 6 deletions(-)
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 4235907b7858..9bb317fb3507 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -228,4 +228,44 @@ config SND_HDA_POWER_SAVE_DEFAULT
endif
+if SND_HDA_INTEL + +config SND_HDA_INTEL_LEGACY_FALLBACK_SKL + bool + help + This option enables HD-audio legacy fallback for + Skylake machines + +config SND_HDA_INTEL_LEGACY_FALLBACK_APL + bool + help + This option enables HD-audio legacy fallback for + Broxton/ApolloLake machines + +config SND_HDA_INTEL_LEGACY_FALLBACK_KBL + bool + help + This option enables HD-audio legacy fallback for + KabyLake machines + +config SND_HDA_INTEL_LEGACY_FALLBACK_GLK + bool + help + This option enables HD-audio legacy fallback for + GeminiLake machines + +config SND_HDA_INTEL_LEGACY_FALLBACK_CNL + bool + help + This option enables HD-audio legacy fallback for + CannonLake machines + +config SND_HDA_INTEL_LEGACY_FALLBACK_CFL + bool + help + This option enables HD-audio legacy fallback for + CoffeeLake machines + +endif ## SND_HDA_INTEL + endmenu diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index eb00e37c1c27..569419242da3 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -360,6 +360,7 @@ enum { AZX_DCAPS_NO_64BIT |\ AZX_DCAPS_4K_BDLE_BOUNDARY | AZX_DCAPS_SNOOP_OFF)
+#define AZX_DCAPS_INTEL_LEGACY_FALLBACK(conf) (IS_ENABLED(conf) ? AZX_DCAPS_INTEL_SHARED : 0) /* * vga_switcheroo support */ @@ -2416,34 +2417,40 @@ static const struct pci_device_id azx_ids[] = { .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, /* Sunrise Point-LP */ { PCI_DEVICE(0x8086, 0x9d70), - .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, + .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE | + AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_SKL) }, /* Kabylake */ { PCI_DEVICE(0x8086, 0xa171), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, /* Kabylake-LP */ { PCI_DEVICE(0x8086, 0x9d71), - .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, + .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE | + AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_KBL) }, /* Kabylake-H */ { PCI_DEVICE(0x8086, 0xa2f0), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, /* Coffelake */ { PCI_DEVICE(0x8086, 0xa348), - .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE}, + .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE | + AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_CFL) }, /* Cannonlake */ { PCI_DEVICE(0x8086, 0x9dc8), - .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE}, + .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE | + AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_CNL) }, /* Icelake */ { PCI_DEVICE(0x8086, 0x34c8), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE}, /* Broxton-P(Apollolake) */ { PCI_DEVICE(0x8086, 0x5a98), - .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON }, + .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON | + AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_APL) }, /* Broxton-T */ { PCI_DEVICE(0x8086, 0x1a98), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON }, /* Gemini-Lake */ { PCI_DEVICE(0x8086, 0x3198), - .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON }, + .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON | + AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_GLK) }, /* Haswell */ { PCI_DEVICE(0x8086, 0x0a0c), .driver_data = AZX_DRIVER_HDMI | AZX_DCAPS_INTEL_HASWELL }, diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index c02d08d31d0d..4c6abdbb0b90 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -197,6 +197,12 @@ config SND_SOC_INTEL_SKYLAKE_COMMON config SND_SOC_INTEL_SKL_LEGACY_SUPPORT bool "Fallback legacy HD-audio binding" depends on SND_HDA_INTEL=y || SND_SOC_INTEL_SKYLAKE_FAMILY=SND_HDA_INTEL + select SND_HDA_INTEL_LEGACY_FALLBACK_SKL if SND_SOC_INTEL_SKL + select SND_HDA_INTEL_LEGACY_FALLBACK_APL if SND_SOC_INTEL_APL + select SND_HDA_INTEL_LEGACY_FALLBACK_KBL if SND_SOC_INTEL_KBL + select SND_HDA_INTEL_LEGACY_FALLBACK_GLK if SND_SOC_INTEL_GLK + select SND_HDA_INTEL_LEGACY_FALLBACK_CNL if SND_SOC_INTEL_CNL + select SND_HDA_INTEL_LEGACY_FALLBACK_CFL if SND_SOC_INTEL_CFL help Fallback binding with the legacy HD-audio driver when no DSP is found
On Tue, Nov 20, 2018 at 03:36:44PM -0600, Pierre-Louis Bossart wrote:
Enable fallback for select PCI IDs
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/pci/hda/Kconfig | 40 +++++++++++++++++++++++++++++++++++++++ sound/pci/hda/hda_intel.c | 19 +++++++++++++------ sound/soc/intel/Kconfig | 6 ++++++ 3 files changed, 59 insertions(+), 6 deletions(-)
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 4235907b7858..9bb317fb3507 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -228,4 +228,44 @@ config SND_HDA_POWER_SAVE_DEFAULT
endif
+if SND_HDA_INTEL
+config SND_HDA_INTEL_LEGACY_FALLBACK_SKL
- bool
- help
This option enables HD-audio legacy fallback for
Skylake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_APL
- bool
- help
This option enables HD-audio legacy fallback for
Broxton/ApolloLake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_KBL
- bool
- help
This option enables HD-audio legacy fallback for
KabyLake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_GLK
- bool
- help
This option enables HD-audio legacy fallback for
GeminiLake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_CNL
- bool
- help
This option enables HD-audio legacy fallback for
CannonLake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_CFL
- bool
- help
This option enables HD-audio legacy fallback for
CoffeeLake machines
+endif ## SND_HDA_INTEL
endmenu diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index eb00e37c1c27..569419242da3 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -360,6 +360,7 @@ enum { AZX_DCAPS_NO_64BIT |\ AZX_DCAPS_4K_BDLE_BOUNDARY | AZX_DCAPS_SNOOP_OFF)
+#define AZX_DCAPS_INTEL_LEGACY_FALLBACK(conf) (IS_ENABLED(conf) ? AZX_DCAPS_INTEL_SHARED : 0) /*
- vga_switcheroo support
*/ @@ -2416,34 +2417,40 @@ static const struct pci_device_id azx_ids[] = { .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, /* Sunrise Point-LP */ { PCI_DEVICE(0x8086, 0x9d70),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_SKL) },
Preprocessor may concatenate the same prefix for you. I expect to see something like ..._FALLBACK(SKL) and so on.
Moreover, you can go further and create a macro that would consolidate all bits together.
/* Kabylake */ { PCI_DEVICE(0x8086, 0xa171), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, /* Kabylake-LP */ { PCI_DEVICE(0x8086, 0x9d71),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
/* Kabylake-H */ { PCI_DEVICE(0x8086, 0xa2f0), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, /* Coffelake */ { PCI_DEVICE(0x8086, 0xa348),AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_KBL) },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
/* Cannonlake */ { PCI_DEVICE(0x8086, 0x9dc8),AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_CFL) },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
/* Icelake */ { PCI_DEVICE(0x8086, 0x34c8), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE}, /* Broxton-P(Apollolake) */ { PCI_DEVICE(0x8086, 0x5a98),AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_CNL) },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON |
/* Broxton-T */ { PCI_DEVICE(0x8086, 0x1a98), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON }, /* Gemini-Lake */ { PCI_DEVICE(0x8086, 0x3198),AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_APL) },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON |
/* Haswell */ { PCI_DEVICE(0x8086, 0x0a0c), .driver_data = AZX_DRIVER_HDMI | AZX_DCAPS_INTEL_HASWELL },AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_GLK) },
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index c02d08d31d0d..4c6abdbb0b90 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -197,6 +197,12 @@ config SND_SOC_INTEL_SKYLAKE_COMMON config SND_SOC_INTEL_SKL_LEGACY_SUPPORT bool "Fallback legacy HD-audio binding" depends on SND_HDA_INTEL=y || SND_SOC_INTEL_SKYLAKE_FAMILY=SND_HDA_INTEL
- select SND_HDA_INTEL_LEGACY_FALLBACK_SKL if SND_SOC_INTEL_SKL
- select SND_HDA_INTEL_LEGACY_FALLBACK_APL if SND_SOC_INTEL_APL
- select SND_HDA_INTEL_LEGACY_FALLBACK_KBL if SND_SOC_INTEL_KBL
- select SND_HDA_INTEL_LEGACY_FALLBACK_GLK if SND_SOC_INTEL_GLK
- select SND_HDA_INTEL_LEGACY_FALLBACK_CNL if SND_SOC_INTEL_CNL
- select SND_HDA_INTEL_LEGACY_FALLBACK_CFL if SND_SOC_INTEL_CFL help Fallback binding with the legacy HD-audio driver when no DSP is found
-- 2.17.1
On Wed, 21 Nov 2018 15:39:39 +0100, Andy Shevchenko wrote:
On Tue, Nov 20, 2018 at 03:36:44PM -0600, Pierre-Louis Bossart wrote:
Enable fallback for select PCI IDs
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/pci/hda/Kconfig | 40 +++++++++++++++++++++++++++++++++++++++ sound/pci/hda/hda_intel.c | 19 +++++++++++++------ sound/soc/intel/Kconfig | 6 ++++++ 3 files changed, 59 insertions(+), 6 deletions(-)
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 4235907b7858..9bb317fb3507 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -228,4 +228,44 @@ config SND_HDA_POWER_SAVE_DEFAULT
endif
+if SND_HDA_INTEL
+config SND_HDA_INTEL_LEGACY_FALLBACK_SKL
- bool
- help
This option enables HD-audio legacy fallback for
Skylake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_APL
- bool
- help
This option enables HD-audio legacy fallback for
Broxton/ApolloLake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_KBL
- bool
- help
This option enables HD-audio legacy fallback for
KabyLake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_GLK
- bool
- help
This option enables HD-audio legacy fallback for
GeminiLake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_CNL
- bool
- help
This option enables HD-audio legacy fallback for
CannonLake machines
+config SND_HDA_INTEL_LEGACY_FALLBACK_CFL
- bool
- help
This option enables HD-audio legacy fallback for
CoffeeLake machines
+endif ## SND_HDA_INTEL
endmenu diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index eb00e37c1c27..569419242da3 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -360,6 +360,7 @@ enum { AZX_DCAPS_NO_64BIT |\ AZX_DCAPS_4K_BDLE_BOUNDARY | AZX_DCAPS_SNOOP_OFF)
+#define AZX_DCAPS_INTEL_LEGACY_FALLBACK(conf) (IS_ENABLED(conf) ? AZX_DCAPS_INTEL_SHARED : 0) /*
- vga_switcheroo support
*/ @@ -2416,34 +2417,40 @@ static const struct pci_device_id azx_ids[] = { .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, /* Sunrise Point-LP */ { PCI_DEVICE(0x8086, 0x9d70),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_SKL) },
Preprocessor may concatenate the same prefix for you. I expect to see something like ..._FALLBACK(SKL) and so on.
It' look shorter and better readable, but OTOH, keeping CONFIG_XYZ allows to search the kconfig more easily over the code. Again, we need to consider some drawback.
thanks,
Takashi
Moreover, you can go further and create a macro that would consolidate all bits together.
/* Kabylake */ { PCI_DEVICE(0x8086, 0xa171), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, /* Kabylake-LP */ { PCI_DEVICE(0x8086, 0x9d71),
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
/* Kabylake-H */ { PCI_DEVICE(0x8086, 0xa2f0), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE }, /* Coffelake */ { PCI_DEVICE(0x8086, 0xa348),AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_KBL) },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
/* Cannonlake */ { PCI_DEVICE(0x8086, 0x9dc8),AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_CFL) },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE},
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE |
/* Icelake */ { PCI_DEVICE(0x8086, 0x34c8), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_SKYLAKE}, /* Broxton-P(Apollolake) */ { PCI_DEVICE(0x8086, 0x5a98),AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_CNL) },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON |
/* Broxton-T */ { PCI_DEVICE(0x8086, 0x1a98), .driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON }, /* Gemini-Lake */ { PCI_DEVICE(0x8086, 0x3198),AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_APL) },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON },
.driver_data = AZX_DRIVER_SKL | AZX_DCAPS_INTEL_BROXTON |
/* Haswell */ { PCI_DEVICE(0x8086, 0x0a0c), .driver_data = AZX_DRIVER_HDMI | AZX_DCAPS_INTEL_HASWELL },AZX_DCAPS_INTEL_LEGACY_FALLBACK(CONFIG_SND_HDA_INTEL_LEGACY_FALLBACK_GLK) },
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index c02d08d31d0d..4c6abdbb0b90 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -197,6 +197,12 @@ config SND_SOC_INTEL_SKYLAKE_COMMON config SND_SOC_INTEL_SKL_LEGACY_SUPPORT bool "Fallback legacy HD-audio binding" depends on SND_HDA_INTEL=y || SND_SOC_INTEL_SKYLAKE_FAMILY=SND_HDA_INTEL
- select SND_HDA_INTEL_LEGACY_FALLBACK_SKL if SND_SOC_INTEL_SKL
- select SND_HDA_INTEL_LEGACY_FALLBACK_APL if SND_SOC_INTEL_APL
- select SND_HDA_INTEL_LEGACY_FALLBACK_KBL if SND_SOC_INTEL_KBL
- select SND_HDA_INTEL_LEGACY_FALLBACK_GLK if SND_SOC_INTEL_GLK
- select SND_HDA_INTEL_LEGACY_FALLBACK_CNL if SND_SOC_INTEL_CNL
- select SND_HDA_INTEL_LEGACY_FALLBACK_CFL if SND_SOC_INTEL_CFL help Fallback binding with the legacy HD-audio driver when no DSP is found
-- 2.17.1
-- With Best Regards, Andy Shevchenko
On Tue, 20 Nov 2018 22:36:38 +0100, Pierre-Louis Bossart wrote:
This patchset is provided as an RFC and should not be merged as is (Turkey break in the USA and more validation needed). This is however a good time to gather comments. This work is the result of multiple email discussions to finally provide more flexibility for distributions to chose whether to stick with the legacy HDaudio driver or to enable the SST/Skylake driver for newer platforms (required e.g. for digital microphone support)
The patches add support for CoffeeLake, simplify the probe for the Skylake driver, introduce more compile-time granularity so that platforms can be selected individually and last provide a dynamic fallback mechanism when two drivers are registered for the same PCI ID.
When the SOF driver is released, the same mechanism will be used to enable the SOF-legacy fallback. There will be no plans to enable an SOF->SST falback.
This looks like a sensible way to go, thanks for working on this!
While the fallback stuff might need more testing, the other patches (addition of CFL-S and split of configs) are rather systematic, so we can merge this at first soonish.
And we may need a bit more comments in Kconfig help for the fallback behavior. Or document it properly and refer to it from Kconfig help. The git commit log isn't present in the released kernel code, after all.
thanks,
Takashi
Pierre-Louis Bossart (4): ASoC: Intel: Skylake: stop init/probe if DSP is not present ASoC: Intel: Skylake: remove useless tests on DSP presence ASoC: Intel: Skylake: Add more platform granularity ALSA: hda: add fallback capabilities for SKL+ platforms
Takashi Iwai (2): ASoC: Intel: Skylake: Add CFL-S support ALSA: hda: Allow fallback binding with legacy HD-audio for Intel SKL+
include/sound/hdaudio.h | 6 + sound/pci/hda/Kconfig | 40 +++++++ sound/pci/hda/hda_controller.h | 2 +- sound/pci/hda/hda_intel.c | 51 ++++++-- sound/soc/intel/Kconfig | 86 ++++++++++++-- sound/soc/intel/boards/Kconfig | 16 ++- sound/soc/intel/skylake/skl-messages.c | 8 ++ sound/soc/intel/skylake/skl.c | 154 +++++++++++++++++++------ 8 files changed, 311 insertions(+), 52 deletions(-)
-- 2.17.1
On 11/21/18 4:32 AM, Takashi Iwai wrote:
On Tue, 20 Nov 2018 22:36:38 +0100, Pierre-Louis Bossart wrote:
This patchset is provided as an RFC and should not be merged as is (Turkey break in the USA and more validation needed). This is however a good time to gather comments. This work is the result of multiple email discussions to finally provide more flexibility for distributions to chose whether to stick with the legacy HDaudio driver or to enable the SST/Skylake driver for newer platforms (required e.g. for digital microphone support)
The patches add support for CoffeeLake, simplify the probe for the Skylake driver, introduce more compile-time granularity so that platforms can be selected individually and last provide a dynamic fallback mechanism when two drivers are registered for the same PCI ID.
When the SOF driver is released, the same mechanism will be used to enable the SOF-legacy fallback. There will be no plans to enable an SOF->SST falback.
This looks like a sensible way to go, thanks for working on this!
While the fallback stuff might need more testing, the other patches (addition of CFL-S and split of configs) are rather systematic, so we can merge this at first soonish.
And we may need a bit more comments in Kconfig help for the fallback behavior. Or document it properly and refer to it from Kconfig help. The git commit log isn't present in the released kernel code, after all.
I will indeed put the dynamic fallback on the back-burner, of course the tests work on NUCs and recent devices but fail on a HP SKL device I tried on, so additional code needs to be added to check if the DSP is present or not, remove silly dependencies on NHLT that make no sense for HDaudio, etc.
I'd like however to follow your idea of a 'shared' caps but use it for static mutual-exclusion to start, e.g. to let distributions use legacy for Broadwell but SST for SKL/KBL without requiring the user to play with blacklists. Once we have this in place, and the additional code added for DSP detection the dynamic fallback is an 'easy' transition.
thanks,
Takashi
Pierre-Louis Bossart (4): ASoC: Intel: Skylake: stop init/probe if DSP is not present ASoC: Intel: Skylake: remove useless tests on DSP presence ASoC: Intel: Skylake: Add more platform granularity ALSA: hda: add fallback capabilities for SKL+ platforms
Takashi Iwai (2): ASoC: Intel: Skylake: Add CFL-S support ALSA: hda: Allow fallback binding with legacy HD-audio for Intel SKL+
include/sound/hdaudio.h | 6 + sound/pci/hda/Kconfig | 40 +++++++ sound/pci/hda/hda_controller.h | 2 +- sound/pci/hda/hda_intel.c | 51 ++++++-- sound/soc/intel/Kconfig | 86 ++++++++++++-- sound/soc/intel/boards/Kconfig | 16 ++- sound/soc/intel/skylake/skl-messages.c | 8 ++ sound/soc/intel/skylake/skl.c | 154 +++++++++++++++++++------ 8 files changed, 311 insertions(+), 52 deletions(-)
-- 2.17.1
participants (3)
-
Andy Shevchenko
-
Pierre-Louis Bossart
-
Takashi Iwai