Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128

From: Thierry Reding <thierry.reding@gmail.com>
To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Mark Brown <broonie@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Takashi Iwai <tiwai@suse.com>, Jaroslav Kysela <perex@perex.cz>,
	Jon Hunter <jonathanh@nvidia.com>,
	alsa-devel@alsa-project.org, linux-tegra@vger.kernel.org
Subject: Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
Date: Mon, 19 Apr 2021 16:51:42 +0200	[thread overview]
Message-ID: <YH2Y/rd2/q5qHvnz@orome.fritz.box> (raw)
In-Reply-To: <8735vn59sw.wl-kuninori.morimoto.gx@renesas.com>

[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]

On Mon, Apr 19, 2021 at 10:42:55AM +0900, Kuninori Morimoto wrote:
> 
> Hi Thierry
> 
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > On Tegra186 and later, the number of links can go up to 72, so bump the
> > maximum number of links to the next power of two (128).
> > 
> > Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> (snip)
> > +#define SNDRV_MAX_LINKS 128
> > +
> >  struct link_info {
> >  	int link; /* number of link */
> >  	int cpu;  /* turn for CPU / Codec */
> > -	struct prop_nums num[SNDRV_MINOR_DEVICES];
> > +	struct prop_nums num[SNDRV_MAX_LINKS];
> >  };
> 
> 
> How many numbers do you really need ?

As I mentioned in the commit message, the maximum I've seen is 72.
Rounding up to the next power of two seemed like a good idea to give a
bit of extra headroom.

> Because simple-card needs many parameters,
> thus I will get below warning.
> # It is not yet happen on upstream kernel, but will be
> 
> 	warning: the frame size of 2280 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> 
> This warning doesn't appear if SNDRV_MAX_LINKS was 64.
> Can we reduce it ?

Reducing from 128 to, say, 80 should eliminate that warning, but I
wonder if perhaps a better solution for the longer term would be to
allocate this structure on the heap rather than on the stack? That
way we don't risk triggering this warning again in the future.

The data structure seems to be only used during initialization, so
something like a kzalloc()/kfree() pair doesn't seem like it would
hurt much performance-wise. Add in the devm_ variants and the code
complexity should also remain moderately low.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]