[PATCH 0/2] Audio client support for VGA-switcheroo

[PATCH 0/2] Audio client support for VGA-switcheroo

[Takashi Iwai]

Dave,

this is a small patch series to add the support for audio clients
to VGA switcheroo.  The background problem is that the HD-audio HDMI
controller of the discrete GPU is also disabled when the graphics core
is disabled by vga-switcheroo.  This leads to a long stall of the
audio driver, or at worst, it Oopses.
      https://bugzilla.kernel.org/show_bug.cgi?id=43155

I tried to work around it only in the audio driver side, but it
doesn't seem to help.  When the graphics is disabled, the whole PCI
entry can't be accessed any longer.  Even reading a config triggers an
Oops.  So, the only option is to implement the VGA switcheroo client
for the audio.

The first patch refactors the vga-switcheroo code to be ready to
accept for more than two clients.  It's mostly a clean up using linked
lists.  The next patch implements the audio client registration.

BTW, if these patches are OK, the question remains how to merge.
Obviously I'd need more patches for HD-audio driver based on the new
vga_switcheroo_register_audio_client().  One option is to apply these
patches to a persistent branch (i.e. no rebase planned) of your tree
and pull it to my tree with more patches for HD-audio.
Does it sound good for you?


thanks,

Takashi

[PATCH 1/2] vga_switcheroo: Refactor using linked list

[Takashi Iwai]

Refactor the code base a bit for the further work to adapt more clients.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/vga/vga_switcheroo.c |  209 ++++++++++++++++++++------------------
 1 file changed, 110 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 58434e8..c91573f 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -37,6 +37,7 @@ struct vga_switcheroo_client {
 	bool (*can_switch)(struct pci_dev *pdev);
 	int id;
 	bool active;
+	struct list_head list;
 };
 
 static DEFINE_MUTEX(vgasr_mutex);
@@ -51,7 +52,7 @@ struct vgasr_priv {
 	struct dentry *switch_file;
 
 	int registered_clients;
-	struct vga_switcheroo_client clients[VGA_SWITCHEROO_MAX_CLIENTS];
+	struct list_head clients;
 
 	struct vga_switcheroo_handler *handler;
 };
@@ -60,7 +61,9 @@ static int vga_switcheroo_debugfs_init(struct vgasr_priv *priv);
 static void vga_switcheroo_debugfs_fini(struct vgasr_priv *priv);
 
 /* only one switcheroo per system */
