| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <15F501D1A78BD343BE8F4D8DB854566B31F8F744@hkemmail01.nvidia.com>
Date: Wed, 3 Sep 2008 19:05:47 +0800
From: "Wei Ni" <wni@...dia.com>
To: "Takashi Iwai" <tiwai@...e.de>
Cc: "alsa-devel" <alsa-devel@...a-project.org>,
"linux-kernel" <linux-kernel@...r.kernel.org>,
"akpm" <akpm@...ux-foundation.org>, "Peer Chen" <pchen@...dia.com>
Subject: RE: [alsa-devel][PATCH] add the nvidia HDMI codec driver for MCP77/79
Thanks for your comments. Some reply below:
-----Original Message-----
From: Takashi Iwai [mailto:tiwai@...e.de]
Sent: Wednesday, September 03, 2008 4:52 PM
To: peerchen
Cc: alsa-devel; linux-kernel; akpm; Peer Chen; Wei Ni
Subject: Re: [alsa-devel][PATCH] add the nvidia HDMI codec driver for
MCP77/79
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@...dia.com>
> Signed-off-by: Peer Chen <peerchen@...il.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.
==========Wei Ni's Answer: ==============
There are 2 codec connect to the nvidia AZA controller, one is realtek,
and the other is nvidia HDMI codec.
In azx_codec_create(), the chip->codec_mask indicate codec connection.
This chip->codec_mask=0x9, bit0 indicate realtek codec, and bit3
indicate hdmi codec.
In this routine, it use:
......
for (c = 0; c < AZX_MAX_CODECS; c++) {
if ((chip->codec_mask & (1 << c)) & codec_probe_mask) {
struct hda_codec *codec;
err = snd_hda_codec_new(chip->bus, c, &codec);
if (err < 0)
continue;
codecs++;
if (codec->afg)
audio_codecs++;
}
}
if (!audio_codecs) {
/* probe additional slots if no codec is found */
for (; c < azx_max_codecs[chip->driver_type]; c++) {
if ((chip->codec_mask & (1 << c)) &
codec_probe_mask) {
err = snd_hda_codec_new(chip->bus, c,
NULL);
if (err < 0)
continue;
codecs++;
}
}
}
......
According to these codes, It only can get the realtek codec and can not
get hdmi codec, if just change azx_max_codecs[AZX_DRIVER_NVIDIA] to 4
I think if remove "if (!audio_codecs)", it will be better, and I only
need to chang 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.
==========Wei Ni's Answer: ==============
There are 2 codec connect to the nvidia AZA controller, one is realtek,
the other is nvidia HDMI codec.
If I use the default SPDIF routine: snd_hda_create_spdif_out_ctls(), the
driver can not be insmod success.
I traced this issue, the realtek's SPDIF used this IEC958 first, and
then HDMI codec can't use IEC958 again.
( I traced it based on linux-2.6.25 )
So I write some routines for HDMI which are same as SPDIF routine, and
the driver can work.
=====================================
> +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?
==========Wei Ni's Answer: ==============
Ditto
=====================================
> +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.
==========Wei Ni's Answer: ==============
Thanks, I will modify it.
=====================================
> +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.
==========Wei Ni's Answer: ==============
Thanks, I will modify it.
=====================================
thanks,
Takashi
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists