[PATCH] drm/amd: Remove errors from sphinx documentation

[Powell, Darren]

Subject: [PATCH 1/2] drm/amd: Remove errors from sphinx documentation

Eliminating the warnings produced by sphinx when processing the sphinx comments 
in
 amdgpu_device.c

Signed-off-by: Darren Powell 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c |  5 +++--
 2 files changed, 15 insertions(+), 10 deletions(-)



Subject: [PATCH 2/2] drm/amd: Add sphinx documentation for amd_ip_funcs

Signed-off-by: Darren Powell 
---
 drivers/gpu/drm/amd/include/amd_shared.h | 81 ++--
 1 file changed, 63 insertions(+), 18 deletions(-)


From 8f9651485adc7e207fe081e2b53b0cfe702cf0f3 Mon Sep 17 00:00:00 2001
From: Darren Powell 
Date: Mon, 25 Jun 2018 19:04:03 -0400
Subject: [PATCH 1/2] drm/amd: Remove errors from sphinx documentation

Eliminating the warnings produced by sphinx when processing the sphinx comments in
 amdgpu_device.c

Signed-off-by: Darren Powell 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c |  5 +++--
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 995dc02ed01c..d4d54553bcc3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1076,7 +1076,7 @@ static const struct vga_switcheroo_client_ops amdgpu_switcheroo_ops = {
 /**
  * amdgpu_device_ip_set_clockgating_state - set the CG state
  *
- * @adev: amdgpu_device pointer
+ * @dev: amdgpu_device pointer
  * @block_type: Type of hardware IP (SMU, GFX, UVD, etc.)
  * @state: clockgating state (gate or ungate)
  *
@@ -1110,7 +1110,7 @@ int amdgpu_device_ip_set_clockgating_state(void *dev,
 /**
  * amdgpu_device_ip_set_powergating_state - set the PG state
  *
- * @adev: amdgpu_device pointer
+ * @dev: amdgpu_device pointer
  * @block_type: Type of hardware IP (SMU, GFX, UVD, etc.)
  * @state: powergating state (gate or ungate)
  *
@@ -1221,7 +1221,7 @@ bool amdgpu_device_ip_is_idle(struct amdgpu_device *adev,
  * amdgpu_device_ip_get_ip_block - get a hw IP pointer
  *
  * @adev: amdgpu_device pointer
- * @block_type: Type of hardware IP (SMU, GFX, UVD, etc.)
+ * @type: Type of hardware IP (SMU, GFX, UVD, etc.)
  *
  * Returns a pointer to the hardware IP block structure
  * if it exists for the asic, otherwise NULL.
@@ -2209,7 +2209,7 @@ bool amdgpu_device_has_dc_support(struct amdgpu_device *adev)
  * amdgpu_device_init - initialize the driver
  *
  * @adev: amdgpu_device pointer
- * @pdev: drm dev pointer
+ * @ddev: drm dev pointer
  * @pdev: pci dev pointer
  * @flags: driver flags
  *
@@ -2582,8 +2582,9 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 /**
  * amdgpu_device_suspend - initiate device suspend
  *
- * @pdev: drm dev pointer
- * @state: suspend state
+ * @dev: drm dev pointer
+ * @suspend: suspend state
+ * @fbcon : notify the fbdev of suspend
  *
  * Puts the hw in the suspend state (all asics).
  * Returns 0 for success or an error on failure.
@@ -2681,7 +2682,9 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon)
 /**
  * amdgpu_device_resume - initiate device resume
  *
- * @pdev: drm dev pointer
+ * @dev: drm dev pointer
+ * @resume: resume state
+ * @fbcon : notify the fbdev of resume
  *
  * Bring the hw back to operating state (all asics).
  * Returns 0 for success or an error on failure.
@@ -3145,6 +3148,7 @@ static int amdgpu_device_reset(struct amdgpu_device *adev)
  * amdgpu_device_reset_sriov - reset ASIC for SR-IOV vf
  *
  * @adev: amdgpu device pointer
+ * @from_hypervisor: request from hypervisor
  *
  * do VF FLR and reinitialize Asic
  * return 0 means successed otherwise failed
@@ -3192,7 +3196,7 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device *adev,
  *
  * @adev: amdgpu device pointer
  * @job: which job trigger hang
- * @force forces reset regardless of amdgpu_gpu_recovery
+ * @force: forces reset regardless of amdgpu_gpu_recovery
  *
  * Attempt to reset the GPU if it has hung (all asics).
  * Returns 0 for success or an error on failure.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index 72a3e8c68876..a365ea2383d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -58,7 +58,8 @@
  *
  * @adev: amdgpu device pointer
  * @mm: process address space
- * @mn: MMU notifier structur
+ * @mn: MMU notifier structure
+ * @type: type of MMU notifier
  * @work: destruction work item
  * @node: hash table node to find structure by adev and mn
  * @lock: rw semaphore protecting the notifier nodes
@@ -266,7 +267,7 @@ static void amdgpu_mn_invalidate_range_start_gfx(struct mmu_notifier *mn,
  * amdgpu_mn_invalidate_range_start_hsa - callback to notify about mm change
  *
  * @mn: our notifier
- * @mn: the mm this callback is about
+ * @mm: the mm this callback is about
  

Re: [PATCH] drm/amdgpu: update uvd_v6_0_ring_vm_funcs to use new nop packet

[Leo Liu]

Have this patch overlooked. This is a good catch.

Reviewed-by: Leo Liu 


On 2018-06-29 10:57 AM, Alex Deucher wrote:

Ping?

On Thu, Jun 28, 2018 at 2:37 PM, Alex Deucher  wrote:

Was missed when updating the uvd 6 module.

Fixes: 1aac3c9180 (drm/amdgpu: fix insert nop for UVD6 ring)
Signed-off-by: Alex Deucher 
---
  drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index 1df1c6115341..8ee1c2eaaa14 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -1569,7 +1569,6 @@ static const struct amdgpu_ring_funcs 
uvd_v6_0_ring_phys_funcs = {
  static const struct amdgpu_ring_funcs uvd_v6_0_ring_vm_funcs = {
 .type = AMDGPU_RING_TYPE_UVD,
 .align_mask = 0xf,
-   .nop = PACKET0(mmUVD_NO_OP, 0),
 .support_64bit_ptrs = false,
 .get_rptr = uvd_v6_0_ring_get_rptr,
 .get_wptr = uvd_v6_0_ring_get_wptr,
@@ -1587,7 +1586,7 @@ static const struct amdgpu_ring_funcs 
uvd_v6_0_ring_vm_funcs = {
 .emit_hdp_flush = uvd_v6_0_ring_emit_hdp_flush,
 .test_ring = uvd_v6_0_ring_test_ring,
 .test_ib = amdgpu_uvd_ring_test_ib,
-   .insert_nop = amdgpu_ring_insert_nop,
+   .insert_nop = uvd_v6_0_ring_insert_nop,
 .pad_ib = amdgpu_ring_generic_pad_ib,
 .begin_use = amdgpu_uvd_ring_begin_use,
 .end_use = amdgpu_uvd_ring_end_use,
--
2.13.6


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm: drop drm_pcie_get_speed_cap_mask and drm_pcie_get_max_link_width

[Dave Airlie]

On 26 June 2018 at 07:06, Alex Deucher  wrote:
> These functions duplicated functionality which was ultimately added
> to the pci core.
>
> All users of these functions have been ported to using the newly
> exposed pci functionality.  These functions are no longer used,
> so drop them.
>
> Signed-off-by: Alex Deucher 

Reviewed-by: Dave Airlie 

Dave.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd: Replace drm_dev_unref with drm_dev_put

[Alex Deucher]

On Thu, Jun 28, 2018 at 10:10 AM, Thomas Zimmermann
 wrote:
> This patch unifies the naming of DRM functions for reference counting
> of struct drm_device. The resulting code is more aligned with the rest
> of the Linux kernel interfaces.
>
> Signed-off-by: Thomas Zimmermann 

Applied.  thanks

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index a549483032b0..2abf3b99a968 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -664,7 +664,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>  err_pci:
> pci_disable_device(pdev);
>  err_free:
> -   drm_dev_unref(dev);
> +   drm_dev_put(dev);
> return ret;
>  }
>
> @@ -674,7 +674,7 @@ amdgpu_pci_remove(struct pci_dev *pdev)
> struct drm_device *dev = pci_get_drvdata(pdev);
>
> drm_dev_unregister(dev);
> -   drm_dev_unref(dev);
> +   drm_dev_put(dev);
> pci_disable_device(pdev);
> pci_set_drvdata(pdev, NULL);
>  }
> --
> 2.14.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu/sdma: simplify sdma instance setup

[Alex Deucher]

On Fri, Jun 29, 2018 at 12:52 PM, Leo Liu  wrote:
> Looks good to me. Both patches are
>
> Reviewed-by: Leo Liu 
>

Thanks.  Can you look at the UVD patch as well?
https://patchwork.freedesktop.org/patch/233507/

Alex

>
>
> On 06/29/2018 10:58 AM, Alex Deucher wrote:
>>
>> Ping on this series?
>>
>> On Mon, Jun 25, 2018 at 1:42 PM, Alex Deucher 
>> wrote:
>>>
>>> Set the me instance in early init and use that rather than
>>> calculating the instance based on the ring pointer.
>>>
>>> Signed-off-by: Alex Deucher 
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/cik_sdma.c  | 12 ++--
>>>   drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 12 ++--
>>>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 14 ++
>>>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 23 +++
>>>   4 files changed, 29 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
>>> b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
>>> index a7576255cc30..dbd553a8d584 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
>>> @@ -177,9 +177,8 @@ static uint64_t cik_sdma_ring_get_rptr(struct
>>> amdgpu_ring *ring)
>>>   static uint64_t cik_sdma_ring_get_wptr(struct amdgpu_ring *ring)
>>>   {
>>>  struct amdgpu_device *adev = ring->adev;
>>> -   u32 me = (ring == >sdma.instance[0].ring) ? 0 : 1;
>>>
>>> -   return (RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me]) & 0x3fffc)
>>> >> 2;
>>> +   return (RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me]) &
>>> 0x3fffc) >> 2;
>>>   }
>>>
>>>   /**
>>> @@ -192,9 +191,8 @@ static uint64_t cik_sdma_ring_get_wptr(struct
>>> amdgpu_ring *ring)
>>>   static void cik_sdma_ring_set_wptr(struct amdgpu_ring *ring)
>>>   {
>>>  struct amdgpu_device *adev = ring->adev;
>>> -   u32 me = (ring == >sdma.instance[0].ring) ? 0 : 1;
>>>
>>> -   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me],
>>> +   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me],
>>>  (lower_32_bits(ring->wptr) << 2) & 0x3fffc);
>>>   }
>>>
>>> @@ -248,7 +246,7 @@ static void cik_sdma_ring_emit_hdp_flush(struct
>>> amdgpu_ring *ring)
>>>SDMA_POLL_REG_MEM_EXTRA_FUNC(3)); /* == */
>>>  u32 ref_and_mask;
>>>
>>> -   if (ring == >adev->sdma.instance[0].ring)
>>> +   if (ring->me == 0)
>>>  ref_and_mask = GPU_HDP_FLUSH_DONE__SDMA0_MASK;
>>>  else
>>>  ref_and_mask = GPU_HDP_FLUSH_DONE__SDMA1_MASK;
>>> @@ -1290,8 +1288,10 @@ static void cik_sdma_set_ring_funcs(struct
>>> amdgpu_device *adev)
>>>   {
>>>  int i;
>>>
>>> -   for (i = 0; i < adev->sdma.num_instances; i++)
>>> +   for (i = 0; i < adev->sdma.num_instances; i++) {
>>>  adev->sdma.instance[i].ring.funcs =
>>> _sdma_ring_funcs;
>>> +   adev->sdma.instance[i].ring.me = i;
>>> +   }
>>>   }
>>>
>>>   static const struct amdgpu_irq_src_funcs cik_sdma_trap_irq_funcs = {
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
>>> index c7190c39c4f5..cee4fae76d20 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
>>> @@ -202,8 +202,7 @@ static uint64_t sdma_v2_4_ring_get_rptr(struct
>>> amdgpu_ring *ring)
>>>   static uint64_t sdma_v2_4_ring_get_wptr(struct amdgpu_ring *ring)
>>>   {
>>>  struct amdgpu_device *adev = ring->adev;
>>> -   int me = (ring == >adev->sdma.instance[0].ring) ? 0 : 1;
>>> -   u32 wptr = RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me]) >> 2;
>>> +   u32 wptr = RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me])
>>> >> 2;
>>>
>>>  return wptr;
>>>   }
>>> @@ -218,9 +217,8 @@ static uint64_t sdma_v2_4_ring_get_wptr(struct
>>> amdgpu_ring *ring)
>>>   static void sdma_v2_4_ring_set_wptr(struct amdgpu_ring *ring)
>>>   {
>>>  struct amdgpu_device *adev = ring->adev;
>>> -   int me = (ring == >adev->sdma.instance[0].ring) ? 0 : 1;
>>>
>>> -   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me],
>>> lower_32_bits(ring->wptr) << 2);
>>> +   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me],
>>> lower_32_bits(ring->wptr) << 2);
>>>   }
>>>
>>>   static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring,
>>> uint32_t count)
>>> @@ -273,7 +271,7 @@ static void sdma_v2_4_ring_emit_hdp_flush(struct
>>> amdgpu_ring *ring)
>>>   {
>>>  u32 ref_and_mask = 0;
>>>
>>> -   if (ring == >adev->sdma.instance[0].ring)
>>> +   if (ring->me == 0)
>>>  ref_and_mask = REG_SET_FIELD(ref_and_mask,
>>> GPU_HDP_FLUSH_DONE, SDMA0, 1);
>>>  else
>>>  ref_and_mask = REG_SET_FIELD(ref_and_mask,
>>> GPU_HDP_FLUSH_DONE, SDMA1, 1);
>>> @@ -1213,8 +1211,10 @@ static void sdma_v2_4_set_ring_funcs(struct
>>> amdgpu_device *adev)
>>>   {
>>>  int i;
>>>
>>> -   for (i = 0; i < adev->sdma.num_instances; i++)
>>> +   for 

Re: [PATCH 1/2] drm/amdgpu/sdma: simplify sdma instance setup

[Leo Liu]

Looks good to me. Both patches are

Reviewed-by: Leo Liu 


On 06/29/2018 10:58 AM, Alex Deucher wrote:

Ping on this series?

On Mon, Jun 25, 2018 at 1:42 PM, Alex Deucher  wrote:

Set the me instance in early init and use that rather than
calculating the instance based on the ring pointer.

Signed-off-by: Alex Deucher 
---
  drivers/gpu/drm/amd/amdgpu/cik_sdma.c  | 12 ++--
  drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 12 ++--
  drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 14 ++
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 23 +++
  4 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
index a7576255cc30..dbd553a8d584 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
@@ -177,9 +177,8 @@ static uint64_t cik_sdma_ring_get_rptr(struct amdgpu_ring 
*ring)
  static uint64_t cik_sdma_ring_get_wptr(struct amdgpu_ring *ring)
  {
 struct amdgpu_device *adev = ring->adev;
-   u32 me = (ring == >sdma.instance[0].ring) ? 0 : 1;

-   return (RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me]) & 0x3fffc) >> 2;
+   return (RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me]) & 0x3fffc) 
>> 2;
  }

  /**
@@ -192,9 +191,8 @@ static uint64_t cik_sdma_ring_get_wptr(struct amdgpu_ring 
*ring)
  static void cik_sdma_ring_set_wptr(struct amdgpu_ring *ring)
  {
 struct amdgpu_device *adev = ring->adev;
-   u32 me = (ring == >sdma.instance[0].ring) ? 0 : 1;

-   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me],
+   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me],
 (lower_32_bits(ring->wptr) << 2) & 0x3fffc);
  }

@@ -248,7 +246,7 @@ static void cik_sdma_ring_emit_hdp_flush(struct amdgpu_ring 
*ring)
   SDMA_POLL_REG_MEM_EXTRA_FUNC(3)); /* == */
 u32 ref_and_mask;