-static struct vgasr_priv vgasr_priv;
+static struct vgasr_priv vgasr_priv = {
+	.clients = LIST_HEAD_INIT(vgasr_priv.clients),
+};
 
 int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler)
 {
@@ -86,17 +89,18 @@ EXPORT_SYMBOL(vga_switcheroo_unregister_handler);
 
 static void vga_switcheroo_enable(void)
 {
-	int i;
 	int ret;
+	struct vga_switcheroo_client *client;
+
 	/* call the handler to init */
 	vgasr_priv.handler->init();
 
-	for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-		ret = vgasr_priv.handler->get_client_id(vgasr_priv.clients[i].pdev);
+	list_for_each_entry(client, &vgasr_priv.clients, list) {
+		ret = vgasr_priv.handler->get_client_id(client->pdev);
 		if (ret < 0)
 			return;
 
-		vgasr_priv.clients[i].id = ret;
+		client->id = ret;
 	}
 	vga_switcheroo_debugfs_init(&vgasr_priv);
 	vgasr_priv.active = true;
@@ -107,28 +111,27 @@ int vga_switcheroo_register_client(struct pci_dev *pdev,
 				   void (*reprobe)(struct pci_dev *pdev),
 				   bool (*can_switch)(struct pci_dev *pdev))
 {
-	int index;
+	struct vga_switcheroo_client *client;
 
-	mutex_lock(&vgasr_mutex);
-	/* don't do IGD vs DIS here */
-	if (vgasr_priv.registered_clients & 1)
-		index = 1;
-	else
-		index = 0;
-
-	vgasr_priv.clients[index].pwr_state = VGA_SWITCHEROO_ON;
-	vgasr_priv.clients[index].pdev = pdev;
-	vgasr_priv.clients[index].set_gpu_state = set_gpu_state;
-	vgasr_priv.clients[index].reprobe = reprobe;
-	vgasr_priv.clients[index].can_switch = can_switch;
-	vgasr_priv.clients[index].id = -1;
+	client = kzalloc(sizeof(*client), GFP_KERNEL);
+	if (!client)
+		return -ENOMEM;
+
+	client->pwr_state = VGA_SWITCHEROO_ON;
+	client->pdev = pdev;
+	client->set_gpu_state = set_gpu_state;
+	client->reprobe = reprobe;
+	client->can_switch = can_switch;
+	client->id = -1;
 	if (pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW)
-		vgasr_priv.clients[index].active = true;
+		client->active = true;
 
-	vgasr_priv.registered_clients |= (1 << index);
+	mutex_lock(&vgasr_mutex);
+	list_add_tail(&client->list, &vgasr_priv.clients);
+	vgasr_priv.registered_clients++;
 
 	/* if we get two clients + handler */
-	if (vgasr_priv.registered_clients == 0x3 && vgasr_priv.handler) {
+	if (vgasr_priv.registered_clients == 2 && vgasr_priv.handler) {
 		printk(KERN_INFO "vga_switcheroo: enabled\n");
 		vga_switcheroo_enable();
 	}
@@ -137,18 +140,47 @@ int vga_switcheroo_register_client(struct pci_dev *pdev,
 }
 EXPORT_SYMBOL(vga_switcheroo_register_client);
 
+static struct vga_switcheroo_client *
+find_client_from_pci(struct list_head *head, struct pci_dev *pdev)
+{
+	struct vga_switcheroo_client *client;
+	list_for_each_entry(client, head, list)
+		if (client->pdev == pdev)
+			return client;
+	return NULL;
+}
+
+static struct vga_switcheroo_client *
+find_client_from_id(struct list_head *head, int client_id)
+{
+	struct vga_switcheroo_client *client;
+	list_for_each_entry(client, head, list)
+		if (client->id == client_id)
+			return client;
+	return NULL;
+}
+
+static struct vga_switcheroo_client *
+find_active_client(struct list_head *head)
+{
+	struct vga_switcheroo_client *client;
+	list_for_each_entry(client, head, list)
+		if (client->active == true)
+			return client;
+	return NULL;
+}
+
 void vga_switcheroo_unregister_client(struct pci_dev *pdev)
 {
-	int i;
+	struct vga_switcheroo_client *client;
 
 	mutex_lock(&vgasr_mutex);
-	for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-		if (vgasr_priv.clients[i].pdev == pdev) {
-			vgasr_priv.registered_clients &= ~(1 << i);
-			break;
-		}
+	client = find_client_from_pci(&vgasr_priv.clients, pdev);
+	if (client) {
+		list_del(&client->list);
+		kfree(client);
+		vgasr_priv.registered_clients--;
 	}
-
 	printk(KERN_INFO "vga_switcheroo: disabled\n");
 	vga_switcheroo_debugfs_fini(&vgasr_priv);
 	vgasr_priv.active = false;
@@ -159,29 +191,28 @@ EXPORT_SYMBOL(vga_switcheroo_unregister_client);
 void vga_switcheroo_client_fb_set(struct pci_dev *pdev,
 				 struct fb_info *info)
 {
-	int i;
+	struct vga_switcheroo_client *client;
 
 	mutex_lock(&vgasr_mutex);
-	for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-		if (vgasr_priv.clients[i].pdev == pdev) {
-			vgasr_priv.clients[i].fb_info = info;
-			break;
-		}
-	}
+	client = find_client_from_pci(&vgasr_priv.clients, pdev);
+	if (client)
+		client->fb_info = info;
 	mutex_unlock(&vgasr_mutex);
 }
 EXPORT_SYMBOL(vga_switcheroo_client_fb_set);
 
 static int vga_switcheroo_show(struct seq_file *m, void *v)
 {
-	int i;
+	struct vga_switcheroo_client *client;
+	int i = 0;
 	mutex_lock(&vgasr_mutex);
-	for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
+	list_for_each_entry(client, &vgasr_priv.clients, list) {
 		seq_printf(m, "%d:%s:%c:%s:%s\n", i,
-			   vgasr_priv.clients[i].id == VGA_SWITCHEROO_DIS ? "DIS" : "IGD",
-			   vgasr_priv.clients[i].active ? '+' : ' ',
-			   vgasr_priv.clients[i].pwr_state ? "Pwr" : "Off",
-			   pci_name(vgasr_priv.clients[i].pdev));
+			   client->id == VGA_SWITCHEROO_DIS ? "DIS" : "IGD",
+			   client->active ? '+' : ' ',
+			   client->pwr_state ? "Pwr" : "Off",
+			   pci_name(client->pdev));
+		i++;
 	}
 	mutex_unlock(&vgasr_mutex);
 	return 0;
@@ -215,15 +246,9 @@ static int vga_switchoff(struct vga_switcheroo_client *client)
 /* stage one happens before delay */
 static int vga_switchto_stage1(struct vga_switcheroo_client *new_client)
 {
-	int i;
-	struct vga_switcheroo_client *active = NULL;
+	struct vga_switcheroo_client *active;
 
-	for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-		if (vgasr_priv.clients[i].active == true) {
-			active = &vgasr_priv.clients[i];
-			break;
-		}
-	}
+	active = find_active_client(&vgasr_priv.clients);
 	if (!active)
 		return 0;
 
@@ -240,15 +265,9 @@ static int vga_switchto_stage1(struct vga_switcheroo_client *new_client)
 static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 {
 	int ret;
-	int i;
-	struct vga_switcheroo_client *active = NULL;
+	struct vga_switcheroo_client *active;
 
-	for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-		if (vgasr_priv.clients[i].active == true) {
-			active = &vgasr_priv.clients[i];
-			break;
-		}
-	}
+	active = find_active_client(&vgasr_priv.clients);
 	if (!active)
 		return 0;
 
@@ -274,13 +293,26 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 	return 0;
 }
 
+static bool check_can_switch(void)
+{
+	struct vga_switcheroo_client *client;
+
+	list_for_each_entry(client, &vgasr_priv.clients, list) {
+		if (!client->can_switch(client->pdev)) {
+			printk(KERN_ERR "vga_switcheroo: client %x refused switch\n", client->id);
+			return false;
+		}
+	}
+	return true;
+}
+
 static ssize_t
 vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
 			     size_t cnt, loff_t *ppos)
 {
 	char usercmd[64];
 	const char *pdev_name;
-	int i, ret;
+	int ret;
 	bool delay = false, can_switch;
 	bool just_mux = false;
 	int client_id = -1;
@@ -301,21 +333,21 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
 
 	/* pwr off the device not in use */
 	if (strncmp(usercmd, "OFF", 3) == 0) {
-		for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-			if (vgasr_priv.clients[i].active)
+		list_for_each_entry(client, &vgasr_priv.clients, list) {
+			if (client->active)
 				continue;
-			if (vgasr_priv.clients[i].pwr_state == VGA_SWITCHEROO_ON)
-				vga_switchoff(&vgasr_priv.clients[i]);
+			if (client->pwr_state == VGA_SWITCHEROO_ON)
+				vga_switchoff(client);
 		}
 		goto out;
 	}
 	/* pwr on the device not in use */
 	if (strncmp(usercmd, "ON", 2) == 0) {
-		for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-			if (vgasr_priv.clients[i].active)
+		list_for_each_entry(client, &vgasr_priv.clients, list) {
+			if (client->active)
 				continue;
-			if (vgasr_priv.clients[i].pwr_state == VGA_SWITCHEROO_OFF)
-				vga_switchon(&vgasr_priv.clients[i]);
+			if (client->pwr_state == VGA_SWITCHEROO_OFF)
+				vga_switchon(client);
 		}
 		goto out;
 	}
@@ -348,13 +380,9 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
 
 	if (client_id == -1)
 		goto out;
-
-	for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-		if (vgasr_priv.clients[i].id == client_id) {
-			client = &vgasr_priv.clients[i];
-			break;
-		}
-	}
+	client = find_client_from_id(&vgasr_priv.clients, client_id);
+	if (!client)
+		goto out;
 
 	vgasr_priv.delayed_switch_active = false;
 
@@ -363,23 +391,16 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
 		goto out;
 	}
 
-	if (client->active == true)
+	if (client->active)
 		goto out;
 
 	/* okay we want a switch - test if devices are willing to switch */
-	can_switch = true;
-	for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-		can_switch = vgasr_priv.clients[i].can_switch(vgasr_priv.clients[i].pdev);
-		if (can_switch == false) {
-			printk(KERN_ERR "vga_switcheroo: client %d refused switch\n", i);
-			break;
-		}
-	}
+	can_switch = check_can_switch();
 
 	if (can_switch == false && delay == false)
 		goto out;
 
