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
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);
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; }
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.
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
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.
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
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.
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
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
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
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
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.
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
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
dri-devel@lists.freedesktop.org
-
Alan Cox -
Dave Airlie -
Paul Menzel -
Takashi Iwai