At Fri, 12 Nov 2010 16:46:34 +0100, David Henningsson wrote:
Since I'm a little new in HDMI land and the patch is non-trivial, I wouldn't mind some comments on this one. Also thanks to Jaroslav who cleared a few things out about the logical and physical devices which helped me figure out what I should do about it :-)
Anyway, here's the patch.
Some newer chips have more than one HDMI output, but usually not all of them are exposed as physical jacks. Removing the unused PCM devices (as indicated by BIOS in the pin config default) will reduce user confusion as they currently have to choose between several HDMI devices, some of them not working anyway.
Signed-off-by: David Henningsson david.henningsson@canonical.com
The patch looks mostly OK. But this gives a few errors and warnings via checkpatch.pl. Could you fix errors, at least?
Also some review comments:
static int hdmi_add_cvt(struct hda_codec *codec, hda_nid_t nid) {
- int i; struct hdmi_spec *spec = codec->spec;
- if (spec->num_cvts >= MAX_HDMI_CVTS) {
snd_printk(KERN_WARNING"HDMI: no space for converter %d\n", nid);
- for (i = 0; spec->pin_cvt[i] != nid; i++)
I'd write with a normal loop like for (i = 0; i < spec->num_pins; i++) This is easier to read.
if (!spec->pin[i]) {snd_printd("HDMI: Skipping node %d (no connection)\n", nid);
Better to use snd_printdd(). snd_printd() is enabled on many distros, and such an unnecessary kernel message may make user worry.
@@ -951,17 +954,33 @@ static int hdmi_parse_codec(struct hda_codec *codec)
...
case AC_WID_PIN: caps = snd_hda_param_read(codec, nid, AC_PAR_PIN_CAP); if (!(caps & (AC_PINCAP_HDMI | AC_PINCAP_DP))) continue;
config = snd_hda_codec_read(codec, nid, 0,AC_VERB_GET_CONFIG_DEFAULT, 0);if (((config & AC_DEFCFG_PORT_CONN) >>AC_DEFCFG_PORT_CONN_SHIFT) == AC_JACK_PORT_NONE)
You can use get_defcfg_connect() here.
thanks,
Takashi