On 06/29/2018 01:59 PM, Qu, Jim wrote:
> If the GFX is headless, the audio codec should be disabled on PCIE bus. the the HDA driver will never probe the audio. right?
After checking the HDA a little, looks HDA could also handle many other audio devices.
In this case, HDA may probe other audio device, e.g. from Braswell.
Just a guess.
Jerry
>
> Thanks
> JimQu
>
> ________________________________________
> å??件人: Alex Deucher <alexdeucher at gmail.com>
> å??é??æ?¶é?´: 2018å¹´6æ??29æ?¥ 12:01
> �件人: Zhang, Jerry
> æ??é??: Qu, Jim; amd-gfx list; Deucher, Alexander
> 主�: Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id
>
> On Thu, Jun 28, 2018 at 10:54 PM, Zhang, Jerry (Junwei)
> <Jerry.Zhang at amd.com> wrote:
>> On 06/28/2018 02:22 PM, 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, so if system runtime pm is enabled,
>>> the audio which is binded to UMA will be suspended.
>>>
>>> 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, let vgaswitchroo make decision.
>>>
>>> Newer dGPUs use PCI_CLASS_DISPLAY_OTHER rather than
>>> PCI_CLASS_DISPLAY_VGA. Make sure we check for both when
>>> checking for the dGPU in get_bound_vga()
>>
>>
>> IIRC, VGA is for iGPU, OTHER is for dGPU.
>> if not, please correct me.
>
> I think the dGPU can be either VGA or OTHER depending on whether there
> are displays wired to it. E.g., iceland and hainan don't even have
> VGA hw on them since they are headless.
>
> Alex
>
>>
>>
>>>
>>> The patch also combine the HDA change to avoid break
>>> building.
>>>
>>> Change-Id: I9906c1bd4dd5b36108d7133bb1cf724d13f1cd6d
>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>>> Signed-off-by: Jim Qu <Jim.Qu at amd.com>
>>> ---
>>> drivers/gpu/vga/vga_switcheroo.c | 11 +++++++++--
>>> include/linux/vga_switcheroo.h | 4 ++--
>>> sound/pci/hda/hda_intel.c | 15 +++++++++------
>>> 3 files changed, 20 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/vga/vga_switcheroo.c
>>> b/drivers/gpu/vga/vga_switcheroo.c
>>> index fc4adf3..7b7b0fd 100644
>>> --- a/drivers/gpu/vga/vga_switcheroo.c
>>> +++ b/drivers/gpu/vga/vga_switcheroo.c
>>> @@ -329,7 +329,7 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
>>> * vga_switcheroo_register_audio_client - register audio client
>>> * @pdev: client pci device
>>> * @ops: client callbacks
>>> - * @id: client identifier
>>> + * @bound_vga: client bound vga pci device
>>> *
>>> * Register audio client (audio device on a GPU). The client is assumed
>>> * to use runtime PM. Beforehand, vga_switcheroo_client_probe_defer()
>>> @@ -339,8 +339,15 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
>>> */
>>> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
>>> const struct vga_switcheroo_client_ops *ops,
>>> - enum vga_switcheroo_client_id id)
>>> + struct pci_dev *bound_vga)
>>> {
>>> + enum vga_switcheroo_client_id id = VGA_SWITCHEROO_DIS;
>>> +
>>> + if (bound_vga) {
>>> + if (vgasr_priv.handler->get_client_id)
>>
>>
>> We may combine 2 "if" together.
>>
>> Anyway, the patch looks fine for me.
>>
>> Reviewed-by: Junwei Zhang <Jerry.Zhang at amd.com>
>>
>> Additionally, better to includes Intel guys mail list and sound mail list if
>> any.
>>
>> Jerry
>>
>>
>>> + id = vgasr_priv.handler->get_client_id(bound_vga);
>>> + }
>>> +
>>> return register_client(pdev, ops, id | ID_BIT_AUDIO, false, true);
>>> }
>>> EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
>>> diff --git a/include/linux/vga_switcheroo.h
>>> b/include/linux/vga_switcheroo.h
>>> index 77f0f0a..a0c7b86 100644
>>> --- a/include/linux/vga_switcheroo.h
>>> +++ b/include/linux/vga_switcheroo.h
>>> @@ -151,7 +151,7 @@ int vga_switcheroo_register_client(struct pci_dev
>>> *dev,
>>> bool driver_power_control);
>>> int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
>>> const struct
>>> vga_switcheroo_client_ops *ops,
>>> - enum vga_switcheroo_client_id
>>> id);
>>> + struct pci_dev *bound_vga);
>>>
>>> void vga_switcheroo_client_fb_set(struct pci_dev *dev,
>>> struct fb_info *info);
>>> @@ -180,7 +180,7 @@ static inline int
>>> vga_switcheroo_register_handler(const struct vga_switcheroo_ha
>>> enum vga_switcheroo_handler_flags_t handler_flags) {
>>> return 0; }
>>> static inline int vga_switcheroo_register_audio_client(struct pci_dev
>>> *pdev,
>>> const struct vga_switcheroo_client_ops *ops,
>>> - enum vga_switcheroo_client_id id) { return 0; }
>>> + struct pci_dev *bound_vga) { return 0; }
>>> static inline void vga_switcheroo_unregister_handler(void) {}
>>> static inline enum vga_switcheroo_handler_flags_t
>>> vga_switcheroo_handler_flags(void) { return 0; }
>>> static inline int vga_switcheroo_lock_ddc(struct pci_dev *pdev) { return
>>> -ENODEV; }
>>> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
>>> index 1ae1850..8f992e6 100644
>>> --- a/sound/pci/hda/hda_intel.c
>>> +++ b/sound/pci/hda/hda_intel.c
>>> @@ -1319,15 +1319,17 @@ static const struct vga_switcheroo_client_ops
>>> azx_vs_ops = {
>>> static int register_vga_switcheroo(struct azx *chip)
>>> {
>>> struct hda_intel *hda = container_of(chip, struct hda_intel,
>>> chip);
>>> + struct pci_dev *p;
>>> int err;
>>>
>>> if (!hda->use_vga_switcheroo)
>>> return 0;
>>> - /* FIXME: currently only handling DIS controller
>>> - * is there any machine with two switchable HDMI audio
>>> controllers?
>>> - */
>>> - err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
>>> - VGA_SWITCHEROO_DIS);
>>> +
>>> + p = get_bound_vga(chip->pci);
>>> + err = vga_switcheroo_register_audio_client(chip->pci, &azx_vs_ops,
>>> p);
>>> +
>>> + if (p)
>>> + pci_dev_put(p);
>>> if (err < 0)
>>> return err;
>>> hda->vga_switcheroo_registered = 1;
>>> @@ -1429,7 +1431,8 @@ static struct pci_dev *get_bound_vga(struct pci_dev
>>> *pci)
>>> p =
>>> pci_get_domain_bus_and_slot(pci_domain_nr(pci->bus),
>>> pci->bus->number,
>>> 0);
>>> if (p) {
>>> - if ((p->class >> 8) ==
>>> PCI_CLASS_DISPLAY_VGA)
>>> + if ((p->class >> 8) ==
>>> PCI_CLASS_DISPLAY_VGA ||
>>> + (p->class >> 8) ==
>>> PCI_CLASS_DISPLAY_OTHER)
>>> return p;
>>> pci_dev_put(p);
>>> }
>>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
