[alsa-devel] [PATCH] add the nvidia HDMI codec driver for MCP77/79

Takashi Iwai tiwai at suse.de
Wed Sep 3 10:51:48 CEST 2008


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 at nvidia.com>
> Signed-off-by: Peer Chen <peerchen at gmail.com>

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


More information about the Alsa-devel mailing list