-   if (ring == >adev->sdma.instance[0].ring)
+   if (ring->me == 0)
 ref_and_mask = GPU_HDP_FLUSH_DONE__SDMA0_MASK;
 else
 ref_and_mask = GPU_HDP_FLUSH_DONE__SDMA1_MASK;
@@ -1290,8 +1288,10 @@ static void cik_sdma_set_ring_funcs(struct amdgpu_device 
*adev)
  {
 int i;

-   for (i = 0; i < adev->sdma.num_instances; i++)
+   for (i = 0; i < adev->sdma.num_instances; i++) {
 adev->sdma.instance[i].ring.funcs = _sdma_ring_funcs;
+   adev->sdma.instance[i].ring.me = i;
+   }
  }

  static const struct amdgpu_irq_src_funcs cik_sdma_trap_irq_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
index c7190c39c4f5..cee4fae76d20 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
@@ -202,8 +202,7 @@ static uint64_t sdma_v2_4_ring_get_rptr(struct amdgpu_ring 
*ring)
  static uint64_t sdma_v2_4_ring_get_wptr(struct amdgpu_ring *ring)
  {
 struct amdgpu_device *adev = ring->adev;
-   int me = (ring == >adev->sdma.instance[0].ring) ? 0 : 1;
-   u32 wptr = RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me]) >> 2;
+   u32 wptr = RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me]) >> 2;

 return wptr;
  }
@@ -218,9 +217,8 @@ static uint64_t sdma_v2_4_ring_get_wptr(struct amdgpu_ring 
*ring)
  static void sdma_v2_4_ring_set_wptr(struct amdgpu_ring *ring)
  {
 struct amdgpu_device *adev = ring->adev;
-   int me = (ring == >adev->sdma.instance[0].ring) ? 0 : 1;

-   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me], lower_32_bits(ring->wptr) 
<< 2);
+   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me], 
lower_32_bits(ring->wptr) << 2);
  }

  static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t 
count)
@@ -273,7 +271,7 @@ static void sdma_v2_4_ring_emit_hdp_flush(struct 
amdgpu_ring *ring)
  {
 u32 ref_and_mask = 0;

-   if (ring == >adev->sdma.instance[0].ring)
+   if (ring->me == 0)
 ref_and_mask = REG_SET_FIELD(ref_and_mask, GPU_HDP_FLUSH_DONE, 
SDMA0, 1);
 else
 ref_and_mask = REG_SET_FIELD(ref_and_mask, GPU_HDP_FLUSH_DONE, 
SDMA1, 1);
@@ -1213,8 +1211,10 @@ static void sdma_v2_4_set_ring_funcs(struct 
amdgpu_device *adev)
  {
 int i;

-   for (i = 0; i < adev->sdma.num_instances; i++)
+   for (i = 0; i < adev->sdma.num_instances; i++) {
 adev->sdma.instance[i].ring.funcs = _v2_4_ring_funcs;
+   adev->sdma.instance[i].ring.me = i;
+   }
  }

  static const struct amdgpu_irq_src_funcs sdma_v2_4_trap_irq_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
index aa9ab299fd32..99616dd9594f 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -365,9 +365,7 @@ static uint64_t sdma_v3_0_ring_get_wptr(struct amdgpu_ring 
*ring)
 /* XXX check if swapping is 

Re: [PATCH] drm/amdgpu: fix user fence write race condition

[Alex Deucher]

On Fri, Jun 29, 2018 at 7:30 AM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> The buffer object backing the user fence is reserved using the non-user
> fence, i.e., as soon as the non-user fence is signaled, the user fence
> buffer object can be moved or even destroyed.
>
> Therefore, emit the user fence first.
>
> Both fences have the same cache invalidation behavior, so this should
> have no user-visible effect.
>
> Signed-off-by: Nicolai Hähnle 
> ---
> There is one aspect to this change that I'm a bit unsure about: what does
> insert_end do? It's only used by UVD & friends, and since those rings
> don't use user fences I guess this patch doesn't really change anything
> for them. And having the insert_end between those fences always looked
> a bit suspicious...

It should be fine I think.  some MM engines just need start and end
markers around a work batch.  They don't support user fences so it
shouldn't be an issue.  Patch looks good to me.

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 829c4d2a33b9..8117b8c2113e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -227,38 +227,38 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, 
> unsigned num_ibs,
> if (ring->funcs->emit_tmz)
> amdgpu_ring_emit_tmz(ring, false);
>
> if (ring->funcs->emit_hdp_invalidate
>  #ifdef CONFIG_X86_64
> && !(adev->flags & AMD_IS_APU)
>  #endif
>)
> amdgpu_ring_emit_hdp_invalidate(ring);
>
> +   /* wrap the last IB with fence */
> +   if (job && job->uf_addr) {
> +   amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence,
> +  AMDGPU_FENCE_FLAG_64BIT);
> +   }
> +
> r = amdgpu_fence_emit(ring, f);
> if (r) {
> dev_err(adev->dev, "failed to emit fence (%d)\n", r);
> if (job && job->vmid)
> amdgpu_vmid_reset(adev, ring->funcs->vmhub, 
> job->vmid);
> amdgpu_ring_undo(ring);
> return r;
> }
>
> if (ring->funcs->insert_end)
> ring->funcs->insert_end(ring);
>
> -   /* wrap the last IB with fence */
> -   if (job && job->uf_addr) {
> -   amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence,
> -  AMDGPU_FENCE_FLAG_64BIT);
> -   }
> -
> if (patch_offset != ~0 && ring->funcs->patch_cond_exec)
> amdgpu_ring_patch_cond_exec(ring, patch_offset);
>
> ring->current_ctx = fence_ctx;
> if (vm && ring->funcs->emit_switch_buffer)
> amdgpu_ring_emit_switch_buffer(ring);
> amdgpu_ring_commit(ring);
> return 0;
>  }
>
> --
> 2.14.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/5] drm: use core pcie functionality for pcie gen/width

