At Fri, 7 Dec 2012 21:35:58 -0800, Ian Minett wrote:
From: Ian Minett ian_minett@creativelabs.com
Add the new switches, virtual nodes, controls and parameters used for configuring the firmware DSP. Update callbacks and other routines to set up the correct streams and use the new effect switches. Add verb tables for DSP init.
Signed-off-by: Ian Minett ian_minett@creativelabs.com
OK, this is a big change in a single shot, and it makes harder to follow the code.
Could you split this to a several patches? The change needed for the headphone/speaker control and the change for the mic are basically independent. Also, the enhancement of PCM (e.g. max_channels became 6 now) is also independent from other changes.
@@ -893,6 +866,8 @@ static int chipio_write_data(struct hda_codec *codec, unsigned int data) data >> 16); }
- spec->curr_chip_addx = (res != -EIO) ?
(spec->curr_chip_addx + 4) : ~0UL;
What does this do at all? A bit more description please. I suppose it increments the address automatically for the word.
@@ -2626,17 +2614,63 @@ static bool dspload_wait_loaded(struct hda_codec *codec) CA0132_CODEC_MUTE_MONO(xname, nid, 3, dir)
/*
- PCM stuffs
- */
+static void ca0132_setup_stream(struct hda_codec *codec, hda_nid_t nid,
u32 stream_tag,
int channel_id, int format)
+{
- unsigned int oldval, newval;
- if (!nid)
return;
- snd_printdd(
"ca0132_setup_stream: NID=0x%x, stream=0x%x, "
"channel=%d, format=0x%x\n",
nid, stream_tag, channel_id, format);
- /* update the format-id if changed */
- oldval = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_STREAM_FORMAT,
0);
- if (oldval != format) {
msleep(20);
snd_hda_codec_write(codec, nid, 0,
AC_VERB_SET_STREAM_FORMAT,
format);
- }
- oldval = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_CONV, 0);
- newval = (stream_tag << 4) | channel_id;
- if (oldval != newval) {
snd_hda_codec_write(codec, nid, 0,
AC_VERB_SET_CHANNEL_STREAMID,
newval);
- }
+}
+static void ca0132_cleanup_stream(struct hda_codec *codec, hda_nid_t nid) +{
- unsigned int val;
- if (!nid)
return;
- snd_printdd(KERN_INFO "ca0132_cleanup_stream: NID=0x%x\n", nid);
- val = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_CONV, 0);
- if (!val)
return;
- snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_STREAM_FORMAT, 0);
- snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_CHANNEL_STREAMID, 0);
+}
Do we still need these two special functions? In the recent version, I already fixed the common helper snd_hda_multi_out_analog_*() for the case like ca0132.
@@ -2653,7 +2689,16 @@ static int ca0132_playback_pcm_cleanup(struct hda_pcm_stream *hinfo, struct snd_pcm_substream *substream) { struct ca0132_spec *spec = codec->spec;
- return snd_hda_multi_out_analog_cleanup(codec, &spec->multiout);
- if (spec->dsp_state == DSP_DOWNLOADING)
return 0;
- /*Allow stream some time to flush effects tail*/
- msleep(50);
This is a long stall. Do we need this unconditionally?
+static int ca0132_select_out(struct hda_codec *codec) +{
- struct ca0132_spec *spec = codec->spec;
- unsigned int pin_ctl;
- int jack_present;
- int auto_jack;
- unsigned int tmp;
- int err;
- snd_printdd(KERN_INFO "ca0132_select_out\n");
- snd_hda_power_up(codec);
- auto_jack = spec->vnode_lswitch[VNID_HP_ASEL - VNODE_START_NID];
- if (auto_jack)
jack_present = snd_hda_jack_detect(codec, spec->out_pins[1]);
- else
jack_present =
spec->vnode_lswitch[VNID_HP_SEL - VNODE_START_NID];
- if (jack_present)
spec->cur_out_type = HEADPHONE_OUT;
- else
spec->cur_out_type = SPEAKER_OUT;
- if (spec->cur_out_type == SPEAKER_OUT) {
snd_printdd(KERN_INFO "ca0132_select_out speaker\n");
/*speaker out config*/
tmp = FLOAT_ONE;
err = dspio_set_param(codec, 0x80, 0x04,
&tmp, sizeof(unsigned int));
Looking through the whole code, dspio_set_param() is called only for a 32bit int. Define the simplified version like:
static int dspio_set_int_param(struct hda_codec *codec, int mod_id, int req, unsigned int val) { return dspio_set_param(codec, mod_id, req, &val, sizeof }
+/*
- When changing Node IDs for Mixer Controls below, make sure to update
- Node IDs in ca0132_config() as well.
- */
+static struct snd_kcontrol_new ca0132_mixer[] = {
- CA0132_CODEC_VOL("Master Playback Volume", VNID_SPK, HDA_OUTPUT),
- CA0132_CODEC_MUTE("Master Playback Switch", VNID_SPK, HDA_OUTPUT),
- CA0132_CODEC_VOL("Capture Volume", VNID_MIC, HDA_INPUT),
- CA0132_CODEC_MUTE("Capture Switch", VNID_MIC, HDA_INPUT),
- HDA_CODEC_VOLUME("Analog-Mic2 Capture Volume", 0x08, 0, HDA_INPUT),
- HDA_CODEC_MUTE("Analog-Mic2 Capture Switch", 0x08, 0, HDA_INPUT),
- HDA_CODEC_VOLUME("What U Hear Capture Volume", 0x0a, 0, HDA_INPUT),
- HDA_CODEC_MUTE("What U Hear Capture Switch", 0x0a, 0, HDA_INPUT),
- CA0132_CODEC_MUTE_MONO("Mic1-Boost (30dB) Capture Switch",
0x12, 1, HDA_INPUT),
- CA0132_CODEC_MUTE_MONO("HP/Speaker Playback Switch",
VNID_HP_SEL, 1, HDA_OUTPUT),
This allows user to choose which output is used, right? Then this should be an enum. Otherwise it's not clear what it serves for.
- CA0132_CODEC_MUTE_MONO("AMic1/DMic Capture Switch",
VNID_AMIC1_SEL, 1, HDA_INPUT),
Ditto.
- CA0132_CODEC_MUTE_MONO("HP/Speaker Auto Detect Playback Switch",
VNID_HP_ASEL, 1, HDA_OUTPUT),
In other codec drivers, it's called as "Auto-Mute Mode".
- CA0132_CODEC_MUTE_MONO("AMic1/DMic Auto Detect Capture Switch",
VNID_AMIC1_ASEL, 1, HDA_INPUT),
There is no standard name for this yet, though...
I'll look into more details once when the patch is split. Otherwise it's really difficult to follow all changes.
thanks,
Takashi