-	if (can_switch == true) {
+	if (can_switch) {
 		pdev_name = pci_name(client->pdev);
 		ret = vga_switchto_stage1(client);
 		if (ret)
@@ -451,10 +472,8 @@ fail:
 
 int vga_switcheroo_process_delayed_switch(void)
 {
-	struct vga_switcheroo_client *client = NULL;
+	struct vga_switcheroo_client *client;
 	const char *pdev_name;
-	bool can_switch = true;
-	int i;
 	int ret;
 	int err = -EINVAL;
 
@@ -464,17 +483,9 @@ int vga_switcheroo_process_delayed_switch(void)
 
 	printk(KERN_INFO "vga_switcheroo: processing delayed switch to %d\n", vgasr_priv.delayed_client_id);
 
-	for (i = 0; i < VGA_SWITCHEROO_MAX_CLIENTS; i++) {
-		if (vgasr_priv.clients[i].id == vgasr_priv.delayed_client_id)
-			client = &vgasr_priv.clients[i];
-		can_switch = vgasr_priv.clients[i].can_switch(vgasr_priv.clients[i].pdev);
-		if (can_switch == false) {
-			printk(KERN_ERR "vga_switcheroo: client %d refused switch\n", i);
-			break;
-		}
-	}
-
-	if (can_switch == false || client == NULL)
+	client = find_client_from_id(&vgasr_priv.clients,
+				     vgasr_priv.delayed_client_id);
+	if (!client || !check_can_switch())
 		goto err;
 
 	pdev_name = pci_name(client->pdev);
-- 
1.7.9.2

[PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Takashi Iwai]

Add the support for audio clients to VGA-switcheroo for handling the
HDMI audio controller together with VGA switching.  The id of the
audio controller should be given explicity at registration time unlike
the video controller.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43155

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/vga/vga_switcheroo.c |   84 ++++++++++++++++++++++++++++++--------
 include/linux/vga_switcheroo.h   |   10 +++++
 2 files changed, 78 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index c91573f..df25ea6 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -57,6 +57,11 @@ struct vgasr_priv {
 	struct vga_switcheroo_handler *handler;
 };
 
+#define ID_BIT_AUDIO		0x100
+#define client_is_audio(c)	((c)->id & ID_BIT_AUDIO)
+#define client_is_vga(c)	((c)->id == -1 || !client_is_audio(c))
+#define client_id(c)		((c)->id & ~ID_BIT_AUDIO)
+
 static int vga_switcheroo_debugfs_init(struct vgasr_priv *priv);
 static void vga_switcheroo_debugfs_fini(struct vgasr_priv *priv);
 
@@ -96,6 +101,8 @@ static void vga_switcheroo_enable(void)
 	vgasr_priv.handler->init();
 
 	list_for_each_entry(client, &vgasr_priv.clients, list) {
+		if (client->id != -1)
+			continue;
 		ret = vgasr_priv.handler->get_client_id(client->pdev);
 		if (ret < 0)
 			return;
@@ -106,10 +113,11 @@ static void vga_switcheroo_enable(void)
 	vgasr_priv.active = true;
 }
 
-int vga_switcheroo_register_client(struct pci_dev *pdev,
-				   void (*set_gpu_state)(struct pci_dev *pdev, enum vga_switcheroo_state),
-				   void (*reprobe)(struct pci_dev *pdev),
-				   bool (*can_switch)(struct pci_dev *pdev))
+static int register_client(struct pci_dev *pdev,
+	void (*set_gpu_state)(struct pci_dev *pdev, enum vga_switcheroo_state),
+	void (*reprobe)(struct pci_dev *pdev),
+	bool (*can_switch)(struct pci_dev *pdev),
+	int id, bool active)
 {
 	struct vga_switcheroo_client *client;
 
@@ -122,24 +130,48 @@ int vga_switcheroo_register_client(struct pci_dev *pdev,
 	client->set_gpu_state = set_gpu_state;
 	client->reprobe = reprobe;
 	client->can_switch = can_switch;
-	client->id = -1;
-	if (pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW)
-		client->active = true;
+	client->id = id;
+	client->active = active;
 
 	mutex_lock(&vgasr_mutex);
 	list_add_tail(&client->list, &vgasr_priv.clients);
-	vgasr_priv.registered_clients++;
+	if (client_is_vga(client))
+		vgasr_priv.registered_clients++;
 
 	/* if we get two clients + handler */
-	if (vgasr_priv.registered_clients == 2 && vgasr_priv.handler) {
+	if (!vgasr_priv.active &&
+	    vgasr_priv.registered_clients == 2 && vgasr_priv.handler) {
 		printk(KERN_INFO "vga_switcheroo: enabled\n");
 		vga_switcheroo_enable();
 	}
 	mutex_unlock(&vgasr_mutex);
 	return 0;
 }
+
+int vga_switcheroo_register_client(struct pci_dev *pdev,
+				   void (*set_gpu_state)(struct pci_dev *pdev, enum vga_switcheroo_state),
+				   void (*reprobe)(struct pci_dev *pdev),
+				   bool (*can_switch)(struct pci_dev *pdev))
+{
+	bool active = false;
+	if (pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW)
+		active = true;
+	return register_client(pdev, set_gpu_state, reprobe, can_switch,
+			       -1, active);
+}
 EXPORT_SYMBOL(vga_switcheroo_register_client);
 
+int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
+	void (*set_gpu_state)(struct pci_dev *pdev, enum vga_switcheroo_state),
+	void (*reprobe)(struct pci_dev *pdev),
+	bool (*can_switch)(struct pci_dev *pdev),
+	int id, bool active)
+{
+	return register_client(pdev, set_gpu_state, reprobe, can_switch,
+			       id | ID_BIT_AUDIO, active);
+}
+EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
+
 static struct vga_switcheroo_client *
 find_client_from_pci(struct list_head *head, struct pci_dev *pdev)
 {
@@ -165,7 +197,7 @@ find_active_client(struct list_head *head)
 {
 	struct vga_switcheroo_client *client;
 	list_for_each_entry(client, head, list)
-		if (client->active == true)
+		if (client->active && client_is_vga(client))
 			return client;
 	return NULL;
 }
@@ -177,13 +209,16 @@ void vga_switcheroo_unregister_client(struct pci_dev *pdev)
 	mutex_lock(&vgasr_mutex);
 	client = find_client_from_pci(&vgasr_priv.clients, pdev);
 	if (client) {
+		if (client_is_vga(client))
+			vgasr_priv.registered_clients--;
 		list_del(&client->list);
 		kfree(client);
-		vgasr_priv.registered_clients--;
 	}
-	printk(KERN_INFO "vga_switcheroo: disabled\n");
-	vga_switcheroo_debugfs_fini(&vgasr_priv);
-	vgasr_priv.active = false;
+	if (vgasr_priv.active && vgasr_priv.registered_clients < 2) {
+		printk(KERN_INFO "vga_switcheroo: disabled\n");
+		vga_switcheroo_debugfs_fini(&vgasr_priv);
+		vgasr_priv.active = false;
+	}
 	mutex_unlock(&vgasr_mutex);
 }
 EXPORT_SYMBOL(vga_switcheroo_unregister_client);
@@ -207,8 +242,9 @@ static int vga_switcheroo_show(struct seq_file *m, void *v)
 	int i = 0;
 	mutex_lock(&vgasr_mutex);
 	list_for_each_entry(client, &vgasr_priv.clients, list) {
-		seq_printf(m, "%d:%s:%c:%s:%s\n", i,
-			   client->id == VGA_SWITCHEROO_DIS ? "DIS" : "IGD",
+		seq_printf(m, "%d:%s%s:%c:%s:%s\n", i,
+			   client_id(client) == VGA_SWITCHEROO_DIS ? "DIS" : "IGD",
+			   client_is_vga(client) ? "" : "-Audio",
 			   client->active ? '+' : ' ',
 			   client->pwr_state ? "Pwr" : "Off",
 			   pci_name(client->pdev));
@@ -243,6 +279,17 @@ static int vga_switchoff(struct vga_switcheroo_client *client)
 	return 0;
 }
 
+static void set_audio_state(int id, int state)
+{
+	struct vga_switcheroo_client *client;
+
+	client = find_client_from_id(&vgasr_priv.clients, id | ID_BIT_AUDIO);
+	if (client && client->pwr_state != state) {
+		client->set_gpu_state(client->pdev, state);
+		client->pwr_state = state;
+	}
+}
+
 /* stage one happens before delay */
 static int vga_switchto_stage1(struct vga_switcheroo_client *new_client)
 {
@@ -258,6 +305,9 @@ static int vga_switchto_stage1(struct vga_switcheroo_client *new_client)
 	/* swap shadow resource to denote boot VGA device has changed so X starts on new device */
 	active->pdev->resource[PCI_ROM_RESOURCE].flags &= ~IORESOURCE_ROM_SHADOW;
 	new_client->pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+
+	set_audio_state(new_client->id, VGA_SWITCHEROO_ON);
+
 	return 0;
 }
 
@@ -286,6 +336,8 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 	if (new_client->reprobe)
 		new_client->reprobe(new_client->pdev);
 
+	set_audio_state(active->id, VGA_SWITCHEROO_OFF);
+
 	if (active->pwr_state == VGA_SWITCHEROO_ON)
 		vga_switchoff(active);
 
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index 4b9a7f5..fcbe175 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -35,6 +35,11 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
 				   void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state),
 				   void (*reprobe)(struct pci_dev *dev),
 				   bool (*can_switch)(struct pci_dev *dev));
+int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
+	void (*set_gpu_state)(struct pci_dev *pdev, enum vga_switcheroo_state),
+	void (*reprobe)(struct pci_dev *pdev),
+	bool (*can_switch)(struct pci_dev *pdev),
+	int id, bool active);
 
 void vga_switcheroo_client_fb_set(struct pci_dev *dev,
 				  struct fb_info *info);
@@ -53,6 +58,11 @@ static inline int vga_switcheroo_register_client(struct pci_dev *dev,
 					  bool (*can_switch)(struct pci_dev *dev)) { return 0; }
 static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info) {}
 static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
+static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
+	void (*set_gpu_state)(struct pci_dev *pdev, enum vga_switcheroo_state),
+	void (*reprobe)(struct pci_dev *pdev),
+	bool (*can_switch)(struct pci_dev *pdev),
+	int id, bool active) { return 0; }
 static inline void vga_switcheroo_unregister_handler(void) {}
 static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
 
