Re: Why open-coding in sof_hda_bus_init()?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux