2011/6/18 Raymond Yau superquad.vortex2@gmail.com:
2011/6/8 alex dot baldacchino dot alsasub at gmail dot com alex.baldacchino.alsasub@gmail.com:
Adding a few printk()'s to file "patch_via.c" showed me function via_hp_build() is called before via_build_pcms() (unless front panel is disabled in BIOS, in which case via_hp_build() isn't invoked at all), therefore, when a control "Independent HP" is created, it should be 'visible' by via_build_pcms(); however, snd_hda_find_mixer_ctl(codec, "Independent HP") fails to find it until the following loop is executed inside via_build_controls:
and via_build_controls() is called after via_build_pcms(), but then it's too late (or seemed to be such) to modify the number of playback substreams. At last, I've tried the following, which worked:
The fix is to check spec->multiout.hp_nid since "Independent HP" need the extra DAC for HP when there is no HP if front panel is disabled by BIOS
static int via_build_pcms(struct hda_codec *codec) { ...
info->name = spec->stream_name_analog; info->stream[SNDRV_PCM_STREAM_PLAYBACK] = *(spec->stream_analog_playback); info->stream[SNDRV_PCM_STREAM_PLAYBACK].nid = spec->multiout.dac_nids[0];
- info->stream[SNDRV_PCM_STREAM_PLAYBACK].substreams =
- spec->multiout.hp_nid ? 2 : 1;
info->stream[SNDRV_PCM_STREAM_CAPTURE] = *(spec->stream_analog_capture); info->stream[SNDRV_PCM_STREAM_CAPTURE].nid = spec->adc_nids[0];
info->stream[SNDRV_PCM_STREAM_PLAYBACK].channels_max = spec->multiout.max_channels;
It might work, but might also fail, I guess. Even if everything else went ok during initialization, and multiout.hp_nid were set in vtxxxx_auto_create_hp_ctls(), via_hp_build() could fail to create an Independent HP control, because of either a failure allocating memory for it, when calling via_clone_control():
knew = via_clone_control(spec, &via_hp_mixer[0]); if (knew == NULL) return -ENOMEM;
or because the nid corresponding to the Independent HP control hasn't got enough connections:
if (spec->codec_type != VT1708) { nums = snd_hda_get_connections(codec, nid, conn, HDA_MAX_CONNECTIONS); if (nums <= 1) return 0; }
As I understand it, without a control "Independent HP" with its proper .get and .put methods, that's not possible to switch between single- and multi-streaming, thus the problem is here, I think.
But perhaps, if there are not enough connections to switch from/to, it might happen that autocfg.hp_pins[0] is 0, so that:
static int vtXXXX_parse_auto_config(struct hda_codec *codec) { ...
err = vtXXX_auto_create_hp_ctls(spec, spec->autocfg.hp_pins[0]); ...
triggered:
static int vtXXX_auto_create_hp_ctls(struct via_spec *spec, hda_nid_t pin) { ...
if (!pin) return 0; ...
before spec->multiout.hp_nid is set, but it may also not happen, because, regardless of support for multi-streaming, there can be support for a front panel line-out in redirected output (refer to commit 3d83e577a8206f0f3822a3840e12f76477142ba2 and ee3c35c0827de02de414d08b2ddcbb910c2263ab, if there is need to solve that problem in via_hp_build(), it should mean that spec->multiout.hp_nid is not null in such scenarios).
However, that wouldn't solve the memory failure risk, or the risk via_hp_build is not called at all because spec->hp_mux wasn't created (though, this latter risk is fixable by moving assignment to spec->multiout.hp_nid at the end of vtXXX_auto_create_hp_ctls, and using local variables before), and anyway, in doubt, I think a safe choice is to add a proper field, I've called independent_front_panel_out, to struct via_spec, initialize it to 0 and set it to 1 in via_hp_build() after via_clone_control(spec, &via_hp_mixer[0]) succeeds, so that in via_build_pcms():
info->stream[SNDRV_PCM_STREAM_PLAYBACK].substreams = 1 + spec->independent_front_panel_out;
I was using it also in patch_vtXXXX() (for a few codecs, not all) to create a "Front Mic Boost Volume" control dynamically only when the front panel exist (but that's wrong, I've modified it to check for spec->autocfg.hp_pins[0], which seems to be 0 when there's no front panel seen by the BIOS).
More code is in my last mail for this thread, before this one, I've changed since then:
front_panel_out renamed independent_front_panel_out, and vt_add_front_mic_boost() now is:
static int vt_add_front_mic_boost( struct via_spec *spec, const struct snd_kcontrol_new *fmb_ctl ){ struct snd_kcontrol_new *knew;
if( spec->autocfg.hp_pins[0] ){ /* if there is a front audio panel */ knew = via_clone_control( spec, fmb_ctl ); if( knew == NULL ) return -ENOMEM; return 1; } return 0; }
(here, it would be the same to check for spec->multiout.hp_nid being set, but only if the position of the assignment to spec->multiout.hp_nid is not moved around in vtxxx_auto_create_hp_ctls)
However there is a bug in using subdevice 1 for "Independent HP" and subdevice 0 for multi-channels
The channel count reported by snd_pcm_hw_params_get_channels_max() or snd_pcm_hw_params_test_channel() for subdevice 1 is also equal to spec->multiout.max_channels but this subdevice 1 only support stereo
I haven't understood all of the pcm layer, but I guess that issue could be hard to fix without touching it at some level, or creating a separate pcm (struct snd_pcm) to handle multi-outs and hp separately, with a careful inspection of the code needed to handle them in tandem (via_playback_pcm_open() and so on).
struct hda_pcm can hold only 2 struct hda_pcm_stream elements (one actually used for playback, the other for capture):
/* for PCM creation */ struct hda_pcm { char *name; struct hda_pcm_stream stream[2]; ...
and at least snd_hda_attach_pcm() (hda_codec.c) counts on it:
static int snd_hda_attach_pcm(struct hda_codec *codec, struct hda_pcm *pcm) { struct hda_bus *bus = codec->bus; struct hda_pcm_stream *info; int stream, err;
if (snd_BUG_ON(!pcm->name)) return -EINVAL; for (stream = 0; stream < 2; stream++) { info = &pcm->stream[stream]; if (info->substreams) { err = set_pcm_default_values(codec, info); if (err < 0) return err; } } return bus->ops.attach_pcm(bus, codec, pcm); }
Moreover, only one pcm is created for a VIA codec, to handle analog outputs, plus one if there are digital connections:
(file patch_via.c)
static int via_build_pcms(struct hda_codec *codec) { struct via_spec *spec = codec->spec; struct hda_pcm *info = spec->pcm_rec;
codec->num_pcms = 1; codec->pcm_info = info; ....
if (spec->multiout.dig_out_nid || spec->dig_in_nid) { codec->num_pcms++; info++; ...
(file hda_codec.c)
int snd_hda_codec_build_pcms(struct hda_codec *codec) { unsigned int pcm; int err; ...
for (pcm = 0; pcm < codec->num_pcms; pcm++) { struct hda_pcm *cpcm = &codec->pcm_info[pcm]; int dev; ...
if (!cpcm->pcm) { dev = get_empty_pcm_device(codec->bus, cpcm->pcm_type); if (dev < 0) continue; /* no fatal error */ cpcm->device = dev; err = snd_hda_attach_pcm(codec, cpcm); ...
snd_hda_attach_pcm() calls bus->ops.attach_pcm(), that is (in file hda_intel.c) azx_attach_pcm_stream():
static int azx_attach_pcm_stream(struct hda_bus *bus, struct hda_codec *codec, struct hda_pcm *cpcm) { struct azx *chip = bus->private_data; struct snd_pcm *pcm; struct azx_pcm *apcm; int pcm_dev = cpcm->device; int s, err; ...
err = snd_pcm_new(chip->card, cpcm->name, pcm_dev, cpcm->stream[SNDRV_PCM_STREAM_PLAYBACK].substreams, cpcm->stream[SNDRV_PCM_STREAM_CAPTURE].substreams, &pcm); ...
apcm->chip = chip; apcm->codec = codec; pcm->private_data = apcm; ...
cpcm->pcm = pcm; for (s = 0; s < 2; s++) { apcm->hinfo[s] = &cpcm->stream[s]; if (cpcm->stream[s].substreams) snd_pcm_set_ops(pcm, s, &azx_pcm_ops); } ...
again counting on two streams only:
(hda_intel.c):
struct azx_pcm { struct azx *chip; struct hda_codec *codec; struct hda_pcm_stream *hinfo[2]; };
(pcm.h):
struct snd_pcm { ... struct snd_pcm_str streams[2]; ... void *private_data; ...
struct snd_pcm_str { int stream; /* stream (direction) */ struct snd_pcm *pcm; /* -- substreams -- */ unsigned int substream_count; unsigned int substream_opened; struct snd_pcm_substream *substream; ...
struct snd_pcm_substream { struct snd_pcm *pcm; struct snd_pcm_str *pstr; void *private_data; /* copied from pcm->private_data */ ...
/* -- hardware operations -- */ struct snd_pcm_ops *ops; /* -- runtime information -- */ struct snd_pcm_runtime *runtime;
(notice above comment about field private_data, from file)
struct snd_pcm_runtime { ... /* -- hardware description -- */ struct snd_pcm_hardware hw; struct snd_pcm_hw_constraints hw_constraints; ...
struct snd_pcm_hardware { ... unsigned int channels_min; /* min channels */ unsigned int channels_max; /* max channels */ ...
(pcm.c):
int snd_pcm_new(struct snd_card *card, const char *id, int device, int playback_count, int capture_count, struct snd_pcm ** rpcm) { struct snd_pcm *pcm; int err; ...
if ((err = snd_pcm_new_stream(pcm, SNDRV_PCM_STREAM_PLAYBACK, playback_count)) < 0) { ... if ((err = snd_pcm_new_stream(pcm, SNDRV_PCM_STREAM_CAPTURE, capture_count)) < 0) { ...
int snd_pcm_new_stream(struct snd_pcm *pcm, int stream, int substream_count) { int idx, err; struct snd_pcm_str *pstr = &pcm->streams[stream]; struct snd_pcm_substream *substream, *prev; ...
pstr->stream = stream; pstr->pcm = pcm; pstr->substream_count = substream_count; ... for (idx = 0, prev = NULL; idx < substream_count; idx++) { substream = kzalloc(sizeof(*substream), GFP_KERNEL); ...
substream->pcm = pcm; substream->pstr = pstr; substream->number = idx; substream->stream = stream; ....
int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, struct file *file, struct snd_pcm_substream **rsubstream) { struct snd_pcm_str * pstr; struct snd_pcm_substream *substream; struct snd_pcm_runtime *runtime; ...
runtime = kzalloc(sizeof(*runtime), GFP_KERNEL); ...
substream->runtime = runtime; substream->private_data = pcm->private_data; ... *rsubstream = substream; return 0; }
This one called by: (pcm_native.c):
int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, struct file *file, struct snd_pcm_substream **rsubstream) { struct snd_pcm_substream *substream; int err;
err = snd_pcm_attach_substream(pcm, stream, file, &substream); ...
if ((err = substream->ops->open(substream)) < 0) goto error;
substream->hw_opened = 1; ...
*rsubstream = substream; ...
called by snd_pcm_open_file(), in turn called by snd_pcm_open(), in turn called by snd_pcm_{playback, capture}_open(), which are 'exported' as file_operations; substream->ops->open(), as set by azx_attach_pcm_stream(), is:
(hda_intel.c):
static int azx_pcm_open(struct snd_pcm_substream *substream) { struct azx_pcm *apcm = snd_pcm_substream_chip(substream); struct hda_pcm_stream *hinfo = apcm->hinfo[substream->stream]; struct azx *chip = apcm->chip; struct azx_dev *azx_dev; struct snd_pcm_runtime *runtime = substream->runtime; ...
runtime->hw.channels_min = hinfo->channels_min; runtime->hw.channels_max = hinfo->channels_max; ...
In file pcm.h:
#define snd_pcm_substream_chip(substream) ((substream)->private_data)
since substream->private_data is shared by all substreams of the same snd_pcm, in case of playback (substream->stream == SNDRV_PCM_STREAM_PLAYBACK), hinfo is spec->stream_analog_playback, and .channel_{min, max} fields hold same values for both multi-out and Independent HP subdevices (if I'm not mistaken, of course).
If my interpretation above is correct, something should change in that chain in order to have different values retrieved for different substreams of the same stream of the same pcm.
On the other hand, using separate pcms could require less invasive, yet careful, changes (affecting only patch_via.c, or one patch at a time, to test it better), but could affect any application eventually counting on analog outputs being handled by the same pcm.
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel