At Mon, 21 Jan 2008 13:48:20 -0500, Marc Boucher wrote:

Hi Takashi,

Here's the sign-off:

Signed-off-by: Marc Boucher marc@linuxant.com

Thanks!

Do you have any specific comments about the code itself?

I have now applied some parts of your patch to HG tree, namely,

- laptop model fixes for Cxt5045 - clean up of Cxt5047 verbs - add missing input elements for cxt5047 test model - a workaround in power-state change - afg and mfg fields in codec preset - ratelimit to timeout messages

I modified the above a bit, mainly for fixing coding issues. So, be careful to rebase your patch again.

The rest, the addition of cxt5051 and the HSF modem are pending.

I checked the patch and found some problems in cxt5051 code:

- the ADC stream may be unswitched even if you plug/unplug the docking station when the PCM is being opened - basically the capture switch has no role - judging the difference of capture prepare/cleanup via the number of adc

The patch below is my revised version with some cleanups. Can someone test with the top of the very latest HG tree (at least changeset 5753)?

About HSF modem, let me clarify. The HSF modem patch can't be applied as is to the kernel tree. The purpose of this patch is exactly to bind with a binary-only object. And, as everybody knows, binary-only objects are refused in the kernel upstream in general, so there is no reason to apply it.

Oh, I also didn't apply the addition of wallclock and lpos. These are basically fine to add but there is no user except for HSF. The lpos callback can be replaced with the normal PCM position read, I guess.

The exit callback is also not applied. Why is this needed? Doesn't it work if you call from free callback?

Takashi

diff -r 7cebcd53be49 Documentation/ALSA-Configuration.txt --- a/Documentation/ALSA-Configuration.txt Tue Jan 22 15:18:08 2008 +0100 +++ b/Documentation/ALSA-Configuration.txt Tue Jan 22 15:19:14 2008 +0100 @@ -960,6 +960,9 @@ Prior to version 0.9.0rc4 options had a test for testing/debugging purpose, almost all controls can be adjusted. Appearing only when compiled with $CONFIG_SND_DEBUG=y + + Conexant 5051 + laptop Basic Laptop config

STAC9200 ref Reference board diff -r 7cebcd53be49 pci/hda/patch_conexant.c --- a/pci/hda/patch_conexant.c Tue Jan 22 15:18:08 2008 +0100 +++ b/pci/hda/patch_conexant.c Tue Jan 22 15:19:14 2008 +0100 @@ -63,6 +63,11 @@ struct conexant_spec { unsigned int num_adc_nids; hda_nid_t *adc_nids; hda_nid_t dig_in_nid; /* digital-in NID; optional */ + + unsigned int cur_adc_idx; + hda_nid_t cur_adc; + unsigned int cur_adc_stream_tag; + unsigned int cur_adc_format;

/* capture source */ const struct hda_input_mux *input_mux; @@ -217,6 +222,41 @@ static struct hda_pcm_stream conexant_pc /* NID is set in alc_build_pcms */ };