-- 
1.7.9.2

Re: [PATCH 0/2] Audio client support for VGA-switcheroo

[Takashi Iwai]

At Thu, 10 May 2012 09:08:38 +0200,
Takashi Iwai wrote:
> 
> Dave,
> 
> this is a small patch series to add the support for audio clients
> to VGA switcheroo.  The background problem is that the HD-audio HDMI
> controller of the discrete GPU is also disabled when the graphics core
> is disabled by vga-switcheroo.  This leads to a long stall of the
> audio driver, or at worst, it Oopses.
>       https://bugzilla.kernel.org/show_bug.cgi?id=43155
> 
> I tried to work around it only in the audio driver side, but it
> doesn't seem to help.  When the graphics is disabled, the whole PCI
> entry can't be accessed any longer.  Even reading a config triggers an
> Oops.  So, the only option is to implement the VGA switcheroo client
> for the audio.
> 
> The first patch refactors the vga-switcheroo code to be ready to
> accept for more than two clients.  It's mostly a clean up using linked
> lists.  The next patch implements the audio client registration.
> 
> BTW, if these patches are OK, the question remains how to merge.
> Obviously I'd need more patches for HD-audio driver based on the new
> vga_switcheroo_register_audio_client().  One option is to apply these
> patches to a persistent branch (i.e. no rebase planned) of your tree
> and pull it to my tree with more patches for HD-audio.
> Does it sound good for you?

