Re: [PATCH] ALSA: hda - HDMI: Fix MCP7x audio infoframe checksums

[Takashi Iwai <tiwai@suse.de>]

At Wed, 6 Apr 2011 17:19:04 -0700,
Aaron Plattner wrote:
> 
> The MCP7x hardware computes the audio infoframe channel count
> automatically, but requires the audio driver to set the audio
> infoframe checksum manually via the Nv_VERB_SET_Info_Frame_Checksum
> control verb.
> 
> When audio starts playing, nvhdmi_8ch_7x_pcm_prepare sets the checksum
> to (0x71 - chan - chanmask).  For example, for 2ch audio, chan == 1
> and chanmask == 0 so the checksum is set to 0x70.  When audio playback
> finishes and the device is closed, nvhdmi_8ch_7x_pcm_close resets the
> channel formats, causing the channel count to revert to 8ch.  Since
> the checksum is not reset, the hardware starts generating audio
> infoframes with invalid checksums.  This causes some displays to blank
> the video.
> 
> Fix this by updating the checksum and channel mask when the device is
> closed and also when it is first initialized.  In addition, make sure
> that the channel mask is appropriate for an 8ch infoframe by setting
> it to 0x13 (FL FR LFE FC RL RR RLC RRC).
> 
> Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
> Acked-by: Stephen Warren <swarren@nvidia.com>

Thanks, applied now.


Takashi

> ---
>  sound/pci/hda/patch_hdmi.c |   70 +++++++++++++++++++++++++++----------------
>  1 files changed, 44 insertions(+), 26 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 251773e..715615a 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1280,6 +1280,39 @@ static int simple_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
>  					     stream_tag, format, substream);
>  }
>  
> +static void nvhdmi_8ch_7x_set_info_frame_parameters(struct hda_codec *codec,
> +						    int channels)
> +{
> +	unsigned int chanmask;
> +	int chan = channels ? (channels - 1) : 1;
> +
> +	switch (channels) {
> +	default:
> +	case 0:
> +	case 2:
> +		chanmask = 0x00;
> +		break;
> +	case 4:
> +		chanmask = 0x08;
> +		break;
> +	case 6:
> +		chanmask = 0x0b;
> +		break;
> +	case 8:
> +		chanmask = 0x13;
> +		break;
> +	}
> +
> +	/* Set the audio infoframe channel allocation and checksum fields.  The
> +	 * channel count is computed implicitly by the hardware. */
> +	snd_hda_codec_write(codec, 0x1, 0,
> +			Nv_VERB_SET_Channel_Allocation, chanmask);
> +
> +	snd_hda_codec_write(codec, 0x1, 0,
> +			Nv_VERB_SET_Info_Frame_Checksum,
> +			(0x71 - chan - chanmask));
> +}
> +
>  static int nvhdmi_8ch_7x_pcm_close(struct hda_pcm_stream *hinfo,
>  				   struct hda_codec *codec,
>  				   struct snd_pcm_substream *substream)
> @@ -1298,6 +1331,10 @@ static int nvhdmi_8ch_7x_pcm_close(struct hda_pcm_stream *hinfo,
>  				AC_VERB_SET_STREAM_FORMAT, 0);
>  	}
>  
> +	/* The audio hardware sends a channel count of 0x7 (8ch) when all the
> +	 * streams are disabled. */
> +	nvhdmi_8ch_7x_set_info_frame_parameters(codec, 8);
> +
>  	return snd_hda_multi_out_dig_close(codec, &spec->multiout);
>  }
>  
> @@ -1308,37 +1345,16 @@ static int nvhdmi_8ch_7x_pcm_prepare(struct hda_pcm_stream *hinfo,
>  				     struct snd_pcm_substream *substream)
>  {
>  	int chs;
> -	unsigned int dataDCC1, dataDCC2, chan, chanmask, channel_id;
> +	unsigned int dataDCC1, dataDCC2, channel_id;
>  	int i;
>  
>  	mutex_lock(&codec->spdif_mutex);
>  
>  	chs = substream->runtime->channels;
> -	chan = chs ? (chs - 1) : 1;
>  
> -	switch (chs) {
> -	default:
> -	case 0:
> -	case 2:
> -		chanmask = 0x00;
> -		break;
> -	case 4:
> -		chanmask = 0x08;
> -		break;
> -	case 6:
> -		chanmask = 0x0b;
> -		break;
> -	case 8:
> -		chanmask = 0x13;
> -		break;
> -	}
>  	dataDCC1 = AC_DIG1_ENABLE | AC_DIG1_COPYRIGHT;
>  	dataDCC2 = 0x2;
>  
> -	/* set the Audio InforFrame Channel Allocation */
> -	snd_hda_codec_write(codec, 0x1, 0,
> -			Nv_VERB_SET_Channel_Allocation, chanmask);
> -
>  	/* turn off SPDIF once; otherwise the IEC958 bits won't be updated */
>  	if (codec->spdif_status_reset && (codec->spdif_ctls & AC_DIG1_ENABLE))
>  		snd_hda_codec_write(codec,
> @@ -1413,10 +1429,7 @@ static int nvhdmi_8ch_7x_pcm_prepare(struct hda_pcm_stream *hinfo,
>  		}
>  	}
>  
> -	/* set the Audio Info Frame Checksum */
> -	snd_hda_codec_write(codec, 0x1, 0,
> -			Nv_VERB_SET_Info_Frame_Checksum,
> -			(0x71 - chan - chanmask));
> +	nvhdmi_8ch_7x_set_info_frame_parameters(codec, chs);
>  
>  	mutex_unlock(&codec->spdif_mutex);
>  	return 0;
> @@ -1512,6 +1525,11 @@ static int patch_nvhdmi_8ch_7x(struct hda_codec *codec)
>  	spec->multiout.max_channels = 8;
>  	spec->pcm_playback = &nvhdmi_pcm_playback_8ch_7x;
>  	codec->patch_ops = nvhdmi_patch_ops_8ch_7x;
> +
> +	/* Initialize the audio infoframe channel mask and checksum to something
> +	 * valid */
> +	nvhdmi_8ch_7x_set_info_frame_parameters(codec, 8);
> +
>  	return 0;
>  }
>  
> -- 
> 1.7.1
>