[SRU,Bionic,OEM-B,v2,0/6] HDMI/DP audio can't work on the laptop of Dell Latitude 5495
mbox series

Message ID 1532914344-7055-1-git-send-email-hui.wang@canonical.com
Headers show
Series
  • HDMI/DP audio can't work on the laptop of Dell Latitude 5495
Related show

Message

Hui Wang July 30, 2018, 1:32 a.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1782689

In the v2:
the patchset is also sent to OEM-B since the OEM project is waiting for the patchset.

The v1 introduced a building error, my investigation is shown as below:
The declaration of "enum vga_switcheroo_client_id (*get_client_id)(struct pci_dev *pdev);"
and the implementation of "static int nouveau_dsm_get_client_id(struct pci_dev *pdev)"
exist in the kernel (bionic, artful) for a long time, but "-Werror=incompatible-pointer-types"
didn't report it as an error, after applied these 2 patches, it is reported as an error,
that is because the patch touches the definition of "enum vga_switcheroo_client_id",
before applying these 2 patches, the vga_switcheroo_client_id includes a negative numer:
VGA_SWITCHEROO_UNKNOWN_ID = -1, but after applying the patches, there is no negative number
anymore in the "enum vga_swicheroo_client_id".
I guess if the definition of enum includes negative number, the c compiler treat it as int type,
that is why artful and bionic did not expose building error before.

To fix the building error, I cherry-picked 4 more patches [3/6-6/6], and the purpose of those
4 patches is straightforward.



In the v1:
This bug comes from the OEM project, and the engineer Jim Qu from AMD took
almost 2 weeks to investigate this problem, then he worte these two patches to
fix this problem and now the patches are merged to sound repository.

According to Jim Qu's investigation, the root cause of this problem is:
1. there is two GPU on the system. iGPU has a audio codec for HDMI output,
   and dGPU is without audio codec it is only for offload rendering.
2. under runtime pm, when dGPU suspend, amdgpu will also call vgaswitchroo
   driver to power off audio which vgaswitchroo client is VGA_SWITCHEROO_DIS.
3. In current HDA audio driver, the iGPU's audio will register to vgaswitchroo
   as VGA_SWITCHEROO_DIS, therefore, the audio will be selected and powered off
   by runtime pm.

[Impact]
On the Dell Latitude 5495, If we plug a monitor with audio capability to the
HDMI/DP ports, we can't find HDMI/DP audio sinks as expected, then we can't
play any sound through HDMI/DP audio.

[Fix]
With these two patches, the driver will not always set vgaswicheroo clients of
HDA audio as VGA_SWITCHEROO_DIS, it will set it to _DIS or _IGD with the help
of callback function of DRM drivers. So on this machine, the vgaswicheroo client
will be set _IGD, then it will not be powered off when discrete gpu is powered off. 

[Test Case]
We tested plug/unplug detection and playback through HDMI/DP audio, everything works
well.

[Regression Potential]
Very low, without these two patches, the vgaswitchroo client of audio will be
set to _DIS unconditionally, it did not expose any problem because in the past,
all the HDMI/DP audio codecs are in the discreate GPU. But on Latitude 5495, the
HDMI/DP audio codec is in the integrated GPU, so we need to change the driver to
let DRM driver decide if it is _DIS or _IGD, it will not introduce regression for
old mahcines on old machines, the client will be set to _DIS as before.

And we have tested these two patches on some old machines with two gpus like A+A
, I+A and I+N, all of them worked well as before.

Jim Qu (2):
  ALSA: hda: use PCI_BASE_CLASS_DISPLAY to replace PCI_CLASS_DISPLAY_VGA
  vga_switcheroo: set audio client id according to bound GPU id

Luc Van Oostenryck (4):
  drm/nouveau: fix nouveau_dsm_get_client_id()'s return type
  drm/radeon: fix radeon_atpx_get_client_id()'s return type
  drm/amdgpu: fix amdgpu_atpx_get_client_id()'s return type
  platform/x86: apple-gmux: fix gmux_get_client_id()'s return type

 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c           |  2 +-
 drivers/gpu/drm/radeon/radeon_atpx_handler.c     |  2 +-
 drivers/gpu/vga/vga_switcheroo.c                 | 62 +++++++++++++++++++-----
 drivers/platform/x86/apple-gmux.c                |  2 +-
 include/linux/vga_switcheroo.h                   |  8 +--
 sound/pci/hda/hda_intel.c                        | 13 ++---
 7 files changed, 66 insertions(+), 25 deletions(-)

Comments

Stefan Bader July 30, 2018, 10:39 a.m. UTC | #1
On 30.07.2018 03:32, Hui Wang wrote:
> BugLink: https://bugs.launchpad.net/bugs/1782689
> 
> In the v2:
> the patchset is also sent to OEM-B since the OEM project is waiting for the patchset.
> 
> The v1 introduced a building error, my investigation is shown as below:
> The declaration of "enum vga_switcheroo_client_id (*get_client_id)(struct pci_dev *pdev);"
> and the implementation of "static int nouveau_dsm_get_client_id(struct pci_dev *pdev)"
> exist in the kernel (bionic, artful) for a long time, but "-Werror=incompatible-pointer-types"
> didn't report it as an error, after applied these 2 patches, it is reported as an error,
> that is because the patch touches the definition of "enum vga_switcheroo_client_id",
> before applying these 2 patches, the vga_switcheroo_client_id includes a negative numer:
> VGA_SWITCHEROO_UNKNOWN_ID = -1, but after applying the patches, there is no negative number
> anymore in the "enum vga_swicheroo_client_id".
> I guess if the definition of enum includes negative number, the c compiler treat it as int type,
> that is why artful and bionic did not expose building error before.
> 
> To fix the building error, I cherry-picked 4 more patches [3/6-6/6], and the purpose of those
> 4 patches is straightforward.
> 
> 
> 
> In the v1:
> This bug comes from the OEM project, and the engineer Jim Qu from AMD took
> almost 2 weeks to investigate this problem, then he worte these two patches to
> fix this problem and now the patches are merged to sound repository.
> 
> According to Jim Qu's investigation, the root cause of this problem is:
> 1. there is two GPU on the system. iGPU has a audio codec for HDMI output,
>    and dGPU is without audio codec it is only for offload rendering.
> 2. under runtime pm, when dGPU suspend, amdgpu will also call vgaswitchroo
>    driver to power off audio which vgaswitchroo client is VGA_SWITCHEROO_DIS.
> 3. In current HDA audio driver, the iGPU's audio will register to vgaswitchroo
>    as VGA_SWITCHEROO_DIS, therefore, the audio will be selected and powered off
>    by runtime pm.
> 
> [Impact]
> On the Dell Latitude 5495, If we plug a monitor with audio capability to the
> HDMI/DP ports, we can't find HDMI/DP audio sinks as expected, then we can't
> play any sound through HDMI/DP audio.
> 
> [Fix]
> With these two patches, the driver will not always set vgaswicheroo clients of
> HDA audio as VGA_SWITCHEROO_DIS, it will set it to _DIS or _IGD with the help
> of callback function of DRM drivers. So on this machine, the vgaswicheroo client
> will be set _IGD, then it will not be powered off when discrete gpu is powered off. 
> 
> [Test Case]
> We tested plug/unplug detection and playback through HDMI/DP audio, everything works
> well.
> 
> [Regression Potential]
> Very low, without these two patches, the vgaswitchroo client of audio will be
> set to _DIS unconditionally, it did not expose any problem because in the past,
> all the HDMI/DP audio codecs are in the discreate GPU. But on Latitude 5495, the
> HDMI/DP audio codec is in the integrated GPU, so we need to change the driver to
> let DRM driver decide if it is _DIS or _IGD, it will not introduce regression for
> old mahcines on old machines, the client will be set to _DIS as before.
> 
> And we have tested these two patches on some old machines with two gpus like A+A
> , I+A and I+N, all of them worked well as before.
> 
> Jim Qu (2):
>   ALSA: hda: use PCI_BASE_CLASS_DISPLAY to replace PCI_CLASS_DISPLAY_VGA
>   vga_switcheroo: set audio client id according to bound GPU id
> 
> Luc Van Oostenryck (4):
>   drm/nouveau: fix nouveau_dsm_get_client_id()'s return type
>   drm/radeon: fix radeon_atpx_get_client_id()'s return type
>   drm/amdgpu: fix amdgpu_atpx_get_client_id()'s return type
>   platform/x86: apple-gmux: fix gmux_get_client_id()'s return type
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_acpi.c           |  2 +-
>  drivers/gpu/drm/radeon/radeon_atpx_handler.c     |  2 +-
>  drivers/gpu/vga/vga_switcheroo.c                 | 62 +++++++++++++++++++-----
>  drivers/platform/x86/apple-gmux.c                |  2 +-
>  include/linux/vga_switcheroo.h                   |  8 +--
>  sound/pci/hda/hda_intel.c                        | 13 ++---
>  7 files changed, 66 insertions(+), 25 deletions(-)
> 
Acked-by: Stefan Bader <stefan.bader@canonical.com>

I think the additional patches need to be applied before at least current patch
#2 (maybe just before both) to avoid compile being broken in between.
Also, you should update the SRU justification in the bug report to match the new
number of patches.

-Stefan
Hui Wang July 31, 2018, 12:20 a.m. UTC | #2
On 2018年07月30日 18:39, Stefan Bader wrote:
> On 30.07.2018 03:32, Hui Wang wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1782689
>>
>> In the v2:
>> the patchset is also sent to OEM-B since the OEM project is waiting for the patchset.
>>
>> The v1 introduced a building error, my investigation is shown as below:
>> The declaration of "enum vga_switcheroo_client_id (*get_client_id)(struct pci_dev *pdev);"
>> and the implementation of "static int nouveau_dsm_get_client_id(struct pci_dev *pdev)"
>> exist in the kernel (bionic, artful) for a long time, but "-Werror=incompatible-pointer-types"
>> didn't report it as an error, after applied these 2 patches, it is reported as an error,
>> that is because the patch touches the definition of "enum vga_switcheroo_client_id",
>> before applying these 2 patches, the vga_switcheroo_client_id includes a negative numer:
>> VGA_SWITCHEROO_UNKNOWN_ID = -1, but after applying the patches, there is no negative number
>> anymore in the "enum vga_swicheroo_client_id".
>> I guess if the definition of enum includes negative number, the c compiler treat it as int type,
>> that is why artful and bionic did not expose building error before.
>>
>> To fix the building error, I cherry-picked 4 more patches [3/6-6/6], and the purpose of those
>> 4 patches is straightforward.
>>
>>
>>
>> In the v1:
>> This bug comes from the OEM project, and the engineer Jim Qu from AMD took
>> almost 2 weeks to investigate this problem, then he worte these two patches to
>> fix this problem and now the patches are merged to sound repository.
>>
>> According to Jim Qu's investigation, the root cause of this problem is:
>> 1. there is two GPU on the system. iGPU has a audio codec for HDMI output,
>>     and dGPU is without audio codec it is only for offload rendering.
>> 2. under runtime pm, when dGPU suspend, amdgpu will also call vgaswitchroo
>>     driver to power off audio which vgaswitchroo client is VGA_SWITCHEROO_DIS.
>> 3. In current HDA audio driver, the iGPU's audio will register to vgaswitchroo
>>     as VGA_SWITCHEROO_DIS, therefore, the audio will be selected and powered off
>>     by runtime pm.
>>
>> [Impact]
>> On the Dell Latitude 5495, If we plug a monitor with audio capability to the
>> HDMI/DP ports, we can't find HDMI/DP audio sinks as expected, then we can't
>> play any sound through HDMI/DP audio.
>>
>> [Fix]
>> With these two patches, the driver will not always set vgaswicheroo clients of
>> HDA audio as VGA_SWITCHEROO_DIS, it will set it to _DIS or _IGD with the help
>> of callback function of DRM drivers. So on this machine, the vgaswicheroo client
>> will be set _IGD, then it will not be powered off when discrete gpu is powered off.
>>
>> [Test Case]
>> We tested plug/unplug detection and playback through HDMI/DP audio, everything works
>> well.
>>
>> [Regression Potential]
>> Very low, without these two patches, the vgaswitchroo client of audio will be
>> set to _DIS unconditionally, it did not expose any problem because in the past,
>> all the HDMI/DP audio codecs are in the discreate GPU. But on Latitude 5495, the
>> HDMI/DP audio codec is in the integrated GPU, so we need to change the driver to
>> let DRM driver decide if it is _DIS or _IGD, it will not introduce regression for
>> old mahcines on old machines, the client will be set to _DIS as before.
>>
>> And we have tested these two patches on some old machines with two gpus like A+A
>> , I+A and I+N, all of them worked well as before.
>>
>> Jim Qu (2):
>>    ALSA: hda: use PCI_BASE_CLASS_DISPLAY to replace PCI_CLASS_DISPLAY_VGA
>>    vga_switcheroo: set audio client id according to bound GPU id
>>
>> Luc Van Oostenryck (4):
>>    drm/nouveau: fix nouveau_dsm_get_client_id()'s return type
>>    drm/radeon: fix radeon_atpx_get_client_id()'s return type
>>    drm/amdgpu: fix amdgpu_atpx_get_client_id()'s return type
>>    platform/x86: apple-gmux: fix gmux_get_client_id()'s return type
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |  2 +-
>>   drivers/gpu/drm/nouveau/nouveau_acpi.c           |  2 +-
>>   drivers/gpu/drm/radeon/radeon_atpx_handler.c     |  2 +-
>>   drivers/gpu/vga/vga_switcheroo.c                 | 62 +++++++++++++++++++-----
>>   drivers/platform/x86/apple-gmux.c                |  2 +-
>>   include/linux/vga_switcheroo.h                   |  8 +--
>>   sound/pci/hda/hda_intel.c                        | 13 ++---
>>   7 files changed, 66 insertions(+), 25 deletions(-)
>>
> Acked-by: Stefan Bader <stefan.bader@canonical.com>
>
> I think the additional patches need to be applied before at least current patch
> #2 (maybe just before both) to avoid compile being broken in between.
Yes, please, move the last 4 patches ahead of first 2 ones.
> Also, you should update the SRU justification in the bug report to match the new
> number of patches.
Got it, and fixed it in the bug description of #1782689

Thanks.
> -Stefan
>
Kleber Souza July 31, 2018, 1:22 p.m. UTC | #3
On 07/30/18 03:32, Hui Wang wrote:
> BugLink: https://bugs.launchpad.net/bugs/1782689
> 
> In the v2:
> the patchset is also sent to OEM-B since the OEM project is waiting for the patchset.
> 
> The v1 introduced a building error, my investigation is shown as below:
> The declaration of "enum vga_switcheroo_client_id (*get_client_id)(struct pci_dev *pdev);"
> and the implementation of "static int nouveau_dsm_get_client_id(struct pci_dev *pdev)"
> exist in the kernel (bionic, artful) for a long time, but "-Werror=incompatible-pointer-types"
> didn't report it as an error, after applied these 2 patches, it is reported as an error,
> that is because the patch touches the definition of "enum vga_switcheroo_client_id",
> before applying these 2 patches, the vga_switcheroo_client_id includes a negative numer:
> VGA_SWITCHEROO_UNKNOWN_ID = -1, but after applying the patches, there is no negative number
> anymore in the "enum vga_swicheroo_client_id".
> I guess if the definition of enum includes negative number, the c compiler treat it as int type,
> that is why artful and bionic did not expose building error before.
> 
> To fix the building error, I cherry-picked 4 more patches [3/6-6/6], and the purpose of those
> 4 patches is straightforward.
> 
> 
> 
> In the v1:
> This bug comes from the OEM project, and the engineer Jim Qu from AMD took
> almost 2 weeks to investigate this problem, then he worte these two patches to
> fix this problem and now the patches are merged to sound repository.
> 
> According to Jim Qu's investigation, the root cause of this problem is:
> 1. there is two GPU on the system. iGPU has a audio codec for HDMI output,
>    and dGPU is without audio codec it is only for offload rendering.
> 2. under runtime pm, when dGPU suspend, amdgpu will also call vgaswitchroo
>    driver to power off audio which vgaswitchroo client is VGA_SWITCHEROO_DIS.
> 3. In current HDA audio driver, the iGPU's audio will register to vgaswitchroo
>    as VGA_SWITCHEROO_DIS, therefore, the audio will be selected and powered off
>    by runtime pm.
> 
> [Impact]
> On the Dell Latitude 5495, If we plug a monitor with audio capability to the
> HDMI/DP ports, we can't find HDMI/DP audio sinks as expected, then we can't
> play any sound through HDMI/DP audio.
> 
> [Fix]
> With these two patches, the driver will not always set vgaswicheroo clients of
> HDA audio as VGA_SWITCHEROO_DIS, it will set it to _DIS or _IGD with the help
> of callback function of DRM drivers. So on this machine, the vgaswicheroo client
> will be set _IGD, then it will not be powered off when discrete gpu is powered off. 
> 
> [Test Case]
> We tested plug/unplug detection and playback through HDMI/DP audio, everything works
> well.
> 
> [Regression Potential]
> Very low, without these two patches, the vgaswitchroo client of audio will be
> set to _DIS unconditionally, it did not expose any problem because in the past,
> all the HDMI/DP audio codecs are in the discreate GPU. But on Latitude 5495, the
> HDMI/DP audio codec is in the integrated GPU, so we need to change the driver to
> let DRM driver decide if it is _DIS or _IGD, it will not introduce regression for
> old mahcines on old machines, the client will be set to _DIS as before.
> 
> And we have tested these two patches on some old machines with two gpus like A+A
> , I+A and I+N, all of them worked well as before.
> 
> Jim Qu (2):
>   ALSA: hda: use PCI_BASE_CLASS_DISPLAY to replace PCI_CLASS_DISPLAY_VGA
>   vga_switcheroo: set audio client id according to bound GPU id
> 
> Luc Van Oostenryck (4):
>   drm/nouveau: fix nouveau_dsm_get_client_id()'s return type
>   drm/radeon: fix radeon_atpx_get_client_id()'s return type
>   drm/amdgpu: fix amdgpu_atpx_get_client_id()'s return type
>   platform/x86: apple-gmux: fix gmux_get_client_id()'s return type
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_acpi.c           |  2 +-
>  drivers/gpu/drm/radeon/radeon_atpx_handler.c     |  2 +-
>  drivers/gpu/vga/vga_switcheroo.c                 | 62 +++++++++++++++++++-----
>  drivers/platform/x86/apple-gmux.c                |  2 +-
>  include/linux/vga_switcheroo.h                   |  8 +--
>  sound/pci/hda/hda_intel.c                        | 13 ++---
>  7 files changed, 66 insertions(+), 25 deletions(-)
> 


Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Kleber Souza July 31, 2018, 1:58 p.m. UTC | #4
On 07/30/18 03:32, Hui Wang wrote:
> BugLink: https://bugs.launchpad.net/bugs/1782689
> 
> In the v2:
> the patchset is also sent to OEM-B since the OEM project is waiting for the patchset.
> 
> The v1 introduced a building error, my investigation is shown as below:
> The declaration of "enum vga_switcheroo_client_id (*get_client_id)(struct pci_dev *pdev);"
> and the implementation of "static int nouveau_dsm_get_client_id(struct pci_dev *pdev)"
> exist in the kernel (bionic, artful) for a long time, but "-Werror=incompatible-pointer-types"
> didn't report it as an error, after applied these 2 patches, it is reported as an error,
> that is because the patch touches the definition of "enum vga_switcheroo_client_id",
> before applying these 2 patches, the vga_switcheroo_client_id includes a negative numer:
> VGA_SWITCHEROO_UNKNOWN_ID = -1, but after applying the patches, there is no negative number
> anymore in the "enum vga_swicheroo_client_id".
> I guess if the definition of enum includes negative number, the c compiler treat it as int type,
> that is why artful and bionic did not expose building error before.
> 
> To fix the building error, I cherry-picked 4 more patches [3/6-6/6], and the purpose of those
> 4 patches is straightforward.
> 
> 
> 
> In the v1:
> This bug comes from the OEM project, and the engineer Jim Qu from AMD took
> almost 2 weeks to investigate this problem, then he worte these two patches to
> fix this problem and now the patches are merged to sound repository.
> 
> According to Jim Qu's investigation, the root cause of this problem is:
> 1. there is two GPU on the system. iGPU has a audio codec for HDMI output,
>    and dGPU is without audio codec it is only for offload rendering.
> 2. under runtime pm, when dGPU suspend, amdgpu will also call vgaswitchroo
>    driver to power off audio which vgaswitchroo client is VGA_SWITCHEROO_DIS.
> 3. In current HDA audio driver, the iGPU's audio will register to vgaswitchroo
>    as VGA_SWITCHEROO_DIS, therefore, the audio will be selected and powered off
>    by runtime pm.
> 
> [Impact]
> On the Dell Latitude 5495, If we plug a monitor with audio capability to the
> HDMI/DP ports, we can't find HDMI/DP audio sinks as expected, then we can't
> play any sound through HDMI/DP audio.
> 
> [Fix]
> With these two patches, the driver will not always set vgaswicheroo clients of
> HDA audio as VGA_SWITCHEROO_DIS, it will set it to _DIS or _IGD with the help
> of callback function of DRM drivers. So on this machine, the vgaswicheroo client
> will be set _IGD, then it will not be powered off when discrete gpu is powered off. 
> 
> [Test Case]
> We tested plug/unplug detection and playback through HDMI/DP audio, everything works
> well.
> 
> [Regression Potential]
> Very low, without these two patches, the vgaswitchroo client of audio will be
> set to _DIS unconditionally, it did not expose any problem because in the past,
> all the HDMI/DP audio codecs are in the discreate GPU. But on Latitude 5495, the
> HDMI/DP audio codec is in the integrated GPU, so we need to change the driver to
> let DRM driver decide if it is _DIS or _IGD, it will not introduce regression for
> old mahcines on old machines, the client will be set to _DIS as before.
> 
> And we have tested these two patches on some old machines with two gpus like A+A
> , I+A and I+N, all of them worked well as before.
> 
> Jim Qu (2):
>   ALSA: hda: use PCI_BASE_CLASS_DISPLAY to replace PCI_CLASS_DISPLAY_VGA
>   vga_switcheroo: set audio client id according to bound GPU id
> 
> Luc Van Oostenryck (4):
>   drm/nouveau: fix nouveau_dsm_get_client_id()'s return type
>   drm/radeon: fix radeon_atpx_get_client_id()'s return type
>   drm/amdgpu: fix amdgpu_atpx_get_client_id()'s return type
>   platform/x86: apple-gmux: fix gmux_get_client_id()'s return type
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_acpi.c           |  2 +-
>  drivers/gpu/drm/radeon/radeon_atpx_handler.c     |  2 +-
>  drivers/gpu/vga/vga_switcheroo.c                 | 62 +++++++++++++++++++-----
>  drivers/platform/x86/apple-gmux.c                |  2 +-
>  include/linux/vga_switcheroo.h                   |  8 +--
>  sound/pci/hda/hda_intel.c                        | 13 ++---
>  7 files changed, 66 insertions(+), 25 deletions(-)
> 

Applied to bionic/master-next branch, in the following order:

3/6 - drm/nouveau: fix nouveau_dsm_get_client_id()'s return type
4/6 - drm/radeon: fix radeon_atpx_get_client_id()'s return type
5/6 - drm/amdgpu: fix amdgpu_atpx_get_client_id()'s return type
6/6 - platform/x86: apple-gmux: fix gmux_get_client_id()'s return type
1/6 - ALSA: hda: use PCI_BASE_CLASS_DISPLAY to replace PCI_CLASS_DISPLAY_VGA
2/6 - (HEAD -> master-next) vga_switcheroo: set audio client id
according to bound GPU id


Thanks,
Kleber
Seth Forshee Aug. 1, 2018, 1:11 p.m. UTC | #5
On Mon, Jul 30, 2018 at 09:32:18AM +0800, Hui Wang wrote:
> BugLink: https://bugs.launchpad.net/bugs/1782689
> 
> In the v2:
> the patchset is also sent to OEM-B since the OEM project is waiting for the patchset.
> 
> The v1 introduced a building error, my investigation is shown as below:
> The declaration of "enum vga_switcheroo_client_id (*get_client_id)(struct pci_dev *pdev);"
> and the implementation of "static int nouveau_dsm_get_client_id(struct pci_dev *pdev)"
> exist in the kernel (bionic, artful) for a long time, but "-Werror=incompatible-pointer-types"
> didn't report it as an error, after applied these 2 patches, it is reported as an error,
> that is because the patch touches the definition of "enum vga_switcheroo_client_id",
> before applying these 2 patches, the vga_switcheroo_client_id includes a negative numer:
> VGA_SWITCHEROO_UNKNOWN_ID = -1, but after applying the patches, there is no negative number
> anymore in the "enum vga_swicheroo_client_id".
> I guess if the definition of enum includes negative number, the c compiler treat it as int type,
> that is why artful and bionic did not expose building error before.
> 
> To fix the building error, I cherry-picked 4 more patches [3/6-6/6], and the purpose of those
> 4 patches is straightforward.
> 
> 
> 
> In the v1:
> This bug comes from the OEM project, and the engineer Jim Qu from AMD took
> almost 2 weeks to investigate this problem, then he worte these two patches to
> fix this problem and now the patches are merged to sound repository.
> 
> According to Jim Qu's investigation, the root cause of this problem is:
> 1. there is two GPU on the system. iGPU has a audio codec for HDMI output,
>    and dGPU is without audio codec it is only for offload rendering.
> 2. under runtime pm, when dGPU suspend, amdgpu will also call vgaswitchroo
>    driver to power off audio which vgaswitchroo client is VGA_SWITCHEROO_DIS.
> 3. In current HDA audio driver, the iGPU's audio will register to vgaswitchroo
>    as VGA_SWITCHEROO_DIS, therefore, the audio will be selected and powered off
>    by runtime pm.
> 
> [Impact]
> On the Dell Latitude 5495, If we plug a monitor with audio capability to the
> HDMI/DP ports, we can't find HDMI/DP audio sinks as expected, then we can't
> play any sound through HDMI/DP audio.
> 
> [Fix]
> With these two patches, the driver will not always set vgaswicheroo clients of
> HDA audio as VGA_SWITCHEROO_DIS, it will set it to _DIS or _IGD with the help
> of callback function of DRM drivers. So on this machine, the vgaswicheroo client
> will be set _IGD, then it will not be powered off when discrete gpu is powered off. 
> 
> [Test Case]
> We tested plug/unplug detection and playback through HDMI/DP audio, everything works
> well.
> 
> [Regression Potential]
> Very low, without these two patches, the vgaswitchroo client of audio will be
> set to _DIS unconditionally, it did not expose any problem because in the past,
> all the HDMI/DP audio codecs are in the discreate GPU. But on Latitude 5495, the
> HDMI/DP audio codec is in the integrated GPU, so we need to change the driver to
> let DRM driver decide if it is _DIS or _IGD, it will not introduce regression for
> old mahcines on old machines, the client will be set to _DIS as before.
> 
> And we have tested these two patches on some old machines with two gpus like A+A
> , I+A and I+N, all of them worked well as before.
> 
> Jim Qu (2):
>   ALSA: hda: use PCI_BASE_CLASS_DISPLAY to replace PCI_CLASS_DISPLAY_VGA
>   vga_switcheroo: set audio client id according to bound GPU id
> 
> Luc Van Oostenryck (4):
>   drm/nouveau: fix nouveau_dsm_get_client_id()'s return type
>   drm/radeon: fix radeon_atpx_get_client_id()'s return type
>   drm/amdgpu: fix amdgpu_atpx_get_client_id()'s return type
>   platform/x86: apple-gmux: fix gmux_get_client_id()'s return type
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_acpi.c           |  2 +-
>  drivers/gpu/drm/radeon/radeon_atpx_handler.c     |  2 +-
>  drivers/gpu/vga/vga_switcheroo.c                 | 62 +++++++++++++++++++-----
>  drivers/platform/x86/apple-gmux.c                |  2 +-
>  include/linux/vga_switcheroo.h                   |  8 +--
>  sound/pci/hda/hda_intel.c                        | 13 ++---
>  7 files changed, 66 insertions(+), 25 deletions(-)

Applied to cosmic/master-next, with the backport for "vga_switcheroo:
set audio client id according to bound GPU id" replaced with a cherry
pick from the maintainer tree. Also applied that patch and "ALSA: hda:
use PCI_BASE_CLASS_DISPLAY to replace PCI_CLASS_DISPLAY_VGA" to
unstable/master, as the other patches are already in 4.18. Thanks!