* [RFC PATCH 0/3] support DP MST audio
@ 2016-09-26 8:35 libin.yang
2016-09-26 8:35 ` [RFC PATCH 1/3] ALSA: hda - codec add DP MST support for connection list libin.yang
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: libin.yang @ 2016-09-26 8:35 UTC (permalink / raw)
To: alsa-devel, tiwai; +Cc: libin.yang, mengdong.lin, Libin Yang
From: Libin Yang <libin.yang@linux.intel.com>
This patchset starts to support DP MST audio.
This patchset is based on drm-intel-nightly on
git://anongit.freedesktop.org/drm-intel
Libin Yang (3):
ALSA: hda - codec add DP MST support for connection list
ALSA: hda - add DP mst verb support
ALSA: hda - add DP mst audio support
sound/pci/hda/hda_codec.c | 129 ++++++++++++++++++-----
sound/pci/hda/hda_codec.h | 18 ++--
sound/pci/hda/hda_generic.c | 18 ++--
sound/pci/hda/hda_proc.c | 2 +-
sound/pci/hda/patch_analog.c | 18 ++--
sound/pci/hda/patch_hdmi.c | 228 +++++++++++++++++++++++++++++++----------
sound/pci/hda/patch_realtek.c | 22 ++--
sound/pci/hda/patch_sigmatel.c | 4 +-
sound/pci/hda/patch_via.c | 10 +-
9 files changed, 328 insertions(+), 121 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 1/3] ALSA: hda - codec add DP MST support for connection list
2016-09-26 8:35 [RFC PATCH 0/3] support DP MST audio libin.yang
@ 2016-09-26 8:35 ` libin.yang
2016-09-26 8:35 ` [RFC PATCH 2/3] ALSA: hda - add DP mst verb support libin.yang
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: libin.yang @ 2016-09-26 8:35 UTC (permalink / raw)
To: alsa-devel, tiwai; +Cc: libin.yang, mengdong.lin, Libin Yang
From: Libin Yang <libin.yang@linux.intel.com>
This patches adds the support of connection list for DP MST.
With this, hdmi driver in DP MST mode can easily reuse
the connection list mechanism.
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
sound/pci/hda/hda_codec.c | 52 +++++++++++++++++++++++++++---------------
sound/pci/hda/hda_codec.h | 15 ++++++------
sound/pci/hda/hda_generic.c | 18 +++++++--------
sound/pci/hda/hda_proc.c | 2 +-
sound/pci/hda/patch_analog.c | 18 ++++++++-------
sound/pci/hda/patch_hdmi.c | 8 ++++---
sound/pci/hda/patch_realtek.c | 22 +++++++++---------
sound/pci/hda/patch_sigmatel.c | 4 ++--
sound/pci/hda/patch_via.c | 10 ++++----
9 files changed, 85 insertions(+), 64 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 9913be8..32e21e8 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -110,23 +110,24 @@ struct hda_conn_list {
struct list_head list;
int len;
hda_nid_t nid;
+ int dev_id;
hda_nid_t conns[0];
};
/* look up the cached results */
static struct hda_conn_list *
-lookup_conn_list(struct hda_codec *codec, hda_nid_t nid)
+lookup_conn_list(struct hda_codec *codec, hda_nid_t nid, int dev_id)
{
struct hda_conn_list *p;
list_for_each_entry(p, &codec->conn_list, list) {
- if (p->nid == nid)
+ if ((p->nid == nid) && (p->dev_id == dev_id))
return p;
}
return NULL;
}
-static int add_conn_list(struct hda_codec *codec, hda_nid_t nid, int len,
- const hda_nid_t *list)
+static int add_conn_list(struct hda_codec *codec, hda_nid_t nid,
+ int dev_id, int len, const hda_nid_t *list)
{
struct hda_conn_list *p;
@@ -135,6 +136,7 @@ static int add_conn_list(struct hda_codec *codec, hda_nid_t nid, int len,
return -ENOMEM;
p->len = len;
p->nid = nid;
+ p->dev_id = dev_id;
memcpy(p->conns, list, len * sizeof(hda_nid_t));
list_add(&p->list, &codec->conn_list);
return 0;
@@ -150,8 +152,13 @@ static void remove_conn_list(struct hda_codec *codec)
}
}
-/* read the connection and add to the cache */
-static int read_and_add_raw_conns(struct hda_codec *codec, hda_nid_t nid)
+/*
+ * read the connection and add to the cache
+ * the caller should select the device entry by sending the
+ * corresponding verb if necessary before calling this function
+ */
+static int read_and_add_raw_conns(struct hda_codec *codec, hda_nid_t nid,
+ int dev_id)
{
hda_nid_t list[32];
hda_nid_t *result = list;
@@ -166,7 +173,8 @@ static int read_and_add_raw_conns(struct hda_codec *codec, hda_nid_t nid)
len = snd_hda_get_raw_connections(codec, nid, result, len);
}
if (len >= 0)
- len = snd_hda_override_conn_list(codec, nid, len, result);
+ len = snd_hda_override_conn_list(codec, nid, dev_id,
+ len, result);
if (result != list)
kfree(result);
return len;
@@ -176,6 +184,7 @@ static int read_and_add_raw_conns(struct hda_codec *codec, hda_nid_t nid)
* snd_hda_get_conn_list - get connection list
* @codec: the HDA codec
* @nid: NID to parse
+ * @dev_id: device entry id
* @listp: the pointer to store NID list
*
* Parses the connection list of the given widget and stores the pointer
@@ -188,7 +197,7 @@ static int read_and_add_raw_conns(struct hda_codec *codec, hda_nid_t nid)
* concurrently, protect with a mutex appropriately.
*/
int snd_hda_get_conn_list(struct hda_codec *codec, hda_nid_t nid,
- const hda_nid_t **listp)
+ int dev_id, const hda_nid_t **listp)
{
bool added = false;
@@ -197,7 +206,7 @@ int snd_hda_get_conn_list(struct hda_codec *codec, hda_nid_t nid,
const struct hda_conn_list *p;
/* if the connection-list is already cached, read it */
- p = lookup_conn_list(codec, nid);
+ p = lookup_conn_list(codec, nid, dev_id);
if (p) {
if (listp)
*listp = p->conns;
@@ -206,7 +215,7 @@ int snd_hda_get_conn_list(struct hda_codec *codec, hda_nid_t nid,
if (snd_BUG_ON(added))
return -EINVAL;
- err = read_and_add_raw_conns(codec, nid);
+ err = read_and_add_raw_conns(codec, nid, dev_id);
if (err < 0)
return err;
added = true;
@@ -218,6 +227,7 @@ EXPORT_SYMBOL_GPL(snd_hda_get_conn_list);
* snd_hda_get_connections - copy connection list
* @codec: the HDA codec
* @nid: NID to parse
+ * @dev_id: device entry id
* @conn_list: connection list array; when NULL, checks only the size
* @max_conns: max. number of connections to store
*
@@ -227,10 +237,11 @@ EXPORT_SYMBOL_GPL(snd_hda_get_conn_list);
* Returns the number of connections, or a negative error code.
*/
int snd_hda_get_connections(struct hda_codec *codec, hda_nid_t nid,
- hda_nid_t *conn_list, int max_conns)
+ int dev_id, hda_nid_t *conn_list,
+ int max_conns)
{
const hda_nid_t *list;
- int len = snd_hda_get_conn_list(codec, nid, &list);
+ int len = snd_hda_get_conn_list(codec, nid, dev_id, &list);
if (len > 0 && conn_list) {
if (len > max_conns) {
@@ -249,6 +260,7 @@ EXPORT_SYMBOL_GPL(snd_hda_get_connections);
* snd_hda_override_conn_list - add/modify the connection-list to cache
* @codec: the HDA codec
* @nid: NID to parse
+ * @dev_id: device entry id
* @len: number of connection list entries
* @list: the list of connection entries
*
@@ -257,18 +269,18 @@ EXPORT_SYMBOL_GPL(snd_hda_get_connections);
*
* Returns zero or a negative error code.
*/
-int snd_hda_override_conn_list(struct hda_codec *codec, hda_nid_t nid, int len,
- const hda_nid_t *list)
+int snd_hda_override_conn_list(struct hda_codec *codec, hda_nid_t nid,
+ int dev_id, int len, const hda_nid_t *list)
{
struct hda_conn_list *p;
- p = lookup_conn_list(codec, nid);
+ p = lookup_conn_list(codec, nid, dev_id);
if (p) {
list_del(&p->list);
kfree(p);
}
- return add_conn_list(codec, nid, len, list);
+ return add_conn_list(codec, nid, dev_id, len, list);
}
EXPORT_SYMBOL_GPL(snd_hda_override_conn_list);
@@ -277,6 +289,7 @@ EXPORT_SYMBOL_GPL(snd_hda_override_conn_list);
* @codec: the HDA codec
* @mux: NID containing the list
* @nid: NID to select
+ * @dev_id: device entry id
* @recursive: 1 when searching NID recursively, otherwise 0
*
* Parses the connection list of the widget @mux and checks whether the
@@ -284,12 +297,12 @@ EXPORT_SYMBOL_GPL(snd_hda_override_conn_list);
* Otherwise it returns -1.
*/
int snd_hda_get_conn_index(struct hda_codec *codec, hda_nid_t mux,
- hda_nid_t nid, int recursive)
+ hda_nid_t nid, int dev_id, int recursive)
{
const hda_nid_t *conn;
int i, nums;
- nums = snd_hda_get_conn_list(codec, mux, &conn);
+ nums = snd_hda_get_conn_list(codec, mux, dev_id, &conn);
for (i = 0; i < nums; i++)
if (conn[i] == nid)
return i;
@@ -304,7 +317,8 @@ int snd_hda_get_conn_index(struct hda_codec *codec, hda_nid_t mux,
unsigned int type = get_wcaps_type(get_wcaps(codec, conn[i]));
if (type == AC_WID_PIN || type == AC_WID_AUD_OUT)
continue;
- if (snd_hda_get_conn_index(codec, conn[i], nid, recursive) >= 0)
+ if (snd_hda_get_conn_index(codec, conn[i], nid, dev_id,
+ recursive) >= 0)
return i;
}
return -1;
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 373fcad..4852c1a 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -329,11 +329,12 @@ snd_hda_codec_write(struct hda_codec *codec, hda_nid_t nid, int flags,
#define snd_hda_get_sub_nodes(codec, nid, start_nid) \
snd_hdac_get_sub_nodes(&(codec)->core, nid, start_nid)
int snd_hda_get_connections(struct hda_codec *codec, hda_nid_t nid,
- hda_nid_t *conn_list, int max_conns);
+ int dev_id, hda_nid_t *conn_list,
+ int max_conns);
static inline int
-snd_hda_get_num_conns(struct hda_codec *codec, hda_nid_t nid)
+snd_hda_get_num_conns(struct hda_codec *codec, hda_nid_t nid, int dev_id)
{
- return snd_hda_get_connections(codec, nid, NULL, 0);
+ return snd_hda_get_connections(codec, nid, dev_id, NULL, 0);
}
#define snd_hda_get_raw_connections(codec, nid, list, max_conns) \
@@ -341,12 +342,12 @@ snd_hda_get_num_conns(struct hda_codec *codec, hda_nid_t nid)
#define snd_hda_get_num_raw_conns(codec, nid) \
snd_hdac_get_connections(&(codec)->core, nid, NULL, 0);
-int snd_hda_get_conn_list(struct hda_codec *codec, hda_nid_t nid,
+int snd_hda_get_conn_list(struct hda_codec *codec, hda_nid_t nid, int dev_id,
const hda_nid_t **listp);
-int snd_hda_override_conn_list(struct hda_codec *codec, hda_nid_t nid, int nums,
- const hda_nid_t *list);
+int snd_hda_override_conn_list(struct hda_codec *codec, hda_nid_t nid,
+ int dev_id, int nums, const hda_nid_t *list);
int snd_hda_get_conn_index(struct hda_codec *codec, hda_nid_t mux,
- hda_nid_t nid, int recursive);
+ hda_nid_t nid, int dev_id, int recursive);
int snd_hda_get_devices(struct hda_codec *codec, hda_nid_t nid,
u8 *dev_list, int max_devices);
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index e7c8f4f..9a6e67b 100644
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -338,7 +338,7 @@ static bool is_reachable_path(struct hda_codec *codec,
{
if (!from_nid || !to_nid)
return false;
- return snd_hda_get_conn_index(codec, to_nid, from_nid, true) >= 0;
+ return snd_hda_get_conn_index(codec, to_nid, from_nid, 0, true) >= 0;
}
/* nid, dir and idx */
@@ -397,7 +397,7 @@ static bool __parse_nid_path(struct hda_codec *codec,
else if (to_nid == (hda_nid_t)(-anchor_nid))
return false; /* hit the exclusive nid */
- nums = snd_hda_get_conn_list(codec, to_nid, &conn);
+ nums = snd_hda_get_conn_list(codec, to_nid, 0, &conn);
for (i = 0; i < nums; i++) {
if (conn[i] != from_nid) {
/* special case: when from_nid is 0,
@@ -696,9 +696,9 @@ static bool is_stereo_amps(struct hda_codec *codec, hda_nid_t nid, int dir)
return true;
if (dir != HDA_INPUT || get_wcaps_type(wcaps) != AC_WID_AUD_MIX)
return false;
- if (snd_hda_get_num_conns(codec, nid) != 1)
+ if (snd_hda_get_num_conns(codec, nid, 0) != 1)
return false;
- if (snd_hda_get_connections(codec, nid, &conn, 1) < 0)
+ if (snd_hda_get_connections(codec, nid, 0, &conn, 1) < 0)
return false;
return !!(get_wcaps(codec, conn) & AC_WCAP_STEREO);
}
@@ -791,7 +791,7 @@ static void activate_amp_in(struct hda_codec *codec, struct nid_path *path,
int type;
hda_nid_t nid = path->path[i];
- nums = snd_hda_get_conn_list(codec, nid, &conn);
+ nums = snd_hda_get_conn_list(codec, nid, 0, &conn);
type = get_wcaps_type(get_wcaps(codec, nid));
if (type == AC_WID_PIN ||
(type == AC_WID_AUD_IN && codec->single_adc_amp)) {
@@ -1060,7 +1060,7 @@ static int add_sw_ctl(struct hda_codec *codec, const char *pfx, int cidx,
val = amp_val_replace_channels(val, chs);
if (get_amp_direction_(val) == HDA_INPUT) {
hda_nid_t nid = get_amp_nid_(val);
- int nums = snd_hda_get_num_conns(codec, nid);
+ int nums = snd_hda_get_num_conns(codec, nid, 0);
if (nums > 1) {
type = HDA_CTL_BIND_MUTE;
val |= nums << 19;
@@ -3002,7 +3002,7 @@ static bool look_for_mix_leaf_ctls(struct hda_codec *codec, hda_nid_t mix_nid,
const hda_nid_t *list;
hda_nid_t nid;
- idx = snd_hda_get_conn_index(codec, mix_nid, pin, true);
+ idx = snd_hda_get_conn_index(codec, mix_nid, pin, 0, true);
if (idx < 0)
return false;
@@ -3015,7 +3015,7 @@ static bool look_for_mix_leaf_ctls(struct hda_codec *codec, hda_nid_t mix_nid,
return true;
/* check leaf node */
- num_conns = snd_hda_get_conn_list(codec, mix_nid, &list);
+ num_conns = snd_hda_get_conn_list(codec, mix_nid, 0, &list);
if (num_conns < idx)
return false;
nid = list[idx];
@@ -4760,7 +4760,7 @@ static void mute_all_mixer_nid(struct hda_codec *codec, hda_nid_t mix)
const hda_nid_t *conn;
bool has_amp;
- nums = snd_hda_get_conn_list(codec, mix, &conn);
+ nums = snd_hda_get_conn_list(codec, mix, 0, &conn);
has_amp = nid_has_mute(codec, mix, HDA_INPUT);
for (i = 0; i < nums; i++) {
if (has_amp)
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c
index 033aa84..c747183 100644
--- a/sound/pci/hda/hda_proc.c
+++ b/sound/pci/hda/hda_proc.c
@@ -636,7 +636,7 @@ static void print_conn_list(struct snd_info_buffer *buffer,
}
/* Get Cache connections info */
- cache_len = snd_hda_get_conn_list(codec, nid, &list);
+ cache_len = snd_hda_get_conn_list(codec, nid, 0, &list);
if (cache_len >= 0 && (cache_len != conn_len ||
memcmp(list, conn, conn_len) != 0)) {
snd_iprintf(buffer, " In-driver Connection: %d\n", cache_len);
diff --git a/sound/pci/hda/patch_analog.c b/sound/pci/hda/patch_analog.c
index e0fb8c6..6f007ff 100644
--- a/sound/pci/hda/patch_analog.c
+++ b/sound/pci/hda/patch_analog.c
@@ -466,7 +466,7 @@ static int ad1983_auto_smux_enum_info(struct snd_kcontrol *kcontrol,
static const char * const texts2[] = { "PCM", "ADC" };
static const char * const texts3[] = { "PCM", "ADC1", "ADC2" };
hda_nid_t dig_out = spec->gen.multiout.dig_out_nid;
- int num_conns = snd_hda_get_num_conns(codec, dig_out);
+ int num_conns = snd_hda_get_num_conns(codec, dig_out, 0);
if (num_conns == 2)
return snd_hda_enum_helper_info(kcontrol, uinfo, 2, texts2);
@@ -493,7 +493,7 @@ static int ad1983_auto_smux_enum_put(struct snd_kcontrol *kcontrol,
struct ad198x_spec *spec = codec->spec;
unsigned int val = ucontrol->value.enumerated.item[0];
hda_nid_t dig_out = spec->gen.multiout.dig_out_nid;
- int num_conns = snd_hda_get_num_conns(codec, dig_out);
+ int num_conns = snd_hda_get_num_conns(codec, dig_out, 0);
if (val >= num_conns)
return -EINVAL;
@@ -521,7 +521,7 @@ static int ad1983_add_spdif_mux_ctl(struct hda_codec *codec)
if (!dig_out)
return 0;
- num_conns = snd_hda_get_num_conns(codec, dig_out);
+ num_conns = snd_hda_get_num_conns(codec, dig_out, 0);
if (num_conns != 2 && num_conns != 3)
return 0;
if (!snd_hda_gen_add_kctl(&spec->gen, NULL, &ad1983_auto_smux_mixer))
@@ -546,8 +546,10 @@ static int patch_ad1983(struct hda_codec *codec)
set_beep_amp(spec, 0x10, 0, HDA_OUTPUT);
/* limit the loopback routes not to confuse the parser */
- snd_hda_override_conn_list(codec, 0x0c, ARRAY_SIZE(conn_0c), conn_0c);
- snd_hda_override_conn_list(codec, 0x0d, ARRAY_SIZE(conn_0d), conn_0d);
+ snd_hda_override_conn_list(codec, 0x0c, 0, ARRAY_SIZE(conn_0c),
+ conn_0c);
+ snd_hda_override_conn_list(codec, 0x0d, 0, ARRAY_SIZE(conn_0d),
+ conn_0d);
err = ad198x_parse_auto_config(codec, false);
if (err < 0)
@@ -745,7 +747,7 @@ static int ad1988_auto_smux_enum_info(struct snd_kcontrol *kcontrol,
static const char * const texts[] = {
"PCM", "ADC1", "ADC2", "ADC3",
};
- int num_conns = snd_hda_get_num_conns(codec, 0x0b) + 1;
+ int num_conns = snd_hda_get_num_conns(codec, 0x0b, 0) + 1;
if (num_conns > 4)
num_conns = 4;
return snd_hda_enum_helper_info(kcontrol, uinfo, num_conns, texts);
@@ -768,7 +770,7 @@ static int ad1988_auto_smux_enum_put(struct snd_kcontrol *kcontrol,
struct ad198x_spec *spec = codec->spec;
unsigned int val = ucontrol->value.enumerated.item[0];
struct nid_path *path;
- int num_conns = snd_hda_get_num_conns(codec, 0x0b) + 1;
+ int num_conns = snd_hda_get_num_conns(codec, 0x0b, 0) + 1;
if (val >= num_conns)
return -EINVAL;
@@ -856,7 +858,7 @@ static int ad1988_add_spdif_mux_ctl(struct hda_codec *codec)
get_wcaps_type(get_wcaps(codec, 0x1d)) != AC_WID_AUD_MIX)
return 0;
- num_conns = snd_hda_get_num_conns(codec, 0x0b) + 1;
+ num_conns = snd_hda_get_num_conns(codec, 0x0b, 0) + 1;
if (num_conns != 3 && num_conns != 4)
return 0;
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index cf9bc042..8a20cf4 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1198,7 +1198,7 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
return -EINVAL;
}
- per_pin->num_mux_nids = snd_hda_get_connections(codec, pin_nid,
+ per_pin->num_mux_nids = snd_hda_get_connections(codec, pin_nid, 0,
per_pin->mux_nids,
HDA_MAX_CONNECTIONS);
@@ -2224,14 +2224,16 @@ static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
hda_nid_t conns[4];
int nconns;
- nconns = snd_hda_get_connections(codec, nid, conns, ARRAY_SIZE(conns));
+ nconns = snd_hda_get_connections(codec, nid, 0, conns,
+ ARRAY_SIZE(conns));
if (nconns == spec->num_cvts &&
!memcmp(conns, spec->cvt_nids, spec->num_cvts * sizeof(hda_nid_t)))
return;
/* override pins connection list */
codec_dbg(codec, "hdmi: haswell: override pin connection 0x%x\n", nid);
- snd_hda_override_conn_list(codec, nid, spec->num_cvts, spec->cvt_nids);
+ snd_hda_override_conn_list(codec, nid, 0, spec->num_cvts,
+ spec->cvt_nids);
}
#define INTEL_VENDOR_NID 0x08
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 1fe8750..51240d6 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -1861,17 +1861,17 @@ static void alc889_fixup_dac_route(struct hda_codec *codec,
/* fake the connections during parsing the tree */
hda_nid_t conn1[2] = { 0x0c, 0x0d };
hda_nid_t conn2[2] = { 0x0e, 0x0f };
- snd_hda_override_conn_list(codec, 0x14, 2, conn1);
- snd_hda_override_conn_list(codec, 0x15, 2, conn1);
- snd_hda_override_conn_list(codec, 0x18, 2, conn2);
- snd_hda_override_conn_list(codec, 0x1a, 2, conn2);
+ snd_hda_override_conn_list(codec, 0x14, 0, 2, conn1);
+ snd_hda_override_conn_list(codec, 0x15, 0, 2, conn1);
+ snd_hda_override_conn_list(codec, 0x18, 0, 2, conn2);
+ snd_hda_override_conn_list(codec, 0x1a, 0, 2, conn2);
} else if (action == HDA_FIXUP_ACT_PROBE) {
/* restore the connections */
hda_nid_t conn[5] = { 0x0c, 0x0d, 0x0e, 0x0f, 0x26 };
- snd_hda_override_conn_list(codec, 0x14, 5, conn);
- snd_hda_override_conn_list(codec, 0x15, 5, conn);
- snd_hda_override_conn_list(codec, 0x18, 5, conn);
- snd_hda_override_conn_list(codec, 0x1a, 5, conn);
+ snd_hda_override_conn_list(codec, 0x14, 0, 5, conn);
+ snd_hda_override_conn_list(codec, 0x15, 0, 5, conn);
+ snd_hda_override_conn_list(codec, 0x18, 0, 5, conn);
+ snd_hda_override_conn_list(codec, 0x1a, 0, 5, conn);
}
}
@@ -4684,8 +4684,8 @@ static void alc290_fixup_mono_speakers(struct hda_codec *codec,
make sure 0x14 (front speaker) and 0x15 (headphones) use the
stereo DAC, while leaving 0x17 (bass speaker) for node 0x03. */
hda_nid_t conn1[2] = { 0x0c };
- snd_hda_override_conn_list(codec, 0x14, 1, conn1);
- snd_hda_override_conn_list(codec, 0x15, 1, conn1);
+ snd_hda_override_conn_list(codec, 0x14, 0, 1, conn1);
+ snd_hda_override_conn_list(codec, 0x15, 0, 1, conn1);
}
}
@@ -4701,7 +4701,7 @@ static void alc298_fixup_speaker_volume(struct hda_codec *codec,
speaker's volume now. */
hda_nid_t conn1[1] = { 0x0c };
- snd_hda_override_conn_list(codec, 0x17, 1, conn1);
+ snd_hda_override_conn_list(codec, 0x17, 0, 1, conn1);
}
}
diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
index 37b70f8..5bff793 100644
--- a/sound/pci/hda/patch_sigmatel.c
+++ b/sound/pci/hda/patch_sigmatel.c
@@ -984,7 +984,7 @@ static int stac_create_spdif_mux_ctls(struct hda_codec *codec)
if (cfg->dig_outs < 1)
return 0;
- num_cons = snd_hda_get_num_conns(codec, cfg->dig_out_pins[0]);
+ num_cons = snd_hda_get_num_conns(codec, cfg->dig_out_pins[0], 0);
if (num_cons <= 1)
return 0;
@@ -4543,7 +4543,7 @@ static int patch_stac92hd73xx(struct hda_codec *codec)
spec->gen.mixer_nid = 0x1d;
spec->have_spdif_mux = 1;
- num_dacs = snd_hda_get_num_conns(codec, 0x0a) - 1;
+ num_dacs = snd_hda_get_num_conns(codec, 0x0a, 0) - 1;
if (num_dacs < 3 || num_dacs > 5) {
codec_warn(codec,
"Could not determine number of channels defaulting to DAC count\n");
diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c
index fc30d1e..14d9010 100644
--- a/sound/pci/hda/patch_via.c
+++ b/sound/pci/hda/patch_via.c
@@ -867,7 +867,7 @@ static int add_secret_dac_path(struct hda_codec *codec)
if (!spec->gen.mixer_nid)
return 0;
- nums = snd_hda_get_connections(codec, spec->gen.mixer_nid, conn,
+ nums = snd_hda_get_connections(codec, spec->gen.mixer_nid, 0, conn,
ARRAY_SIZE(conn) - 1);
for (i = 0; i < nums; i++) {
if (get_wcaps_type(get_wcaps(codec, conn[i])) == AC_WID_AUD_OUT)
@@ -882,7 +882,7 @@ static int add_secret_dac_path(struct hda_codec *codec)
conn[nums++] = nid;
return snd_hda_override_conn_list(codec,
spec->gen.mixer_nid,
- nums, conn);
+ 0, nums, conn);
}
}
return 0;
@@ -1082,8 +1082,10 @@ static void fix_vt1802_connections(struct hda_codec *codec)
static hda_nid_t conn_24[] = { 0x14, 0x1c };
static hda_nid_t conn_33[] = { 0x1c };
- snd_hda_override_conn_list(codec, 0x24, ARRAY_SIZE(conn_24), conn_24);
- snd_hda_override_conn_list(codec, 0x33, ARRAY_SIZE(conn_33), conn_33);
+ snd_hda_override_conn_list(codec, 0x24, 0, ARRAY_SIZE(conn_24),
+ conn_24);
+ snd_hda_override_conn_list(codec, 0x33, 0, ARRAY_SIZE(conn_33),
+ conn_33);
}
/* patch for vt2002P */
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC PATCH 2/3] ALSA: hda - add DP mst verb support
2016-09-26 8:35 [RFC PATCH 0/3] support DP MST audio libin.yang
2016-09-26 8:35 ` [RFC PATCH 1/3] ALSA: hda - codec add DP MST support for connection list libin.yang
@ 2016-09-26 8:35 ` libin.yang
2016-09-26 8:35 ` [RFC PATCH 3/3] ALSA: hda - add DP mst audio support libin.yang
2016-09-26 15:10 ` [RFC PATCH 0/3] support DP MST audio Takashi Iwai
3 siblings, 0 replies; 14+ messages in thread
From: libin.yang @ 2016-09-26 8:35 UTC (permalink / raw)
To: alsa-devel, tiwai; +Cc: libin.yang, mengdong.lin, Libin Yang
From: Libin Yang <libin.yang@linux.intel.com>
Add snd_hda_get_dev_select() and snd_hda_set_dev_select() functions
for DP MST audio support.
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
sound/pci/hda/hda_codec.c | 73 ++++++++++++++++++++++++++++++++++++++++++++---
sound/pci/hda/hda_codec.h | 3 ++
2 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 32e21e8..57c25b7 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -325,9 +325,16 @@ int snd_hda_get_conn_index(struct hda_codec *codec, hda_nid_t mux,
}
EXPORT_SYMBOL_GPL(snd_hda_get_conn_index);
-
-/* return DEVLIST_LEN parameter of the given widget */
-static unsigned int get_num_devices(struct hda_codec *codec, hda_nid_t nid)
+/**
+ * snd_hda_get_num_devices - get DEVLIST_LEN parameter of the given widget
+ * @codec: the HDA codec
+ * @nid: NID of the pin to parse
+ *
+ * Get the device entry number on the given widget.
+ * This is a feature of DP MST audio. Each pin can
+ * have several device entries in it.
+ */
+unsigned int snd_hda_get_num_devices(struct hda_codec *codec, hda_nid_t nid)
{
unsigned int wcaps = get_wcaps(codec, nid);
unsigned int parm;
@@ -341,6 +348,7 @@ static unsigned int get_num_devices(struct hda_codec *codec, hda_nid_t nid)
parm = 0;
return parm & AC_DEV_LIST_LEN_MASK;
}
+EXPORT_SYMBOL_GPL(snd_hda_get_num_devices);
/**
* snd_hda_get_devices - copy device list without cache
@@ -358,7 +366,7 @@ int snd_hda_get_devices(struct hda_codec *codec, hda_nid_t nid,
unsigned int parm;
int i, dev_len, devices;
- parm = get_num_devices(codec, nid);
+ parm = snd_hda_get_num_devices(codec, nid);
if (!parm) /* not multi-stream capable */
return 0;
@@ -382,6 +390,63 @@ int snd_hda_get_devices(struct hda_codec *codec, hda_nid_t nid,
return devices;
}
+/**
+ * snd_hda_get_dev_select - get device entry select on the pin
+ * @codec: the HDA codec
+ * @nid: NID of the pin to get device entry select
+ *
+ * Get the devcie entry select on the pin. Return the device entry
+ * id selected on the pin. Return 0 means the first device entry
+ * is selected or MST is not supported.
+ */
+int snd_hda_get_dev_select(struct hda_codec *codec, hda_nid_t nid)
+{
+ /* not support dp_mst will always return 0, using first dev_entry */
+ if (!codec->dp_mst)
+ return 0;
+
+ return snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_DEVICE_SEL, 0);
+}
+EXPORT_SYMBOL_GPL(snd_hda_get_dev_select);
+
+/**
+ * snd_hda_set_dev_select - set device entry select on the pin
+ * @codec: the HDA codec
+ * @nid: NID of the pin to set device entry select
+ * @dev_id: device entry id to be set
+ *
+ * Set the device entry select on the pin nid.
+ */
+int snd_hda_set_dev_select(struct hda_codec *codec, hda_nid_t nid, int dev_id)
+{
+ int ret, num_devices;
+
+ /* not support dp_mst will always return 0, using first dev_entry */
+ if (!codec->dp_mst)
+ return 0;
+
+ /* AC_PAR_DEVLIST_LEN is 0 based. */
+ num_devices = snd_hda_get_num_devices(codec, nid) + 1;
+ /* If Device List Length is 0 (num_device = 1),
+ * the pin is not multi stream capable.
+ * Do nothing in this case.
+ */
+ if (num_devices == 1)
+ return 0;
+
+ /* Behavior of setting index being equal to or greater than
+ * Device List Length is not predictable
+ */
+ if (num_devices <= dev_id)
+ return -EINVAL;
+
+ ret = snd_hda_codec_write(codec, nid, 0,
+ AC_VERB_SET_DEVICE_SEL, dev_id);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(snd_hda_set_dev_select);
+
/*
* read widget caps for each widget and store in cache
*/
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 4852c1a..57d1659 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -348,8 +348,11 @@ int snd_hda_override_conn_list(struct hda_codec *codec, hda_nid_t nid,
int dev_id, int nums, const hda_nid_t *list);
int snd_hda_get_conn_index(struct hda_codec *codec, hda_nid_t mux,
hda_nid_t nid, int dev_id, int recursive);
+unsigned int snd_hda_get_num_devices(struct hda_codec *codec, hda_nid_t nid);
int snd_hda_get_devices(struct hda_codec *codec, hda_nid_t nid,
u8 *dev_list, int max_devices);
+int snd_hda_get_dev_select(struct hda_codec *codec, hda_nid_t nid);
+int snd_hda_set_dev_select(struct hda_codec *codec, hda_nid_t nid, int dev_id);
struct hda_verb {
hda_nid_t nid;
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC PATCH 3/3] ALSA: hda - add DP mst audio support
2016-09-26 8:35 [RFC PATCH 0/3] support DP MST audio libin.yang
2016-09-26 8:35 ` [RFC PATCH 1/3] ALSA: hda - codec add DP MST support for connection list libin.yang
2016-09-26 8:35 ` [RFC PATCH 2/3] ALSA: hda - add DP mst verb support libin.yang
@ 2016-09-26 8:35 ` libin.yang
2016-09-26 15:10 ` [RFC PATCH 0/3] support DP MST audio Takashi Iwai
3 siblings, 0 replies; 14+ messages in thread
From: libin.yang @ 2016-09-26 8:35 UTC (permalink / raw)
To: alsa-devel, tiwai; +Cc: libin.yang, mengdong.lin, Libin Yang
From: Libin Yang <libin.yang@linux.intel.com>
This patch adds the DP mst audio support.
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
sound/pci/hda/hda_codec.c | 4 +
sound/pci/hda/patch_hdmi.c | 226 ++++++++++++++++++++++++++++++++++-----------
2 files changed, 174 insertions(+), 56 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 57c25b7..d624d77 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -482,6 +482,10 @@ static int read_pin_defaults(struct hda_codec *codec)
pin->nid = nid;
pin->cfg = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_CONFIG_DEFAULT, 0);
+ /*
+ * all device entries are the same widget control so far
+ * fixme: if any codec is different, need fix here
+ */
pin->ctrl = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_PIN_WIDGET_CONTROL,
0);
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 8a20cf4..e584c5a 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -76,6 +76,7 @@ struct hdmi_spec_per_cvt {
struct hdmi_spec_per_pin {
hda_nid_t pin_nid;
+ int dev_id;
/* pin idx, different device entries on the same pin use the same idx */
int pin_nid_idx;
int num_mux_nids;
@@ -130,7 +131,23 @@ struct hdmi_spec {
struct snd_array cvts; /* struct hdmi_spec_per_cvt */
hda_nid_t cvt_nids[4]; /* only for haswell fix */
+ /*
+ * num_pins is the number of virtual pins
+ * for example, there are 3 pins, and each pin
+ * has 4 device entries, then the num_pins is 12
+ */
int num_pins;
+ /*
+ * num_nids is the number of real pins
+ * In the above example, num_nids is 3
+ */
+ int num_nids;
+ /*
+ * dev_num is the number of device entries
+ * on each pin.
+ * In the above example, dev_num is 4
+ */
+ int dev_num;
struct snd_array pins; /* struct hdmi_spec_per_pin */
struct hdmi_pcm pcm_rec[16];
struct mutex pcm_lock;
@@ -217,14 +234,26 @@ union audio_infoframe {
/* obtain hda_pcm object assigned to idx */
#define get_pcm_rec(spec, idx) (get_hdmi_pcm(spec, idx)->pcm)
-static int pin_nid_to_pin_index(struct hda_codec *codec, hda_nid_t pin_nid)
+static int pin_id_to_pin_index(struct hda_codec *codec,
+ hda_nid_t pin_nid, int dev_id)
{
struct hdmi_spec *spec = codec->spec;
int pin_idx;
+ struct hdmi_spec_per_pin *per_pin;
- for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++)
- if (get_pin(spec, pin_idx)->pin_nid == pin_nid)
+ /*
+ * (dev_id == -1) means it is NON-MST pin
+ * return the first virtual pin on this port
+ */
+ if (dev_id == -1)
+ dev_id = 0;
+
+ for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
+ per_pin = get_pin(spec, pin_idx);
+ if ((per_pin->pin_nid == pin_nid) &&
+ (per_pin->dev_id == dev_id))
return pin_idx;
+ }
codec_warn(codec, "HDMI: pin nid %d not registered\n", pin_nid);
return -EINVAL;
@@ -724,10 +753,11 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
-static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid)
+static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid,
+ int dev_id)
{
struct hdmi_spec *spec = codec->spec;
- int pin_idx = pin_nid_to_pin_index(codec, nid);
+ int pin_idx = pin_id_to_pin_index(codec, nid, dev_id);
if (pin_idx < 0)
return;
@@ -738,7 +768,8 @@ static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid)
static void jack_callback(struct hda_codec *codec,
struct hda_jack_callback *jack)
{
- check_presence_and_report(codec, jack->nid);
+ /* hda_jack don't support DP MST */
+ check_presence_and_report(codec, jack->nid, 0);
}
static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
@@ -747,6 +778,12 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
struct hda_jack_tbl *jack;
int dev_entry = (res & AC_UNSOL_RES_DE) >> AC_UNSOL_RES_DE_SHIFT;
+ /*
+ * assume DP MST uses dyn_pcm_assign and acomp and
+ * never comes here
+ * if DP MST supports unsol event, below code need
+ * consider dev_entry
+ */
jack = snd_hda_jack_tbl_get_from_tag(codec, tag);
if (!jack)
return;
@@ -757,7 +794,8 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
codec->addr, jack->nid, dev_entry, !!(res & AC_UNSOL_RES_IA),
!!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV));
- check_presence_and_report(codec, jack->nid);
+ /* hda_jack don't support DP MST */
+ check_presence_and_report(codec, jack->nid, 0);
}
static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
@@ -970,28 +1008,55 @@ static int intel_cvt_id_to_mux_idx(struct hdmi_spec *spec,
* by any other pins.
*/
static void intel_not_share_assigned_cvt(struct hda_codec *codec,
- hda_nid_t pin_nid, int mux_idx)
+ hda_nid_t pin_nid,
+ int dev_id, int mux_idx)
{
struct hdmi_spec *spec = codec->spec;
hda_nid_t nid;
int cvt_idx, curr;
struct hdmi_spec_per_cvt *per_cvt;
+ struct hdmi_spec_per_pin *per_pin;
+ int pin_idx;
/* configure all pins, including "no physical connection" ones */
- for_each_hda_codec_node(nid, codec) {
- unsigned int wid_caps = get_wcaps(codec, nid);
- unsigned int wid_type = get_wcaps_type(wid_caps);
+ for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
+ int dev_id_saved;
+ int dev_num;
+
+ per_pin = get_pin(spec, pin_idx);
+ /*
+ * pin not connected to monitor
+ * no need to operate on it
+ */
+ if (!per_pin->pcm)
+ continue;
- if (wid_type != AC_WID_PIN)
+ if ((per_pin->pin_nid == pin_nid) &&
+ (per_pin->dev_id == dev_id))
continue;
- if (nid == pin_nid)
+ /*
+ * if per_pin->dev_id >= dev_num,
+ * snd_hda_get_dev_select() will fail,
+ * and the following operation is unpredictable.
+ * So skip this situation.
+ */
+ dev_num = snd_hda_get_num_devices(codec, per_pin->pin_nid) + 1;
+ if (per_pin->dev_id >= dev_num)
continue;
+ nid = per_pin->pin_nid;
+
+ /* save the dev id for each pin, and restore it when return */
+ dev_id_saved = snd_hda_get_dev_select(codec, nid);
+ snd_hda_set_dev_select(codec, nid, per_pin->dev_id);
curr = snd_hda_codec_read(codec, nid, 0,
AC_VERB_GET_CONNECT_SEL, 0);
- if (curr != mux_idx)
+ if (curr != mux_idx) {
+ snd_hda_set_dev_select(codec, nid, dev_id_saved);
continue;
+ }
+
/* choose an unassigned converter. The conveters in the
* connection list are in the same order as in the codec.
@@ -1008,12 +1073,13 @@ static void intel_not_share_assigned_cvt(struct hda_codec *codec,
break;
}
}
+ snd_hda_set_dev_select(codec, nid, dev_id_saved);
}
}
/* A wrapper of intel_not_share_asigned_cvt() */
static void intel_not_share_assigned_cvt_nid(struct hda_codec *codec,
- hda_nid_t pin_nid, hda_nid_t cvt_nid)
+ hda_nid_t pin_nid, int dev_id, hda_nid_t cvt_nid)
{
int mux_idx;
struct hdmi_spec *spec = codec->spec;
@@ -1025,7 +1091,7 @@ static void intel_not_share_assigned_cvt_nid(struct hda_codec *codec,
*/
mux_idx = intel_cvt_id_to_mux_idx(spec, cvt_nid);
if (mux_idx >= 0)
- intel_not_share_assigned_cvt(codec, pin_nid, mux_idx);
+ intel_not_share_assigned_cvt(codec, pin_nid, dev_id, mux_idx);
}
/* skeleton caller of pin_cvt_fixup ops */
@@ -1140,6 +1206,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
per_pin->cvt_nid = per_cvt->cvt_nid;
hinfo->nid = per_cvt->cvt_nid;
+ snd_hda_set_dev_select(codec, per_pin->pin_nid, per_pin->dev_id);
snd_hda_codec_write_cache(codec, per_pin->pin_nid, 0,
AC_VERB_SET_CONNECT_SEL,
per_pin->mux_idx);
@@ -1215,13 +1282,13 @@ static int hdmi_find_pcm_slot(struct hdmi_spec *spec,
return per_pin->pin_nid_idx;
/* have a second try; check the "reserved area" over num_pins */
- for (i = spec->num_pins; i < spec->pcm_used; i++) {
+ for (i = spec->num_nids; i < spec->pcm_used; i++) {
if (!test_bit(i, &spec->pcm_bitmap))
return i;
}
/* the last try; check the empty slots in pins */
- for (i = 0; i < spec->num_pins; i++) {
+ for (i = 0; i < spec->num_nids; i++) {
if (!test_bit(i, &spec->pcm_bitmap))
return i;
}
@@ -1296,10 +1363,13 @@ static void hdmi_pcm_setup_pin(struct hdmi_spec *spec,
per_pin->cvt_nid = hinfo->nid;
mux_idx = hdmi_get_pin_cvt_mux(spec, per_pin, hinfo->nid);
- if (mux_idx < per_pin->num_mux_nids)
+ if (mux_idx < per_pin->num_mux_nids) {
+ snd_hda_set_dev_select(codec, per_pin->pin_nid,
+ per_pin->dev_id);
snd_hda_codec_write_cache(codec, per_pin->pin_nid, 0,
AC_VERB_SET_CONNECT_SEL,
mux_idx);
+ }
snd_hda_spdif_ctls_assign(codec, per_pin->pcm_idx, hinfo->nid);
non_pcm = check_non_pcm_per_cvt(codec, hinfo->nid);
@@ -1467,6 +1537,7 @@ static struct snd_jack *pin_idx_to_jack(struct hda_codec *codec,
if (per_pin->pcm_idx >= 0 && spec->dyn_pcm_assign)
jack = spec->pcm_rec[per_pin->pcm_idx].jack;
else if (!spec->dyn_pcm_assign) {
+ //jack tbl not support mst
jack_tbl = snd_hda_jack_tbl_get(codec, per_pin->pin_nid);
if (jack_tbl)
jack = jack_tbl->jack;
@@ -1485,9 +1556,9 @@ static void sync_eld_via_acomp(struct hda_codec *codec,
mutex_lock(&per_pin->lock);
eld->monitor_present = false;
- size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid, -1,
- &eld->monitor_present, eld->eld_buffer,
- ELD_MAX_SIZE);
+ size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid,
+ per_pin->dev_id, &eld->monitor_present,
+ eld->eld_buffer, ELD_MAX_SIZE);
if (size > 0) {
size = min(size, ELD_MAX_SIZE);
if (snd_hdmi_parse_eld(codec, &eld->info,
@@ -1556,7 +1627,7 @@ static void hdmi_repoll_eld(struct work_struct *work)
}
static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
- hda_nid_t nid);
+ hda_nid_t nid, int dev_id);
static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
{
@@ -1565,38 +1636,64 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
int pin_idx;
struct hdmi_spec_per_pin *per_pin;
int err;
+ int dev_num, i;
caps = snd_hda_query_pin_caps(codec, pin_nid);
if (!(caps & (AC_PINCAP_HDMI | AC_PINCAP_DP)))
return 0;
+ /*
+ * For DP MST audio, Configuration Default is the same for
+ * all device entries on the same pin
+ */
config = snd_hda_codec_get_pincfg(codec, pin_nid);
if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
return 0;
- if (is_haswell_plus(codec))
- intel_haswell_fixup_connect_list(codec, pin_nid);
-
- pin_idx = spec->num_pins;
- per_pin = snd_array_new(&spec->pins);
- if (!per_pin)
- return -ENOMEM;
-
- per_pin->pin_nid = pin_nid;
- per_pin->non_pcm = false;
- if (spec->dyn_pcm_assign)
- per_pin->pcm_idx = -1;
- else {
- per_pin->pcm = get_hdmi_pcm(spec, pin_idx);
- per_pin->pcm_idx = pin_idx;
+ if (is_haswell_plus(codec)) {
+ dev_num = 3;
+ spec->dev_num = 3;
+ } else if (spec->dyn_pcm_assign && codec->dp_mst) {
+ dev_num = snd_hda_get_num_devices(codec, pin_nid) + 1;
+ /*
+ * spec->dev_num is the maxinum number of device entries
+ * among all the pins
+ */
+ spec->dev_num = (spec->dev_num > dev_num) ?
+ spec->dev_num : dev_num;
+ } else {
+ dev_num = 1;
+ spec->dev_num = 1;
}
- per_pin->pin_nid_idx = pin_idx;
- err = hdmi_read_pin_conn(codec, pin_idx);
- if (err < 0)
- return err;
+ for (i = 0; i < dev_num; i++) {
+ pin_idx = spec->num_pins;
+ per_pin = snd_array_new(&spec->pins);
+
+ if (!per_pin)
+ return -ENOMEM;
- spec->num_pins++;
+ if (spec->dyn_pcm_assign) {
+ per_pin->pcm = NULL;
+ per_pin->pcm_idx = -1;
+ } else {
+ per_pin->pcm = get_hdmi_pcm(spec, pin_idx);
+ per_pin->pcm_idx = pin_idx;
+ }
+ per_pin->pin_nid = pin_nid;
+ per_pin->pin_nid_idx = spec->num_nids;
+ per_pin->dev_id = i;
+ per_pin->non_pcm = false;
+ snd_hda_set_dev_select(codec, pin_nid, i);
+ if (is_haswell_plus(codec))
+ intel_haswell_fixup_connect_list(codec, pin_nid, i);
+ //need check here
+ err = hdmi_read_pin_conn(codec, pin_idx);
+ if (err < 0)
+ return err;
+ spec->num_pins++;
+ }
+ spec->num_nids++;
return 0;
}
@@ -1744,7 +1841,7 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
/* Call sync_audio_rate to set the N/CTS/M manually if necessary */
/* Todo: add DP1.2 MST audio support later */
if (codec_has_acomp(codec))
- snd_hdac_sync_audio_rate(&codec->core, pin_nid, -1,
+ snd_hdac_sync_audio_rate(&codec->core, pin_nid, per_pin->dev_id,
runtime->rate);
non_pcm = check_non_pcm_per_cvt(codec, cvt_nid);
@@ -1762,6 +1859,7 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
pinctl | PIN_OUT);
}
+ //need call snd_hda_set_dev_select()???
err = spec->ops.setup_stream(codec, cvt_nid, pin_nid,
stream_tag, format);
mutex_unlock(&spec->pcm_lock);
@@ -1897,17 +1995,23 @@ static bool is_hdmi_pcm_attached(struct hdac_device *hdac, int pcm_idx)
static int generic_hdmi_build_pcms(struct hda_codec *codec)
{
struct hdmi_spec *spec = codec->spec;
- int pin_idx;
+ int idx;
- for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
+ /*
+ * for non-mst mode, pcm number is the same as before
+ * for DP MST mode, pcm number is (nid number + dev_num - 1)
+ * dev_num is the device entry number in a pin
+ *
+ */
+ for (idx = 0; idx < spec->num_nids + spec->dev_num - 1; idx++) {
struct hda_pcm *info;
struct hda_pcm_stream *pstr;
- info = snd_hda_codec_pcm_new(codec, "HDMI %d", pin_idx);
+ info = snd_hda_codec_pcm_new(codec, "HDMI %d", idx);
if (!info)
return -ENOMEM;
- spec->pcm_rec[pin_idx].pcm = info;
+ spec->pcm_rec[idx].pcm = info;
spec->pcm_used++;
info->pcm_type = HDA_PCM_TYPE_HDMI;
info->own_chmap = true;
@@ -1915,6 +2019,9 @@ static int generic_hdmi_build_pcms(struct hda_codec *codec)
pstr = &info->stream[SNDRV_PCM_STREAM_PLAYBACK];
pstr->substreams = 1;
pstr->ops = generic_ops;
+ /* pcm number is less than 16 */
+ if (spec->pcm_used >= 16)
+ break;
/* other pstr fields are set in open */
}
@@ -2070,7 +2177,9 @@ static int generic_hdmi_init(struct hda_codec *codec)
for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
hda_nid_t pin_nid = per_pin->pin_nid;
+ int dev_id = per_pin->dev_id;
+ snd_hda_set_dev_select(codec, pin_nid, dev_id);
hdmi_init_pin(codec, pin_nid);
if (!codec_has_acomp(codec))
snd_hda_jack_detect_enable_callback(codec, pin_nid,
@@ -2178,6 +2287,7 @@ static int alloc_generic_hdmi(struct hda_codec *codec)
return -ENOMEM;
spec->ops = generic_standard_hdmi_ops;
+ spec->dev_num = 1; /* initialize to 1 */
mutex_init(&spec->pcm_lock);
snd_hdac_register_chmap_ops(&codec->core, &spec->chmap);
@@ -2218,21 +2328,21 @@ static int patch_generic_hdmi(struct hda_codec *codec)
*/
static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
- hda_nid_t nid)
+ hda_nid_t nid, int dev_id)
{
struct hdmi_spec *spec = codec->spec;
hda_nid_t conns[4];
int nconns;
- nconns = snd_hda_get_connections(codec, nid, 0, conns,
- ARRAY_SIZE(conns));
+ nconns = snd_hda_get_connections(codec, nid, dev_id,
+ conns, ARRAY_SIZE(conns));
if (nconns == spec->num_cvts &&
!memcmp(conns, spec->cvt_nids, spec->num_cvts * sizeof(hda_nid_t)))
return;
/* override pins connection list */
codec_dbg(codec, "hdmi: haswell: override pin connection 0x%x\n", nid);
- snd_hda_override_conn_list(codec, nid, 0, spec->num_cvts,
+ snd_hda_override_conn_list(codec, nid, dev_id, spec->num_cvts,
spec->cvt_nids);
}
@@ -2297,6 +2407,7 @@ static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe)
{
struct hda_codec *codec = audio_ptr;
int pin_nid;
+ int dev_id = pipe;
/* we assume only from port-B to port-D */
if (port < 1 || port > 3)
@@ -2323,7 +2434,7 @@ static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe)
return;
snd_hdac_i915_set_bclk(&codec->bus->core);
- check_presence_and_report(codec, pin_nid);
+ check_presence_and_report(codec, pin_nid, dev_id);
}
/* register i915 component pin_eld_notify callback */
@@ -2356,11 +2467,13 @@ static void i915_pin_cvt_fixup(struct hda_codec *codec,
hda_nid_t cvt_nid)
{
if (per_pin) {
+ snd_hda_set_dev_select(codec, per_pin->pin_nid,
+ per_pin->dev_id);
intel_verify_pin_cvt_connect(codec, per_pin);
intel_not_share_assigned_cvt(codec, per_pin->pin_nid,
- per_pin->mux_idx);
+ per_pin->dev_id, per_pin->mux_idx);
} else {
- intel_not_share_assigned_cvt_nid(codec, 0, cvt_nid);
+ intel_not_share_assigned_cvt_nid(codec, 0, 0, cvt_nid);
}
}
@@ -2380,6 +2493,8 @@ static int patch_i915_hsw_hdmi(struct hda_codec *codec)
if (err < 0)
return err;
spec = codec->spec;
+ codec->dp_mst = true;
+ spec->dyn_pcm_assign = true;
intel_haswell_enable_all_pins(codec, true);
intel_haswell_fixup_enable_dp12(codec);
@@ -2391,7 +2506,6 @@ static int patch_i915_hsw_hdmi(struct hda_codec *codec)
codec->core.link_power_control = 1;
codec->patch_ops.set_power_state = haswell_set_power_state;
- codec->dp_mst = true;
codec->depop_delay = 0;
codec->auto_runtime_pm = 1;
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-26 8:35 [RFC PATCH 0/3] support DP MST audio libin.yang
` (2 preceding siblings ...)
2016-09-26 8:35 ` [RFC PATCH 3/3] ALSA: hda - add DP mst audio support libin.yang
@ 2016-09-26 15:10 ` Takashi Iwai
2016-09-26 16:07 ` Yang, Libin
3 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2016-09-26 15:10 UTC (permalink / raw)
To: libin.yang; +Cc: libin.yang, mengdong.lin, alsa-devel
On Mon, 26 Sep 2016 10:35:35 +0200,
libin.yang@linux.intel.com wrote:
>
> From: Libin Yang <libin.yang@linux.intel.com>
>
> This patchset starts to support DP MST audio.
>
> This patchset is based on drm-intel-nightly on
> git://anongit.freedesktop.org/drm-intel
>
> Libin Yang (3):
> ALSA: hda - codec add DP MST support for connection list
> ALSA: hda - add DP mst verb support
> ALSA: hda - add DP mst audio support
I read the patchset again, and I'm still not convinced enough by this
change.
The connection list is coded in the current way under the assumption
that connections are more or less static. The connections are cached
in snd_array, which is only for growing up, and not suitable for the
entries that might be frequently removed. The removal is done only at
overriding, and it happens only once at boot.
OTOH, with DP-MST case, the connection list is basically dynamic. It
may increase or decrease depending on the connected monitors... Is my
understanding correct? Or is the DP-MST connection list is static,
too? Then I do wonder how it covers the whole connections with
arbitrary device indices.
So, the connection cache management is one thing. Another thing is
that the patchset doesn't consider about the pin ELD notification.
Basically we switched to ELD notification instead of the pin unsol
event for Intel chips. How does it fit?
thanks,
Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-26 15:10 ` [RFC PATCH 0/3] support DP MST audio Takashi Iwai
@ 2016-09-26 16:07 ` Yang, Libin
2016-09-26 17:50 ` Takashi Iwai
0 siblings, 1 reply; 14+ messages in thread
From: Yang, Libin @ 2016-09-26 16:07 UTC (permalink / raw)
To: Takashi Iwai, libin.yang; +Cc: Lin, Mengdong, alsa-devel
Hi Takashi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Monday, September 26, 2016 11:11 PM
> To: libin.yang@linux.intel.com
> Cc: alsa-devel@alsa-project.org; Lin, Mengdong <mengdong.lin@intel.com>;
> Yang, Libin <libin.yang@intel.com>
> Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
>
> On Mon, 26 Sep 2016 10:35:35 +0200,
> libin.yang@linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > This patchset starts to support DP MST audio.
> >
> > This patchset is based on drm-intel-nightly on
> > git://anongit.freedesktop.org/drm-intel
> >
> > Libin Yang (3):
> > ALSA: hda - codec add DP MST support for connection list
> > ALSA: hda - add DP mst verb support
> > ALSA: hda - add DP mst audio support
>
> I read the patchset again, and I'm still not convinced enough by this change.
Yes and I'm sorry that this patchset is delayed for a long time.
Our gfx driver has been redesigned and some APIs have been changed.
The change is done in Pandiyan, Dhinakaran's patches:
Patchset: Prep. for DP audio MST support
drm/i915: Standardize port type for DVO encoders
drm/i915: Store port enum in intel_encoder
drm/i915: Switch to using port stored in intel_encoder
drm/i915: Move audio_connector to intel_encoder
drm/i915: start adding dp mst audio
Patch: drm/i915/dp: DP audio API changes for MST
>
> The connection list is coded in the current way under the assumption that
> connections are more or less static. The connections are cached in snd_array,
> which is only for growing up, and not suitable for the entries that might be
> frequently removed. The removal is done only at overriding, and it happens
> only once at boot.
>
> OTOH, with DP-MST case, the connection list is basically dynamic. It may
> increase or decrease depending on the connected monitors... Is my
> understanding correct? Or is the DP-MST connection list is static, too? Then I
> do wonder how it covers the whole connections with arbitrary device indices.
For the connection list, the driver will setup all the connection list at the beginning
for each device entry, even it is non-MST. We statically allocate the connection
list and will never remove them.
>
> So, the connection cache management is one thing. Another thing is that the
> patchset doesn't consider about the pin ELD notification.
> Basically we switched to ELD notification instead of the pin unsol event for
> Intel chips. How does it fit?
For the ELD notification, in the patch 0003, if there is monitor hotplug,
gfx driver will notify audio driver with acomp. Gfx driver will tell which
port (pin), pipe (device entry) occurs the hotplug event. And then audio
driver will call snd_hdac_acomp_get_eld() to get the eld information.
Regards,
Libin
>
>
> thanks,
>
> Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-26 16:07 ` Yang, Libin
@ 2016-09-26 17:50 ` Takashi Iwai
2016-09-27 7:47 ` Yang, Libin
0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2016-09-26 17:50 UTC (permalink / raw)
To: Yang, Libin; +Cc: Lin, Mengdong, libin.yang, alsa-devel
On Mon, 26 Sep 2016 18:07:51 +0200,
Yang, Libin wrote:
>
> Hi Takashi,
>
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Monday, September 26, 2016 11:11 PM
> > To: libin.yang@linux.intel.com
> > Cc: alsa-devel@alsa-project.org; Lin, Mengdong <mengdong.lin@intel.com>;
> > Yang, Libin <libin.yang@intel.com>
> > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> >
> > On Mon, 26 Sep 2016 10:35:35 +0200,
> > libin.yang@linux.intel.com wrote:
> > >
> > > From: Libin Yang <libin.yang@linux.intel.com>
> > >
> > > This patchset starts to support DP MST audio.
> > >
> > > This patchset is based on drm-intel-nightly on
> > > git://anongit.freedesktop.org/drm-intel
> > >
> > > Libin Yang (3):
> > > ALSA: hda - codec add DP MST support for connection list
> > > ALSA: hda - add DP mst verb support
> > > ALSA: hda - add DP mst audio support
> >
> > I read the patchset again, and I'm still not convinced enough by this change.
>
> Yes and I'm sorry that this patchset is delayed for a long time.
> Our gfx driver has been redesigned and some APIs have been changed.
> The change is done in Pandiyan, Dhinakaran's patches:
> Patchset: Prep. for DP audio MST support
> drm/i915: Standardize port type for DVO encoders
> drm/i915: Store port enum in intel_encoder
> drm/i915: Switch to using port stored in intel_encoder
> drm/i915: Move audio_connector to intel_encoder
> drm/i915: start adding dp mst audio
> Patch: drm/i915/dp: DP audio API changes for MST
>
> >
> > The connection list is coded in the current way under the assumption that
> > connections are more or less static. The connections are cached in snd_array,
> > which is only for growing up, and not suitable for the entries that might be
> > frequently removed. The removal is done only at overriding, and it happens
> > only once at boot.
> >
> > OTOH, with DP-MST case, the connection list is basically dynamic. It may
> > increase or decrease depending on the connected monitors... Is my
> > understanding correct? Or is the DP-MST connection list is static, too? Then I
> > do wonder how it covers the whole connections with arbitrary device indices.
>
> For the connection list, the driver will setup all the connection list at the beginning
> for each device entry, even it is non-MST. We statically allocate the connection
> list and will never remove them.
Hrm, so how will it actually be? Are the device indices fixed through
the whole operations, no matter which monitor is connected? How many
will they be?
It'd be helpful if you give an example of the actual typical setup.
Then we can judge whether the proposed implementation is suitable.
> > So, the connection cache management is one thing. Another thing is that the
> > patchset doesn't consider about the pin ELD notification.
> > Basically we switched to ELD notification instead of the pin unsol event for
> > Intel chips. How does it fit?
>
> For the ELD notification, in the patch 0003, if there is monitor hotplug,
> gfx driver will notify audio driver with acomp. Gfx driver will tell which
> port (pin), pipe (device entry) occurs the hotplug event. And then audio
> driver will call snd_hdac_acomp_get_eld() to get the eld information.
OK, then drop the patch 1 and try to implement without messing around
the connection list. The patch 1 is what I really don't like from the
beginning. It makes things complicated.
Is there anything else than haswell fixup that touches the connection
list with the dev id? If it's the only place, there can be an
alternative way to hack it.
thanks,
Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-26 17:50 ` Takashi Iwai
@ 2016-09-27 7:47 ` Yang, Libin
2016-09-27 7:58 ` Takashi Iwai
0 siblings, 1 reply; 14+ messages in thread
From: Yang, Libin @ 2016-09-27 7:47 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Lin, Mengdong, libin.yang, alsa-devel
Hi Takashi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, September 27, 2016 1:51 AM
> To: Yang, Libin <libin.yang@intel.com>
> Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin, Mengdong
> <mengdong.lin@intel.com>
> Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
>
> On Mon, 26 Sep 2016 18:07:51 +0200,
> Yang, Libin wrote:
> >
> > Hi Takashi,
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Monday, September 26, 2016 11:11 PM
> > > To: libin.yang@linux.intel.com
> > > Cc: alsa-devel@alsa-project.org; Lin, Mengdong
> > > <mengdong.lin@intel.com>; Yang, Libin <libin.yang@intel.com>
> > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> > >
> > > On Mon, 26 Sep 2016 10:35:35 +0200,
> > > libin.yang@linux.intel.com wrote:
> > > >
> > > > From: Libin Yang <libin.yang@linux.intel.com>
> > > >
> > > > This patchset starts to support DP MST audio.
> > > >
> > > > This patchset is based on drm-intel-nightly on
> > > > git://anongit.freedesktop.org/drm-intel
> > > >
> > > > Libin Yang (3):
> > > > ALSA: hda - codec add DP MST support for connection list
> > > > ALSA: hda - add DP mst verb support
> > > > ALSA: hda - add DP mst audio support
> > >
> > > I read the patchset again, and I'm still not convinced enough by this
> change.
> >
> > Yes and I'm sorry that this patchset is delayed for a long time.
> > Our gfx driver has been redesigned and some APIs have been changed.
> > The change is done in Pandiyan, Dhinakaran's patches:
> > Patchset: Prep. for DP audio MST support
> > drm/i915: Standardize port type for DVO encoders
> > drm/i915: Store port enum in intel_encoder
> > drm/i915: Switch to using port stored in intel_encoder
> > drm/i915: Move audio_connector to intel_encoder
> > drm/i915: start adding dp mst audio
> > Patch: drm/i915/dp: DP audio API changes for MST
> >
> > >
> > > The connection list is coded in the current way under the assumption
> > > that connections are more or less static. The connections are
> > > cached in snd_array, which is only for growing up, and not suitable
> > > for the entries that might be frequently removed. The removal is
> > > done only at overriding, and it happens only once at boot.
> > >
> > > OTOH, with DP-MST case, the connection list is basically dynamic.
> > > It may increase or decrease depending on the connected monitors...
> > > Is my understanding correct? Or is the DP-MST connection list is
> > > static, too? Then I do wonder how it covers the whole connections with
> arbitrary device indices.
> >
> > For the connection list, the driver will setup all the connection list
> > at the beginning for each device entry, even it is non-MST. We
> > statically allocate the connection list and will never remove them.
>
> Hrm, so how will it actually be? Are the device indices fixed through the
> whole operations, no matter which monitor is connected? How many will
> they be?
>
> It'd be helpful if you give an example of the actual typical setup.
> Then we can judge whether the proposed implementation is suitable.
Even it is in non-MST mode, the connection list is setup with the per_pin
initialization. And as the device entries under the same pin have the
same connection list, I use the device entry 0's connection list for other
device entries. This avoids connection list is invalid for other device entries
in non-MST. After initialization, the connection list will never change even
the non-MST/MST mode is changed.
>
>
> > > So, the connection cache management is one thing. Another thing is
> > > that the patchset doesn't consider about the pin ELD notification.
> > > Basically we switched to ELD notification instead of the pin unsol
> > > event for Intel chips. How does it fit?
> >
> > For the ELD notification, in the patch 0003, if there is monitor
> > hotplug, gfx driver will notify audio driver with acomp. Gfx driver
> > will tell which port (pin), pipe (device entry) occurs the hotplug
> > event. And then audio driver will call snd_hdac_acomp_get_eld() to get the
> eld information.
>
> OK, then drop the patch 1 and try to implement without messing around the
> connection list. The patch 1 is what I really don't like from the beginning. It
> makes things complicated.
As the connection list is the same for the device entries under same pin,
what do you think if we use the original APIs, ignoring the device id. This
will not change the connection list code or be a very small change.
If you agree on that, I will have a try on it.
On the same time, I will check with our silicon if my understanding
"the connection list is the same for the device entries under same pin"
is right.
>
> Is there anything else than haswell fixup that touches the connection list with
> the dev id? If it's the only place, there can be an alternative way to hack it.
If we always use the device entry 0 as all the device entries under the same pin,
I think we can ignore the dev id now.
Regards,
Libin
>
>
> thanks,
>
> Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-27 7:47 ` Yang, Libin
@ 2016-09-27 7:58 ` Takashi Iwai
2016-09-27 8:18 ` Yang, Libin
0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2016-09-27 7:58 UTC (permalink / raw)
To: Yang, Libin; +Cc: Lin, Mengdong, libin.yang, alsa-devel
On Tue, 27 Sep 2016 09:47:38 +0200,
Yang, Libin wrote:
>
> Hi Takashi,
>
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Tuesday, September 27, 2016 1:51 AM
> > To: Yang, Libin <libin.yang@intel.com>
> > Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin, Mengdong
> > <mengdong.lin@intel.com>
> > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> >
> > On Mon, 26 Sep 2016 18:07:51 +0200,
> > Yang, Libin wrote:
> > >
> > > Hi Takashi,
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Monday, September 26, 2016 11:11 PM
> > > > To: libin.yang@linux.intel.com
> > > > Cc: alsa-devel@alsa-project.org; Lin, Mengdong
> > > > <mengdong.lin@intel.com>; Yang, Libin <libin.yang@intel.com>
> > > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> > > >
> > > > On Mon, 26 Sep 2016 10:35:35 +0200,
> > > > libin.yang@linux.intel.com wrote:
> > > > >
> > > > > From: Libin Yang <libin.yang@linux.intel.com>
> > > > >
> > > > > This patchset starts to support DP MST audio.
> > > > >
> > > > > This patchset is based on drm-intel-nightly on
> > > > > git://anongit.freedesktop.org/drm-intel
> > > > >
> > > > > Libin Yang (3):
> > > > > ALSA: hda - codec add DP MST support for connection list
> > > > > ALSA: hda - add DP mst verb support
> > > > > ALSA: hda - add DP mst audio support
> > > >
> > > > I read the patchset again, and I'm still not convinced enough by this
> > change.
> > >
> > > Yes and I'm sorry that this patchset is delayed for a long time.
> > > Our gfx driver has been redesigned and some APIs have been changed.
> > > The change is done in Pandiyan, Dhinakaran's patches:
> > > Patchset: Prep. for DP audio MST support
> > > drm/i915: Standardize port type for DVO encoders
> > > drm/i915: Store port enum in intel_encoder
> > > drm/i915: Switch to using port stored in intel_encoder
> > > drm/i915: Move audio_connector to intel_encoder
> > > drm/i915: start adding dp mst audio
> > > Patch: drm/i915/dp: DP audio API changes for MST
> > >
> > > >
> > > > The connection list is coded in the current way under the assumption
> > > > that connections are more or less static. The connections are
> > > > cached in snd_array, which is only for growing up, and not suitable
> > > > for the entries that might be frequently removed. The removal is
> > > > done only at overriding, and it happens only once at boot.
> > > >
> > > > OTOH, with DP-MST case, the connection list is basically dynamic.
> > > > It may increase or decrease depending on the connected monitors...
> > > > Is my understanding correct? Or is the DP-MST connection list is
> > > > static, too? Then I do wonder how it covers the whole connections with
> > arbitrary device indices.
> > >
> > > For the connection list, the driver will setup all the connection list
> > > at the beginning for each device entry, even it is non-MST. We
> > > statically allocate the connection list and will never remove them.
> >
> > Hrm, so how will it actually be? Are the device indices fixed through the
> > whole operations, no matter which monitor is connected? How many will
> > they be?
> >
> > It'd be helpful if you give an example of the actual typical setup.
> > Then we can judge whether the proposed implementation is suitable.
>
> Even it is in non-MST mode, the connection list is setup with the per_pin
> initialization. And as the device entries under the same pin have the
> same connection list, I use the device entry 0's connection list for other
> device entries. This avoids connection list is invalid for other device entries
> in non-MST. After initialization, the connection list will never change even
> the non-MST/MST mode is changed.
The devid=0 case is fine. This is identical with the normal widget
connections. But you wrote that these connections are static even for
devid!=0. Is it correct?
If so, now my question again: for devid!=0, which connections are
given statically? In other words, how many devid would be passed?
> > > > So, the connection cache management is one thing. Another thing is
> > > > that the patchset doesn't consider about the pin ELD notification.
> > > > Basically we switched to ELD notification instead of the pin unsol
> > > > event for Intel chips. How does it fit?
> > >
> > > For the ELD notification, in the patch 0003, if there is monitor
> > > hotplug, gfx driver will notify audio driver with acomp. Gfx driver
> > > will tell which port (pin), pipe (device entry) occurs the hotplug
> > > event. And then audio driver will call snd_hdac_acomp_get_eld() to get the
> > eld information.
> >
> > OK, then drop the patch 1 and try to implement without messing around the
> > connection list. The patch 1 is what I really don't like from the beginning. It
> > makes things complicated.
>
> As the connection list is the same for the device entries under same pin,
> what do you think if we use the original APIs, ignoring the device id. This
> will not change the connection list code or be a very small change.
>
> If you agree on that, I will have a try on it.
>
> On the same time, I will check with our silicon if my understanding
> "the connection list is the same for the device entries under same pin"
> is right.
>
> > Is there anything else than haswell fixup that touches the connection list with
> > the dev id? If it's the only place, there can be an alternative way to hack it.
>
> If we always use the device entry 0 as all the device entries under the same pin,
> I think we can ignore the dev id now.
Well, it is only about the pin/cvt connection inside HDMI/DP codec,
and it should be easily covered in a different way even with devid!=0,
I suppose. That said, the patchset looks way too complex than
necessary.
Though, it might be a good idea to start with devid=0 assumption if it
makes the changes simpler.
Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-27 7:58 ` Takashi Iwai
@ 2016-09-27 8:18 ` Yang, Libin
2016-09-27 8:45 ` Takashi Iwai
0 siblings, 1 reply; 14+ messages in thread
From: Yang, Libin @ 2016-09-27 8:18 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Lin, Mengdong, libin.yang, alsa-devel
Hi Takashi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, September 27, 2016 3:59 PM
> To: Yang, Libin <libin.yang@intel.com>
> Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin, Mengdong
> <mengdong.lin@intel.com>
> Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
>
> On Tue, 27 Sep 2016 09:47:38 +0200,
> Yang, Libin wrote:
> >
> > Hi Takashi,
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Tuesday, September 27, 2016 1:51 AM
> > > To: Yang, Libin <libin.yang@intel.com>
> > > Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin,
> > > Mengdong <mengdong.lin@intel.com>
> > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> > >
> > > On Mon, 26 Sep 2016 18:07:51 +0200,
> > > Yang, Libin wrote:
> > > >
> > > > Hi Takashi,
> > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > Sent: Monday, September 26, 2016 11:11 PM
> > > > > To: libin.yang@linux.intel.com
> > > > > Cc: alsa-devel@alsa-project.org; Lin, Mengdong
> > > > > <mengdong.lin@intel.com>; Yang, Libin <libin.yang@intel.com>
> > > > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> > > > >
> > > > > On Mon, 26 Sep 2016 10:35:35 +0200, libin.yang@linux.intel.com
> > > > > wrote:
> > > > > >
> > > > > > From: Libin Yang <libin.yang@linux.intel.com>
> > > > > >
> > > > > > This patchset starts to support DP MST audio.
> > > > > >
> > > > > > This patchset is based on drm-intel-nightly on
> > > > > > git://anongit.freedesktop.org/drm-intel
> > > > > >
> > > > > > Libin Yang (3):
> > > > > > ALSA: hda - codec add DP MST support for connection list
> > > > > > ALSA: hda - add DP mst verb support
> > > > > > ALSA: hda - add DP mst audio support
> > > > >
> > > > > I read the patchset again, and I'm still not convinced enough by
> > > > > this
> > > change.
> > > >
> > > > Yes and I'm sorry that this patchset is delayed for a long time.
> > > > Our gfx driver has been redesigned and some APIs have been changed.
> > > > The change is done in Pandiyan, Dhinakaran's patches:
> > > > Patchset: Prep. for DP audio MST support
> > > > drm/i915: Standardize port type for DVO encoders
> > > > drm/i915: Store port enum in intel_encoder
> > > > drm/i915: Switch to using port stored in intel_encoder
> > > > drm/i915: Move audio_connector to intel_encoder
> > > > drm/i915: start adding dp mst audio
> > > > Patch: drm/i915/dp: DP audio API changes for MST
> > > >
> > > > >
> > > > > The connection list is coded in the current way under the
> > > > > assumption that connections are more or less static. The
> > > > > connections are cached in snd_array, which is only for growing
> > > > > up, and not suitable for the entries that might be frequently
> > > > > removed. The removal is done only at overriding, and it happens only
> once at boot.
> > > > >
> > > > > OTOH, with DP-MST case, the connection list is basically dynamic.
> > > > > It may increase or decrease depending on the connected monitors...
> > > > > Is my understanding correct? Or is the DP-MST connection list
> > > > > is static, too? Then I do wonder how it covers the whole
> > > > > connections with
> > > arbitrary device indices.
> > > >
> > > > For the connection list, the driver will setup all the connection
> > > > list at the beginning for each device entry, even it is non-MST.
> > > > We statically allocate the connection list and will never remove them.
> > >
> > > Hrm, so how will it actually be? Are the device indices fixed
> > > through the whole operations, no matter which monitor is connected?
> > > How many will they be?
> > >
> > > It'd be helpful if you give an example of the actual typical setup.
> > > Then we can judge whether the proposed implementation is suitable.
> >
> > Even it is in non-MST mode, the connection list is setup with the
> > per_pin initialization. And as the device entries under the same pin
> > have the same connection list, I use the device entry 0's connection
> > list for other device entries. This avoids connection list is invalid
> > for other device entries in non-MST. After initialization, the
> > connection list will never change even the non-MST/MST mode is changed.
>
> The devid=0 case is fine. This is identical with the normal widget connections.
> But you wrote that these connections are static even for devid!=0. Is it
> correct?
>
> If so, now my question again: for devid!=0, which connections are given
> statically? In other words, how many devid would be passed?
For the connection list, it is static. It will be initialized at bootup.
For the pin and cvt actual connection, it is dynamically assigned.
In hdmi_pcm_open(), it will call hdmi_choose_cvt() to pick up
a cvt (based on the connection list and the cvt is used or not).
Actually, I think the connection list is only useful when picking
up the cvt (or other nodes). Please correct me if I'm wrong.
When we try to connect device entry 2, pin 1 to cvt A, for example,
we will call snd_hda_set_dev_select(pin_nid, dev_id) to select the
device entry on the pin. And then we will use the verb
AC_VERB_SET_CONNECT_SEL to setup the connection as before.
>
> > > > > So, the connection cache management is one thing. Another thing
> > > > > is that the patchset doesn't consider about the pin ELD notification.
> > > > > Basically we switched to ELD notification instead of the pin
> > > > > unsol event for Intel chips. How does it fit?
> > > >
> > > > For the ELD notification, in the patch 0003, if there is monitor
> > > > hotplug, gfx driver will notify audio driver with acomp. Gfx
> > > > driver will tell which port (pin), pipe (device entry) occurs the
> > > > hotplug event. And then audio driver will call
> > > > snd_hdac_acomp_get_eld() to get the
> > > eld information.
> > >
> > > OK, then drop the patch 1 and try to implement without messing
> > > around the connection list. The patch 1 is what I really don't like
> > > from the beginning. It makes things complicated.
> >
> > As the connection list is the same for the device entries under same
> > pin, what do you think if we use the original APIs, ignoring the
> > device id. This will not change the connection list code or be a very small
> change.
> >
> > If you agree on that, I will have a try on it.
> >
> > On the same time, I will check with our silicon if my understanding
> > "the connection list is the same for the device entries under same pin"
> > is right.
> >
> > > Is there anything else than haswell fixup that touches the
> > > connection list with the dev id? If it's the only place, there can be an
> alternative way to hack it.
> >
> > If we always use the device entry 0 as all the device entries under
> > the same pin, I think we can ignore the dev id now.
>
> Well, it is only about the pin/cvt connection inside HDMI/DP codec, and it
> should be easily covered in a different way even with devid!=0, I suppose.
> That said, the patchset looks way too complex than necessary.
>
> Though, it might be a good idea to start with devid=0 assumption if it makes
> the changes simpler.
Yes, the patches are too complex for this simple usage. I will try to see if it
works without touching the connection list core.
Regards,
Libin
>
>
> Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-27 8:18 ` Yang, Libin
@ 2016-09-27 8:45 ` Takashi Iwai
2016-09-28 2:50 ` Yang, Libin
0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2016-09-27 8:45 UTC (permalink / raw)
To: Yang, Libin; +Cc: Lin, Mengdong, libin.yang, alsa-devel
On Tue, 27 Sep 2016 10:18:13 +0200,
Yang, Libin wrote:
>
> Hi Takashi,
>
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Tuesday, September 27, 2016 3:59 PM
> > To: Yang, Libin <libin.yang@intel.com>
> > Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin, Mengdong
> > <mengdong.lin@intel.com>
> > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> >
> > On Tue, 27 Sep 2016 09:47:38 +0200,
> > Yang, Libin wrote:
> > >
> > > Hi Takashi,
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Tuesday, September 27, 2016 1:51 AM
> > > > To: Yang, Libin <libin.yang@intel.com>
> > > > Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin,
> > > > Mengdong <mengdong.lin@intel.com>
> > > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> > > >
> > > > On Mon, 26 Sep 2016 18:07:51 +0200,
> > > > Yang, Libin wrote:
> > > > >
> > > > > Hi Takashi,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > > Sent: Monday, September 26, 2016 11:11 PM
> > > > > > To: libin.yang@linux.intel.com
> > > > > > Cc: alsa-devel@alsa-project.org; Lin, Mengdong
> > > > > > <mengdong.lin@intel.com>; Yang, Libin <libin.yang@intel.com>
> > > > > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> > > > > >
> > > > > > On Mon, 26 Sep 2016 10:35:35 +0200, libin.yang@linux.intel.com
> > > > > > wrote:
> > > > > > >
> > > > > > > From: Libin Yang <libin.yang@linux.intel.com>
> > > > > > >
> > > > > > > This patchset starts to support DP MST audio.
> > > > > > >
> > > > > > > This patchset is based on drm-intel-nightly on
> > > > > > > git://anongit.freedesktop.org/drm-intel
> > > > > > >
> > > > > > > Libin Yang (3):
> > > > > > > ALSA: hda - codec add DP MST support for connection list
> > > > > > > ALSA: hda - add DP mst verb support
> > > > > > > ALSA: hda - add DP mst audio support
> > > > > >
> > > > > > I read the patchset again, and I'm still not convinced enough by
> > > > > > this
> > > > change.
> > > > >
> > > > > Yes and I'm sorry that this patchset is delayed for a long time.
> > > > > Our gfx driver has been redesigned and some APIs have been changed.
> > > > > The change is done in Pandiyan, Dhinakaran's patches:
> > > > > Patchset: Prep. for DP audio MST support
> > > > > drm/i915: Standardize port type for DVO encoders
> > > > > drm/i915: Store port enum in intel_encoder
> > > > > drm/i915: Switch to using port stored in intel_encoder
> > > > > drm/i915: Move audio_connector to intel_encoder
> > > > > drm/i915: start adding dp mst audio
> > > > > Patch: drm/i915/dp: DP audio API changes for MST
> > > > >
> > > > > >
> > > > > > The connection list is coded in the current way under the
> > > > > > assumption that connections are more or less static. The
> > > > > > connections are cached in snd_array, which is only for growing
> > > > > > up, and not suitable for the entries that might be frequently
> > > > > > removed. The removal is done only at overriding, and it happens only
> > once at boot.
> > > > > >
> > > > > > OTOH, with DP-MST case, the connection list is basically dynamic.
> > > > > > It may increase or decrease depending on the connected monitors...
> > > > > > Is my understanding correct? Or is the DP-MST connection list
> > > > > > is static, too? Then I do wonder how it covers the whole
> > > > > > connections with
> > > > arbitrary device indices.
> > > > >
> > > > > For the connection list, the driver will setup all the connection
> > > > > list at the beginning for each device entry, even it is non-MST.
> > > > > We statically allocate the connection list and will never remove them.
> > > >
> > > > Hrm, so how will it actually be? Are the device indices fixed
> > > > through the whole operations, no matter which monitor is connected?
> > > > How many will they be?
> > > >
> > > > It'd be helpful if you give an example of the actual typical setup.
> > > > Then we can judge whether the proposed implementation is suitable.
> > >
> > > Even it is in non-MST mode, the connection list is setup with the
> > > per_pin initialization. And as the device entries under the same pin
> > > have the same connection list, I use the device entry 0's connection
> > > list for other device entries. This avoids connection list is invalid
> > > for other device entries in non-MST. After initialization, the
> > > connection list will never change even the non-MST/MST mode is changed.
> >
> > The devid=0 case is fine. This is identical with the normal widget connections.
> > But you wrote that these connections are static even for devid!=0. Is it
> > correct?
> >
> > If so, now my question again: for devid!=0, which connections are given
> > statically? In other words, how many devid would be passed?
>
> For the connection list, it is static. It will be initialized at bootup.
So this is only about devid=0, right? For devid!=0, there wouldn't be
anything else than pin/cvt connections, correct?
> For the pin and cvt actual connection, it is dynamically assigned.
... but you still want to cache this to an (ever-growing) array for
the standard connection lists. That's what I was concerned.
> In hdmi_pcm_open(), it will call hdmi_choose_cvt() to pick up
> a cvt (based on the connection list and the cvt is used or not).
> Actually, I think the connection list is only useful when picking
> up the cvt (or other nodes). Please correct me if I'm wrong.
Yes, since HDMI/DP codec has only pretty simple connections.
> When we try to connect device entry 2, pin 1 to cvt A, for example,
> we will call snd_hda_set_dev_select(pin_nid, dev_id) to select the
> device entry on the pin. And then we will use the verb
> AC_VERB_SET_CONNECT_SEL to setup the connection as before.
OK, that sounds straightforward. So we just need to set up these
verbs at opening the device (and likely at suspend/resume, too).
> > > > > > So, the connection cache management is one thing. Another thing
> > > > > > is that the patchset doesn't consider about the pin ELD notification.
> > > > > > Basically we switched to ELD notification instead of the pin
> > > > > > unsol event for Intel chips. How does it fit?
> > > > >
> > > > > For the ELD notification, in the patch 0003, if there is monitor
> > > > > hotplug, gfx driver will notify audio driver with acomp. Gfx
> > > > > driver will tell which port (pin), pipe (device entry) occurs the
> > > > > hotplug event. And then audio driver will call
> > > > > snd_hdac_acomp_get_eld() to get the
> > > > eld information.
> > > >
> > > > OK, then drop the patch 1 and try to implement without messing
> > > > around the connection list. The patch 1 is what I really don't like
> > > > from the beginning. It makes things complicated.
> > >
> > > As the connection list is the same for the device entries under same
> > > pin, what do you think if we use the original APIs, ignoring the
> > > device id. This will not change the connection list code or be a very small
> > change.
> > >
> > > If you agree on that, I will have a try on it.
> > >
> > > On the same time, I will check with our silicon if my understanding
> > > "the connection list is the same for the device entries under same pin"
> > > is right.
> > >
> > > > Is there anything else than haswell fixup that touches the
> > > > connection list with the dev id? If it's the only place, there can be an
> > alternative way to hack it.
> > >
> > > If we always use the device entry 0 as all the device entries under
> > > the same pin, I think we can ignore the dev id now.
> >
> > Well, it is only about the pin/cvt connection inside HDMI/DP codec, and it
> > should be easily covered in a different way even with devid!=0, I suppose.
> > That said, the patchset looks way too complex than necessary.
> >
> > Though, it might be a good idea to start with devid=0 assumption if it makes
> > the changes simpler.
>
> Yes, the patches are too complex for this simple usage. I will try to see if it
> works without touching the connection list core.
Yes, please. Let's keep KISS principle.
thanks,
Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-27 8:45 ` Takashi Iwai
@ 2016-09-28 2:50 ` Yang, Libin
2016-09-28 7:57 ` Takashi Iwai
0 siblings, 1 reply; 14+ messages in thread
From: Yang, Libin @ 2016-09-28 2:50 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Lin, Mengdong, libin.yang, alsa-devel
Hi Takashi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, September 27, 2016 4:46 PM
> To: Yang, Libin <libin.yang@intel.com>
> Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin, Mengdong
> <mengdong.lin@intel.com>
> Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
>
> On Tue, 27 Sep 2016 10:18:13 +0200,
> Yang, Libin wrote:
> >
> > Hi Takashi,
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Tuesday, September 27, 2016 3:59 PM
> > > To: Yang, Libin <libin.yang@intel.com>
> > > Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin,
> > > Mengdong <mengdong.lin@intel.com>
> > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> > >
> > > On Tue, 27 Sep 2016 09:47:38 +0200,
> > > Yang, Libin wrote:
> > > >
> > > > Hi Takashi,
> > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > Sent: Tuesday, September 27, 2016 1:51 AM
> > > > > To: Yang, Libin <libin.yang@intel.com>
> > > > > Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org;
> > > > > Lin, Mengdong <mengdong.lin@intel.com>
> > > > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> > > > >
> > > > > On Mon, 26 Sep 2016 18:07:51 +0200, Yang, Libin wrote:
> > > > > >
> > > > > > Hi Takashi,
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > > > Sent: Monday, September 26, 2016 11:11 PM
> > > > > > > To: libin.yang@linux.intel.com
> > > > > > > Cc: alsa-devel@alsa-project.org; Lin, Mengdong
> > > > > > > <mengdong.lin@intel.com>; Yang, Libin <libin.yang@intel.com>
> > > > > > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST
> > > > > > > audio
> > > > > > >
> > > > > > > On Mon, 26 Sep 2016 10:35:35 +0200,
> > > > > > > libin.yang@linux.intel.com
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > From: Libin Yang <libin.yang@linux.intel.com>
> > > > > > > >
> > > > > > > > This patchset starts to support DP MST audio.
> > > > > > > >
> > > > > > > > This patchset is based on drm-intel-nightly on
> > > > > > > > git://anongit.freedesktop.org/drm-intel
> > > > > > > >
> > > > > > > > Libin Yang (3):
> > > > > > > > ALSA: hda - codec add DP MST support for connection list
> > > > > > > > ALSA: hda - add DP mst verb support
> > > > > > > > ALSA: hda - add DP mst audio support
> > > > > > >
> > > > > > > I read the patchset again, and I'm still not convinced
> > > > > > > enough by this
> > > > > change.
> > > > > >
> > > > > > Yes and I'm sorry that this patchset is delayed for a long time.
> > > > > > Our gfx driver has been redesigned and some APIs have been
> changed.
> > > > > > The change is done in Pandiyan, Dhinakaran's patches:
> > > > > > Patchset: Prep. for DP audio MST support
> > > > > > drm/i915: Standardize port type for DVO encoders
> > > > > > drm/i915: Store port enum in intel_encoder
> > > > > > drm/i915: Switch to using port stored in intel_encoder
> > > > > > drm/i915: Move audio_connector to intel_encoder
> > > > > > drm/i915: start adding dp mst audio
> > > > > > Patch: drm/i915/dp: DP audio API changes for MST
> > > > > >
> > > > > > >
> > > > > > > The connection list is coded in the current way under the
> > > > > > > assumption that connections are more or less static. The
> > > > > > > connections are cached in snd_array, which is only for
> > > > > > > growing up, and not suitable for the entries that might be
> > > > > > > frequently removed. The removal is done only at overriding,
> > > > > > > and it happens only
> > > once at boot.
> > > > > > >
> > > > > > > OTOH, with DP-MST case, the connection list is basically dynamic.
> > > > > > > It may increase or decrease depending on the connected
> monitors...
> > > > > > > Is my understanding correct? Or is the DP-MST connection
> > > > > > > list is static, too? Then I do wonder how it covers the
> > > > > > > whole connections with
> > > > > arbitrary device indices.
> > > > > >
> > > > > > For the connection list, the driver will setup all the
> > > > > > connection list at the beginning for each device entry, even it is non-
> MST.
> > > > > > We statically allocate the connection list and will never remove them.
> > > > >
> > > > > Hrm, so how will it actually be? Are the device indices fixed
> > > > > through the whole operations, no matter which monitor is connected?
> > > > > How many will they be?
> > > > >
> > > > > It'd be helpful if you give an example of the actual typical setup.
> > > > > Then we can judge whether the proposed implementation is suitable.
> > > >
> > > > Even it is in non-MST mode, the connection list is setup with the
> > > > per_pin initialization. And as the device entries under the same
> > > > pin have the same connection list, I use the device entry 0's
> > > > connection list for other device entries. This avoids connection
> > > > list is invalid for other device entries in non-MST. After
> > > > initialization, the connection list will never change even the non-
> MST/MST mode is changed.
> > >
> > > The devid=0 case is fine. This is identical with the normal widget
> connections.
> > > But you wrote that these connections are static even for devid!=0.
> > > Is it correct?
> > >
> > > If so, now my question again: for devid!=0, which connections are
> > > given statically? In other words, how many devid would be passed?
> >
> > For the connection list, it is static. It will be initialized at bootup.
>
> So this is only about devid=0, right? For devid!=0, there wouldn't be anything
> else than pin/cvt connections, correct?
Devid != 0 is the same as devid = 0. And there is only pin/cvt connections.
My basic idea is:
1. Every device entry use a virtual pin
2. Each device entry will be initialized at bootup even it is not in MST mode.
This means after bootup, mode change will not add/remove the per_pin.
3. All device entries under the same pin will use the same connection list.
And the connection list is initialized at bootup. It will be saved in
per_pin->mux_nids. This will never be changed after bootup.
(However the pin-cvt connection is dynamically assigned when necessary.)
>
> > For the pin and cvt actual connection, it is dynamically assigned.
>
> ... but you still want to cache this to an (ever-growing) array for the standard
> connection lists. That's what I was concerned.
Do you mean codec->conn_list? If we don't use the dev_id and all
the device entries under the same pin use the same one, it will be
exactly the same as before.
Or do you mean the array of spec->pins? spec->pins is setup at bootup
and will not be changed later.
Regards,
Libin
>
> > In hdmi_pcm_open(), it will call hdmi_choose_cvt() to pick up a cvt
> > (based on the connection list and the cvt is used or not).
> > Actually, I think the connection list is only useful when picking up
> > the cvt (or other nodes). Please correct me if I'm wrong.
>
> Yes, since HDMI/DP codec has only pretty simple connections.
>
> > When we try to connect device entry 2, pin 1 to cvt A, for example, we
> > will call snd_hda_set_dev_select(pin_nid, dev_id) to select the device
> > entry on the pin. And then we will use the verb
> > AC_VERB_SET_CONNECT_SEL to setup the connection as before.
>
> OK, that sounds straightforward. So we just need to set up these verbs at
> opening the device (and likely at suspend/resume, too).
>
>
> > > > > > > So, the connection cache management is one thing. Another
> > > > > > > thing is that the patchset doesn't consider about the pin ELD
> notification.
> > > > > > > Basically we switched to ELD notification instead of the pin
> > > > > > > unsol event for Intel chips. How does it fit?
> > > > > >
> > > > > > For the ELD notification, in the patch 0003, if there is
> > > > > > monitor hotplug, gfx driver will notify audio driver with
> > > > > > acomp. Gfx driver will tell which port (pin), pipe (device
> > > > > > entry) occurs the hotplug event. And then audio driver will
> > > > > > call
> > > > > > snd_hdac_acomp_get_eld() to get the
> > > > > eld information.
> > > > >
> > > > > OK, then drop the patch 1 and try to implement without messing
> > > > > around the connection list. The patch 1 is what I really don't
> > > > > like from the beginning. It makes things complicated.
> > > >
> > > > As the connection list is the same for the device entries under
> > > > same pin, what do you think if we use the original APIs, ignoring
> > > > the device id. This will not change the connection list code or be
> > > > a very small
> > > change.
> > > >
> > > > If you agree on that, I will have a try on it.
> > > >
> > > > On the same time, I will check with our silicon if my
> > > > understanding "the connection list is the same for the device entries
> under same pin"
> > > > is right.
> > > >
> > > > > Is there anything else than haswell fixup that touches the
> > > > > connection list with the dev id? If it's the only place, there
> > > > > can be an
> > > alternative way to hack it.
> > > >
> > > > If we always use the device entry 0 as all the device entries
> > > > under the same pin, I think we can ignore the dev id now.
> > >
> > > Well, it is only about the pin/cvt connection inside HDMI/DP codec,
> > > and it should be easily covered in a different way even with devid!=0, I
> suppose.
> > > That said, the patchset looks way too complex than necessary.
> > >
> > > Though, it might be a good idea to start with devid=0 assumption if
> > > it makes the changes simpler.
> >
> > Yes, the patches are too complex for this simple usage. I will try to
> > see if it works without touching the connection list core.
>
> Yes, please. Let's keep KISS principle.
>
>
> thanks,
>
> Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-28 2:50 ` Yang, Libin
@ 2016-09-28 7:57 ` Takashi Iwai
2016-09-28 8:18 ` Yang, Libin
0 siblings, 1 reply; 14+ messages in thread
From: Takashi Iwai @ 2016-09-28 7:57 UTC (permalink / raw)
To: Yang, Libin; +Cc: Lin, Mengdong, libin.yang, alsa-devel
On Wed, 28 Sep 2016 04:50:53 +0200,
Yang, Libin wrote:
>
> Hi Takashi,
>
> > > > If so, now my question again: for devid!=0, which connections are
> > > > given statically? In other words, how many devid would be passed?
> > >
> > > For the connection list, it is static. It will be initialized at bootup.
> >
> > So this is only about devid=0, right? For devid!=0, there wouldn't be anything
> > else than pin/cvt connections, correct?
>
> Devid != 0 is the same as devid = 0. And there is only pin/cvt connections.
>
> My basic idea is:
> 1. Every device entry use a virtual pin
OK.
> 2. Each device entry will be initialized at bootup even it is not in MST mode.
Here starts unclearness. How "*each* device entry" will be enumerated
at boot up time? The device id can be arbitrary numbers, per spec.
And "even it is not in MST" means the devices that aren't capable MST
(like SandyBridge HDMI/DP)? Or it's meant "even when no monitor is
connected via MST"?
> This means after bootup, mode change will not add/remove the per_pin.
What here "mode" means exactly?
> 3. All device entries under the same pin will use the same connection list.
> And the connection list is initialized at bootup. It will be saved in
> per_pin->mux_nids. This will never be changed after bootup.
> (However the pin-cvt connection is dynamically assigned when necessary.)
>
> >
> > > For the pin and cvt actual connection, it is dynamically assigned.
> >
> > ... but you still want to cache this to an (ever-growing) array for the standard
> > connection lists. That's what I was concerned.
>
> Do you mean codec->conn_list? If we don't use the dev_id and all
> the device entries under the same pin use the same one, it will be
> exactly the same as before.
But you touch the whole codes doing the connection list caching even
by adding the dev_id field. Why this is needed if you don't use the
dev_id...?
Again, try not to mess up the core code as a start. The virtual pin
concept is OK as long as it's self-contained in the HDMI codec
driver. And adding some basic calls to handle the device id is fine.
But the way to change as in patch 1 can't be included without the
enough justification.
thanks,
Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 0/3] support DP MST audio
2016-09-28 7:57 ` Takashi Iwai
@ 2016-09-28 8:18 ` Yang, Libin
0 siblings, 0 replies; 14+ messages in thread
From: Yang, Libin @ 2016-09-28 8:18 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Lin, Mengdong, libin.yang, alsa-devel
Hi Takashi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, September 28, 2016 3:57 PM
> To: Yang, Libin <libin.yang@intel.com>
> Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin, Mengdong
> <mengdong.lin@intel.com>
> Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
>
> On Wed, 28 Sep 2016 04:50:53 +0200,
> Yang, Libin wrote:
> >
> > Hi Takashi,
> >
> > > > > If so, now my question again: for devid!=0, which connections
> > > > > are given statically? In other words, how many devid would be
> passed?
> > > >
> > > > For the connection list, it is static. It will be initialized at bootup.
> > >
> > > So this is only about devid=0, right? For devid!=0, there wouldn't
> > > be anything else than pin/cvt connections, correct?
> >
> > Devid != 0 is the same as devid = 0. And there is only pin/cvt connections.
> >
> > My basic idea is:
> > 1. Every device entry use a virtual pin
>
> OK.
>
> > 2. Each device entry will be initialized at bootup even it is not in MST mode.
>
> Here starts unclearness. How "*each* device entry" will be enumerated at
> boot up time? The device id can be arbitrary numbers, per spec.
This is a good question. We really met such issue. We currently is to judge
whether it is support MST or not (if it is haswell_plus, then it supports MST).
We didn't use snd_hda_get_num_devices() to get the device number.
And if it is MST, then everything works well, each per_pin represents a device
entry.
If it is NON-MST, the device id means nothing, and snd_hda_set_dev_select()
does nothing. So we will use the first device entry (dev_id = 0) to represent
the pin.
>
> And "even it is not in MST" means the devices that aren't capable MST (like
> SandyBridge HDMI/DP)? Or it's meant "even when no monitor is connected
> via MST"?
I mean " even when no monitor is connected via MST". Actually, when
initialization, driver will judge the platform supports MST or not (for intel,
only haswell_plus support MST).
>
> > This means after bootup, mode change will not add/remove the per_pin.
>
> What here "mode" means exactly?
Mode means MST mode or NON_MST mode.
>
> > 3. All device entries under the same pin will use the same connection list.
> > And the connection list is initialized at bootup. It will be saved in
> > per_pin->mux_nids. This will never be changed after bootup.
> > (However the pin-cvt connection is dynamically assigned when
> > necessary.)
> >
> > >
> > > > For the pin and cvt actual connection, it is dynamically assigned.
> > >
> > > ... but you still want to cache this to an (ever-growing) array for
> > > the standard connection lists. That's what I was concerned.
> >
> > Do you mean codec->conn_list? If we don't use the dev_id and all the
> > device entries under the same pin use the same one, it will be exactly
> > the same as before.
>
> But you touch the whole codes doing the connection list caching even by
> adding the dev_id field. Why this is needed if you don't use the dev_id...?
>
> Again, try not to mess up the core code as a start. The virtual pin concept is
> OK as long as it's self-contained in the HDMI codec driver. And adding some
> basic calls to handle the device id is fine.
> But the way to change as in patch 1 can't be included without the enough
> justification.
Yes, I understand and I will refine the patch and not touch the
connection list core code in next patchset. In the new patch, dev_id
will not be used in connection list. I will totally remove the patch 1.
Regards,
Libin
>
>
> thanks,
>
> Takashi
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-09-28 8:18 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-26 8:35 [RFC PATCH 0/3] support DP MST audio libin.yang
2016-09-26 8:35 ` [RFC PATCH 1/3] ALSA: hda - codec add DP MST support for connection list libin.yang
2016-09-26 8:35 ` [RFC PATCH 2/3] ALSA: hda - add DP mst verb support libin.yang
2016-09-26 8:35 ` [RFC PATCH 3/3] ALSA: hda - add DP mst audio support libin.yang
2016-09-26 15:10 ` [RFC PATCH 0/3] support DP MST audio Takashi Iwai
2016-09-26 16:07 ` Yang, Libin
2016-09-26 17:50 ` Takashi Iwai
2016-09-27 7:47 ` Yang, Libin
2016-09-27 7:58 ` Takashi Iwai
2016-09-27 8:18 ` Yang, Libin
2016-09-27 8:45 ` Takashi Iwai
2016-09-28 2:50 ` Yang, Libin
2016-09-28 7:57 ` Takashi Iwai
2016-09-28 8:18 ` Yang, Libin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.