Re: [PATCH] ALSA: hda: Use maple tree register cache

[Takashi Iwai <tiwai@suse.de>]

On Tue, 13 Jun 2023 18:41:15 +0200,
Mark Brown wrote:
> 
> On Tue, Jun 13, 2023 at 06:15:12PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> > > On Tue, Jun 13, 2023 at 04:24:28PM +0200, Takashi Iwai wrote:
> 
> > > > Since HD-audio codec has no known default values unlike normal codecs,
> > > > it needs to initialize itself only at the first access, and this
> > > > helper does it.
> 
> > > Ah, if it's just suppressing the write the code should just be removed.
> > > regmap_update_bits() already suppresses noop writes so unless we might
> > > write a different value to the register later the effect will be the
> > > same.  I can send a patch.
> 
> > Oh, I'm afraid that we're seeing different things.  The code there is
> > rather to *set* some initial value for each amp register (but only
> > once), and it's not about optimization for writing a same value
> > again.
> 
> > That is, the function helps to set an initial (mute) value on each amp
> > when the driver parses the topology and finds an amp.  But if the
> > driver already has parsed this amp beforehand by other paths, it skips
> > the initialization, as the other path may have already unmuted the
> > amp.
> 
> > Or I might have misunderstood what you mean about _update_bits()...
> 
> So it is possible that we might set two distinct values during setup
> then and we're doing this intentionally?  It's not obvious that this
> might happen.  A comment wouldn't hurt, and a big part of this is
> confusing is that in the non-regmap case all we're doing is suppressing
> duplicate writes, in that path it's just checking for changes in the
> register value.
> 
> None of this is what the non-regmap path does, it just suppresses noop
> writes to the hardware.

Actually, many of HD-audio codec driver code heavily relies on the
regmap, more or less mandatory.  The snd_hda_codec_amp_init() is one
of such.  You may write a codec driver without the regmap, but some
helpers won't work as expected.


Takashi