| Message ID | 1430405556-19166-1-git-send-email-vinod.koul@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 4/30/15 9:52 AM, Vinod Koul wrote: > Some Intel HDA controllers sport a DSP. These systems can also be enabled > with ASoC HDA driver as well. So add a flag in hda-core to enable/disable > aDSP This flag for now is false, and should be true once the ASoC based > systems mature. The integrators/OS vendors can configure this flag based on > system preference. This choice is contingent on the BIOS options, you can't enable the DSP if the BIOS said no DSP... > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > --- > include/sound/hdaudio.h | 11 +++++++++++ > sound/hda/hda_bus_type.c | 4 ++++ > 2 files changed, 15 insertions(+) > > diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h > index b97c59eab7ab..015bec1079f9 100644 > --- a/include/sound/hdaudio.h > +++ b/include/sound/hdaudio.h > @@ -12,6 +12,17 @@ > #include <sound/memalloc.h> > #include <sound/hda_verbs.h> > > +/* > + * hdac_adsp_enable: exported HD-A aDSP enable configuration. > + * > + * Some Intel HDA controllers sport a DSP, for these platform we can bypass > + * aDSP and use as regular HDA controller or enable aDSP and use aDSP > + * along with I2S codecs etc. > + * hdac_adsp_enable would enable the aDSP based HDA controller if the > + * platform supports it > + */ > +extern bool hdac_adsp_enable; > + > /* codec node id */ > typedef u16 hda_nid_t; > > diff --git a/sound/hda/hda_bus_type.c b/sound/hda/hda_bus_type.c > index 519914a12e8a..80e0570ffbf4 100644 > --- a/sound/hda/hda_bus_type.c > +++ b/sound/hda/hda_bus_type.c > @@ -10,6 +10,10 @@ > MODULE_DESCRIPTION("HD-audio bus"); > MODULE_LICENSE("GPL"); > > +bool hdac_adsp_enable = false; > +module_param(hdac_adsp_enable, bool, 0444); > +MODULE_PARM_DESC(hdac_adsp_enable, "Enable aDSP on Intel HDA based systems"); > + > static int hda_bus_match(struct device *dev, struct device_driver *drv) > { > struct hdac_device *hdev = dev_to_hdac_dev(dev); >
On Thu, Apr 30, 2015 at 11:02:19AM -0500, Pierre-Louis Bossart wrote: > On 4/30/15 9:52 AM, Vinod Koul wrote: > >Some Intel HDA controllers sport a DSP. These systems can also be enabled > >with ASoC HDA driver as well. So add a flag in hda-core to enable/disable > >aDSP This flag for now is false, and should be true once the ASoC based > >systems mature. The integrators/OS vendors can configure this flag based on > >system preference. > This choice is contingent on the BIOS options, you can't enable the DSP if > the BIOS said no DSP... Presumably this flag should be coming from the BIOS by default and this providing a disable only setting for test/development?
At Thu, 30 Apr 2015 20:27:26 +0100, Mark Brown wrote: > > On Thu, Apr 30, 2015 at 11:02:19AM -0500, Pierre-Louis Bossart wrote: > > On 4/30/15 9:52 AM, Vinod Koul wrote: > > > >Some Intel HDA controllers sport a DSP. These systems can also be enabled > > >with ASoC HDA driver as well. So add a flag in hda-core to enable/disable > > >aDSP This flag for now is false, and should be true once the ASoC based > > >systems mature. The integrators/OS vendors can configure this flag based on > > >system preference. > > > This choice is contingent on the BIOS options, you can't enable the DSP if > > the BIOS said no DSP... > > Presumably this flag should be coming from the BIOS by default and this > providing a disable only setting for test/development? It's hard to know exactly how many models will be with DSP and how many without... The problem we're dealing with is that we'll have two individual drivers supporting the very same PCI ID. And, there is no good mechanism to prioritize the driver load on Linux, so far. So, this is the consequence after lengthy discussions: it's fairly simple but effective enough. Distros may build both drivers, but which driver to use can be decided by this option. The default value of this switch should be set via a Kconfig, but it can be overridden dynamically via either a static module option or boot option. Takashi
At Thu, 30 Apr 2015 20:22:34 +0530, Vinod Koul wrote: > > Some Intel HDA controllers sport a DSP. These systems can also be enabled > with ASoC HDA driver as well. So add a flag in hda-core to enable/disable > aDSP This flag for now is false, and should be true once the ASoC based > systems mature. The integrators/OS vendors can configure this flag based on > system preference. > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > --- > include/sound/hdaudio.h | 11 +++++++++++ > sound/hda/hda_bus_type.c | 4 ++++ > 2 files changed, 15 insertions(+) > > diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h > index b97c59eab7ab..015bec1079f9 100644 > --- a/include/sound/hdaudio.h > +++ b/include/sound/hdaudio.h > @@ -12,6 +12,17 @@ > #include <sound/memalloc.h> > #include <sound/hda_verbs.h> > > +/* > + * hdac_adsp_enable: exported HD-A aDSP enable configuration. > + * > + * Some Intel HDA controllers sport a DSP, for these platform we can bypass > + * aDSP and use as regular HDA controller or enable aDSP and use aDSP > + * along with I2S codecs etc. > + * hdac_adsp_enable would enable the aDSP based HDA controller if the > + * platform supports it > + */ > +extern bool hdac_adsp_enable; I prefer snd_ prefix for exported symbols, partly for consistency and partly for safety reason. > + > /* codec node id */ > typedef u16 hda_nid_t; > > diff --git a/sound/hda/hda_bus_type.c b/sound/hda/hda_bus_type.c > index 519914a12e8a..80e0570ffbf4 100644 > --- a/sound/hda/hda_bus_type.c > +++ b/sound/hda/hda_bus_type.c > @@ -10,6 +10,10 @@ > MODULE_DESCRIPTION("HD-audio bus"); > MODULE_LICENSE("GPL"); > > +bool hdac_adsp_enable = false; > +module_param(hdac_adsp_enable, bool, 0444); Try to reduce to a shorter option name, e.g. enable_adsp. With module_param_named(), you can have different option- and variable names. thanks, Takashi
On Thu, Apr 30, 2015 at 10:44:21PM +0200, Takashi Iwai wrote: > At Thu, 30 Apr 2015 20:27:26 +0100, > Mark Brown wrote: > > > > On Thu, Apr 30, 2015 at 11:02:19AM -0500, Pierre-Louis Bossart wrote: > > > On 4/30/15 9:52 AM, Vinod Koul wrote: > > > > > >Some Intel HDA controllers sport a DSP. These systems can also be enabled > > > >with ASoC HDA driver as well. So add a flag in hda-core to enable/disable > > > >aDSP This flag for now is false, and should be true once the ASoC based > > > >systems mature. The integrators/OS vendors can configure this flag based on > > > >system preference. > > > > > This choice is contingent on the BIOS options, you can't enable the DSP if > > > the BIOS said no DSP... That is right, but by default DSP in On. Btw this is not like older HSW platform where HDA and DSP and muxed and BIOS plays key role. > > > > Presumably this flag should be coming from the BIOS by default and this > > providing a disable only setting for test/development? > > It's hard to know exactly how many models will be with DSP and how > many without... The problem we're dealing with is that we'll have two > individual drivers supporting the very same PCI ID. And, there is no > good mechanism to prioritize the driver load on Linux, so far. > > So, this is the consequence after lengthy discussions: it's fairly > simple but effective enough. Distros may build both drivers, but > which driver to use can be decided by this option. The default value > of this switch should be set via a Kconfig, but it can be overridden > dynamically via either a static module option or boot option. Okay i will respin the series based on latest discussion Thanks
On Thu, Apr 30, 2015 at 10:44:21PM +0200, Takashi Iwai wrote: > Mark Brown wrote: > > On Thu, Apr 30, 2015 at 11:02:19AM -0500, Pierre-Louis Bossart wrote: j > > > This choice is contingent on the BIOS options, you can't enable the DSP if > > > the BIOS said no DSP... > > Presumably this flag should be coming from the BIOS by default and this > > providing a disable only setting for test/development? > It's hard to know exactly how many models will be with DSP and how > many without... The problem we're dealing with is that we'll have two > individual drivers supporting the very same PCI ID. And, there is no > good mechanism to prioritize the driver load on Linux, so far. Well, I'm a bit confused why we're loading separate drivers at all here. Shouldn't we be loading one driver and then figuring out what to do with the hardware based on something like DMI matching (you'd hope that there would be some config readback from the firmware but recent performance suggests not)? > So, this is the consequence after lengthy discussions: it's fairly > simple but effective enough. Distros may build both drivers, but > which driver to use can be decided by this option. The default value > of this switch should be set via a Kconfig, but it can be overridden > dynamically via either a static module option or boot option. I don't understand how it's going to work well for users, especially distros where you're building something to be installed on a wide range of hardware, and it doesn't seem idiomatic.
On 4/30/15 11:39 PM, Vinod Koul wrote: > On Thu, Apr 30, 2015 at 10:44:21PM +0200, Takashi Iwai wrote: >> At Thu, 30 Apr 2015 20:27:26 +0100, >> Mark Brown wrote: >>> >>> On Thu, Apr 30, 2015 at 11:02:19AM -0500, Pierre-Louis Bossart wrote: >>>> On 4/30/15 9:52 AM, Vinod Koul wrote: >>> >>>>> Some Intel HDA controllers sport a DSP. These systems can also be enabled >>>>> with ASoC HDA driver as well. So add a flag in hda-core to enable/disable >>>>> aDSP This flag for now is false, and should be true once the ASoC based >>>>> systems mature. The integrators/OS vendors can configure this flag based on >>>>> system preference. >>> >>>> This choice is contingent on the BIOS options, you can't enable the DSP if >>>> the BIOS said no DSP... > That is right, but by default DSP in On. Btw this is not like older HSW > platform where HDA and DSP and muxed and BIOS plays key role. >>> >>> Presumably this flag should be coming from the BIOS by default and this >>> providing a disable only setting for test/development? >> >> It's hard to know exactly how many models will be with DSP and how >> many without... The problem we're dealing with is that we'll have two >> individual drivers supporting the very same PCI ID. And, there is no >> good mechanism to prioritize the driver load on Linux, so far. >> >> So, this is the consequence after lengthy discussions: it's fairly >> simple but effective enough. Distros may build both drivers, but >> which driver to use can be decided by this option. The default value >> of this switch should be set via a Kconfig, but it can be overridden >> dynamically via either a static module option or boot option. > Okay i will respin the series based on latest discussion Maybe I confused everyone, it's not complicated: there is a register that indicates if the DSP is enabled and that can be queried before launching the DSP driver. There is no guessing or need for DMI-based quirks, the capabilities are exposed and that should be used. You can then add a driver parameter to fall back to legacy mode, e.g. for testing, but that would be a second level disable. For once the hardware does tell us what to do, we need to use the information...
On Fri, May 01, 2015 at 09:15:10AM -0500, Pierre-Louis Bossart wrote: > Maybe I confused everyone, it's not complicated: there is a register that > indicates if the DSP is enabled and that can be queried before launching the > DSP driver. There is no guessing or need for DMI-based quirks, the > capabilities are exposed and that should be used. You can then add a driver > parameter to fall back to legacy mode, e.g. for testing, but that would be a > second level disable. For once the hardware does tell us what to do, we need > to use the information... Oh, OK. That sounds sensible! If that's the case then surely we should just have one driver for the PCI ID which just skips enabling the DSP bits based on a combination of the register and the driver parameter? We shouldn't need this multiple drivers for the same PCI ID stuff.
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index b97c59eab7ab..015bec1079f9 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -12,6 +12,17 @@ #include <sound/memalloc.h> #include <sound/hda_verbs.h> +/* + * hdac_adsp_enable: exported HD-A aDSP enable configuration. + * + * Some Intel HDA controllers sport a DSP, for these platform we can bypass + * aDSP and use as regular HDA controller or enable aDSP and use aDSP + * along with I2S codecs etc. + * hdac_adsp_enable would enable the aDSP based HDA controller if the + * platform supports it + */ +extern bool hdac_adsp_enable; + /* codec node id */ typedef u16 hda_nid_t; diff --git a/sound/hda/hda_bus_type.c b/sound/hda/hda_bus_type.c index 519914a12e8a..80e0570ffbf4 100644 --- a/sound/hda/hda_bus_type.c +++ b/sound/hda/hda_bus_type.c @@ -10,6 +10,10 @@ MODULE_DESCRIPTION("HD-audio bus"); MODULE_LICENSE("GPL"); +bool hdac_adsp_enable = false; +module_param(hdac_adsp_enable, bool, 0444); +MODULE_PARM_DESC(hdac_adsp_enable, "Enable aDSP on Intel HDA based systems"); + static int hda_bus_match(struct device *dev, struct device_driver *drv) { struct hdac_device *hdev = dev_to_hdac_dev(dev);
Some Intel HDA controllers sport a DSP. These systems can also be enabled with ASoC HDA driver as well. So add a flag in hda-core to enable/disable aDSP This flag for now is false, and should be true once the ASoC based systems mature. The integrators/OS vendors can configure this flag based on system preference. Signed-off-by: Vinod Koul <vinod.koul@intel.com> --- include/sound/hdaudio.h | 11 +++++++++++ sound/hda/hda_bus_type.c | 4 ++++ 2 files changed, 15 insertions(+)