* Why open-coding in sof_hda_bus_init()? @ 2019-05-31 17:11 Takashi Iwai 2019-05-31 17:43 ` Pierre-Louis Bossart 0 siblings, 1 reply; 11+ messages in thread From: Takashi Iwai @ 2019-05-31 17:11 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: alsa-devel, Keyon Jie 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? The only thing that differs is the handling of bus->id number, where SOF fixes to zero (which was actually done in a patch after the first commit). But judging from the comment, this doesn't seem like a big matter, as we assume a single bus, so far... thanks, Takashi --- diff --git a/sound/soc/sof/intel/Makefile b/sound/soc/sof/intel/Makefile index b8f58e006e29..f42450a9a7b6 100644 --- a/sound/soc/sof/intel/Makefile +++ b/sound/soc/sof/intel/Makefile @@ -7,7 +7,7 @@ snd-sof-intel-ipc-objs := intel-ipc.o snd-sof-intel-hda-common-objs := hda.o hda-loader.o hda-stream.o hda-trace.o \ hda-dsp.o hda-ipc.o hda-ctrl.o hda-pcm.o \ - hda-dai.o hda-bus.o \ + hda-dai.o \ apl.o cnl.o snd-sof-intel-hda-objs := hda-codec.o diff --git a/sound/soc/sof/intel/hda-bus.c b/sound/soc/sof/intel/hda-bus.c deleted file mode 100644 index a7e6d8227df6..000000000000 --- a/sound/soc/sof/intel/hda-bus.c +++ /dev/null @@ -1,111 +0,0 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) -// -// This file is provided under a dual BSD/GPLv2 license. When using or -// redistributing this file, you may do so under either license. -// -// Copyright(c) 2018 Intel Corporation. All rights reserved. -// -// Authors: Keyon Jie <yang.jie@linux.intel.com> - -#include <linux/io.h> -#include <sound/hdaudio.h> -#include "../sof-priv.h" -#include "hda.h" - -#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - -static const struct hdac_bus_ops bus_ops = { - .command = snd_hdac_bus_send_cmd, - .get_response = snd_hdac_bus_get_response, -}; - -#endif - -static void sof_hda_writel(u32 value, u32 __iomem *addr) -{ - writel(value, addr); -} - -static u32 sof_hda_readl(u32 __iomem *addr) -{ - return readl(addr); -} - -static void sof_hda_writew(u16 value, u16 __iomem *addr) -{ - writew(value, addr); -} - -static u16 sof_hda_readw(u16 __iomem *addr) -{ - return readw(addr); -} - -static void sof_hda_writeb(u8 value, u8 __iomem *addr) -{ - writeb(value, addr); -} - -static u8 sof_hda_readb(u8 __iomem *addr) -{ - return readb(addr); -} - -static int sof_hda_dma_alloc_pages(struct hdac_bus *bus, int type, - size_t size, struct snd_dma_buffer *buf) -{ - return snd_dma_alloc_pages(type, bus->dev, size, buf); -} - -static void sof_hda_dma_free_pages(struct hdac_bus *bus, - struct snd_dma_buffer *buf) -{ - snd_dma_free_pages(buf); -} - -static const struct hdac_io_ops io_ops = { - .reg_writel = sof_hda_writel, - .reg_readl = sof_hda_readl, - .reg_writew = sof_hda_writew, - .reg_readw = sof_hda_readw, - .reg_writeb = sof_hda_writeb, - .reg_readb = sof_hda_readb, - .dma_alloc_pages = sof_hda_dma_alloc_pages, - .dma_free_pages = sof_hda_dma_free_pages, -}; - -/* - * This can be used for both with/without hda link support. - */ -void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, - const struct hdac_ext_bus_ops *ext_ops) -{ - memset(bus, 0, sizeof(*bus)); - bus->dev = dev; - - bus->io_ops = &io_ops; - INIT_LIST_HEAD(&bus->stream_list); - - bus->irq = -1; - bus->ext_ops = ext_ops; - - /* - * There is only one HDA bus atm. keep the index as 0. - * Need to fix when there are more than one HDA bus. - */ - bus->idx = 0; - - spin_lock_init(&bus->reg_lock); - -#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - INIT_LIST_HEAD(&bus->codec_list); - INIT_LIST_HEAD(&bus->hlink_list); - - mutex_init(&bus->cmd_mutex); - mutex_init(&bus->lock); - bus->ops = &bus_ops; - INIT_WORK(&bus->unsol_work, snd_hdac_bus_process_unsol_events); - bus->cmd_dma_state = true; -#endif - -} diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 92d45c43b4b1..a9de6a297b56 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -532,8 +532,11 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset); /* * HDA bus operations. */ -void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, - const struct hdac_ext_bus_ops *ext_ops); +static inline void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, + const struct hdac_ext_bus_ops *ext_ops) +{ + snd_hdac_ext_bus_init(bus, dev, NULL, NULL, ext_ops); +} #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) /* ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 17:11 Why open-coding in sof_hda_bus_init()? Takashi Iwai @ 2019-05-31 17:43 ` Pierre-Louis Bossart 2019-05-31 18:22 ` Takashi Iwai 0 siblings, 1 reply; 11+ messages in thread From: Pierre-Louis Bossart @ 2019-05-31 17:43 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, Keyon Jie 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? > > The only thing that differs is the handling of bus->id number, where > SOF fixes to zero (which was actually done in a patch after the first > commit). But judging from the comment, this doesn't seem like a big > matter, as we assume a single bus, so far... > > > thanks, > > Takashi > > --- > diff --git a/sound/soc/sof/intel/Makefile b/sound/soc/sof/intel/Makefile > index b8f58e006e29..f42450a9a7b6 100644 > --- a/sound/soc/sof/intel/Makefile > +++ b/sound/soc/sof/intel/Makefile > @@ -7,7 +7,7 @@ snd-sof-intel-ipc-objs := intel-ipc.o > > snd-sof-intel-hda-common-objs := hda.o hda-loader.o hda-stream.o hda-trace.o \ > hda-dsp.o hda-ipc.o hda-ctrl.o hda-pcm.o \ > - hda-dai.o hda-bus.o \ > + hda-dai.o \ > apl.o cnl.o > > snd-sof-intel-hda-objs := hda-codec.o > diff --git a/sound/soc/sof/intel/hda-bus.c b/sound/soc/sof/intel/hda-bus.c > deleted file mode 100644 > index a7e6d8227df6..000000000000 > --- a/sound/soc/sof/intel/hda-bus.c > +++ /dev/null > @@ -1,111 +0,0 @@ > -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) > -// > -// This file is provided under a dual BSD/GPLv2 license. When using or > -// redistributing this file, you may do so under either license. > -// > -// Copyright(c) 2018 Intel Corporation. All rights reserved. > -// > -// Authors: Keyon Jie <yang.jie@linux.intel.com> > - > -#include <linux/io.h> > -#include <sound/hdaudio.h> > -#include "../sof-priv.h" > -#include "hda.h" > - > -#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) > - > -static const struct hdac_bus_ops bus_ops = { > - .command = snd_hdac_bus_send_cmd, > - .get_response = snd_hdac_bus_get_response, > -}; > - > -#endif > - > -static void sof_hda_writel(u32 value, u32 __iomem *addr) > -{ > - writel(value, addr); > -} > - > -static u32 sof_hda_readl(u32 __iomem *addr) > -{ > - return readl(addr); > -} > - > -static void sof_hda_writew(u16 value, u16 __iomem *addr) > -{ > - writew(value, addr); > -} > - > -static u16 sof_hda_readw(u16 __iomem *addr) > -{ > - return readw(addr); > -} > - > -static void sof_hda_writeb(u8 value, u8 __iomem *addr) > -{ > - writeb(value, addr); > -} > - > -static u8 sof_hda_readb(u8 __iomem *addr) > -{ > - return readb(addr); > -} > - > -static int sof_hda_dma_alloc_pages(struct hdac_bus *bus, int type, > - size_t size, struct snd_dma_buffer *buf) > -{ > - return snd_dma_alloc_pages(type, bus->dev, size, buf); > -} > - > -static void sof_hda_dma_free_pages(struct hdac_bus *bus, > - struct snd_dma_buffer *buf) > -{ > - snd_dma_free_pages(buf); > -} > - > -static const struct hdac_io_ops io_ops = { > - .reg_writel = sof_hda_writel, > - .reg_readl = sof_hda_readl, > - .reg_writew = sof_hda_writew, > - .reg_readw = sof_hda_readw, > - .reg_writeb = sof_hda_writeb, > - .reg_readb = sof_hda_readb, > - .dma_alloc_pages = sof_hda_dma_alloc_pages, > - .dma_free_pages = sof_hda_dma_free_pages, > -}; > - > -/* > - * This can be used for both with/without hda link support. > - */ > -void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, > - const struct hdac_ext_bus_ops *ext_ops) > -{ > - memset(bus, 0, sizeof(*bus)); > - bus->dev = dev; > - > - bus->io_ops = &io_ops; > - INIT_LIST_HEAD(&bus->stream_list); > - > - bus->irq = -1; > - bus->ext_ops = ext_ops; > - > - /* > - * There is only one HDA bus atm. keep the index as 0. > - * Need to fix when there are more than one HDA bus. > - */ > - bus->idx = 0; > - > - spin_lock_init(&bus->reg_lock); > - > -#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) > - INIT_LIST_HEAD(&bus->codec_list); > - INIT_LIST_HEAD(&bus->hlink_list); > - > - mutex_init(&bus->cmd_mutex); > - mutex_init(&bus->lock); > - bus->ops = &bus_ops; > - INIT_WORK(&bus->unsol_work, snd_hdac_bus_process_unsol_events); > - bus->cmd_dma_state = true; > -#endif > - > -} > diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h > index 92d45c43b4b1..a9de6a297b56 100644 > --- a/sound/soc/sof/intel/hda.h > +++ b/sound/soc/sof/intel/hda.h > @@ -532,8 +532,11 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset); > /* > * HDA bus operations. > */ > -void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, > - const struct hdac_ext_bus_ops *ext_ops); > +static inline void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, > + const struct hdac_ext_bus_ops *ext_ops) > +{ > + snd_hdac_ext_bus_init(bus, dev, NULL, NULL, ext_ops); > +} > > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) > /* > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 17:43 ` Pierre-Louis Bossart @ 2019-05-31 18:22 ` Takashi Iwai 2019-05-31 18:31 ` Pierre-Louis Bossart 0 siblings, 1 reply; 11+ messages in thread From: Takashi Iwai @ 2019-05-31 18:22 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: alsa-devel, Keyon Jie 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 } In genral, the open-code is very bad from the maintenance POV. And, even worse, currently the hda-bus.c does only initialization, and the release is with the hda-bus code. thanks, Takashi > > > > > The only thing that differs is the handling of bus->id number, where > > SOF fixes to zero (which was actually done in a patch after the first > > commit). But judging from the comment, this doesn't seem like a big > > matter, as we assume a single bus, so far... > > > > > > thanks, > > > > Takashi > > > > --- > > diff --git a/sound/soc/sof/intel/Makefile b/sound/soc/sof/intel/Makefile > > index b8f58e006e29..f42450a9a7b6 100644 > > --- a/sound/soc/sof/intel/Makefile > > +++ b/sound/soc/sof/intel/Makefile > > @@ -7,7 +7,7 @@ snd-sof-intel-ipc-objs := intel-ipc.o > > snd-sof-intel-hda-common-objs := hda.o hda-loader.o hda-stream.o > > hda-trace.o \ > > hda-dsp.o hda-ipc.o hda-ctrl.o hda-pcm.o \ > > - hda-dai.o hda-bus.o \ > > + hda-dai.o \ > > apl.o cnl.o > > snd-sof-intel-hda-objs := hda-codec.o > > diff --git a/sound/soc/sof/intel/hda-bus.c b/sound/soc/sof/intel/hda-bus.c > > deleted file mode 100644 > > index a7e6d8227df6..000000000000 > > --- a/sound/soc/sof/intel/hda-bus.c > > +++ /dev/null > > @@ -1,111 +0,0 @@ > > -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) > > -// > > -// This file is provided under a dual BSD/GPLv2 license. When using or > > -// redistributing this file, you may do so under either license. > > -// > > -// Copyright(c) 2018 Intel Corporation. All rights reserved. > > -// > > -// Authors: Keyon Jie <yang.jie@linux.intel.com> > > - > > -#include <linux/io.h> > > -#include <sound/hdaudio.h> > > -#include "../sof-priv.h" > > -#include "hda.h" > > - > > -#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) > > - > > -static const struct hdac_bus_ops bus_ops = { > > - .command = snd_hdac_bus_send_cmd, > > - .get_response = snd_hdac_bus_get_response, > > -}; > > - > > -#endif > > - > > -static void sof_hda_writel(u32 value, u32 __iomem *addr) > > -{ > > - writel(value, addr); > > -} > > - > > -static u32 sof_hda_readl(u32 __iomem *addr) > > -{ > > - return readl(addr); > > -} > > - > > -static void sof_hda_writew(u16 value, u16 __iomem *addr) > > -{ > > - writew(value, addr); > > -} > > - > > -static u16 sof_hda_readw(u16 __iomem *addr) > > -{ > > - return readw(addr); > > -} > > - > > -static void sof_hda_writeb(u8 value, u8 __iomem *addr) > > -{ > > - writeb(value, addr); > > -} > > - > > -static u8 sof_hda_readb(u8 __iomem *addr) > > -{ > > - return readb(addr); > > -} > > - > > -static int sof_hda_dma_alloc_pages(struct hdac_bus *bus, int type, > > - size_t size, struct snd_dma_buffer *buf) > > -{ > > - return snd_dma_alloc_pages(type, bus->dev, size, buf); > > -} > > - > > -static void sof_hda_dma_free_pages(struct hdac_bus *bus, > > - struct snd_dma_buffer *buf) > > -{ > > - snd_dma_free_pages(buf); > > -} > > - > > -static const struct hdac_io_ops io_ops = { > > - .reg_writel = sof_hda_writel, > > - .reg_readl = sof_hda_readl, > > - .reg_writew = sof_hda_writew, > > - .reg_readw = sof_hda_readw, > > - .reg_writeb = sof_hda_writeb, > > - .reg_readb = sof_hda_readb, > > - .dma_alloc_pages = sof_hda_dma_alloc_pages, > > - .dma_free_pages = sof_hda_dma_free_pages, > > -}; > > - > > -/* > > - * This can be used for both with/without hda link support. > > - */ > > -void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, > > - const struct hdac_ext_bus_ops *ext_ops) > > -{ > > - memset(bus, 0, sizeof(*bus)); > > - bus->dev = dev; > > - > > - bus->io_ops = &io_ops; > > - INIT_LIST_HEAD(&bus->stream_list); > > - > > - bus->irq = -1; > > - bus->ext_ops = ext_ops; > > - > > - /* > > - * There is only one HDA bus atm. keep the index as 0. > > - * Need to fix when there are more than one HDA bus. > > - */ > > - bus->idx = 0; > > - > > - spin_lock_init(&bus->reg_lock); > > - > > -#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) > > - INIT_LIST_HEAD(&bus->codec_list); > > - INIT_LIST_HEAD(&bus->hlink_list); > > - > > - mutex_init(&bus->cmd_mutex); > > - mutex_init(&bus->lock); > > - bus->ops = &bus_ops; > > - INIT_WORK(&bus->unsol_work, snd_hdac_bus_process_unsol_events); > > - bus->cmd_dma_state = true; > > -#endif > > - > > -} > > diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h > > index 92d45c43b4b1..a9de6a297b56 100644 > > --- a/sound/soc/sof/intel/hda.h > > +++ b/sound/soc/sof/intel/hda.h > > @@ -532,8 +532,11 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset); > > /* > > * HDA bus operations. > > */ > > -void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, > > - const struct hdac_ext_bus_ops *ext_ops); > > +static inline void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, > > + const struct hdac_ext_bus_ops *ext_ops) > > +{ > > + snd_hdac_ext_bus_init(bus, dev, NULL, NULL, ext_ops); > > +} > > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) > > /* > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 18:22 ` Takashi Iwai @ 2019-05-31 18:31 ` Pierre-Louis Bossart 2019-05-31 19:06 ` Takashi Iwai 0 siblings, 1 reply; 11+ messages in thread From: Pierre-Louis Bossart @ 2019-05-31 18:31 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, Keyon Jie 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. > > In genral, the open-code is very bad from the maintenance POV. And, > even worse, currently the hda-bus.c does only initialization, and the > release is with the hda-bus code. I agree, it's not ideal at all, but the snd_hdac library isn't great either... We'll see what we can do, the hda code in SOF is being revisited since there's just too much duplications between the two modes, we can rework the init while we're at it. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 18:31 ` Pierre-Louis Bossart @ 2019-05-31 19:06 ` Takashi Iwai 2019-05-31 19:44 ` Takashi Iwai 0 siblings, 1 reply; 11+ messages in thread From: Takashi Iwai @ 2019-05-31 19:06 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: alsa-devel, Keyon Jie 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. thanks, Takashi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 19:06 ` Takashi Iwai @ 2019-05-31 19:44 ` Takashi Iwai 2019-05-31 20:23 ` Pierre-Louis Bossart 0 siblings, 1 reply; 11+ messages in thread From: Takashi Iwai @ 2019-05-31 19:44 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: alsa-devel, Keyon Jie 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 19:44 ` Takashi Iwai @ 2019-05-31 20:23 ` Pierre-Louis Bossart 2019-05-31 20:36 ` Takashi Iwai 0 siblings, 1 reply; 11+ messages in thread From: Pierre-Louis Bossart @ 2019-05-31 20:23 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, Keyon Jie On 5/31/19 2:44 PM, Takashi Iwai wrote: > 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. we need everything that was removed in your proposal :-) - memset(bus, 0, sizeof(*bus)); - bus->dev = dev; - - bus->io_ops = &io_ops; - INIT_LIST_HEAD(&bus->stream_list); - - bus->irq = -1; - bus->ext_ops = ext_ops; - - /* - * There is only one HDA bus atm. keep the index as 0. - * Need to fix when there are more than one HDA bus. - */ - bus->idx = 0; - - spin_lock_init(&bus->reg_lock); This is the smallest set of initialization needed when you don't need hdmi/hdaudio codec support. > > 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. It's actually not open-coding based on copy/paste, it took us a long time to figure out what was strictly necessary. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 20:23 ` Pierre-Louis Bossart @ 2019-05-31 20:36 ` Takashi Iwai 2019-05-31 20:59 ` Pierre-Louis Bossart 0 siblings, 1 reply; 11+ messages in thread From: Takashi Iwai @ 2019-05-31 20:36 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: alsa-devel, Keyon Jie On Fri, 31 May 2019 22:23:37 +0200, Pierre-Louis Bossart wrote: > > > > On 5/31/19 2:44 PM, Takashi Iwai wrote: > > 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. > > we need everything that was removed in your proposal :-) > > - memset(bus, 0, sizeof(*bus)); > - bus->dev = dev; > - > - bus->io_ops = &io_ops; > - INIT_LIST_HEAD(&bus->stream_list); > - > - bus->irq = -1; > - bus->ext_ops = ext_ops; > - > - /* > - * There is only one HDA bus atm. keep the index as 0. > - * Need to fix when there are more than one HDA bus. > - */ > - bus->idx = 0; > - > - spin_lock_init(&bus->reg_lock); > > This is the smallest set of initialization needed when you don't need > hdmi/hdaudio codec support. I don't understand it... Why SOF core needs to initialize the content of HD-audio bus object even if you won't use it? IOW, what's the merit of having hda-bus.c with the copy of snd-hda-core code? As far as I see, both hda.c and hda-bus.c are linked into the same snd-sof-intel-hda-common module. And, the former has the direct calls of HD-audio core API (with CONFIG_SND_SOC_SOF_HDA); i.e. snd-sof-intel-hda-common already depends on snd-hda-core if CONFIG_SND_SOC_SOF_HDA is on, no matter how you code hda-bus.c. thanks, Takashi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 20:36 ` Takashi Iwai @ 2019-05-31 20:59 ` Pierre-Louis Bossart 2019-05-31 21:20 ` Takashi Iwai 0 siblings, 1 reply; 11+ messages in thread From: Pierre-Louis Bossart @ 2019-05-31 20:59 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, Keyon Jie >> we need everything that was removed in your proposal :-) >> >> - memset(bus, 0, sizeof(*bus)); >> - bus->dev = dev; >> - >> - bus->io_ops = &io_ops; >> - INIT_LIST_HEAD(&bus->stream_list); >> - >> - bus->irq = -1; >> - bus->ext_ops = ext_ops; >> - >> - /* >> - * There is only one HDA bus atm. keep the index as 0. >> - * Need to fix when there are more than one HDA bus. >> - */ >> - bus->idx = 0; >> - >> - spin_lock_init(&bus->reg_lock); >> >> This is the smallest set of initialization needed when you don't need >> hdmi/hdaudio codec support. > > I don't understand it... Why SOF core needs to initialize the content > of HD-audio bus object even if you won't use it? we do use it left and right, but we only use the 'controller/DMA' parts of that structure. we have zero use for CORB/RIRB and codec-specific stuff when I2S and DMIC are the only connections to 3rd party chips > > IOW, what's the merit of having hda-bus.c with the copy of > snd-hda-core code? As far as I see, both hda.c and hda-bus.c are > linked into the same snd-sof-intel-hda-common module. And, the former > has the direct calls of HD-audio core API (with > CONFIG_SND_SOC_SOF_HDA); i.e. snd-sof-intel-hda-common already depends > on snd-hda-core if CONFIG_SND_SOC_SOF_HDA is on, no matter how you > code hda-bus.c. I agree we could implement hda-bus in a cleaner way - but it's a very small file. A larger core repartitioning would take quite a bit of time, and in the mean time we already have to sort out all the deltas between legacy driver and hdac library. Anyways, that's it for me this week, enjoy your vacation! ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 20:59 ` Pierre-Louis Bossart @ 2019-05-31 21:20 ` Takashi Iwai 2019-06-03 5:58 ` Keyon Jie 0 siblings, 1 reply; 11+ messages in thread From: Takashi Iwai @ 2019-05-31 21:20 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: alsa-devel, Keyon Jie On Fri, 31 May 2019 22:59:17 +0200, Pierre-Louis Bossart wrote: > > > >> we need everything that was removed in your proposal :-) > >> > >> - memset(bus, 0, sizeof(*bus)); > >> - bus->dev = dev; > >> - > >> - bus->io_ops = &io_ops; > >> - INIT_LIST_HEAD(&bus->stream_list); > >> - > >> - bus->irq = -1; > >> - bus->ext_ops = ext_ops; > >> - > >> - /* > >> - * There is only one HDA bus atm. keep the index as 0. > >> - * Need to fix when there are more than one HDA bus. > >> - */ > >> - bus->idx = 0; > >> - > >> - spin_lock_init(&bus->reg_lock); > >> > >> This is the smallest set of initialization needed when you don't need > >> hdmi/hdaudio codec support. > > > > I don't understand it... Why SOF core needs to initialize the content > > of HD-audio bus object even if you won't use it? > > we do use it left and right, but we only use the 'controller/DMA' > parts of that structure. we have zero use for CORB/RIRB and > codec-specific stuff when I2S and DMIC are the only connections to 3rd > party chips So you want to avoid the dependency on snd-hda-core if the system is with only I2S/DMIC? But, snd-sof-intel-hda-common already has the hard dependency on snd-hda-core if CONFIG_SND_SOC_SOF_HDA is enabled. So the attempt makes little sense as long as CONFIG_SND_SOC_SOF_HDA is set. And, if CONFIG_SND_SOC_SOF_HDA isn't set -- i.e. CONFIG_SND_SOC_HF_HDA_LINK isn't set either; it implies that there can be no user of HD-audio bus. So, I see no big reason we need to care about the stuff... > > IOW, what's the merit of having hda-bus.c with the copy of > > snd-hda-core code? As far as I see, both hda.c and hda-bus.c are > > linked into the same snd-sof-intel-hda-common module. And, the former > > has the direct calls of HD-audio core API (with > > CONFIG_SND_SOC_SOF_HDA); i.e. snd-sof-intel-hda-common already depends > > on snd-hda-core if CONFIG_SND_SOC_SOF_HDA is on, no matter how you > > code hda-bus.c. > > I agree we could implement hda-bus in a cleaner way - but it's a very > small file. Yes, but this is a kind of layer violation. Imagine you do initialize the struct pci_dev or whatever else device object because you want to reduce the dependency on other core helpers; it would be nightmare from the maintenance POV. I've had already hard time to figure out why SOF HDA initializes the HD-audio bus, because it misses the explicit snd_hdac_*_bus_init() call. That's the starting point of this thread. So, let's avoid ugly hacks. Make it more straightforward. Again, if the module size matters, we can split and reduce the part of HD-audio core stuff that is directly linked to SOF core. > A larger core repartitioning would take quite a bit of > time, and in the mean time we already have to sort out all the deltas > between legacy driver and hdac library. > > Anyways, that's it for me this week, enjoy your vacation! Thanks! Takashi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Why open-coding in sof_hda_bus_init()? 2019-05-31 21:20 ` Takashi Iwai @ 2019-06-03 5:58 ` Keyon Jie 0 siblings, 0 replies; 11+ messages in thread From: Keyon Jie @ 2019-06-03 5:58 UTC (permalink / raw) To: Takashi Iwai, Pierre-Louis Bossart; +Cc: alsa-devel On 2019/6/1 上午5:20, Takashi Iwai wrote: > On Fri, 31 May 2019 22:59:17 +0200, > Pierre-Louis Bossart wrote: >> >> >>>> we need everything that was removed in your proposal :-) >>>> >>>> - memset(bus, 0, sizeof(*bus)); >>>> - bus->dev = dev; >>>> - >>>> - bus->io_ops = &io_ops; >>>> - INIT_LIST_HEAD(&bus->stream_list); >>>> - >>>> - bus->irq = -1; >>>> - bus->ext_ops = ext_ops; >>>> - >>>> - /* >>>> - * There is only one HDA bus atm. keep the index as 0. >>>> - * Need to fix when there are more than one HDA bus. >>>> - */ >>>> - bus->idx = 0; >>>> - >>>> - spin_lock_init(&bus->reg_lock); >>>> >>>> This is the smallest set of initialization needed when you don't need >>>> hdmi/hdaudio codec support. >>> >>> I don't understand it... Why SOF core needs to initialize the content >>> of HD-audio bus object even if you won't use it? >> >> we do use it left and right, but we only use the 'controller/DMA' >> parts of that structure. we have zero use for CORB/RIRB and >> codec-specific stuff when I2S and DMIC are the only connections to 3rd >> party chips > > So you want to avoid the dependency on snd-hda-core if the system is > with only I2S/DMIC? Hi Takashi, yes, that's true. We want to reuse the hdac registers access part(bus->io_ops, bus->stream_list), for host DMA stream management(e.g. in sof/intel/hda-stream.c), without the dependency on snd-hda-core. > > But, snd-sof-intel-hda-common already has the hard dependency on > snd-hda-core if CONFIG_SND_SOC_SOF_HDA is enabled. So the attempt > makes little sense as long as CONFIG_SND_SOC_SOF_HDA is set. > > And, if CONFIG_SND_SOC_SOF_HDA isn't set -- i.e. > CONFIG_SND_SOC_HF_HDA_LINK isn't set either; it implies that there can > be no user of HD-audio bus. So, I see no big reason we need to care > about the stuff... > > >>> IOW, what's the merit of having hda-bus.c with the copy of >>> snd-hda-core code? As far as I see, both hda.c and hda-bus.c are >>> linked into the same snd-sof-intel-hda-common module. And, the former >>> has the direct calls of HD-audio core API (with >>> CONFIG_SND_SOC_SOF_HDA); i.e. snd-sof-intel-hda-common already depends >>> on snd-hda-core if CONFIG_SND_SOC_SOF_HDA is on, no matter how you >>> code hda-bus.c. >> >> I agree we could implement hda-bus in a cleaner way - but it's a very >> small file. > > Yes, but this is a kind of layer violation. Imagine you do initialize > the struct pci_dev or whatever else device object because you want to > reduce the dependency on other core helpers; it would be nightmare > from the maintenance POV. > > I've had already hard time to figure out why SOF HDA initializes the > HD-audio bus, because it misses the explicit snd_hdac_*_bus_init() > call. That's the starting point of this thread. > > So, let's avoid ugly hacks. Make it more straightforward. Again, if > the module size matters, we can split and reduce the part of HD-audio > core stuff that is directly linked to SOF core. If we can split HD-audio core stuff to provide a host DMA/stream only module for I2S/DMIC, that would be great. Thanks, ~Keyon > >> A larger core repartitioning would take quite a bit of >> time, and in the mean time we already have to sort out all the deltas >> between legacy driver and hdac library. >> >> Anyways, that's it for me this week, enjoy your vacation! > > Thanks! > > > Takashi > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-06-03 5:53 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-31 17:11 Why open-coding in sof_hda_bus_init()? Takashi Iwai 2019-05-31 17:43 ` Pierre-Louis Bossart 2019-05-31 18:22 ` Takashi Iwai 2019-05-31 18:31 ` Pierre-Louis Bossart 2019-05-31 19:06 ` Takashi Iwai 2019-05-31 19:44 ` Takashi Iwai 2019-05-31 20:23 ` Pierre-Louis Bossart 2019-05-31 20:36 ` Takashi Iwai 2019-05-31 20:59 ` Pierre-Louis Bossart 2019-05-31 21:20 ` Takashi Iwai 2019-06-03 5:58 ` Keyon Jie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.