[Alex Deucher]

Ping on the drm and amdgpu patches.

Alex

On Mon, Jun 25, 2018 at 5:06 PM, Alex Deucher  wrote:
> This series exports some pcie helper functions for use by drivers and
> fixes up the amdgpu and radeon drivers to use this core functionality
> rather than the duplicated functionality in the drm.  Finally we remove
> the drm helpers since the duplicate the pcie functionality of the core.
> This also adds proper pcie gen4 detection for amdgpu.
>
> Alex Deucher (5):
>   pci: export pcie_get_speed_cap and pcie_get_width_cap
>   drm/amdgpu: update amd_pcie.h to include gen4 speeds
>   drm/amdgpu: use pcie functions for link width and speed
>   drm/radeon: use pcie functions for link width
>   drm: drop drm_pcie_get_speed_cap_mask and drm_pcie_get_max_link_width
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 83 
> +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c|  7 ++-
>  drivers/gpu/drm/amd/amdgpu/ci_dpm.c|  3 +-
>  drivers/gpu/drm/amd/amdgpu/si_dpm.c|  3 +-
>  drivers/gpu/drm/amd/include/amd_pcie.h |  2 +
>  drivers/gpu/drm/drm_pci.c  | 58 -
>  drivers/gpu/drm/radeon/ci_dpm.c| 20 +--
>  drivers/gpu/drm/radeon/cik.c   | 22 
>  drivers/gpu/drm/radeon/r600_dpm.c  |  4 +-
>  drivers/gpu/drm/radeon/radeon.h|  4 ++
>  drivers/gpu/drm/radeon/si.c| 22 
>  drivers/gpu/drm/radeon/si_dpm.c| 20 +--
>  drivers/pci/pci.c  |  2 +
>  include/drm/drm_pci.h  |  7 ---
>  include/linux/pci.h|  3 ++
>  15 files changed, 132 insertions(+), 128 deletions(-)
>
> --
> 2.13.6
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu/sdma: simplify sdma instance setup

[Alex Deucher]

Ping on this series?

