[alsa-devel] [PATCH 2/2] ASoC: hdmi-codec: add channel mapping control

[Arnaud Pouliquen]

On 12/13/2016 02:58 PM, Takashi Sakamoto wrote:
> On 2016年12月13日 22:23, Takashi Sakamoto wrote:
>> Hi Arnaud,
>>>> This table is invariant in lifetime of the storage object, as well.
>>>> Let's put into .rodata section, too.
>>>>
>>> This table is updated in hdmi_codec_cea_init_channel_alloc so can not be
>>> constant. In theory i could declare all field instead of computing some.
>>> But for lisibility, i would prefer to just declare ca_index  and
>>> speakers allocation field in this table (i will declared both as const)
>>
>> You should pay enough attention to a case that one system has several
>> GPUs to which relevant GPU drivers register HDMI_CODEC_DRV_NAME platform
>> device. The 'static' modifier has an effect to keep just one storage
>> object, thus your code causes bugs in the case.
> 
> Oops, the bug is unrelated to the static modifier. The modifier is for 
> reference scope. I'll correct as the file local symbol has just one 
> storage object.
> 
> (I might be tired tonight...)
> 
i understood in this way :-)
As 'ca_index' and "speakers" fiel are constants
the fields computed in hdmi_codec_cea_init_channel_alloc will
not change if the functions is called several time ( multi-instances).
but agree that it is not very clean, but this should work.
To avoid to compute several time, i can also add a test in
hdmi_codec_cea_init_channel_alloc to do it only one time (by testing
hdmi_codec_channel_alloc[0].channels), or perhaps better an atomic local
variable to avoid side effect for Multiprocessor archs.

Now the second approach is to define all the field. This indeeds a big
table (around 256 lines instead of 32 lines):

static const struct hdmi_codec_cea_spk_alloc hdmi_codec_channel_alloc[] = {
	{
		.ca_index = 0x00,
		.speakers = {   0,    0,   0,   0,   0,    0,  FR,  FL } },
		.channels = 2,
		.spk_mask = FR | FL,
		.spk_na_mask = 0x00,
	},
	{ /* 2.1 */
		.ca_index = 0x01,
		.speakers = {   0,    0,   0,   0,   0,  LFE,  FR,  FL } },
		.channels = 3,
		.spk_mask = FR | FL | LFE,
		.spk_na_mask = 0x00,
	},
	{ /* Dolby Surround */
		.ca_index = 0x02,
		.speakers = {   0,    0,   0,   0,  FC,    0,  FR,  FL } },
		.channels = 4,
		.spk_mask = FR | FL | FC,
		.spk_na_mask = BIT(2),
	},
[...]

	{
		.ca_index = 0x1f,
		.speakers = { FRC,  FLC,  RR,  RL,  FC,  LFE,  FR,  FL } },
		.channels = 8,
		.spk_mask = FRC | FLC | RR | RL | FC | LFE | FR | FL,
		.spk_na_mask = 0x0,
	},

I would prefer the first one to save lines, but i can implement the
second one (or another one if you have an alternative to propose).

Regards
Arnaud