Or, if you prefer, just pull from topic/vga-switcheroo branch of my tree:
  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git topic/vga-switcheroo


Takashi

Re: [PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Paul Menzel]

[-- Attachment #1.1: Type: text/plain, Size: 772 bytes --]

Dear Takashi,


thank you for your patches.

Am Donnerstag, den 10.05.2012, 09:10 +0200 schrieb Takashi Iwai:
> Add the support for audio clients to VGA-switcheroo for handling the
> HDMI audio controller together with VGA switching.  The id of the
> audio controller should be given explicity at registration time unlike

explicit*l*y

> the video controller.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43155

Is there a way (command sequence) to test this patch?

> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/vga/vga_switcheroo.c |   84 ++++++++++++++++++++++++++++++--------
>  include/linux/vga_switcheroo.h   |   10 +++++
>  2 files changed, 78 insertions(+), 16 deletions(-)

[…]


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Takashi Iwai]

At Thu, 10 May 2012 10:23:26 +0200,
Paul Menzel wrote:
> 
> Dear Takashi,
> 
> 
> thank you for your patches.
> 
> Am Donnerstag, den 10.05.2012, 09:10 +0200 schrieb Takashi Iwai:
> > Add the support for audio clients to VGA-switcheroo for handling the
> > HDMI audio controller together with VGA switching.  The id of the
> > audio controller should be given explicity at registration time unlike
> 
> explicit*l*y

