在 2018/7/13 17:27, Lukas Wunner 写道:
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@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.
In genernic speaking, there are two cases , a. audio client is the third client. b. audio client is not the third client.
if audio is third client. vga_switcheroo_ready() is true, vga_switcheroo_enable() can be called in audio client register fuction. if audio is not the third client, vga_switcheroo_enable() will be called in the second GFX client register. In vga_switcheroo_enable() , the first list loop will set two GPU clients' id, and the second list loop will select audio client, set id by its bound gpu pci dev.
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.
OK, will do it in next v2.
@@ -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.
Will update it in v2.
--- 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?
Before , we suppose VGA_SWITCHEROO_UNKNOWN_ID can be only used for gpu client, and now we also use it on the audio client. if we set VGA_SWITCHEROO_UNKNOWN_ID to gpu clients, client_is_audio(client) will be true, since -1 in memory is presented as 0xffffffffffffffff(if it is 64bit asic).
thanks JimQu
Thanks,
Lukas