<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">在 2018/7/14 0:38, Takashi Iwai 写道: </div> <blockquote type="cite" cite="mid:s5hbmbbm6q9.wl-tiwai@suse.de"> <pre wrap="">On Fri, 13 Jul 2018 17:25:39 +0200, jimqu wrote: </pre> <blockquote type="cite"> <pre wrap=""> 在 2018/7/13 16:46, Takashi Iwai 写道: </pre> <blockquote type="cite"> <pre wrap="">On Fri, 13 Jul 2018 10:06:02 +0200, Jim Qu wrote: </pre> <blockquote type="cite"> <pre wrap="">On modern laptop, there are more and more platforms have two GPUs, and each of them maybe have audio codec for HDMP/DP output. For some dGPU which is no output, audio codec usually is disabled. In currect HDA audio driver, it will set all codec as VGA_SWITCHEROO_DIS, the audio which is binded to UMA will be suspended if user use debugfs to contorl power In HDA driver side, it is difficult to know which GPU the audio has binded to. So set the bound gpu pci dev to vgaswitchroo, the correct audio id will be set in vgaswitchreoo enable function. Signed-off-by: Jim Qu <a class="moz-txt-link-rfc2396E" href="mailto:Jim.Qu@amd.com"><Jim.Qu@amd.com></a> </pre> </blockquote> <pre wrap="">The idea looks good to me. Just a nitpick: </pre> <blockquote type="cite"> <pre wrap="">@@ -1319,15 +1319,16 @@ static const struct vga_switcheroo_client_ops azx_vs_ops = { static int register_vga_switcheroo(struct azx *chip) { struct hda_intel *hda = container_of(chip, struct hda_intel, chip); + struct pci_dev * p; int err; if (!hda->use_vga_switcheroo) return 0; - /* FIXME: currently only handling DIS controller - * is there any machine with two switchable HDMI audio controllers? - */ - err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops, - VGA_SWITCHEROO_DIS); + + p = get_bound_vga(chip->pci); + err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops, p); + if (p) + pci_dev_put(p); </pre> </blockquote> <pre wrap="">Passing a NULL vga_dev won't work, so the NULL check after calling vga_switcheroo_register_audio_client() is moot. If any, it should be checked before that. </pre> </blockquote> <pre wrap=""> The hda->use_vga_switcheroo is set to 1 in init_vga_switcheroo() if bound vga pci dev is not NULL. Since the hda->use_vga_switcheroo is checked at beginning of the function. so the p which pass into vga_switcheroo_register_audio_client() is constant valid value. </pre> </blockquote> <pre wrap=""> So the NULL-check in the patch is also useless, no? :) </pre> </blockquote> Ha-ha, indeed, the NULL-check of p is unnecessary. will update in v2. Thanks JimQu <span style="color: rgb(255, 255, 255); font-family: Arial, "PingFang SC", "Hiragino Sans GB", STHeiti, "Microsoft YaHei", "WenQuanYi Micro Hei", sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: 2; text-align: left; text-indent: 0px; text-transform: none; white-space: nowrap; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(67, 149, 255); display: inline !important; float: none;"> <span style="color: rgb(255, 255, 255); font-family: Arial, "PingFang SC", "Hiragino Sans GB", STHeiti, "Microsoft YaHei", "WenQuanYi Micro Hei", sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: 2; text-align: left; text-indent: 0px; text-transform: none; white-space: nowrap; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(67, 149, 255); display: inline !important; float: none;"> <blockquote type="cite" cite="mid:s5hbmbbm6q9.wl-tiwai@suse.de"> <pre wrap=""> Takashi </pre> </blockquote> </body> </html>