Re: [PATCH] ASoC: rsnd: Calculate DALIGN inversion at run-time

From: Eugeniu Rosca <erosca@de.adit-jv.com>
To: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Nilkanth Ahirrao <external.anilkanth@jp.adit-jv.com>,
	<alsa-devel@alsa-project.org>,
	<linux-renesas-soc@vger.kernel.org>,
	Eugeniu Rosca <erosca@de.adit-jv.com>,
	Eugeniu Rosca <roscaeugeniu@gmail.com>
Subject: Re: [PATCH] ASoC: rsnd: Calculate DALIGN inversion at run-time
Date: Mon, 2 Dec 2019 18:38:27 +0100	[thread overview]
Message-ID: <20191202173827.GA13630@vmlxhi-102.adit-jv.com> (raw)
In-Reply-To: <20191202155834.22582-1-geert+renesas@glider.be>

Hi Geert,

On Mon, Dec 02, 2019 at 04:58:34PM +0100, Geert Uytterhoeven wrote:
> There is no need to store the inverted DALIGN values in the table, as
> they can easily be calculated at run-time.  This also protects against
> the introduction of inconsistencies between normal and inverted values
> by a future table modification.
> 
> Reorder the two subexpressions in the AND check, to perform the least
> expensive check first.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com>

> ---
> Compile-tested only.
> 
> Interestingly, this decreases code size on arm64, but increases on arm32
> (both gcc version 7.4.0 (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1)).
> 
> arm32:
> 
>        text	   data	    bss	    dec	    hex	filename
>       16186	    276	     84	  16546	   40a2	sound/soc/sh/rcar/core.o.orig
>       16194	    276	     84	  16554	   40aa	sound/soc/sh/rcar/core.o
> 
> arm64:
> 
>        text	   data	    bss	    dec	    hex	filename
>       17426	    392	    104	  17922	   4602	sound/soc/sh/rcar/core.o.orig
>       17414	    392	    104	  17910	   45f6	sound/soc/sh/rcar/core.o
> ---
>  sound/soc/sh/rcar/core.c | 31 +++++++++++++------------------

jFTR, using ARM's aarch64 gcc-8.3-2019.03 and v5.4-10271-g596cf45cbf6e:

$ size sound/soc/sh/rcar/core.o.before sound/soc/sh/rcar/core.o.after
   text	   data	    bss	    dec	    hex	filename
  21433	    380	     88	  21901	   558d	sound/soc/sh/rcar/core.o.before
  21505	    380	     88	  21973	   55d5	sound/soc/sh/rcar/core.o.after

>  1 file changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
> index 399dc6e9bde5b042..d20f03dfdee66643 100644
> --- a/sound/soc/sh/rcar/core.c
> +++ b/sound/soc/sh/rcar/core.c
> @@ -376,20 +376,15 @@ u32 rsnd_get_adinr_bit(struct rsnd_mod *mod, struct rsnd_dai_stream *io)
>   */
>  u32 rsnd_get_dalign(struct rsnd_mod *mod, struct rsnd_dai_stream *io)
>  {
> -	static const u32 dalign_values[8][2] = {
> -		{0x76543210, 0x67452301},
> -		{0x00000032, 0x00000023},
> -		{0x00007654, 0x00006745},
> -		{0x00000076, 0x00000067},
> -		{0xfedcba98, 0xefcdab89},
> -		{0x000000ba, 0x000000ab},
> -		{0x0000fedc, 0x0000efcd},
> -		{0x000000fe, 0x000000ef},
> +	static const u32 dalign_values[8] = {
> +		0x76543210, 0x00000032, 0x00007654, 0x00000076,
> +		0xfedcba98, 0x000000ba, 0x0000fedc, 0x000000fe,

FWIW, using below test procedure, I can reconstruct the original
two-dimensional dalign_values[8][2] array on the host:

#include <stdio.h>
#include <stdint.h>

typedef uint32_t u32;

void main(void)
{
	u32 dalign, i;

	static const u32 dalign_values[8] = {
		0x76543210, 0x00000032, 0x00007654, 0x00000076,
		0xfedcba98, 0x000000ba, 0x0000fedc, 0x000000fe,
	};

	for (i = 0; i < 8; i++) {
		u32 keep = dalign = dalign_values[i];
		dalign = (dalign & 0xf0f0f0f0) >> 4 |
			 (dalign & 0x0f0f0f0f) << 4;
		printf("{0x%08x, 0x%08x},\n", keep, dalign);
	}
}

-- 
Best Regards,
Eugeniu