On Mon, Jun 25, 2018 at 1:42 PM, Alex Deucher  wrote:
> Set the me instance in early init and use that rather than
> calculating the instance based on the ring pointer.
>
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/cik_sdma.c  | 12 ++--
>  drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 12 ++--
>  drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 14 ++
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 23 +++
>  4 files changed, 29 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c 
> b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> index a7576255cc30..dbd553a8d584 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> @@ -177,9 +177,8 @@ static uint64_t cik_sdma_ring_get_rptr(struct amdgpu_ring 
> *ring)
>  static uint64_t cik_sdma_ring_get_wptr(struct amdgpu_ring *ring)
>  {
> struct amdgpu_device *adev = ring->adev;
> -   u32 me = (ring == >sdma.instance[0].ring) ? 0 : 1;
>
> -   return (RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me]) & 0x3fffc) >> 
> 2;
> +   return (RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me]) & 
> 0x3fffc) >> 2;
>  }
>
>  /**
> @@ -192,9 +191,8 @@ static uint64_t cik_sdma_ring_get_wptr(struct amdgpu_ring 
> *ring)
>  static void cik_sdma_ring_set_wptr(struct amdgpu_ring *ring)
>  {
> struct amdgpu_device *adev = ring->adev;
> -   u32 me = (ring == >sdma.instance[0].ring) ? 0 : 1;
>
> -   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me],
> +   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me],
> (lower_32_bits(ring->wptr) << 2) & 0x3fffc);
>  }
>
> @@ -248,7 +246,7 @@ static void cik_sdma_ring_emit_hdp_flush(struct 
> amdgpu_ring *ring)
>   SDMA_POLL_REG_MEM_EXTRA_FUNC(3)); /* == */
> u32 ref_and_mask;
>
> -   if (ring == >adev->sdma.instance[0].ring)
> +   if (ring->me == 0)
> ref_and_mask = GPU_HDP_FLUSH_DONE__SDMA0_MASK;
> else
> ref_and_mask = GPU_HDP_FLUSH_DONE__SDMA1_MASK;
> @@ -1290,8 +1288,10 @@ static void cik_sdma_set_ring_funcs(struct 
> amdgpu_device *adev)
>  {
> int i;
>
> -   for (i = 0; i < adev->sdma.num_instances; i++)
> +   for (i = 0; i < adev->sdma.num_instances; i++) {
> adev->sdma.instance[i].ring.funcs = _sdma_ring_funcs;
> +   adev->sdma.instance[i].ring.me = i;
> +   }
>  }
>
>  static const struct amdgpu_irq_src_funcs cik_sdma_trap_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c 
> b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index c7190c39c4f5..cee4fae76d20 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -202,8 +202,7 @@ static uint64_t sdma_v2_4_ring_get_rptr(struct 
> amdgpu_ring *ring)
>  static uint64_t sdma_v2_4_ring_get_wptr(struct amdgpu_ring *ring)
>  {
> struct amdgpu_device *adev = ring->adev;
> -   int me = (ring == >adev->sdma.instance[0].ring) ? 0 : 1;
> -   u32 wptr = RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me]) >> 2;
> +   u32 wptr = RREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me]) >> 2;
>
> return wptr;
>  }
> @@ -218,9 +217,8 @@ static uint64_t sdma_v2_4_ring_get_wptr(struct 
> amdgpu_ring *ring)
>  static void sdma_v2_4_ring_set_wptr(struct amdgpu_ring *ring)
>  {
> struct amdgpu_device *adev = ring->adev;
> -   int me = (ring == >adev->sdma.instance[0].ring) ? 0 : 1;
>
> -   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[me], 
> lower_32_bits(ring->wptr) << 2);
> +   WREG32(mmSDMA0_GFX_RB_WPTR + sdma_offsets[ring->me], 
> lower_32_bits(ring->wptr) << 2);
>  }
>
>  static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t 
> count)
> @@ -273,7 +271,7 @@ static void sdma_v2_4_ring_emit_hdp_flush(struct 
> amdgpu_ring *ring)
>  {
> u32 ref_and_mask = 0;
>
> -   if (ring == >adev->sdma.instance[0].ring)
> +   if (ring->me == 0)
> ref_and_mask = REG_SET_FIELD(ref_and_mask, 
> GPU_HDP_FLUSH_DONE, SDMA0, 1);
> else
> ref_and_mask = REG_SET_FIELD(ref_and_mask, 
> GPU_HDP_FLUSH_DONE, SDMA1, 1);
> @@ -1213,8 +1211,10 @@ static void sdma_v2_4_set_ring_funcs(struct 
> amdgpu_device *adev)
>  {
> int i;
>
> -   for (i = 0; i < adev->sdma.num_instances; i++)
> +   for (i = 0; i < adev->sdma.num_instances; i++) {
> adev->sdma.instance[i].ring.funcs = _v2_4_ring_funcs;
> +   adev->sdma.instance[i].ring.me = i;
> +   }
>  }
>
>  static const struct amdgpu_irq_src_funcs sdma_v2_4_trap_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c 
> b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index aa9ab299fd32..99616dd9594f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -365,9 +365,7 @@ static uint64_t 

Re: [PATCH] drm/amdgpu: update uvd_v6_0_ring_vm_funcs to use new nop packet

[Alex Deucher]

Ping?

On Thu, Jun 28, 2018 at 2:37 PM, Alex Deucher  wrote:
> Was missed when updating the uvd 6 module.
>
> Fixes: 1aac3c9180 (drm/amdgpu: fix insert nop for UVD6 ring)
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 1df1c6115341..8ee1c2eaaa14 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -1569,7 +1569,6 @@ static const struct amdgpu_ring_funcs 
> uvd_v6_0_ring_phys_funcs = {
>  static const struct amdgpu_ring_funcs uvd_v6_0_ring_vm_funcs = {
> .type = AMDGPU_RING_TYPE_UVD,
> .align_mask = 0xf,
> -   .nop = PACKET0(mmUVD_NO_OP, 0),
> .support_64bit_ptrs = false,
> .get_rptr = uvd_v6_0_ring_get_rptr,
> .get_wptr = uvd_v6_0_ring_get_wptr,
> @@ -1587,7 +1586,7 @@ static const struct amdgpu_ring_funcs 
> uvd_v6_0_ring_vm_funcs = {
> .emit_hdp_flush = uvd_v6_0_ring_emit_hdp_flush,
> .test_ring = uvd_v6_0_ring_test_ring,
> .test_ib = amdgpu_uvd_ring_test_ib,
> -   .insert_nop = amdgpu_ring_insert_nop,
> +   .insert_nop = uvd_v6_0_ring_insert_nop,
> .pad_ib = amdgpu_ring_generic_pad_ib,
> .begin_use = amdgpu_uvd_ring_begin_use,
> .end_use = amdgpu_uvd_ring_end_use,
> --
> 2.13.6
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: fix user fence write race condition

[Nicolai Hähnle]

From: Nicolai Hähnle 

The buffer object backing the user fence is reserved using the non-user
fence, i.e., as soon as the non-user fence is signaled, the user fence
buffer object can be moved or even destroyed.

Therefore, emit the user fence first.

Both fences have the same cache invalidation behavior, so this should
have no user-visible effect.

Signed-off-by: Nicolai Hähnle 
---
There is one aspect to this change that I'm a bit unsure about: what does
insert_end do? It's only used by UVD & friends, and since those rings
don't use user fences I guess this patch doesn't really change anything
for them. And having the insert_end between those fences always looked
a bit suspicious...
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 829c4d2a33b9..8117b8c2113e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -227,38 +227,38 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
num_ibs,
if (ring->funcs->emit_tmz)
amdgpu_ring_emit_tmz(ring, false);
 
if (ring->funcs->emit_hdp_invalidate
 #ifdef CONFIG_X86_64
&& !(adev->flags & AMD_IS_APU)
 #endif
   )
amdgpu_ring_emit_hdp_invalidate(ring);
 
+   /* wrap the last IB with fence */
+   if (job && job->uf_addr) {
+   amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence,
+  AMDGPU_FENCE_FLAG_64BIT);
+   }
+
r = amdgpu_fence_emit(ring, f);
if (r) {
dev_err(adev->dev, "failed to emit fence (%d)\n", r);
if (job && job->vmid)
amdgpu_vmid_reset(adev, ring->funcs->vmhub, job->vmid);
amdgpu_ring_undo(ring);
return r;
}
 
if (ring->funcs->insert_end)
ring->funcs->insert_end(ring);
 
-   /* wrap the last IB with fence */
-   if (job && job->uf_addr) {
-   amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence,
-  AMDGPU_FENCE_FLAG_64BIT);
-   }
-
if (patch_offset != ~0 && ring->funcs->patch_cond_exec)
amdgpu_ring_patch_cond_exec(ring, patch_offset);
 
ring->current_ctx = fence_ctx;
if (vm && ring->funcs->emit_switch_buffer)
amdgpu_ring_emit_switch_buffer(ring);
amdgpu_ring_commit(ring);
return 0;
 }
 
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Lukas Wunner]

