On Fri, Jul 13, 2018 at 04:06:02PM +0800, Jim Qu wrote:
> 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 <Jim.Qu at amd.com>
The approach you've taken in this patch won't work if the HDA controller
is bound to a driver after both GPUs have already been bound to a driver
and the handler has already been registered.
That's because vga_switcheroo_enable() is run when two GPUs have registered
as clients and the handler has also registered. If the HDA controller is
probed afterwards, its ID will be stuck at VGA_SWITCHEROO_UNKNOWN_ID.
Before resubmitting this patch, please run it through scripts/checkpatch.pl,
there are multiple trivial issues here.
Also, please use scripts/get_maintainers.pl on all files you're touching
and cc the patch to the listed maintainers and reviewers.
> @@ -161,9 +163,8 @@ struct vgasr_priv {
> };
>
> #define ID_BIT_AUDIO 0x100
> -#define client_is_audio(c) ((c)->id & ID_BIT_AUDIO)
> -#define client_is_vga(c) ((c)->id == VGA_SWITCHEROO_UNKNOWN_ID || \
> - !client_is_audio(c))
> +#define client_is_audio(c) ((c)->id & ID_BIT_AUDIO)
> +#define client_is_vga(c) (!client_is_audio(c))
> #define client_id(c) ((c)->id & ~ID_BIT_AUDIO)
There's a spurious whitespace change in the client_is_audio() line,
please get rid of it.
> --- a/include/linux/vga_switcheroo.h
> +++ b/include/linux/vga_switcheroo.h
> @@ -84,8 +84,8 @@ enum vga_switcheroo_state {
> * Client identifier. Audio clients use the same identifier & 0x100.
> */
> enum vga_switcheroo_client_id {
> - VGA_SWITCHEROO_UNKNOWN_ID = -1,
> - VGA_SWITCHEROO_IGD,
> + VGA_SWITCHEROO_UNKNOWN_ID = 0x1000,
> + VGA_SWITCHEROO_IGD = 0,
Hm, why is this change necessary?
Thanks,
Lukas
