[PATCH 2/2] vgaswitchreoo: set audio client id in vgaswitchreoo enable function
jimqu
jimqu at amd.com
Fri Jul 13 15:34:53 UTC 2018
在 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.
>
> 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.
>
Yeah, I will correct it in later update.
>> --- 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, but now we also use it on audio client. Because of -1 in memory
is presented as 0xffffffffffffffff, so the 0xffffffffffffffff &
ID_BIT_AUDIO = ID_BIT_AUDIO, this will confuse people if it used on gpu
client. that audio identify bit is set.
Thanks
JimQu
>
> Thanks,
>
> Lukas
More information about the dri-devel
mailing list