On Fri, Jun 29, 2018 at 10:40:50AM +, Qu, Jim wrote:
> > That is no longer the case since v4.17.  The HDA controller now runtime
> > suspends autonomously, see commit 07f4f97d7b4b ("vga_switcheroo: Use
> > device link for HDA controller").
> > 
> > Your patch appears to be geared towards an older kernel version.
> > Please retest on the laptop in question with a v4.17+ kernel.
> 
> indeed? I used 4.13 on the platform. let me have a try with the patch
> you mentioned

The commit can't be cherry-picked by itself onto v4.13, it was part of
a larger series.  It's best to use a stock v4.17 kernel.

Note, a fix went into Linus' tree yesterday, commit 57cb54e53bdd
("ALSA: hda - Force to link down at runtime suspend on ATI/AMD HDMI").
Not sure if it's needed on your machine.

Thanks,

Lukas
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Qu, Jim]

indeed?I used 4.13 on the platform. let me have a try with the patch you 
mentioned

thinks
jimqu

?? Outlook for Android


From: Lukas Wunner 
Sent: Friday, June 29, 2018 5:21:38 PM
To: Qu, Jim
Cc: alsa-de...@alsa-project.org; dri-de...@lists.freedesktop.org; Deucher, 
Alexander; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu 
client id

On Fri, Jun 29, 2018 at 08:55:40AM +, Qu, Jim wrote:
> When our dGPU does suspend by runtime pm. amdgpu driver for dgpu will
> also call vgaswtichroo to power off its audio. vgaswitchroo driver will
> find audio codec by vgaswitchroo dgpu client id(VGA_SWITCHEROO_DIS).

That is no longer the case since v4.17.  The HDA controller now runtime
suspends autonomously, see commit 07f4f97d7b4b ("vga_switcheroo: Use
device link for HDA controller").

Your patch appears to be geared towards an older kernel version.
Please retest on the laptop in question with a v4.17+ kernel.


> I think the issue should be observed on both Intel+AMD or AMD+AMD
> platform which has the same HW configuration.
>
> 1.if dGPU has no audio codec. the issue should be always observed.
> 2.if both iGPU and dGPU has audio codecs, the issue should be random,
> it depends on the first audio found by vgaswitchroo driver is on iGPU
> or dGPU.

On discrete AMD and Nvidia GPUs, the HDA controller is function 1 and
the GPU is function 0 in the same PCI slot.

On Intel chipsets, the HDA controller and the GPU have completely
different PCI device numbers, e.g. the GPU might be :00:02.0 and
the HDA controller might be :00:1b.0.

get_bound_vga() checks that the HDA controller is function 1 and there's
a GPU in function 0 of the same slot.

Thus get_bound_vga() always returns NULL for an Intel HDA controller and
the controller is never registered with vga_switcheroo (which is fine).

Thanks,

Lukas
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: update documentation for amdgpu_drv.c

[Michel Dänzer]

On 2018-06-28 08:31 PM, Alex Deucher wrote:
> On Thu, Jun 28, 2018 at 2:02 PM, Jiang, Sonny  wrote:
>> Hi Alex,
>>
>>
>> What's your opinion about Michel's suggestion?
> 
> You should definitely update amdgpu.rst to include the new
> documentation section.  As for whether to have separate sections or
> one big section for the parameters, I could go either way.  If we have
> one big section, people will likely forget to update it when they add
> a new parameter.  On the other hand, if we have separate sections for
> each option, people will likely forget to update amdgpu.rst to add the
> new parameter.

I think it's easier to catch the latter in review, and while it is
slightly annoying needing an entry for each parameter in amdgpu.rst,
maybe it can serve as incentive against needlessly adding new module
parameters. :)

Right now there's a third possibility, see the diff below as an example. By
referencing amdgpu_drv.c without a :doc: or :internal: stanza, each DOC
comment is picked up automatically, with the text after DOC as the
heading. However, this will break down if we want to add general DOC
comments or function comments in amdgpu_drv.c.


P.S. I cherry-picked some upstream fixes from the Documentation tree to
amd-staging-drm-next, make htmldocs works again.


diff --git a/Documentation/gpu/amdgpu.rst b/Documentation/gpu/amdgpu.rst
index 765c2a32938f..a740e491dfcc 100644
--- a/Documentation/gpu/amdgpu.rst
+++ b/Documentation/gpu/amdgpu.rst
@@ -5,6 +5,13 @@
 The drm/amdgpu driver supports all AMD Radeon GPUs based on the Graphics Core
 Next (GCN) architecture.

+Module Parameters
+=
+
+The amdgpu driver supports the following module parameters:
+
+.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+
 Core Driver Infrastructure
 ==

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index dcdc97d6dc44..ff0299879601 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1,10 +1,3 @@
-/**
- * \file amdgpu_drv.c
- * AMD Amdgpu driver
- *
- * \author Gareth Hughes 
- */
-
 /*
  * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
  * All Rights Reserved.
@@ -136,6 +129,11 @@ int amdgpu_gpu_recovery = -1; /* auto */
 int amdgpu_emu_mode = 0;
 uint amdgpu_smu_memory_pool_size = 0;

+/**
+ * DOC: vramlimit
+ *
+ * Restrict VRAM for testing, in megabytes
+ */
 MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
 module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Remove amdgpu_gem_map_attach target_dev documentation

[Zhang, Jerry (Junwei)]

On 06/29/2018 05:29 PM, Michel Dänzer wrote:

From: Michel Dänzer 

Reviewed-by: Junwei Zhang 



The parameter was removed.

Fixes: a19741e5e5a9 "dma_buf: remove device parameter from attach
  callback v2"