Thanks, fixed now in topic/vga-switcheroo branch.
Will resend the patch if necessary.

> > the video controller.
> > 
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43155
> 
> Is there a way (command sequence) to test this patch?

The normal on/off of D-GPU via debugfs should work.


Takashi

> 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/gpu/vga/vga_switcheroo.c |   84 ++++++++++++++++++++++++++++++--------
> >  include/linux/vga_switcheroo.h   |   10 +++++
> >  2 files changed, 78 insertions(+), 16 deletions(-)
> 
> […]
> 
> 
> Thanks,
> 
> Paul
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Dave Airlie]

On Thu, May 10, 2012 at 8:10 AM, Takashi Iwai <tiwai@suse.de> wrote:
> Add the support for audio clients to VGA-switcheroo for handling the
> HDMI audio controller together with VGA switching.  The id of the
> audio controller should be given explicity at registration time unlike
> the video controller.

The only question I really have is how you assign the audio ids.

Currently we mostly don't see > 1 HDMI audio on a GPU, but that
should be changing with the latest ones.

Otherwise I'm happy to pull that branch from you.

Dave.

Re: [PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Alan Cox]

On Thu, 10 May 2012 09:10:16 +0200
Takashi Iwai <tiwai@suse.de> wrote:

> Add the support for audio clients to VGA-switcheroo for handling the
> HDMI audio controller together with VGA switching.  The id of the
> audio controller should be given explicity at registration time unlike
> the video controller.

It would I think be a lot cleaner and more future proof to pass a 

struct hdmi_audio_switch_ops

or some similar named object with an array of function pointers ?

Alan

Re: [PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Takashi Iwai]

At Thu, 10 May 2012 12:20:05 +0100,
Alan Cox wrote:
> 
> On Thu, 10 May 2012 09:10:16 +0200
> Takashi Iwai <tiwai@suse.de> wrote:
> 
> > Add the support for audio clients to VGA-switcheroo for handling the
> > HDMI audio controller together with VGA switching.  The id of the
> > audio controller should be given explicity at registration time unlike
> > the video controller.
> 
> It would I think be a lot cleaner and more future proof to pass a 
> 
> struct hdmi_audio_switch_ops
> 
> or some similar named object with an array of function pointers ?

That would be a good option, indeed.

In my patch series, I just didn't want to break the existing API,
so I kept the current style.


Takashi

Re: [PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Takashi Iwai]

At Thu, 10 May 2012 11:40:50 +0100,
Dave Airlie wrote:
> 
> On Thu, May 10, 2012 at 8:10 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > Add the support for audio clients to VGA-switcheroo for handling the
> > HDMI audio controller together with VGA switching.  The id of the
> > audio controller should be given explicity at registration time unlike
> > the video controller.
> 
> The only question I really have is how you assign the audio ids.
> 
> Currently we mostly don't see > 1 HDMI audio on a GPU, but that
> should be changing with the latest ones.

There is one HDMI audio "controller" entry usually per graphics.
There are more than one codecs can be connected to the controller,
e.g. Nvidia has up to 8 codecs, but this is an issue inside HD-audio
driver, and irrelevant from VGA-switcheroo.


thanks,

Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Takashi Iwai]

