All of lore.kernel.org
 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 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.