Signed-off-by: Michel Dänzer 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index b2286bc41aec..df7226ad64b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -191,7 +191,6 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
  /**
   * amdgpu_gem_map_attach - _buf_ops.attach implementation
   * @dma_buf: shared DMA buffer
- * @target_dev: target device
   * @attach: DMA-buf attachment
   *
   * Makes sure that the shared DMA buffer can be accessed by the target device.


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Remove amdgpu_gem_map_attach target_dev documentation

[Michel Dänzer]

From: Michel Dänzer 

The parameter was removed.

Fixes: a19741e5e5a9 "dma_buf: remove device parameter from attach
 callback v2"

Signed-off-by: Michel Dänzer 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index b2286bc41aec..df7226ad64b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -191,7 +191,6 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
 /**
  * amdgpu_gem_map_attach - _buf_ops.attach implementation
  * @dma_buf: shared DMA buffer
- * @target_dev: target device
  * @attach: DMA-buf attachment
  *
  * Makes sure that the shared DMA buffer can be accessed by the target device.
-- 
2.18.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Lukas Wunner]

On Fri, Jun 29, 2018 at 08:55:40AM +, Qu, Jim wrote:
> When our dGPU does suspend by runtime pm. amdgpu driver for dgpu will
> also call vgaswtichroo to power off its audio. vgaswitchroo driver will
> find audio codec by vgaswitchroo dgpu client id(VGA_SWITCHEROO_DIS).

That is no longer the case since v4.17.  The HDA controller now runtime
suspends autonomously, see commit 07f4f97d7b4b ("vga_switcheroo: Use
device link for HDA controller").

Your patch appears to be geared towards an older kernel version.
Please retest on the laptop in question with a v4.17+ kernel.


> I think the issue should be observed on both Intel+AMD or AMD+AMD
> platform which has the same HW configuration.
> 
> 1.if dGPU has no audio codec. the issue should be always observed.
> 2.if both iGPU and dGPU has audio codecs, the issue should be random,
> it depends on the first audio found by vgaswitchroo driver is on iGPU
> or dGPU.

On discrete AMD and Nvidia GPUs, the HDA controller is function 1 and
the GPU is function 0 in the same PCI slot.

On Intel chipsets, the HDA controller and the GPU have completely
different PCI device numbers, e.g. the GPU might be :00:02.0 and
the HDA controller might be :00:1b.0.

get_bound_vga() checks that the HDA controller is function 1 and there's
a GPU in function 0 of the same slot.

Thus get_bound_vga() always returns NULL for an Intel HDA controller and
the controller is never registered with vga_switcheroo (which is fine).

Thanks,

Lukas
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Zhang, Jerry (Junwei)]

On 06/29/2018 04:58 PM, Qu, Jim wrote:

Hi Jerry,

No, the issue is dGPU has no audio. and the HDA driver register iGPU audio as 
VGA_SWITCHEROO_DIS. So when dGPU suspend by runtime pm, iGPU audio will be 
powerd off, since its has the same client id with dGPU.


Thanks to clarify the situation.
anyway, HDA should separate audio from iGPU and dGPU.

>> The Key pointer is the problem: does it exist the GFX device is defined as 
PCI_CLASS_DISPLAY_OTHER with audio codec.

IIRC, for HG case, the dGPU will be registered as OTHER.
if it has audio codec, suppose yes.

Jerry




Thanks
JimQu


发件人: Zhang, Jerry
发送时间: 2018年6月29日 16:54
收件人: Qu, Jim; Alex Deucher
抄送: Deucher, Alexander; amd-gfx list
主题: Re: 答复: 答复: [PATCH] vgaswitchroo: set audio client id according to bound 
gpu client id

On 06/29/2018 04:06 PM, Qu, Jim wrote:

I mean if the VGA_OTHER is only for headless, and there is no audio on the 
headless GFX. therefore, the PCI_CLASS_DISPLAY_OTHER is unnecessary in 
get_bound_gpu() on this case.

The Key pointer is the problem: does it exist the GFX device is defined as 
PCI_CLASS_DISPLAY_OTHER with audio codec.


In my view, headless is likely to have no audio,
while dGPU with display could be registered as OTHER, in this case, audio is 
controlled by HDA driver.

To my understanding, this issue is about the dGPU has audio codec registered as 
OTHER, right?

Jerry



Thanks
JimQu


发件人: amd-gfx  代表 Zhang, Jerry (Junwei) 

发送时间: 2018年6月29日 15:40:14
收件人: Qu, Jim; Alex Deucher
抄送: Deucher, Alexander; amd-gfx list
主题: Re: 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu 
client id

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 
发送时间: 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)
 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 
Signed-off-by: Jim Qu 
---
 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 

Additionally, better to includes Intel guys mail list and sound mail 

答复: 答复: 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Qu, Jim]

Hi Jerry,

No, the issue is dGPU has no audio. and the HDA driver register iGPU audio as 
VGA_SWITCHEROO_DIS. So when dGPU suspend by runtime pm, iGPU audio will be 
powerd off, since its has the same client id with dGPU.

Thanks
JimQu


发件人: Zhang, Jerry
发送时间: 2018年6月29日 16:54
收件人: Qu, Jim; Alex Deucher
抄送: Deucher, Alexander; amd-gfx list
主题: Re: 答复: 答复: [PATCH] vgaswitchroo: set audio client id according to bound 
gpu client id

On 06/29/2018 04:06 PM, Qu, Jim wrote:
> I mean if the VGA_OTHER is only for headless, and there is no audio on the 
> headless GFX. therefore, the PCI_CLASS_DISPLAY_OTHER is unnecessary in 
> get_bound_gpu() on this case.
>
> The Key pointer is the problem: does it exist the GFX device is defined as 
> PCI_CLASS_DISPLAY_OTHER with audio codec.

In my view, headless is likely to have no audio,
while dGPU with display could be registered as OTHER, in this case, audio is 
controlled by HDA driver.

To my understanding, this issue is about the dGPU has audio codec registered as 
OTHER, right?

Jerry

>
> Thanks
> JimQu
>
> 
> 发件人: amd-gfx  代表 Zhang, Jerry (Junwei) 
> 
> 发送时间: 2018年6月29日 15:40:14
> 收件人: Qu, Jim; Alex Deucher
> 抄送: Deucher, Alexander; amd-gfx list
> 主题: Re: 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu 
> client id
>
> 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 
>> 发送时间: 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)
>>  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 
 Signed-off-by: Jim Qu 
 ---
 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 
>>>
>>> Additionally, 

答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Qu, Jim]

Hi Lukas,

Thanks to your response, please see comments in line.

Thanks
JimQu


发件人: Lukas Wunner 
发送时间: 2018年6月29日 16:06
收件人: Qu, Jim
抄送: alsa-de...@alsa-project.org; dri-de...@lists.freedesktop.org; Deucher, 
Alexander; amd-gfx@lists.freedesktop.org
主题: Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu client 
id

> > On Thu, Jun 28, 2018 at 2:22 AM, 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.

> Please explain what you mean by UMA here.  Are you referring to
> dual GPU systems where both GPUs are by AMD?  And the integrated
> GPU is incorrectly assigned VGA_SWITCHEROO_DIS?

> Starting with v4.17, the only reason the audio driver registers
> with vga_switcheroo is because it can be powered down if manual
> power control is used.  However manual power control is not the
> default, it's primary use case nowadays is MacBook Pros, and
> there are no MacBook Pros with dual AMD GPUs.  So I don't
> understand what real world use case you're trying to fix.
> Please explain.

Jim: Yeah, this a laptop which has two GPUs, iGPU has a HD audio for HDMI 
output. dGPU is headless which is only for offload render. This is a grapchis 
offloading solution for hybird GFX.

When our dGPU does suspend by runtime pm. amdgpu driver for dgpu will also call 
vgaswtichroo to power off its audio. vgaswitchroo driver will find audio codec 
by vgaswitchroo dgpu client id(VGA_SWITCHEROO_DIS).

Current HDA driver will register iGPU's audio client to vgaswitchroo as 
VGA_SWITCHEROO_DIS. so iGPU's audio has been powered off. It causes the issue 
that no sound on the HDMI device.

I think the issue should be observed on both Intel+AMD or AMD+AMD platform 
which has the same HW
configuration.

1.if dGPU has no audio codec. the issue should be always observed.
2.if both iGPU and dGPU has audio codecs, the issue should be random, it 
depends on the first audio found by vgaswitchroo driver is on iGPU or dGPU.

> > 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()

> Unlike the other changes in your patch, this one would seem
> perfectly valid to me.  However it's a separate issue, so
> please put it in a separate patch.

> Instead of adding a check for PCI_CLASS_DISPLAY_OTHER, I'd prefer
> you replacing the existing check for PCI_CLASS_DISPLAY_VGA with a
> check for PCI_BASE_CLASS_DISPLAY, like so:

> -   if ((p->class >> 8) == PCI_CLASS_DISPLAY_VGA)
> +   if ((p->class >> 16) == 
> PCI_BASE_CLASS_DISPLAY)

Jim: That is fine, will update it.

Thanks,

Lukas
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Zhang, Jerry (Junwei)]

On 06/29/2018 04:06 PM, Qu, Jim wrote:

I mean if the VGA_OTHER is only for headless, and there is no audio on the 
headless GFX. therefore, the PCI_CLASS_DISPLAY_OTHER is unnecessary in 
get_bound_gpu() on this case.

The Key pointer is the problem: does it exist the GFX device is defined as 
PCI_CLASS_DISPLAY_OTHER with audio codec.


In my view, headless is likely to have no audio,
while dGPU with display could be registered as OTHER, in this case, audio is 
controlled by HDA driver.

To my understanding, this issue is about the dGPU has audio codec registered as 
OTHER, right?

Jerry



Thanks
JimQu


发件人: amd-gfx  代表 Zhang, Jerry (Junwei) 

发送时间: 2018年6月29日 15:40:14
收件人: Qu, Jim; Alex Deucher
抄送: Deucher, Alexander; amd-gfx list
主题: Re: 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu 
client id

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 
发送时间: 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)
 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 
Signed-off-by: Jim Qu 
---
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 

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);
+

Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Lukas Wunner]

> On Thu, Jun 28, 2018 at 2:22 AM, 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.

Please explain what you mean by UMA here.  Are you referring to
dual GPU systems where both GPUs are by AMD?  And the integrated
GPU is incorrectly assigned VGA_SWITCHEROO_DIS?

Starting with v4.17, the only reason the audio driver registers
with vga_switcheroo is because it can be powered down if manual
power control is used.  However manual power control is not the
default, it's primary use case nowadays is MacBook Pros, and
there are no MacBook Pros with dual AMD GPUs.  So I don't
understand what real world use case you're trying to fix.
Please explain.


> > 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()

Unlike the other changes in your patch, this one would seem
perfectly valid to me.  However it's a separate issue, so
please put it in a separate patch.

Instead of adding a check for PCI_CLASS_DISPLAY_OTHER, I'd prefer
you replacing the existing check for PCI_CLASS_DISPLAY_VGA with a
check for PCI_BASE_CLASS_DISPLAY, like so:

-   if ((p->class >> 8) == PCI_CLASS_DISPLAY_VGA)
+   if ((p->class >> 16) == PCI_BASE_CLASS_DISPLAY)

Thanks,

Lukas
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

答复: 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Qu, Jim]

I mean if the VGA_OTHER is only for headless, and there is no audio on the 
headless GFX. therefore, the PCI_CLASS_DISPLAY_OTHER is unnecessary in 
get_bound_gpu() on this case.

The Key pointer is the problem: does it exist the GFX device is defined as 
PCI_CLASS_DISPLAY_OTHER with audio codec.

Thanks
JimQu


发件人: amd-gfx  代表 Zhang, Jerry (Junwei) 

发送时间: 2018年6月29日 15:40:14
收件人: Qu, Jim; Alex Deucher
抄送: Deucher, Alexander; amd-gfx list
主题: Re: 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu 
client id

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 
> 发送时间: 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)
>  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 
>>> Signed-off-by: Jim Qu 
>>> ---
>>>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 
>>
>> 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
>>> 

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

[Zhang, Jerry (Junwei)]

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 
发送时间: 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)
 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 
Signed-off-by: Jim Qu 
---
   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 

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 

答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Qu, Jim]

HI,

Any comments for the patch?

Thanks
JimQu


发件人: Alex Deucher 
发送时间: 2018年6月28日 20:43
收件人: Qu, Jim; alsa-de...@alsa-project.org; Maling list - DRI developers
抄送: amd-gfx list; Deucher, Alexander
主题: Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu client 
id

On Thu, Jun 28, 2018 at 2:22 AM, 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()
>
> The patch also combine the HDA change to avoid break
> building.
>
> Change-Id: I9906c1bd4dd5b36108d7133bb1cf724d13f1cd6d
> Signed-off-by: Alex Deucher 
> Signed-off-by: Jim Qu 
> ---
>  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(-)
>

Please resend and send it to dri-devel and alsa-de...@alsa-project.org
since this patch touches multiple subsystems.  I would propose we
merge this through the drm tree if possible.

Alex

> 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)
> +   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 

Re: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Zhang, Jerry (Junwei)]

On 06/29/2018 12:01 PM, Alex Deucher wrote:

On Thu, Jun 28, 2018 at 10:54 PM, Zhang, Jerry (Junwei)
 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.


Yeah, I can get that.

While the atpx detection also confuse me then.
It seems must have one VGA and one OTHER, but OTHER(dGPU) could also be 
connected
to a external display(e.g. via HDMI).
Or should we say OTHER could has display but not default one?

Jerry



Alex






The patch also combine the HDA change to avoid break
building.

Change-Id: I9906c1bd4dd5b36108d7133bb1cf724d13f1cd6d
Signed-off-by: Alex Deucher 
Signed-off-by: Jim Qu 
---
   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 

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, 

答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Qu, Jim]

If the GFX is headless, the audio codec should be disabled on PCIE bus. the the 
HDA driver will never probe the audio. right?

Thanks
JimQu


发件人: Alex Deucher 
发送时间: 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)
 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 
>> Signed-off-by: Jim Qu 
>> ---
>>   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 
>
> 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