Re: [PATCH RFC v3 3/7] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux