Hi Russell,
Am Montag, den 04.01.2016, 22:29 +0000 schrieb Russell King - ARM Linux:
> On Mon, Jan 04, 2016 at 08:09:06PM +0100, Philipp Zabel wrote:
> > Add the interface needed by Jyri's generic hdmi-codec driver [1] to start
> > or stop audio playback and to retrieve ELD (EDID like data) to limit the
> > supported audio formats to the HDMI sink capabilities.
>
> Some of this makes me rather suspicious.
Thanks for being suspicious, then.
> > + switch (params->sample_rate) {
> > + case 32000:
> > + hdmi_params.aud_hdmi_fs = HDMI_AUDIO_SAMPLE_FREQUENCY_32000;
> > + hdmi_params.iec_frame_fs = HDMI_IEC_32K;
> > + /* channel status byte 3: fs and clock accuracy */
> > + hdmi_params.hdmi_l_channel_state[3] = 0x3;
> > + break;
> > + case 44100:
> > + hdmi_params.aud_hdmi_fs = HDMI_AUDIO_SAMPLE_FREQUENCY_44100;
> > + hdmi_params.iec_frame_fs = HDMI_IEC_44K;
> > + /* channel status byte 3: fs and clock accuracy */
> > + hdmi_params.hdmi_l_channel_state[3] = 0;
> > + break;
> > + case 48000:
> > + hdmi_params.aud_hdmi_fs = HDMI_AUDIO_SAMPLE_FREQUENCY_48000;
> > + hdmi_params.iec_frame_fs = HDMI_IEC_48K;
> > + /* channel status byte 3: fs and clock accuracy */
> > + hdmi_params.hdmi_l_channel_state[3] = 0x2;
> > + break;
> > + case 88200:
> > + hdmi_params.aud_hdmi_fs = HDMI_AUDIO_SAMPLE_FREQUENCY_88200;
> > + hdmi_params.iec_frame_fs = HDMI_IEC_88K;
> > + /* channel status byte 3: fs and clock accuracy */
> > + hdmi_params.hdmi_l_channel_state[3] = 0x8;
> > + break;
> > + case 96000:
> > + hdmi_params.aud_hdmi_fs = HDMI_AUDIO_SAMPLE_FREQUENCY_96000;
> > + hdmi_params.iec_frame_fs = HDMI_IEC_96K;
> > + /* channel status byte 3: fs and clock accuracy */
> > + hdmi_params.hdmi_l_channel_state[3] = 0xa;
> > + break;
> > + case 176400:
> > + hdmi_params.aud_hdmi_fs = HDMI_AUDIO_SAMPLE_FREQUENCY_176400;
> > + hdmi_params.iec_frame_fs = HDMI_IEC_176K;
> > + /* channel status byte 3: fs and clock accuracy */
> > + hdmi_params.hdmi_l_channel_state[3] = 0xc;
> > + break;
> > + case 192000:
> > + hdmi_params.aud_hdmi_fs = HDMI_AUDIO_SAMPLE_FREQUENCY_192000;
> > + hdmi_params.iec_frame_fs = HDMI_IEC_192K;
> > + /* channel status byte 3: fs and clock accuracy */
> > + hdmi_params.hdmi_l_channel_state[3] = 0xe;
>
> For exmaple, all the above. The HDMI standards say that the audio info
> frame "shall" always set the sample frequency to "refer to stream" for
> L-PCM and IEC61937 compressed audio. The above looks like it violates
> the HDMI specification as I'm willing to bet that hdmi_params.aud_hdmi_fs
> gets passed over into the audio info frame.
>
> Many of the fields in the audio info frame are supposed to be set as
> "refer to stream". I'd suggest ensuring that you're compliant with
> these.
The audio inforame is generated using
struct hdmi_audio_infoframe frame;
hdmi_audio_infoframe_init(&frame);
frame.coding_type = HDMI_AUDIO_CODING_TYPE_STREAM;
frame.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM;
frame.sample_size = HDMI_AUDIO_SAMPLE_SIZE_STREAM;
frame.channels =
mtk_hdmi_aud_get_chnl_count(
hdmi->aud_param.aud_input_chan_type);
hdmi_audio_infoframe_pack(&frame, buffer, sizeof(buffer));
later. aud_hdmi_fs/iec_frame_fs are used to select the values written to
the N/CTS register. Instead of those I could just pass the sample_rate
down to the lower layers.
> The second thing is that "hdmi_params.hdmi_l_channel_state" looks like
> it's the IEC958 bytes. These must be correct for some of the higher
> end HDMI receivers (eg, Yamaha RX-V677) to correctly process the audio.
I think you are right. These values are written to 24-byte register sets
"L_STATUS" and "R_STATUS" (padded with zeroes) as well as to the 5-byte
register set "I2S_C_STA" for the first five bytes.
> > + break;
> > + default:
> > + dev_err(hdmi->dev, "rate[%d] not supported!\n",
> > + params->sample_rate);
> > + return -EINVAL;
> > + }
> > +
> > + switch (daifmt->fmt) {
> > + case HDMI_I2S:
> > + hdmi_params.aud_codec = HDMI_AUDIO_CODING_TYPE_PCM;
> > + hdmi_params.aud_sampe_size = HDMI_AUDIO_SAMPLE_SIZE_16;
> > + hdmi_params.aud_input_type = HDMI_AUD_INPUT_I2S;
> > + hdmi_params.aud_i2s_fmt = HDMI_I2S_MODE_I2S_24BIT;
> > + hdmi_params.aud_mclk = HDMI_AUD_MCLK_128FS;
> > + break;
> > + default:
> > + dev_err(hdmi->dev, "%s: Invalid format %d\n", __func__,
> > + daifmt->fmt);
> > + return -EINVAL;
> > + }
> > +
> > + /* channel status */
> > + /* byte 0: no copyright is asserted, mode 0 */
> > + hdmi_params.hdmi_l_channel_state[0] = 1 << 2;
> > + /* byte 1: category code */
> > + hdmi_params.hdmi_l_channel_state[1] = 0;
> > + /* byte 2: source/channel number don't take into account */
> > + hdmi_params.hdmi_l_channel_state[2] = 0;
> > + /* byte 4: word length 16bits */
> > + hdmi_params.hdmi_l_channel_state[4] = 0x2;
> > + memcpy(hdmi_params.hdmi_r_channel_state,
> > + hdmi_params.hdmi_l_channel_state,
> > + sizeof(hdmi_params.hdmi_l_channel_state));
>
> Hmm, yes, it is the IEC958 channel status bytes. We have a helper
> for this - snd_pcm_create_iec958_consumer().
hdmi-codec already calls snd_pcm_create_iec958_consumer and provides the
result via params->iec.state, so I'll try to just use that.
regards
Philipp
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
