On 08/14/15 12:57, Russell King - ARM Linux wrote:
> On Fri, Aug 14, 2015 at 12:30:41PM +0300, Jyri Sarha wrote:
>> +static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
>> + struct snd_pcm_hw_params *params,
>> + struct snd_soc_dai *dai)
>> +{
>> + struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
>> + struct hdmi_codec_params hp = {
>> + .cea = { 0 },
>
> Unnecessary initialisation - because you are initialising this structure,
> all unnamed fields will be zeroed.
>
True, I just tried to be explicit.
>> + .iec = {
>> + .status = {
>> + IEC958_AES0_CON_NOT_COPYRIGHT,
>> + IEC958_AES1_CON_GENERAL,
>> + IEC958_AES2_CON_SOURCE_UNSPEC,
>> + IEC958_AES3_CON_CLOCK_VARIABLE,
>> + },
>
> ...
>
>> + hdmi_audio_infoframe_init(&hp.cea);
>> + hp.cea.coding_type = HDMI_AUDIO_CODING_TYPE_PCM;
>
> Something tells me here that you haven't read the HDMI specification.
> HDMI says that the coding type will be zero (refer to stream header).
> The same goes for much of the CEA audio infoframe. Please see the
> Audio InfoFrame details in the HDMI specification.
>
Must admit, that I have not read it end to end. Obviously I have missed
a relevant piece of information here. I'll fix that and check the
related items too.
>> + hp.cea.channels = params_channels(params);
>> +
>> + switch (params_width(params)) {
>> + case 16:
>> + hp.iec.status[4] |= IEC958_AES4_CON_WORDLEN_20_16;
>> + hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_16;
>> + break;
>> + case 18:
>> + hp.iec.status[4] |= IEC958_AES4_CON_WORDLEN_22_18;
>> + hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_20;
>> + break;
>> + case 20:
>> + hp.iec.status[4] |= IEC958_AES4_CON_WORDLEN_24_20;
>> + hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_20;
>> + break;
>> + case 24:
>> + case 32:
>> + hp.iec.status[4] |= IEC958_AES4_CON_MAX_WORDLEN_24 |
>> + IEC958_AES4_CON_WORDLEN_24_20;
>
> Why not use the generic code to generate the AES channel status bits?
> See sound/core/pcm_iec958.c.
>
Thanks, I did not know that exist. I'll make use of that.
Best regards,
Jyri
