Linux Sound subsystem development
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: linux-sound@vger.kernel.org
Cc: "Richard Fitzgerald" <rf@opensource.cirrus.com>,
	Kailang <kailang@realtek.com>,
	"Kai Vehmanen" <kai.vehmanen@linux.intel.com>,
	"Cezary Rojewski" <cezary.rojewski@intel.com>,
	"Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>
Subject: Re: [RFC] Reorganizing HD-audio driver code?
Date: Wed, 02 Jul 2025 16:53:51 +0200	[thread overview]
Message-ID: <87ikkave2o.wl-tiwai@suse.de> (raw)
In-Reply-To: <87h5zygawe.wl-tiwai@suse.de>

On Sun, 29 Jun 2025 11:22:25 +0200,
Takashi Iwai wrote:
> 
> On Fri, 27 Jun 2025 15:22:20 +0200,
> Richard Fitzgerald wrote:
> > 
> > On 27/06/2025 1:04 pm, Takashi Iwai wrote:
> > > Hi,
> > > 
> > > HD-audio driver is known to be quite messy in both file structures and
> > > its design, but until now I haven't touched its files paths so much
> > > because I set a higher priority for the easiness of backport to stable
> > > kernels.  But, you can't leave garbages forever, it's been already
> > > high time for a large clean up.
> > > 
> > > So I tried a quick code reorganization, and put the result in
> > > test/hda-reorg branch of sound.git tree.
> > >    https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/log/?h=test/hda-reorg
> > > 
> > > The basic idea is to move the code from sound/pci/hda/* into different
> > > subdirectories in sound/hda/ per functionality, as most of the stuff
> > > are independent from PCI, but rather HD-audio bus specific.
> > > 
> > 
> > This all seems reasonable to me. I always thought it strange that there
> > is a sound/hda directory but most of the HDA support isn't in that
> > directory.
> 
> Thanks for your review!
> 
> It's a long story: at the time of ASoC Intel HD-audio support, we
> thought to implement more ASoC-friendly way of HD-audio controller and
> codec drivers, e.g. adapting DAPM and others.  So we began with the
> factoring out the HD-audio basic core stuff to the common directory
> sound/hda/*, while keeping the rest legacy stuff almost as is.  But
> ASoC implementation didn't fly in the end from various reasons, and
> the legacy HD-audio stuff was good enough for the actual use cases;
> it's too bit to fail, after all.
> 
> > > The Realtek codec is split further to smaller pieces (which was really
> > > huge).
> > 
> > Yes, that file was very confusing having support and quirks for so many
> > Realtek parts all in one file.
> > 
> > > The Cirrus and TI sub-codec drivers are moved to codecs/side-codecs
> > > subdirectory:
> > 
> > That's ok
> > 
> > > They can be put to each own directory and drop the file name prefix,
> > > if we want, too.  Let me know if Cirrus and TI people would like to
> > > split to more subdirectories.
> > 
> > I don't mind either way.
> > The hda_component* files are common to the amps and the realtek driver
> > so I wonder whether they belong in helpers. They are only utility
> > wrappers around the kernel component-binding APIs.
> 
> Right.  So far, just because it's basically only binding with
> side-codecs, I put into side-codecs subdirectory.
> 
> > > *HOWEVER* the biggest question is: whether it's worth?
> > > 
> > > Essentially, this makes almost impossible to make a patch for stable
> > > trees from the original commit as is; one has to translate the file
> > > paths and adjust manually in each patch.
> > 
> > Depends which file you are patching and how much it has changed.
> > Git can figure out file renames (and changing directory is a rename).
> 
> I'm afraid that the Realtek codec changes might be hard to track
> automatically.  Maybe the Cirrus side-codecs stuff would work.
> 
> > > Also, of course, if anyone is working on HD-audio stuff right now, the
> > > work had to be adjusted to the new file path.  It'd be one-off action,
> > > though.
> > 
> > Speaking only for Cirrus, I don't think this makes much extra effort
> > for us. Our amp drivers have only changed location, so that should be
> > trivial. The other file we change is patch_realtek.c but that typically
> > is only 1-line quirk entries.
> 
> OK, thanks for confirmation!

... and now I worked further on this, resulting in larger set of
changes.  It's not only the file location changes but also the
HD-audio codec driver binding changes.  So I put more people to Cc for
catch their cautions.

To recap:

- The all HD-audio driver code are moved under sound/hda.

  % ls sound/hda 
  codecs/  common/  controllers/  core/  Kconfig  Makefile

  * The former hda core code is found in sound/hda/core.
  * The former snd-hda-codec code is found in sound/hda/common.
  * The former snd-hda-intel, tegra and acpi are put in
    sound/hda/controllers.
  * The former patch_* and co are put to sound/hda/codecs.
  * Realtek codec driver is split to several modules as
    sound/hda/codecs/realtek/alc*.
  * Cirrus codec driver is split to cs420x and cs421x, put under
    sound/hda/codecs/cirrus together with cs8409.
  * HDMI codec driver is split to several modules under
    sound/hda/codecs/hdmi
  * Cirrus and TI sub-codecs are put under
    sound/hda/codecs/side-codecs

- The HD-audio codec driver binding is changed from the embedded
  hda_codec.patch_ops to hda_codec_driver.ops.
  (As of now, hda_codec_driver.ops is a pointer, but it can be
   embedded later, too.)

  This change required some code to be modified without the dynamic
  override of callbacks.

- In future, we may convert the runtime PM handling to use the
  standard pm_ops, too.  This was raised some time ago during the
  discussion with Realtek devs.

The current patches are found in test/hda-reorg branch of sound git
tree:
  https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/log/?h=test/hda-reorg

It's based on master branch for taking all changes of for-linus and
for-next, hence the branch may be still frequently rebased.

So, please let me know if you see any issues or suggestions for this
conversion.  I have no concrete plan for merging, but if everything
looks fine, it can be even on the next 6.17, too.


thanks,

Takashi

  reply	other threads:[~2025-07-02 14:53 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-27 12:04 [RFC] Reorganizing HD-audio driver code? Takashi Iwai
2025-06-27 13:22 ` Richard Fitzgerald
2025-06-29  9:22   ` Takashi Iwai
2025-07-02 14:53     ` Takashi Iwai [this message]
2025-07-03 13:29       ` Amadeusz Sławiński
2025-07-03 13:38         ` Takashi Iwai
2025-07-03 13:57           ` Amadeusz Sławiński
2025-07-03 14:32             ` Takashi Iwai
2025-07-03 14:43               ` Takashi Iwai
2025-07-03 14:52                 ` Amadeusz Sławiński
2025-07-03 15:12                   ` Takashi Iwai
2025-07-08 10:56                     ` Amadeusz Sławiński
2025-07-08 11:15                       ` Takashi Iwai
2025-07-03 14:04     ` Richard Fitzgerald
2025-07-03 14:34       ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ikkave2o.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=amadeuszx.slawinski@linux.intel.com \
    --cc=cezary.rojewski@intel.com \
    --cc=kai.vehmanen@linux.intel.com \
    --cc=kailang@realtek.com \
    --cc=linux-sound@vger.kernel.org \
    --cc=rf@opensource.cirrus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox