[alsa-devel] ASoC cache code (looking toward 2.6.38 merge window)

[Takashi Iwai]

At Mon, 20 Dec 2010 16:34:36 +0000,
Mark Brown wrote:
> 
> On Mon, Dec 20, 2010 at 04:56:03PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > It wasn't portability, it was specifically the fact that some drivers
> > > want to do block operations which means they depend very strongly on the
> > > exact memory layout of the cache (obviously this only works for
> > > uncompressed caches).
> 
> > But this is exactly what hinders the portability :)  Which part of
> > soc_4_12_spi_read/write specifies that this must be aligned to be BE
> > while spi_7_9 to be LE?  Why soc_16_16_i2c converts to BE internally
> > while soc_16_8_i2c doesn't?  I mean, such a difference isn't visible 
> > in the API level but hard-coded in the implementation although the
> > API appears as if it's portable.
> 
> They should all be specifying something, quite what they should be
> specifying varies - it just comes down to whatever the standard wire
> format that the chip vendors have come up with.  Mostly if it's not
> specified it should be little endian.
> 
> It's not really been an issue as pretty much all the CPUs have the same
> endianness.

Yeah, I know it's working now.  But it's hidden inside and merely
called portable ;)


> > > There's more potential users than currently have it enabled, it's just
> > > that the WM8994 driver is the only driver that turns it on by default
> > > right now (several other CODEC drivers should but need testing just to
> > > confirm that it's OK).  It's only a few kilobytes and Kconfig really
> > > isn't good at supporting this sort of selection, I'd worry about the
> > > usability issues.
> 
> > Well, it drags LZO COMPRESS and DECOMPRESS unconditionally, which I
> > feel uneasy.  Usually the consumer of such lib stuff has a Kconfig,
> > especially the feature can be optional (and in this case it is).
> 
> > The necessary change is simple like below.  The code that requires a
> > compressed cache just needs to select SND_SOC_LZO_CACHE or whatever.
> 
> That's one way of doing it, but it does hurt usability.

That's a drawback, sure.

> > It's no urgent issue, yeah.  It's just what I feel uneasy to see such
> > a thing...
> 
> If someone's worried about memory usage in ASoC they'd be much better
> going after the strings than this TBH.
> 
> > +#ifdef CONFIG_SND_SOC_LZO_CACHE
> > +	SND_SOC_LZO_COMPRESSION = 2,
> > +#endif
> > +#ifdef CONFIG_SND_SOC_RBTREE_CACHE
> > +	SND_SOC_RBTREE_COMPRESSION = 3,
> > +#endif
> 
> If we were going to make this user selectable we should keep the defines
> and then fall back to a flat cache if they're turned off otherwise you
> end up having too much faff with Kconfig interdependencies.

My intention of the chunk above was to give a build error for
unsupported cache compressions.  But fallback sounds also reasonable,
too.

> >  config SND_SOC_WM8994
> >         tristate
> > +       select SND_SOC_RBTREE_CACHE
> 
> Except the machine driver can override this...

Hmm, I thought the codec is always selected not depended?
Can it be overridden?


thanks,

Takashi