+static int cx5051_capture_pcm_prepare(struct hda_pcm_stream *hinfo, + struct hda_codec *codec, + unsigned int stream_tag, + unsigned int format, + struct snd_pcm_substream *substream) +{ + struct conexant_spec *spec = codec->spec; + spec->cur_adc = spec->adc_nids[spec->cur_adc_idx]; + spec->cur_adc_stream_tag = stream_tag; + spec->cur_adc_format = format; + snd_hda_codec_setup_stream(codec, spec->cur_adc, stream_tag, 0, format); + return 0; +} + +static int cx5051_capture_pcm_cleanup(struct hda_pcm_stream *hinfo, + struct hda_codec *codec, + struct snd_pcm_substream *substream) +{ + struct conexant_spec *spec = codec->spec; + snd_hda_codec_setup_stream(codec, spec->cur_adc, 0, 0, 0); + spec->cur_adc = 0; + return 0; +} + +static struct hda_pcm_stream cx5051_pcm_analog_capture = { + .substreams = 1, + .channels_min = 2, + .channels_max = 2, + .nid = 0, /* fill later */ + .ops = { + .prepare = cx5051_capture_pcm_prepare, + .cleanup = cx5051_capture_pcm_cleanup + }, +}; + static int conexant_build_pcms(struct hda_codec *codec) { struct conexant_spec *spec = codec->spec; @@ -231,7 +271,12 @@ static int conexant_build_pcms(struct hd spec->multiout.max_channels; info->stream[SNDRV_PCM_STREAM_PLAYBACK].nid = spec->multiout.dac_nids[0]; - info->stream[SNDRV_PCM_STREAM_CAPTURE] = conexant_pcm_analog_capture; + if (codec->vendor_id == 0x14f15051) + info->stream[SNDRV_PCM_STREAM_CAPTURE] = + cx5051_pcm_analog_capture; + else + info->stream[SNDRV_PCM_STREAM_CAPTURE] = + conexant_pcm_analog_capture; info->stream[SNDRV_PCM_STREAM_CAPTURE].substreams = spec->num_adc_nids; info->stream[SNDRV_PCM_STREAM_CAPTURE].nid = spec->adc_nids[0];

@@ -1421,10 +1466,325 @@ static int patch_cxt5047(struct hda_code return 0; }

+/* Conexant 5051 specific */ +static hda_nid_t cxt5051_dac_nids[1] = { 0x10 }; +static hda_nid_t cxt5051_adc_nids[2] = { 0x14, 0x15 }; +static hda_nid_t cxt5051_capsrc_nids[2] = { 0x14, 0x15 }; +#define CXT5051_SPDIF_OUT 0x1C +#define CXT5051_PORTB_EVENT 0x38 +#define CXT5051_PORTC_EVENT 0x39 + +static struct hda_channel_mode cxt5051_modes[1] = { + { 2, NULL }, +}; + +static struct hda_input_mux cxt5051_capture_source = { + .num_items = 2, + .items = { + { "IntMic", 0x1 }, + { "ExtMic", 0x2 }, + } +}; + +/* turn on/off EAPD (+ mute HP) as a master switch */ +static int cxt5051_hp_master_sw_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + static struct hda_verb spk_on[] = { + {0x1a, AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_OUT}, + {} + }; + static struct hda_verb spk_off[] = { + {0x1a, AC_VERB_SET_PIN_WIDGET_CONTROL, 0}, + {} + }; + static struct hda_verb hp_on[] = { + {0x16, AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_OUT}, + {} + }; + static struct hda_verb hp_off[] = { + {0x16, AC_VERB_SET_PIN_WIDGET_CONTROL, 0}, + {} + }; + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + struct conexant_spec *spec = codec->spec; + + if (!cxt_eapd_put(kcontrol, ucontrol)) + return 0; + + /* toggle internal speakers mute depending of presence of + * the headphone jack + */ + if (!spec->hp_present && spec->cur_eapd) + snd_hda_sequence_write(codec, spk_on); + else + snd_hda_sequence_write(codec, spk_off); + + if (spec->cur_eapd) + snd_hda_sequence_write(codec, hp_on); + else + snd_hda_sequence_write(codec, hp_off); + + return 1; +} + +/* bind volumes of both NID 0x10 and 0x11 */ +static int cxt5051_hp_master_vol_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + long *valp = ucontrol->value.integer.value; + int change; + + change = snd_hda_codec_amp_update(codec, 0x10, 0, HDA_OUTPUT, 0, + HDA_AMP_VOLMASK, valp[0]); + change |= snd_hda_codec_amp_update(codec, 0x10, 1, HDA_OUTPUT, 0, + HDA_AMP_VOLMASK, valp[1]); + return change; +} + +/* toggle input of built-in and mic jack appropriately */ +static void cxt5051_portb_automic(struct hda_codec *codec) +{ + static struct hda_verb mic_jack_on[] = { + {0x14, AC_VERB_SET_CONNECT_SEL, 0x1}, + {} + }; + static struct hda_verb mic_jack_off[] = { + {0x14, AC_VERB_SET_CONNECT_SEL, 0x0}, + {} + }; + unsigned int present; + + present = snd_hda_codec_read(codec, 0x17, 0, + AC_VERB_GET_PIN_SENSE, 0) & 0x80000000; + + if (present) + snd_hda_sequence_write(codec, mic_jack_on); + else + snd_hda_sequence_write(codec, mic_jack_off); +} + +static void cxt5051_portc_automic(struct hda_codec *codec) +{ + struct conexant_spec *spec = codec->spec; + unsigned int present; + hda_nid_t new_adc; + + present = snd_hda_codec_read(codec, 0x18, 0, + AC_VERB_GET_PIN_SENSE, 0) & 0x80000000; + if (present) + spec->cur_adc_idx = 1; + else + spec->cur_adc_idx = 0; + new_adc = spec->adc_nids[spec->cur_adc_idx]; + if (spec->cur_adc && spec->cur_adc != new_adc) { + snd_hda_codec_setup_stream(codec, spec->cur_adc, 0, 0, 0); + spec->cur_adc = new_adc; + snd_hda_codec_setup_stream(codec, new_adc, + spec->cur_adc_stream_tag, 0, + spec->cur_adc_format); + } +} + +/* mute internal speaker if HP is plugged */ +static void cxt5051_hp_automute(struct hda_codec *codec) +{ + static struct hda_verb spk_on[] = { + {0x1A, AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_OUT}, + {} + }; + static struct hda_verb spk_off[] = { + {0x1A, AC_VERB_SET_PIN_WIDGET_CONTROL, 0}, + {} + }; + + struct conexant_spec *spec = codec->spec; + unsigned int bits; + + spec->hp_present = snd_hda_codec_read(codec, 0x16, 0, + AC_VERB_GET_PIN_SENSE, 0) & 0x80000000; + + + bits = (spec->hp_present || !spec->cur_eapd) ? 0x80 : 0; + if (bits) + snd_hda_sequence_write(codec, spk_off); + else + snd_hda_sequence_write(codec, spk_on); + + printk(KERN_DEBUG"cxt5051_hp_automute bits =%x\n",bits); +} + +/* unsolicited event for HP jack sensing */ +static void cxt5051_hp_unsol_event(struct hda_codec *codec, + unsigned int res) +{ + switch (res >> 26) { + case CONEXANT_HP_EVENT: + cxt5051_hp_automute(codec); + break; + case CXT5051_PORTB_EVENT: + cxt5051_portb_automic(codec); + break; + case CXT5051_PORTC_EVENT: + cxt5051_portc_automic(codec); + break; + } +} + +static struct snd_kcontrol_new cxt5051_mixers[] = { + HDA_CODEC_VOLUME("Internal Mic Volume", 0x14, 0x00, HDA_INPUT), + HDA_CODEC_MUTE("Internal Mic Switch", 0x14, 0x00, HDA_INPUT), + HDA_CODEC_VOLUME("External Mic Volume", 0x14, 0x01, HDA_INPUT), + HDA_CODEC_MUTE("External Mic Switch", 0x14, 0x01, HDA_INPUT), + HDA_CODEC_VOLUME("Docking Mic Volume", 0x15, 0x00, HDA_INPUT), + HDA_CODEC_MUTE("Docking Mic Switch", 0x15, 0x00, HDA_INPUT), + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Master Playback Volume", + .info = snd_hda_mixer_amp_volume_info, + .get = snd_hda_mixer_amp_volume_get, + .put = cxt5051_hp_master_vol_put, + .private_value = HDA_COMPOSE_AMP_VAL(0x10, 3, 0, HDA_OUTPUT), + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Master Playback Switch", + .info = cxt_eapd_info, + .get = cxt_eapd_get, + .put = cxt5051_hp_master_sw_put, + .private_value = 0x1a, + }, + + {} +}; + +static struct hda_verb cxt5051_init_verbs[] = { + /* Line in, Mic */ + {0x17, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(0) | 0x03}, + {0x17, AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_VREF80}, + {0x18, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(0) | 0x03}, + {0x18, AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_VREF80}, + {0x1d, AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_IN}, + {0x1d, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(0) | 0x03}, + /* SPK */ + {0x1a, AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_OUT}, + {0x1a, AC_VERB_SET_CONNECT_SEL, 0x00}, + /* HP, Amp */ + {0x16, AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_HP}, + {0x16, AC_VERB_SET_CONNECT_SEL, 0x00}, + /* DAC1 */ + {0x10, AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_UNMUTE}, + /* Record selector: Int mic */ + {0x14, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(0) | 0x44}, + {0x14, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(1) | 0x44}, + {0x15, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(0) | 0x44}, + /* SPDIF route: PCM */ + {0x1c, AC_VERB_SET_CONNECT_SEL, 0x0}, + /* EAPD */ + {0x1a, AC_VERB_SET_EAPD_BTLENABLE, 0x2}, /* default on */ + {0x16, AC_VERB_SET_UNSOLICITED_ENABLE, AC_USRSP_EN|CONEXANT_HP_EVENT}, + {0x17, AC_VERB_SET_UNSOLICITED_ENABLE, AC_USRSP_EN|CXT5051_PORTB_EVENT}, + {0x18, AC_VERB_SET_UNSOLICITED_ENABLE, AC_USRSP_EN|CXT5051_PORTC_EVENT}, + { } /* end */ +}; + + +/* For HP's configuration always use portB as Internal and External Mic */ +static struct hda_verb cxt5051_mic_wid17_init_verbs[] = { + /* pin sensing on HP jack */ + {0x14, AC_VERB_SET_CONNECT_SEL,0x1}, + { } /* end */ +}; + + +/* initialize jack-sensing, too */ +static int cxt5051_init(struct hda_codec *codec) +{ + conexant_init(codec); + if (codec->patch_ops.unsol_event) { + cxt5051_hp_automute(codec); + cxt5051_portb_automic(codec); + cxt5051_portc_automic(codec); + } + return 0; +} + + +enum { + CXT5051_LAPTOP, /* Laptops w/ EAPD support */ + CXT5051_MODELS +}; + +static const char *cxt5051_models[CXT5051_MODELS] = { + [CXT5051_LAPTOP] = "laptop", +}; + +static struct snd_pci_quirk cxt5051_cfg_tbl[] = { + SND_PCI_QUIRK(0x14f1, 0x0101, "Conexant Reference board", + CXT5051_LAPTOP), + {} +}; + +static int patch_cxt5051(struct hda_codec *codec) +{ + struct conexant_spec *spec; + int board_config; + + spec = kzalloc(sizeof(*spec), GFP_KERNEL); + if (!spec) + return -ENOMEM; + mutex_init(&spec->amp_mutex); + codec->spec = spec; + + spec->multiout.max_channels = 2; + spec->multiout.num_dacs = ARRAY_SIZE(cxt5051_dac_nids); + spec->multiout.dac_nids = cxt5051_dac_nids; + spec->multiout.dig_out_nid = CXT5051_SPDIF_OUT; + spec->num_adc_nids = 2; + spec->adc_nids = cxt5051_adc_nids; + spec->capsrc_nids = cxt5051_capsrc_nids; + spec->input_mux = &cxt5051_capture_source; + spec->num_mixers = 1; + spec->mixers[0] = cxt5051_mixers; + spec->num_init_verbs = 1; + spec->init_verbs[0] = cxt5051_init_verbs; + spec->spdif_route = 0; + spec->num_channel_mode = ARRAY_SIZE(cxt5051_modes); + spec->channel_mode = cxt5051_modes; + spec->cur_adc = 0; + spec->cur_adc_idx = 0; + + codec->patch_ops = conexant_patch_ops; + + board_config = snd_hda_check_board_config(codec, CXT5051_MODELS, + cxt5051_models, + cxt5051_cfg_tbl); + switch (board_config) { + default: + case CXT5051_LAPTOP: + codec->patch_ops.unsol_event = cxt5051_hp_unsol_event; + spec->input_mux = &cxt5051_capture_source; + spec->num_init_verbs = 2; + spec->init_verbs[1] = cxt5051_mic_wid17_init_verbs; + spec->mixers[0] = cxt5051_mixers; + codec->patch_ops.init = cxt5051_init; + break; + } + + return 0; +} + + +/* + */ + struct hda_codec_preset snd_hda_preset_conexant[] = { { .id = 0x14f15045, .name = "CX20549 (Venice)", .patch = patch_cxt5045 }, { .id = 0x14f15047, .name = "CX20551 (Waikiki)", .patch = patch_cxt5047 }, + { .id = 0x14f15051, .name = "CX20561 (Hermosa)", + .patch = patch_cxt5051 }, {} /* terminator */ };

Em Thursday 24 January 2008 14:27:19 Takashi Iwai escreveu:

At Thu, 24 Jan 2008 16:50:54 +0100,

I wrote:

...

Good to hear. I'll try to clean up more a bit before merging.

I did more rewrite than expected in the end... The revised patch is below. Now it has model=hp for your machine. Give it a try.

Works ok as previously, and the mixer matches it fine, just had to add this on top of your patch:

--- linux-2.6.24-rc8.2mdv/sound/pci/hda/patch_conexant.c.orig 2008-01-24 16:08:26.000000000 -0200 +++ linux-2.6.24-0.rc8.2mdv/sound/pci/hda/patch_conexant.c 2008-01-24 16:50:54.000000000 -0200 @@ -1623,11 +1623,14 @@

static const char *cxt5051_models[CXT5051_MODELS] = { [CXT5051_LAPTOP] = "laptop", + [CXT5051_HP] = "hp", };

static struct snd_pci_quirk cxt5051_cfg_tbl[] = { SND_PCI_QUIRK(0x14f1, 0x0101, "Conexant Reference board", CXT5051_LAPTOP), + SND_PCI_QUIRK(0x14f1, 0x5051, "HP Spartan 1.1", + CXT5051_HP), {} };

If you look at the quirk, it's strange here that this HP laptop has the same vendor/device ids of Conexant instead of using an HP specific one, this can cause problems I think, or may be its something like just a unmodified Conexant reference implementation.

I also tested the laptop model and it's ok as before. I just noticed that with model=hp I don't get anymore the PCM slider in alsamixer, I'm just reporting as I don't know about it and if it's ok this way then fine :)

Takashi