On Fri, 31 May 2019 21:06:25 +0200, Takashi Iwai wrote:
On Fri, 31 May 2019 20:31:33 +0200, Pierre-Louis Bossart wrote:
On 5/31/19 1:22 PM, Takashi Iwai wrote:
On Fri, 31 May 2019 19:43:59 +0200, Pierre-Louis Bossart wrote:
On 5/31/19 12:11 PM, Takashi Iwai wrote:
Hi,
while looking at SOF code due to the recent debugging session, I noticed that sof_hda_bus_init() is basically an open-code of the existing snd_hdac_ext_bus_init(). Why don't we simply call snd_hdac_ext_bus_init() like below?
It's intentional. We've been asked since Day1 of SOF on ApolloLake to provide a 'self-contained' controller-only support that has no dependency on the snd_hdac library for solutions where HDaudio links+codecs are not used (typically IOT devices). This was driven by the lack of separation between layers in that library as well as a desire to have a dual-license. That's why you see the init and some of the basic utilities re-implemented for SOF.
However for cases where HDaudio+HDMI are required, we didn't want to reinvent the wheel - HDaudio is complicated enough - and do make use of this snd_hdac library.
We have a config SND_SOC_SOF_HDA that controls in which mode we operate, and it enables HDMI by default (for I2S+HDMI solutions). To get external HDaudio codecs you need the additional SOF_HDAUDIO_CODEC kconfig.
Does this help?
Well, what's wrong with the conditional build with Kconfig? You can just wrap the call snd_hdac_ext_bus_init() with #if/endif, e.g. in soc/sof/intel/hda.h,
static inline void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, const struct hdac_ext_bus_ops *ext_ops) { #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) snd_hdac_ext_bus_init(bus, dev, NULL, NULL, ext_ops); #endif }
We still need initializations for some of the data structures when SOF_HDA is not defined.
Which data structure? The function above is only initializing the given struct hdac_bus object. I'm not suggesting to change the caller site, hda_init() of sound/soc/sof/intel/hda.c.
Also, if the size matters: we can split hda-core code between the thin bus accessor and the rest. Basically what you need unconditionally is the functions in sound/hda/hdac_bus.c, and they are fairly independent from other HD-audio functios, so it can be its own module. And the recent ext_bus init and exit implementations are very close to the bare init/exit code, too, so they can be simply moved into the hdac-core-bus or provided as a static inline, too.
In anyway, my point is that there are tons better way than the open code of such a complex object initialization.
thanks,
Takashi