[PATCH 2/2] vgaswitchreoo: set audio client id in vgaswitchreoo enable function

[jimqu]

在 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