Re: [alsa-devel][PATCH] add the nvidia HDMI codec driver for MCP77/79
From: Takashi Iwai
Date: Wed Sep 03 2008 - 04:52:01 EST
At Wed, 3 Sep 2008 15:58:17 +0800,
peerchen wrote:
>
> Add the nvidia HDMI codec driver for MCP77/79.
> The patch based on kernel 2.6.27-rc5.
>
> Signed-off-by: Wei Ni <wni@xxxxxxxxxx>
> Signed-off-by: Peer Chen <peerchen@xxxxxxxxx>
Thanks for the patch. Some comments below.
> diff -uprN -X linux-2.6.26-Orig/Documentation/dontdiff linux-2.6.26-Orig/sound/pci/hda/hda_intel.c linux-2.6.26-New/sound/pci/hda/hda_intel.c
> --- linux-2.6.26-Orig/sound/pci/hda/hda_intel.c 2008-08-28 13:47:20.000000000 +0800
> +++ linux-2.6.26-New/sound/pci/hda/hda_intel.c 2008-08-28 15:33:51.000000000 +0800
> @@ -223,8 +223,8 @@ enum { SDI0, SDI1, SDI2, SDI3, SDO0, SDO
> #define RIRB_INT_MASK 0x05
>
> /* STATESTS int mask: SD2,SD1,SD0 */
> -#define AZX_MAX_CODECS 3
> -#define STATESTS_INT_MASK 0x07
> +#define AZX_MAX_CODECS 4
> +#define STATESTS_INT_MASK 0x0f
This may break other platforms. Expanding STATESTS_INT_MASK is OK and
would be harmless, but AZX_MAX_CODECS is not.
AZX_MAX_CODECS is the default number of max codecs. The chipset
specific max number of codecs is defined in azx_max_codecs[].
If Nvidia chipsets supports properly more than 3, change
azx_max_codecs[AZX_DRIVER_NVIDIA] to 4.
> +static unsigned short convert_from_spdif_status_nv(unsigned int sbits)
> +{
> + unsigned short val = 0;
> +
> + if (sbits & IEC958_AES0_PROFESSIONAL)
> + val |= AC_DIG1_PROFESSIONAL;
> + if (sbits & IEC958_AES0_NONAUDIO)
> + val |= AC_DIG1_NONAUDIO;
> + if (sbits & IEC958_AES0_PROFESSIONAL) {
> + if ((sbits & IEC958_AES0_PRO_EMPHASIS) ==
> + IEC958_AES0_PRO_EMPHASIS_5015)
> + val |= AC_DIG1_EMPHASIS;
> + } else {
> + if ((sbits & IEC958_AES0_CON_EMPHASIS) ==
> + IEC958_AES0_CON_EMPHASIS_5015)
> + val |= AC_DIG1_EMPHASIS;
> + if (!(sbits & IEC958_AES0_CON_NOT_COPYRIGHT))
> + val |= AC_DIG1_COPYRIGHT;
> + if (sbits & (IEC958_AES1_CON_ORIGINAL << 8))
> + val |= AC_DIG1_LEVEL;
> + val |= sbits & (IEC958_AES1_CON_CATEGORY << 8);
> + }
> + return val;
> +}
> +
> +static unsigned int convert_to_spdif_status_nv(unsigned short val)
> +{
> + unsigned int sbits = 0;
> +
> + if (val & AC_DIG1_NONAUDIO)
> + sbits |= IEC958_AES0_NONAUDIO;
> + if (val & AC_DIG1_PROFESSIONAL)
> + sbits |= IEC958_AES0_PROFESSIONAL;
> + if (sbits & IEC958_AES0_PROFESSIONAL) {
> + if (sbits & AC_DIG1_EMPHASIS)
> + sbits |= IEC958_AES0_PRO_EMPHASIS_5015;
> + } else {
> + if (val & AC_DIG1_EMPHASIS)
> + sbits |= IEC958_AES0_CON_EMPHASIS_5015;
> + if (!(val & AC_DIG1_COPYRIGHT))
> + sbits |= IEC958_AES0_CON_NOT_COPYRIGHT;
> + if (val & AC_DIG1_LEVEL)
> + sbits |= (IEC958_AES1_CON_ORIGINAL << 8);
> + sbits |= val & (0x7f << 8);
> + }
> + return sbits;
> +}
> +
> +static int snd_hda_hdmi_cmask_get(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + ucontrol->value.iec958.status[0] = IEC958_AES0_PROFESSIONAL |
> + IEC958_AES0_NONAUDIO |
> + IEC958_AES0_CON_EMPHASIS_5015 |
> + IEC958_AES0_CON_NOT_COPYRIGHT;
> + ucontrol->value.iec958.status[1] = IEC958_AES1_CON_CATEGORY |
> + IEC958_AES1_CON_ORIGINAL;
> + return 0;
> +}
> +
> +static int snd_hda_hdmi_pmask_get(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + ucontrol->value.iec958.status[0] = IEC958_AES0_PROFESSIONAL |
> + IEC958_AES0_NONAUDIO |
> + IEC958_AES0_PRO_EMPHASIS_5015;
> + return 0;
> +}
> +
> +static int snd_hda_hdmi_mask_info(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_info *uinfo)
> +{
> + uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
> + uinfo->count = 1;
> + return 0;
> +}
> +
> +static int snd_hda_hdmi_default_get(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> +
> + ucontrol->value.iec958.status[0] = codec->spdif_status & 0xff;
> + ucontrol->value.iec958.status[1] = (codec->spdif_status >> 8) & 0xff;
> + ucontrol->value.iec958.status[2] = (codec->spdif_status >> 16) & 0xff;
> + ucontrol->value.iec958.status[3] = (codec->spdif_status >> 24) & 0xff;
> +
> + return 0;
> +}
> +
> +static int snd_hda_hdmi_default_put(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> + hda_nid_t nid = kcontrol->private_value;
> + unsigned short val;
> + int change;
> +
> + mutex_lock(&codec->spdif_mutex);
> + codec->spdif_status = ucontrol->value.iec958.status[0] |
> + ((unsigned int)ucontrol->value.iec958.status[1] << 8) |
> + ((unsigned int)ucontrol->value.iec958.status[2] << 16) |
> + ((unsigned int)ucontrol->value.iec958.status[3] << 24);
> + val = convert_from_spdif_status_nv(codec->spdif_status);
> + val |= codec->spdif_ctls & 1;
> + change = codec->spdif_ctls != val;
> + codec->spdif_ctls = val;
> +
> + if (change) {
> + snd_hda_codec_write_cache(codec, nid, 0,
> + AC_VERB_SET_DIGI_CONVERT_1,
> + val & 0xff);
> + snd_hda_codec_write_cache(codec, nid, 0,
> + AC_VERB_SET_DIGI_CONVERT_2,
> + val >> 8);
> + }
> +
> + mutex_unlock(&codec->spdif_mutex);
> + return change;
> +}
> +
> +#define snd_hda_hdmi_out_switch_info snd_ctl_boolean_mono_info
> +
> +static int snd_hda_hdmi_out_switch_get(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> +
> + ucontrol->value.integer.value[0] = codec->spdif_ctls & AC_DIG1_ENABLE;
> + return 0;
> +}
> +
> +static int snd_hda_hdmi_out_switch_put(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> + hda_nid_t nid = kcontrol->private_value;
> + unsigned short val;
> + int change;
> +
> + mutex_lock(&codec->spdif_mutex);
> + val = codec->spdif_ctls & ~AC_DIG1_ENABLE;
> + if (ucontrol->value.integer.value[0])
> + val |= AC_DIG1_ENABLE;
> + change = codec->spdif_ctls != val;
> + if (change) {
> + codec->spdif_ctls = val;
> + snd_hda_codec_write_cache(codec, nid, 0,
> + AC_VERB_SET_DIGI_CONVERT_1,
> + val & 0xff);
> + /* unmute amp switch (if any) */
> + if ((get_wcaps(codec, nid) & AC_WCAP_OUT_AMP) &&
> + (val & AC_DIG1_ENABLE))
> + snd_hda_codec_amp_stereo(codec, nid, HDA_OUTPUT, 0,
> + HDA_AMP_MUTE, 0);
> + }
> + mutex_unlock(&codec->spdif_mutex);
> + return change;
> +}
These are almost identical with the functions in hda_codec.c.
Is your intention to allow a different set of IEC958 status bits
controls for HDMI *in addition* to SPDIF? If so, we should change
the codes in hda_codec.c to allow other control names.
> +struct snd_kcontrol_new hdmi_mixes[] = {
> + {
> + .access = SNDRV_CTL_ELEM_ACCESS_READ,
> + .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> + .name = "IEC958 Playback Con Mask HDMI",
> + .info = snd_hda_hdmi_mask_info,
> + .get = snd_hda_hdmi_cmask_get,
> + },
These have also different names from the standard ones.
Any reason?
> +static struct hda_pcm_stream nvhdmi_pcm_digital_playback = {
> + .substreams = 1,
> + .channels_min = 2,
> + .channels_max = 2,
> + .nid = 0x4, /* NID to query formats and rates and setup streams */
> + .rates = SNDRV_PCM_RATE_48000,
> + .maxbps = 16,
> + .formats = SNDRV_PCM_FMTBIT_S16_LE,
Use tabs please.
> +static int nvhdmi_build_pcms(struct hda_codec *codec)
> +{
> + struct nvhdmi_spec *spec = codec->spec;
> + struct hda_pcm *info = &spec->pcm_rec;
> +
> + codec->num_pcms = 1;
> + codec->pcm_info = info;
> +
> + info->name = "NVIDIA HDMI";
> + info->pcm_type = HDA_PCM_TYPE_HDMI
Ditto.
thanks,
Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/