Re: 答复: 答复: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[jimqu]

On 2018年07月11日 17:04, Takashi Iwai wrote:
> On Wed, 11 Jul 2018 10:41:38 +0200,
> jimqu wrote:
>>
>>
>> On 2018年07月11日 15:19, Takashi Iwai wrote:
>>> On Tue, 10 Jul 2018 13:21:00 +0200,
>>> Takashi Iwai wrote:
>>>>> revert the fix of amdgpu suspend issue, audio issue also can be observed.
>>>> Did you check the behavior with the single AMD GPU hardware?
>>>> If confirmed, we can forget about vga_switcheroo.
>>> ... and taking a look back at the recent changes, I guess it can be
>>> the forced runtime PM enablement, not directly with vga_switcheroo
>>> action itself.
>> Yeah, the function vga_switcheroo_set_dynamic_switch() has discarded,
>> so there is no way GFX driver to control audio power. However, keep in
>> mind, current audio is bound to iGPU, that mean the issue should be
>> nothing about
>> vgaswtichreoo. since current audio pci bus is different from dGPU,
>> that means the pci_bus_set_current_state() in
>> vga_switcheroo_runtime_suspend() and pci_wakeup_bus() in
>> vga_switcheroo_runtime_resume() could not touch the audio pci power
>> state from dGPU instance.
>>
>> This is a feedback got from our OEM developer, it is the overview of
>> audio detect process.
>>
>>> First, the kernel  audio driver will be triggered to read ELD, if the
>>>> ELD is valid, it will report a jack event (on or available) to sound
>>>> core driver; the pulseaudio subscribe all jack events, if it is told
>>>> that the hdmi jack is plugged in (on), the pulseaudio will set this
>>>> port to available, then the pa-card or pa-sink has available port, it
>>>> can be selected (manually, some daemons or policy in
>>>> /usr/share/pulseaudio/alsa-mixer/) as default output card/default sink.
>> If the description is correct. I think there are maybe two problems.
>>
>> 1. audio will auto power off after setup device link duo to usage_count=0.
>> 2. duo to audio is power down, it could not get the HDMI jack insert event.
>>
>> How do you think?
>>
>>> Jim, could you tell me which PCI devices are handled as vga_switcheroo
>>> audio client?  The kernel should show all messages "xxx: Handle
>>> vga_switcheroo audio client".
>> [    4.311095] snd_hda_intel 0000:06:00.1: enabling device (0000 -> 0002)
>> [    4.314286] snd_hda_intel 0000:06:00.1: Handle vga_switcheroo audio
>> client
>> [    4.314822] snd_hda_intel 0000:06:00.6: enabling device (0000 -> 0002)
>>
>> 01:00.0 VGA compatible controller [0300]: Advanced Micro Devices,
>> Inc. [AMD/ATI] Device [1002:699f] (rev c3)
>> 06:00.0 VGA compatible controller [0300]: Advanced Micro Devices,
>> Inc. [AMD/ATI] Device [1002:15dd] (rev d1)
>> 06:00.1 Audio device [0403]: Advanced Micro Devices, Inc. [AMD/ATI]
>> Device [1002:15de]
> OK this sheds a brighter light, finally.
>
> If my understanding is correct, the issue is a false vga_switcheroo
> audio detection, after all.  This is the primary GPU and it shouldn't
> be registered as a vga_switcheroo discrete GPU.
>
> Below is a very ugly workaround for this particular case.  It assumes
> that the AMD+AMD combo will never have audio outputs on both but only
> for the primary, and it's possibly wrong.
>
> Is there a handy way to identify whether the given VGA PCI entry is
> a discrete GPU or not?  The amdgpu and radeon seem checking ATPX
> ACPI.

This is no issue about this topic, in amdgpu driver, both iGPU/dGPU will 
register as VGA_SWITCHEROO_UNKNOWN_ID, and the client id will be 
re-initialized in vgaswitchreoo_enable() via ATPX call. Then, iGPU will 
set as VGA_SWITCHEROO_IGD, and dGPU will set as VGA_SWITCHEROO_DIS.

I think current focus should be how to detect HDMI audio device under 
audio suspend state.

Thanks
JimQu
>
> Takashi
>
> ---
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1418,8 +1418,18 @@ static int azx_dev_free(struct snd_device *device)
>    */
>   static struct pci_dev *get_bound_vga(struct pci_dev *pci)
>   {
> +	static const struct pci_device_id ids[] = {
> +		{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_ANY_ID),
> +		  .class = PCI_BASE_CLASS_DISPLAY << 16,
> +		  .class_mask = 0xff << 16 },
> +		{}
> +	};
>   	struct pci_dev *p;
>   
> +	/* check whether Intel graphics is present as primary GPU */
> +	if (!pci_dev_present(ids))
> +		return NULL;
> +
>   	/* check only discrete GPU */
>   	switch (pci->vendor) {
>   	case PCI_VENDOR_ID_ATI: