å?¨ 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 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.
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