At Thu, 10 May 2012 13:42:09 +0200,
Takashi Iwai wrote:
> 
> At Thu, 10 May 2012 12:20:05 +0100,
> Alan Cox wrote:
> > 
> > On Thu, 10 May 2012 09:10:16 +0200
> > Takashi Iwai <tiwai@suse.de> wrote:
> > 
> > > Add the support for audio clients to VGA-switcheroo for handling the
> > > HDMI audio controller together with VGA switching.  The id of the
> > > audio controller should be given explicity at registration time unlike
> > > the video controller.
> > 
> > It would I think be a lot cleaner and more future proof to pass a 
> > 
> > struct hdmi_audio_switch_ops
> > 
> > or some similar named object with an array of function pointers ?
> 
> That would be a good option, indeed.
> 
> In my patch series, I just didn't want to break the existing API,
> so I kept the current style.

Dave, do you prefer the way with an ops struct as Alan suggested?

For example, I can make like

struct vga_switcheroo_client_ops {
       void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state),
       void (*reprobe)(struct pci_dev *dev),
       bool (*can_switch)(struct pci_dev *dev));
};

and pass the pointer to vga_switcher_register_client() and
vga_switcher_register_audio_client().

If it's preferred, I'll work on it and resend you a pull request later.

BTW, I modified topic/vga-switcheroo branch of sound git tree again.
Now it contains only two commits I posted here.  The rest commits for
HD-audio are found in topic/hda-switcheroo branch.


thanks,

Takashi

Re: [PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Dave Airlie]

On Thu, May 10, 2012 at 7:42 PM, Takashi Iwai <tiwai@suse.de> wrote:
> At Thu, 10 May 2012 13:42:09 +0200,
> Takashi Iwai wrote:
>>
>> At Thu, 10 May 2012 12:20:05 +0100,
>> Alan Cox wrote:
>> >
>> > On Thu, 10 May 2012 09:10:16 +0200
>> > Takashi Iwai <tiwai@suse.de> wrote:
>> >
>> > > Add the support for audio clients to VGA-switcheroo for handling the
>> > > HDMI audio controller together with VGA switching.  The id of the
>> > > audio controller should be given explicity at registration time unlike
>> > > the video controller.
>> >
>> > It would I think be a lot cleaner and more future proof to pass a
>> >
>> > struct hdmi_audio_switch_ops
>> >
>> > or some similar named object with an array of function pointers ?
>>
>> That would be a good option, indeed.
>>
>> In my patch series, I just didn't want to break the existing API,
>> so I kept the current style.
>
> Dave, do you prefer the way with an ops struct as Alan suggested?
>
> For example, I can make like
>
> struct vga_switcheroo_client_ops {
>       void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state),
>       void (*reprobe)(struct pci_dev *dev),
>       bool (*can_switch)(struct pci_dev *dev));
> };

Yeah I suppose we should go and just do that now, it kinda grew a bit
much previously.

If you want to do patches for it I'd be happy to take them.

Dave.

Re: [PATCH 2/2] vga_switcheroo: Add the support for audio clients

[Takashi Iwai]

At Thu, 10 May 2012 20:05:25 +0100,
Dave Airlie wrote:
> 
> On Thu, May 10, 2012 at 7:42 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > At Thu, 10 May 2012 13:42:09 +0200,
> > Takashi Iwai wrote:
> >>
> >> At Thu, 10 May 2012 12:20:05 +0100,
> >> Alan Cox wrote:
> >> >
> >> > On Thu, 10 May 2012 09:10:16 +0200
> >> > Takashi Iwai <tiwai@suse.de> wrote:
> >> >
> >> > > Add the support for audio clients to VGA-switcheroo for handling the
> >> > > HDMI audio controller together with VGA switching.  The id of the
> >> > > audio controller should be given explicity at registration time unlike
> >> > > the video controller.
> >> >
> >> > It would I think be a lot cleaner and more future proof to pass a
> >> >
> >> > struct hdmi_audio_switch_ops
> >> >
> >> > or some similar named object with an array of function pointers ?
> >>
> >> That would be a good option, indeed.
> >>
> >> In my patch series, I just didn't want to break the existing API,
> >> so I kept the current style.
> >
> > Dave, do you prefer the way with an ops struct as Alan suggested?
> >
> > For example, I can make like
> >
> > struct vga_switcheroo_client_ops {
> >       void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state),
> >       void (*reprobe)(struct pci_dev *dev),
> >       bool (*can_switch)(struct pci_dev *dev));
> > };
> 
> Yeah I suppose we should go and just do that now, it kinda grew a bit
> much previously.
> 
> If you want to do patches for it I'd be happy to take them.

OK, I'll work on it tomorrow.


Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel