Re: [PATCH v2 0/3] Resolve warnings from AMDGPU

2023-02-20 Thread Christian König

Acked-by: Christian König  for the whole series.

Am 17.02.23 um 19:14 schrieb Arthur Grillo:

Hi,

This series resolve some of the warnings that appear when compiling AMDGPU
with W=1.

Each patch is focused in a specific warning.

This is my First Patch for the GSoC Project Idea about increasing code
coverage of the DRM code[1].

Thanks for reviewing!

Best regards,
Arthur Grillo

[1]: https://www.x.org/wiki/DRMcoverage2023/#firstpatch

---

v1 -> v2: 
https://lore.kernel.org/all/20230213204923.111948-1-arthurgri...@riseup.net/

- Use dm_odm_combine_mode_disabled dm_odm_combine_mode_2to1 instead of an enum 
casting
- Maintain register read

---

Arthur Grillo (3):
   drm/amd/display: Fix implicit enum conversion
   drm/amd/display: Remove unused local variables
   drm/amd/display: Remove unused local variables and function

  .../amd/display/dc/dcn10/dcn10_link_encoder.c |  3 +-
  .../drm/amd/display/dc/dcn201/dcn201_dpp.c|  7 
  .../drm/amd/display/dc/dcn201/dcn201_hwseq.c  |  2 -
  .../gpu/drm/amd/display/dc/dcn30/dcn30_afmt.c |  2 -
  .../gpu/drm/amd/display/dc/dcn30/dcn30_hubp.c |  4 --
  .../drm/amd/display/dc/dcn30/dcn30_hwseq.c|  3 --
  .../gpu/drm/amd/display/dc/dcn31/dcn31_apg.c  | 41 ---
  .../drm/amd/display/dc/dcn32/dcn32_resource.c |  5 +--
  .../display/dc/dcn32/dcn32_resource_helpers.c |  4 --
  .../dc/dml/dcn20/display_mode_vba_20.c|  9 ++--
  .../dc/dml/dcn20/display_mode_vba_20v2.c  | 11 ++---
  .../dc/dml/dcn21/display_mode_vba_21.c| 12 +++---
  .../dc/dml/dcn31/display_rq_dlg_calc_31.c |  2 -
  .../dc/link/protocols/link_dp_capability.c|  4 --
  14 files changed, 19 insertions(+), 90 deletions(-)





Re: [PATCH] drm/gem: Expose the buffer object handle to userspace last

2023-02-20 Thread Christian König

Am 20.02.23 um 10:55 schrieb Tvrtko Ursulin:


Hi,

On 14/02/2023 13:59, Christian König wrote:

Am 14.02.23 um 13:50 schrieb Tvrtko Ursulin:

From: Tvrtko Ursulin 

Currently drm_gem_handle_create_tail exposes the handle to userspace
before the buffer object constructions is complete. This allowing
of working against a partially constructed object, which may also be in
the process of having its creation fail, can have a range of negative
outcomes.

A lot of those will depend on what the individual drivers are doing in
their obj->funcs->open() callbacks, and also with a common failure mode
being -ENOMEM from drm_vma_node_allow.

We can make sure none of this can happen by allocating a handle last,
although with a downside that more of the function now runs under the
dev->object_name_lock.

Looking into the individual drivers open() hooks, we have
amdgpu_gem_object_open which seems like it could have a potential 
security

issue without this change.

A couple drivers like qxl_gem_object_open and vmw_gem_object_open
implement no-op hooks so no impact for them.

A bunch of other require a deeper look by individual owners to asses 
for

impact. Those are lima_gem_object_open, nouveau_gem_object_open,
panfrost_gem_open, radeon_gem_object_open and 
virtio_gpu_gem_object_open.


Putting aside the risk assesment of the above, some common scenarios to
think about are along these lines:

1)
Userspace closes a handle by speculatively "guessing" it from a second
thread.

This results in an unreachable buffer object so, a memory leak.

2)
Same as 1), but object is in the process of getting closed (failed
creation).

The second thread is then able to re-cycle the handle and idr_remove 
would
in the first thread would then remove the handle it does not own 
from the

idr.

3)
Going back to the earlier per driver problem space - individual impact
assesment of allowing a second thread to access and operate on a 
partially

constructed handle / object. (Can something crash? Leak information?)

In terms of identifying when the problem started I will tag some 
patches

as references, but not all, if even any, of them actually point to a
broken state. I am just identifying points at which more opportunity 
for

issues to arise was added.


Yes I've looked into this once as well, but couldn't completely solve 
it for some reason.


Give me a day or two to get this tested and all the logic swapped 
back into my head again.


Managed to recollect what the problem with earlier attempts was?


Nope, that's way to long ago. I can only assume that I ran into problems 
with the object_name_lock.


Probably best to double check if that doesn't result in a lock inversion 
when somebody grabs the reservation lock in their ->load() callback.


Regards,
Christian.



Regards,

Tvrtko


Christian.



References: 304eda32920b ("drm/gem: add hooks to notify driver when 
object handle is created/destroyed")

References: ca481c9b2a3a ("drm/gem: implement vma access management")
References: b39b5394fabc ("drm/gem: Add drm_gem_object_funcs")
Cc: dri-de...@lists.freedesktop.org
Cc: Rob Clark 
Cc: Ben Skeggs 
Cc: David Herrmann 
Cc: Noralf Trønnes 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: l...@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Cc: Steven Price 
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-de...@lists.freedesktop.org
Cc: Zack Rusin 
---
  drivers/gpu/drm/drm_gem.c | 48 
+++

  1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index aa15c52ae182..e3d897bca0f2 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -356,52 +356,52 @@ drm_gem_handle_create_tail(struct drm_file 
*file_priv,

 u32 *handlep)
  {
  struct drm_device *dev = obj->dev;
-    u32 handle;
  int ret;
WARN_ON(!mutex_is_locked(>object_name_lock));
  if (obj->handle_count++ == 0)
  drm_gem_object_get(obj);
+    ret = drm_vma_node_allow(>vma_node, file_priv);
+    if (ret)
+    goto err_put;
+
+    if (obj->funcs->open) {
+    ret = obj->funcs->open(obj, file_priv);
+    if (ret)
+    goto err_revoke;
+    }
+
  /*
- * Get the user-visible handle using idr.  Preload and perform
- * allocation under our spinlock.
+ * Get the user-visible handle using idr as the _last_ step.
+ * Preload and perform allocation under our spinlock.
   */
  idr_preload(GFP_KERNEL);
  spin_lock(_priv->table_lock);
-
  ret = idr_alloc(_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
-
  spin_unlock(_priv->table_lock);
  idr_preload_end();
-    mutex_unlock(>object_name_lock);
  if (ret < 0)
-    goto err_unref;
-
-    handle = ret;
+    goto err_close;
-    ret = drm_vma_node_allow(>vma_node, file_priv);
-    if (ret)
-    goto err_remove;
+    mutex_unlock(>object_name_lock);
-    if (obj->funcs->open) {
-   

Re: [PATCH] drm/gem: Expose the buffer object handle to userspace last

2023-02-20 Thread Tvrtko Ursulin



Hi,

On 14/02/2023 13:59, Christian König wrote:

Am 14.02.23 um 13:50 schrieb Tvrtko Ursulin:

From: Tvrtko Ursulin 

Currently drm_gem_handle_create_tail exposes the handle to userspace
before the buffer object constructions is complete. This allowing
of working against a partially constructed object, which may also be in
the process of having its creation fail, can have a range of negative
outcomes.

A lot of those will depend on what the individual drivers are doing in
their obj->funcs->open() callbacks, and also with a common failure mode
being -ENOMEM from drm_vma_node_allow.

We can make sure none of this can happen by allocating a handle last,
although with a downside that more of the function now runs under the
dev->object_name_lock.

Looking into the individual drivers open() hooks, we have
amdgpu_gem_object_open which seems like it could have a potential 
security

issue without this change.

A couple drivers like qxl_gem_object_open and vmw_gem_object_open
implement no-op hooks so no impact for them.

A bunch of other require a deeper look by individual owners to asses for
impact. Those are lima_gem_object_open, nouveau_gem_object_open,
panfrost_gem_open, radeon_gem_object_open and virtio_gpu_gem_object_open.

Putting aside the risk assesment of the above, some common scenarios to
think about are along these lines:

1)
Userspace closes a handle by speculatively "guessing" it from a second
thread.

This results in an unreachable buffer object so, a memory leak.

2)
Same as 1), but object is in the process of getting closed (failed
creation).

The second thread is then able to re-cycle the handle and idr_remove 
would

in the first thread would then remove the handle it does not own from the
idr.

3)
Going back to the earlier per driver problem space - individual impact
assesment of allowing a second thread to access and operate on a 
partially

constructed handle / object. (Can something crash? Leak information?)

In terms of identifying when the problem started I will tag some patches
as references, but not all, if even any, of them actually point to a
broken state. I am just identifying points at which more opportunity for
issues to arise was added.


Yes I've looked into this once as well, but couldn't completely solve it 
for some reason.


Give me a day or two to get this tested and all the logic swapped back 
into my head again.


Managed to recollect what the problem with earlier attempts was?

Regards,

Tvrtko


Christian.



References: 304eda32920b ("drm/gem: add hooks to notify driver when 
object handle is created/destroyed")

References: ca481c9b2a3a ("drm/gem: implement vma access management")
References: b39b5394fabc ("drm/gem: Add drm_gem_object_funcs")
Cc: dri-de...@lists.freedesktop.org
Cc: Rob Clark 
Cc: Ben Skeggs 
Cc: David Herrmann 
Cc: Noralf Trønnes 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: l...@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Cc: Steven Price 
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-de...@lists.freedesktop.org
Cc: Zack Rusin 
---
  drivers/gpu/drm/drm_gem.c | 48 +++
  1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index aa15c52ae182..e3d897bca0f2 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -356,52 +356,52 @@ drm_gem_handle_create_tail(struct drm_file 
*file_priv,

 u32 *handlep)
  {
  struct drm_device *dev = obj->dev;
-    u32 handle;
  int ret;
  WARN_ON(!mutex_is_locked(>object_name_lock));
  if (obj->handle_count++ == 0)
  drm_gem_object_get(obj);
+    ret = drm_vma_node_allow(>vma_node, file_priv);
+    if (ret)
+    goto err_put;
+
+    if (obj->funcs->open) {
+    ret = obj->funcs->open(obj, file_priv);
+    if (ret)
+    goto err_revoke;
+    }
+
  /*
- * Get the user-visible handle using idr.  Preload and perform
- * allocation under our spinlock.
+ * Get the user-visible handle using idr as the _last_ step.
+ * Preload and perform allocation under our spinlock.
   */
  idr_preload(GFP_KERNEL);
  spin_lock(_priv->table_lock);
-
  ret = idr_alloc(_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
-
  spin_unlock(_priv->table_lock);
  idr_preload_end();
-    mutex_unlock(>object_name_lock);
  if (ret < 0)
-    goto err_unref;
-
-    handle = ret;
+    goto err_close;
-    ret = drm_vma_node_allow(>vma_node, file_priv);
-    if (ret)
-    goto err_remove;
+    mutex_unlock(>object_name_lock);
-    if (obj->funcs->open) {
-    ret = obj->funcs->open(obj, file_priv);
-    if (ret)
-    goto err_revoke;
-    }
+    *handlep = ret;
-    *handlep = handle;
  return 0;
+err_close:
+    if (obj->funcs->close)
+    obj->funcs->close(obj, file_priv);
  err_revoke:
  drm_vma_node_revoke(>vma_node, file_priv);
-err_remove:
-    

Re: [PATCH] drm/gem: Expose the buffer object handle to userspace last

2023-02-20 Thread Tvrtko Ursulin



On 20/02/2023 10:01, Christian König wrote:

Am 20.02.23 um 10:55 schrieb Tvrtko Ursulin:


Hi,

On 14/02/2023 13:59, Christian König wrote:

Am 14.02.23 um 13:50 schrieb Tvrtko Ursulin:

From: Tvrtko Ursulin 

Currently drm_gem_handle_create_tail exposes the handle to userspace
before the buffer object constructions is complete. This allowing
of working against a partially constructed object, which may also be in
the process of having its creation fail, can have a range of negative
outcomes.

A lot of those will depend on what the individual drivers are doing in
their obj->funcs->open() callbacks, and also with a common failure mode
being -ENOMEM from drm_vma_node_allow.

We can make sure none of this can happen by allocating a handle last,
although with a downside that more of the function now runs under the
dev->object_name_lock.

Looking into the individual drivers open() hooks, we have
amdgpu_gem_object_open which seems like it could have a potential 
security

issue without this change.

A couple drivers like qxl_gem_object_open and vmw_gem_object_open
implement no-op hooks so no impact for them.

A bunch of other require a deeper look by individual owners to asses 
for

impact. Those are lima_gem_object_open, nouveau_gem_object_open,
panfrost_gem_open, radeon_gem_object_open and 
virtio_gpu_gem_object_open.


Putting aside the risk assesment of the above, some common scenarios to
think about are along these lines:

1)
Userspace closes a handle by speculatively "guessing" it from a second
thread.

This results in an unreachable buffer object so, a memory leak.

2)
Same as 1), but object is in the process of getting closed (failed
creation).

The second thread is then able to re-cycle the handle and idr_remove 
would
in the first thread would then remove the handle it does not own 
from the

idr.

3)
Going back to the earlier per driver problem space - individual impact
assesment of allowing a second thread to access and operate on a 
partially

constructed handle / object. (Can something crash? Leak information?)

In terms of identifying when the problem started I will tag some 
patches

as references, but not all, if even any, of them actually point to a
broken state. I am just identifying points at which more opportunity 
for

issues to arise was added.


Yes I've looked into this once as well, but couldn't completely solve 
it for some reason.


Give me a day or two to get this tested and all the logic swapped 
back into my head again.


Managed to recollect what the problem with earlier attempts was?


Nope, that's way to long ago. I can only assume that I ran into problems 
with the object_name_lock.


Probably best to double check if that doesn't result in a lock inversion 
when somebody grabs the reservation lock in their ->load() callback.


Hmm I don't immediately follow the connection. But I have only found 
radeon_driver_load_kms as using the load callback. Is there any lockdep 
enabled CI for that driver which could tell us if there is a problem there?


Regards,

Tvrtko



Regards,
Christian.



Regards,

Tvrtko


Christian.



References: 304eda32920b ("drm/gem: add hooks to notify driver when 
object handle is created/destroyed")

References: ca481c9b2a3a ("drm/gem: implement vma access management")
References: b39b5394fabc ("drm/gem: Add drm_gem_object_funcs")
Cc: dri-de...@lists.freedesktop.org
Cc: Rob Clark 
Cc: Ben Skeggs 
Cc: David Herrmann 
Cc: Noralf Trønnes 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: l...@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Cc: Steven Price 
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-de...@lists.freedesktop.org
Cc: Zack Rusin 
---
  drivers/gpu/drm/drm_gem.c | 48 
+++

  1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index aa15c52ae182..e3d897bca0f2 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -356,52 +356,52 @@ drm_gem_handle_create_tail(struct drm_file 
*file_priv,

 u32 *handlep)
  {
  struct drm_device *dev = obj->dev;
-    u32 handle;
  int ret;
WARN_ON(!mutex_is_locked(>object_name_lock));
  if (obj->handle_count++ == 0)
  drm_gem_object_get(obj);
+    ret = drm_vma_node_allow(>vma_node, file_priv);
+    if (ret)
+    goto err_put;
+
+    if (obj->funcs->open) {
+    ret = obj->funcs->open(obj, file_priv);
+    if (ret)
+    goto err_revoke;
+    }
+
  /*
- * Get the user-visible handle using idr.  Preload and perform
- * allocation under our spinlock.
+ * Get the user-visible handle using idr as the _last_ step.
+ * Preload and perform allocation under our spinlock.
   */
  idr_preload(GFP_KERNEL);
  spin_lock(_priv->table_lock);
-
  ret = idr_alloc(_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
-
  spin_unlock(_priv->table_lock);
  idr_preload_end();
-    

Re: [PATCH v2] drm/amd: Don't allow s0ix on APUs older than Raven

2023-02-20 Thread Alex Deucher
On Mon, Feb 20, 2023 at 11:56 AM Limonciello, Mario
 wrote:
>
> [Public]
>
>
>
> > -Original Message-
> > From: Limonciello, Mario 
> > Sent: Monday, February 13, 2023 15:11
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Limonciello, Mario ; Deucher, Alexander
> > 
> > Subject: [PATCH v2] drm/amd: Don't allow s0ix on APUs older than Raven
> >
> > APUs before Raven didn't support s0ix.  As we just relieved some
> > of the safety checks for s0ix to improve power consumption on
> > APUs that support it but that are missing BIOS support a new
> > blind spot was introduced that a user could "try" to run s0ix.
> >
> > Plug this hole so that if users try to run s0ix on anything older
> > than Raven it will just skip suspend of the GPU.
> >
> > Fixes: cf488dcd0ab7 ("drm/amd: Allow s0ix without BIOS support")
> > Suggested-by: Alexander Deucher 
> > Signed-off-by: Mario Limonciello 
> > ---
> > v1->v2:
> >  * Don't run any suspend code or resume code in this case
>
> Any feedback for this patch?

Reviewed-by: Alex Deucher 

I think for S0ix and dGPUs, we probably need some additional work as
well.  If the user tries s2idle and the platform doesn't actually
support s0ix (i.e., doesn't actually turn off the power rails), we
should be using the runtime suspend routines for BACO/BOCO rather than
the S3 suspend routines.

Alex


>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 3 +++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 7 ++-
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > index fa7375b97fd47..25e902077caf6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > @@ -1073,6 +1073,9 @@ bool amdgpu_acpi_is_s0ix_active(struct
> > amdgpu_device *adev)
> >   (pm_suspend_target_state != PM_SUSPEND_TO_IDLE))
> >   return false;
> >
> > + if (adev->asic_type < CHIP_RAVEN)
> > + return false;
> > +
> >   /*
> >* If ACPI_FADT_LOW_POWER_S0 is not set in the FADT, it is
> > generally
> >* risky to do any special firmware-related preparations for entering
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 6c2fe50b528e0..1f6d93dc3d72b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -2414,8 +2414,10 @@ static int amdgpu_pmops_suspend(struct device
> > *dev)
> >
> >   if (amdgpu_acpi_is_s0ix_active(adev))
> >   adev->in_s0ix = true;
> > - else
> > + else if (amdgpu_acpi_is_s3_active(adev))
> >   adev->in_s3 = true;
> > + if (!adev->in_s0ix && !adev->in_s3)
> > + return 0;
> >   return amdgpu_device_suspend(drm_dev, true);
> >  }
> >
> > @@ -2436,6 +2438,9 @@ static int amdgpu_pmops_resume(struct device
> > *dev)
> >   struct amdgpu_device *adev = drm_to_adev(drm_dev);
> >   int r;
> >
> > + if (!adev->in_s0ix && !adev->in_s3)
> > + return 0;
> > +
> >   /* Avoids registers access if device is physically gone */
> >   if (!pci_device_is_present(adev->pdev))
> >   adev->no_hw_access = true;
> > --
> > 2.25.1


[PATCH 2/3] drm/amd: Use runtime suspend in lieu regular suspend on supported dGPUs

2023-02-20 Thread Mario Limonciello
The PMFW on dGPUs that support BACO will transition them in and out
of BACO when video/audio move in out of D3/D0.

On the Linux side users can configure what sleep mode to use in
`/sys/power/mem_sleep`, but if the host hardware doesn't cut the
power rails during this state then calling suspend from Linux may
cause a mismatch of behavior.

To avoid this, only run the runtime suspend and resume callbacks
when the dGPU supports BACO or BOCO and the smart flags didn't return
to skip these stages (because already runtime suspended).

Cc: Peter Kopec 
Signed-off-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index c3d3a042946d..fdc1cbf8ad10 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2418,8 +2418,11 @@ static int amdgpu_pmops_suspend(struct device *dev)
adev->in_s0ix = true;
else if (amdgpu_acpi_is_s3_active(adev))
adev->in_s3 = true;
-   if (!adev->in_s0ix && !adev->in_s3)
+   if (!adev->in_s0ix && !adev->in_s3) {
+   pm_runtime_mark_last_busy(dev);
+   pm_runtime_autosuspend(dev);
return 0;
+   }
return amdgpu_device_suspend(drm_dev, true);
 }
 
@@ -2440,8 +2443,10 @@ static int amdgpu_pmops_resume(struct device *dev)
struct amdgpu_device *adev = drm_to_adev(drm_dev);
int r;
 
-   if (!adev->in_s0ix && !adev->in_s3)
+   if (!adev->in_s0ix && !adev->in_s3) {
+   pm_runtime_resume(dev);
return 0;
+   }
 
/* Avoids registers access if device is physically gone */
if (!pci_device_is_present(adev->pdev))
-- 
2.34.1



[RFC 0/3] Adjust suspend flow for dGPUs

2023-02-20 Thread Mario Limonciello
Currently when a dGPU supports BOCO and is runtime suspended when running
the prepare() callback, the rest of the suspend and resume will be skipped.
This skipping is due to "Smart prepare", "Smart suspend" and "Smart resume"
flags.

dGPUs that support BACO are normally suspended via PMFW and lack of video
or audio, but this policy doesn't apply to them.

Furthermore even if a dGPU is runtime suspended the system wide suspend
callbacks always wake them even if the power rails won't be cut.

This series adjust the flow around runtime suspend handling for dGPUs.

Mario Limonciello (3):
  drm/amd: Don't always set s3 for dGPUs in all sleep modes
  drm/amd: Allow dGPUs that support BACO to use smart suspend
  drm/amd: Use runtime suspend in lieu regular suspend on supported
dGPUs

 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c |  9 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 17 -
 2 files changed, 19 insertions(+), 7 deletions(-)

-- 
2.34.1



[PATCH 1/3] drm/amd: Allow dGPUs that support BACO to use smart suspend

2023-02-20 Thread Mario Limonciello
If a dGPU is already runtime suspended using BACO, there is no point
to waking it up to run regular suspend callbacks.

Cc: Peter Kopec 
Signed-off-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1f6d93dc3d72..c3d3a042946d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2187,8 +2187,9 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
/* only need to skip on ATPX */
if (amdgpu_device_supports_px(ddev))
dev_pm_set_driver_flags(ddev->dev, 
DPM_FLAG_NO_DIRECT_COMPLETE);
-   /* we want direct complete for BOCO */
-   if (amdgpu_device_supports_boco(ddev))
+   /* we want direct complete for BOCO and for BACO */
+   if (amdgpu_device_supports_boco(ddev) ||
+   amdgpu_device_supports_baco(ddev))
dev_pm_set_driver_flags(ddev->dev, 
DPM_FLAG_SMART_PREPARE |
DPM_FLAG_SMART_SUSPEND |
DPM_FLAG_MAY_SKIP_RESUME);
@@ -2389,7 +2390,8 @@ static int amdgpu_pmops_prepare(struct device *dev)
/* Return a positive number here so
 * DPM_FLAG_SMART_SUSPEND works properly
 */
-   if (amdgpu_device_supports_boco(drm_dev))
+   if (amdgpu_device_supports_boco(drm_dev) ||
+   amdgpu_device_supports_baco(drm_dev))
return pm_runtime_suspended(dev);
 
/* if we will not support s3 or s2i for the device
-- 
2.34.1



[PATCH 3/3] drm/amd: Don't always set s3 for dGPUs in all sleep modes

2023-02-20 Thread Mario Limonciello
dGPUs that will be using BACO or BOCO shouldn't be put into S3
when the system is being put into s2idle.

Cc: Peter Kopec 
Signed-off-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 25e902077caf..5c69116bc883 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1038,8 +1038,13 @@ void amdgpu_acpi_detect(void)
  */
 bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev)
 {
-   return !(adev->flags & AMD_IS_APU) ||
-   (pm_suspend_target_state == PM_SUSPEND_MEM);
+   if (pm_suspend_target_state == PM_SUSPEND_MEM)
+   return true;
+   if (adev->flags & AMD_IS_APU)
+   return false;
+   return !amdgpu_device_supports_baco(>ddev) &&
+   !amdgpu_device_supports_boco(>ddev);
+
 }
 
 /**
-- 
2.34.1



[PATCH] drm/amd: Fix initialization for nbio 7.5.1

2023-02-20 Thread Mario Limonciello
A mistake has been made in the BIOS for some ASICs with NBIO 7.5.1
where some NBIO registers aren't properly setup.

Ensure that they're set during initialization.

Tested-by: Richard Gong 
Signed-off-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c 
b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
index 31776b12e4c45..4b0d563c6522c 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
@@ -382,6 +382,11 @@ static void nbio_v7_2_init_registers(struct amdgpu_device 
*adev)
if (def != data)
WREG32_PCIE_PORT(SOC15_REG_OFFSET(NBIO, 0, 
regBIF1_PCIE_MST_CTRL_3), data);
break;
+   case IP_VERSION(7, 5, 1):
+   data = RREG32_SOC15(NBIO, 0, regRCC_DEV2_EPF0_STRAP2);
+   data &= ~RCC_DEV2_EPF0_STRAP2__STRAP_NO_SOFT_RESET_DEV2_F0_MASK;
+   WREG32_SOC15(NBIO, 0, regRCC_DEV2_EPF0_STRAP2, data);
+   fallthrough;
default:
def = data = RREG32_PCIE_PORT(SOC15_REG_OFFSET(NBIO, 0, 
regPCIE_CONFIG_CNTL));
data = REG_SET_FIELD(data, PCIE_CONFIG_CNTL,
-- 
2.25.1



Re: [PATCH] drm/amd: Fix initialization for nbio 7.5.1

2023-02-20 Thread Deucher, Alexander
[AMD Official Use Only - General]

Acked-by: Alex Deucher 

From: amd-gfx  on behalf of Mario 
Limonciello 
Sent: Monday, February 20, 2023 1:10 PM
To: amd-gfx@lists.freedesktop.org 
Cc: Gong, Richard ; Limonciello, Mario 

Subject: [PATCH] drm/amd: Fix initialization for nbio 7.5.1

A mistake has been made in the BIOS for some ASICs with NBIO 7.5.1
where some NBIO registers aren't properly setup.

Ensure that they're set during initialization.

Tested-by: Richard Gong 
Signed-off-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c 
b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
index 31776b12e4c45..4b0d563c6522c 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
@@ -382,6 +382,11 @@ static void nbio_v7_2_init_registers(struct amdgpu_device 
*adev)
 if (def != data)
 WREG32_PCIE_PORT(SOC15_REG_OFFSET(NBIO, 0, 
regBIF1_PCIE_MST_CTRL_3), data);
 break;
+   case IP_VERSION(7, 5, 1):
+   data = RREG32_SOC15(NBIO, 0, regRCC_DEV2_EPF0_STRAP2);
+   data &= ~RCC_DEV2_EPF0_STRAP2__STRAP_NO_SOFT_RESET_DEV2_F0_MASK;
+   WREG32_SOC15(NBIO, 0, regRCC_DEV2_EPF0_STRAP2, data);
+   fallthrough;
 default:
 def = data = RREG32_PCIE_PORT(SOC15_REG_OFFSET(NBIO, 0, 
regPCIE_CONFIG_CNTL));
 data = REG_SET_FIELD(data, PCIE_CONFIG_CNTL,
--
2.25.1



[linux-next:master] BUILD REGRESSION d2af0fa4bfa4ec29d03b449ccd43fee39501112d

2023-02-20 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch HEAD: d2af0fa4bfa4ec29d03b449ccd43fee39501112d  Add linux-next specific 
files for 20230220

Error/Warning reports:

https://lore.kernel.org/oe-kbuild-all/202302062224.byzetxh1-...@intel.com
https://lore.kernel.org/oe-kbuild-all/202302092211.54eydhyh-...@intel.com
https://lore.kernel.org/oe-kbuild-all/202302111601.jty4lkra-...@intel.com
https://lore.kernel.org/oe-kbuild-all/202302170355.ljqlzucu-...@intel.com
https://lore.kernel.org/oe-kbuild-all/202302210017.xt59wvsm-...@intel.com
https://lore.kernel.org/oe-kbuild-all/202302210350.lynwcl4t-...@intel.com

Error/Warning: (recently discovered and may have been fixed)

Documentation/sphinx/templates/kernel-toc.html: 1:36 Invalid token: #}
ERROR: modpost: "__umoddi3" [fs/btrfs/btrfs.ko] undefined!
ERROR: modpost: "devm_platform_ioremap_resource" [drivers/dma/fsl-edma.ko] 
undefined!
ERROR: modpost: "devm_platform_ioremap_resource" [drivers/dma/idma64.ko] 
undefined!
FAILED: load BTF from vmlinux: No data available
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_optc.c:294:6: warning: no 
previous prototype for 'optc3_wait_drr_doublebuffer_pending_clear' 
[-Wmissing-prototypes]
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_resource_helpers.c:62:18: 
warning: variable 'cursor_bpp' set but not used [-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_detection.c:1199: warning: 
expecting prototype for dc_link_detect_connection_type(). Prototype was for 
link_detect_connection_type() instead
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_capability.c:1292:32:
 warning: variable 'result_write_min_hblank' set but not used 
[-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_training.c:1586:38:
 warning: variable 'result' set but not used [-Wunused-but-set-variable]
drivers/net/ethernet/sfc/ef100_nic.c:1197:9: warning: variable 'rc' is 
uninitialized when used here [-Wuninitialized]
drivers/net/ethernet/sfc/efx_devlink.c:326:58: error: expected ')' before 
'build_id'
drivers/net/ethernet/sfc/efx_devlink.c:338:55: error: expected ';' before '}' 
token
drivers/of/unittest.c:3042:41: error: 'struct device_node' has no member named 
'kobj'
drivers/pwm/pwm-dwc.c:314:1: error: type defaults to 'int' in declaration of 
'module_pci_driver' [-Werror=implicit-int]
include/asm-generic/div64.h:238:36: error: passing argument 1 of '__div64_32' 
from incompatible pointer type [-Werror=incompatible-pointer-types]

Unverified Error/Warning (likely false positive, please contact us if 
interested):

drivers/infiniband/hw/hfi1/verbs.c:1661 hfi1_alloc_hw_device_stats() error: we 
previously assumed 'dev_cntr_descs' could be null (see line 1650)
drivers/net/phy/phy-c45.c:712 genphy_c45_write_eee_adv() error: uninitialized 
symbol 'changed'.
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 
rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.
drivers/usb/gadget/composite.c:2082:33: sparse: sparse: restricted __le16 
degrades to integer
drivers/virtio/virtio_ring.c:1585 virtqueue_add_packed_vring() error: 
uninitialized symbol 'prev'.
drivers/virtio/virtio_ring.c:1593 virtqueue_add_packed_vring() error: 
uninitialized symbol 'head_flags'.
drivers/virtio/virtio_ring.c:697 virtqueue_add_split_vring() error: 
uninitialized symbol 'prev'.
pahole: .tmp_vmlinux.btf: No such file or directory

Error/Warning ids grouped by kconfigs:

gcc_recent_errors
|-- alpha-allyesconfig
|   |-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_detection.c:warning:expecting-prototype-for-dc_link_detect_connection_type().-Prototype-was-for-link_detect_connection_type()-instead
|   |-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-protocols-link_dp_capability.c:warning:variable-result_write_min_hblank-set-but-not-used
|   `-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-protocols-link_dp_training.c:warning:variable-result-set-but-not-used
|-- alpha-buildonly-randconfig-r006-20230219
|   |-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_detection.c:warning:expecting-prototype-for-dc_link_detect_connection_type().-Prototype-was-for-link_detect_connection_type()-instead
|   |-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-protocols-link_dp_capability.c:warning:variable-result_write_min_hblank-set-but-not-used
|   `-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-protocols-link_dp_training.c:warning:variable-result-set-but-not-used
|-- arc-allyesconfig
|   |-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_detection.c:warning:expecting-prototype-for-dc_link_detect_connection_type().-Prototype-was-for-link_detect_connection_type()-instead
|   |-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-protocols-link_dp_capability.c:warning:variable-result_write_min_hblank-set-but-not-used
|   |-- 
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-protocols-link_dp_trai

RE: [PATCH v2] drm/amd: Don't allow s0ix on APUs older than Raven

2023-02-20 Thread Limonciello, Mario
[Public]



> -Original Message-
> From: Limonciello, Mario 
> Sent: Monday, February 13, 2023 15:11
> To: amd-gfx@lists.freedesktop.org
> Cc: Limonciello, Mario ; Deucher, Alexander
> 
> Subject: [PATCH v2] drm/amd: Don't allow s0ix on APUs older than Raven
> 
> APUs before Raven didn't support s0ix.  As we just relieved some
> of the safety checks for s0ix to improve power consumption on
> APUs that support it but that are missing BIOS support a new
> blind spot was introduced that a user could "try" to run s0ix.
> 
> Plug this hole so that if users try to run s0ix on anything older
> than Raven it will just skip suspend of the GPU.
> 
> Fixes: cf488dcd0ab7 ("drm/amd: Allow s0ix without BIOS support")
> Suggested-by: Alexander Deucher 
> Signed-off-by: Mario Limonciello 
> ---
> v1->v2:
>  * Don't run any suspend code or resume code in this case

Any feedback for this patch?

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 3 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 7 ++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index fa7375b97fd47..25e902077caf6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -1073,6 +1073,9 @@ bool amdgpu_acpi_is_s0ix_active(struct
> amdgpu_device *adev)
>   (pm_suspend_target_state != PM_SUSPEND_TO_IDLE))
>   return false;
> 
> + if (adev->asic_type < CHIP_RAVEN)
> + return false;
> +
>   /*
>* If ACPI_FADT_LOW_POWER_S0 is not set in the FADT, it is
> generally
>* risky to do any special firmware-related preparations for entering
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6c2fe50b528e0..1f6d93dc3d72b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2414,8 +2414,10 @@ static int amdgpu_pmops_suspend(struct device
> *dev)
> 
>   if (amdgpu_acpi_is_s0ix_active(adev))
>   adev->in_s0ix = true;
> - else
> + else if (amdgpu_acpi_is_s3_active(adev))
>   adev->in_s3 = true;
> + if (!adev->in_s0ix && !adev->in_s3)
> + return 0;
>   return amdgpu_device_suspend(drm_dev, true);
>  }
> 
> @@ -2436,6 +2438,9 @@ static int amdgpu_pmops_resume(struct device
> *dev)
>   struct amdgpu_device *adev = drm_to_adev(drm_dev);
>   int r;
> 
> + if (!adev->in_s0ix && !adev->in_s3)
> + return 0;
> +
>   /* Avoids registers access if device is physically gone */
>   if (!pci_device_is_present(adev->pdev))
>   adev->no_hw_access = true;
> --
> 2.25.1


RE: [PATCH v2] drm/amd: Don't allow s0ix on APUs older than Raven

2023-02-20 Thread Limonciello, Mario
[AMD Official Use Only - General]



> -Original Message-
> From: Alex Deucher 
> Sent: Monday, February 20, 2023 11:10
> To: Limonciello, Mario 
> Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> 
> Subject: Re: [PATCH v2] drm/amd: Don't allow s0ix on APUs older than Raven
> 
> On Mon, Feb 20, 2023 at 11:56 AM Limonciello, Mario
>  wrote:
> >
> > [Public]
> >
> >
> >
> > > -Original Message-
> > > From: Limonciello, Mario 
> > > Sent: Monday, February 13, 2023 15:11
> > > To: amd-gfx@lists.freedesktop.org
> > > Cc: Limonciello, Mario ; Deucher,
> Alexander
> > > 
> > > Subject: [PATCH v2] drm/amd: Don't allow s0ix on APUs older than Raven
> > >
> > > APUs before Raven didn't support s0ix.  As we just relieved some
> > > of the safety checks for s0ix to improve power consumption on
> > > APUs that support it but that are missing BIOS support a new
> > > blind spot was introduced that a user could "try" to run s0ix.
> > >
> > > Plug this hole so that if users try to run s0ix on anything older
> > > than Raven it will just skip suspend of the GPU.
> > >
> > > Fixes: cf488dcd0ab7 ("drm/amd: Allow s0ix without BIOS support")
> > > Suggested-by: Alexander Deucher 
> > > Signed-off-by: Mario Limonciello 
> > > ---
> > > v1->v2:
> > >  * Don't run any suspend code or resume code in this case
> >
> > Any feedback for this patch?
> 
> Reviewed-by: Alex Deucher 
> 

Thanks.

> I think for S0ix and dGPUs, we probably need some additional work as
> well.  If the user tries s2idle and the platform doesn't actually
> support s0ix (i.e., doesn't actually turn off the power rails), we
> should be using the runtime suspend routines for BACO/BOCO rather than
> the S3 suspend routines.

OK - I'll review the framework code for that case and see what makes sense.

> 
> Alex
> 
> 
> >
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 3 +++
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 7 ++-
> > >  2 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > > index fa7375b97fd47..25e902077caf6 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> > > @@ -1073,6 +1073,9 @@ bool amdgpu_acpi_is_s0ix_active(struct
> > > amdgpu_device *adev)
> > >   (pm_suspend_target_state != PM_SUSPEND_TO_IDLE))
> > >   return false;
> > >
> > > + if (adev->asic_type < CHIP_RAVEN)
> > > + return false;
> > > +
> > >   /*
> > >* If ACPI_FADT_LOW_POWER_S0 is not set in the FADT, it is
> > > generally
> > >* risky to do any special firmware-related preparations for 
> > > entering
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > index 6c2fe50b528e0..1f6d93dc3d72b 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > @@ -2414,8 +2414,10 @@ static int amdgpu_pmops_suspend(struct
> device
> > > *dev)
> > >
> > >   if (amdgpu_acpi_is_s0ix_active(adev))
> > >   adev->in_s0ix = true;
> > > - else
> > > + else if (amdgpu_acpi_is_s3_active(adev))
> > >   adev->in_s3 = true;
> > > + if (!adev->in_s0ix && !adev->in_s3)
> > > + return 0;
> > >   return amdgpu_device_suspend(drm_dev, true);
> > >  }
> > >
> > > @@ -2436,6 +2438,9 @@ static int amdgpu_pmops_resume(struct device
> > > *dev)
> > >   struct amdgpu_device *adev = drm_to_adev(drm_dev);
> > >   int r;
> > >
> > > + if (!adev->in_s0ix && !adev->in_s3)
> > > + return 0;
> > > +
> > >   /* Avoids registers access if device is physically gone */
> > >   if (!pci_device_is_present(adev->pdev))
> > >   adev->no_hw_access = true;
> > > --
> > > 2.25.1


[PATCH] drm/amd/pm: downgrade log level upon SMU IF version mismatch

2023-02-20 Thread Guchun Chen
SMU IF version mismatch as a warning message exists widely
after asic production, however, due to this log level setting,
such mismatch warning will be caught by automation test like
IGT and reported as a fake error after checking. As such mismatch
does not break anything, to reduce confusion, downgrade it from
dev_warn to dev_info.

Signed-off-by: Guchun Chen 
---
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 4 ++--
 drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c | 4 ++--
 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 6492d69e2e60..e1ef88ee1ed3 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -256,7 +256,7 @@ int smu_v11_0_check_fw_version(struct smu_context *smu)
 * to be backward compatible.
 * 2. New fw usually brings some optimizations. But that's visible
 * only on the paired driver.
-* Considering above, we just leave user a warning message instead
+* Considering above, we just leave user a verbal message instead
 * of halt driver loading.
 */
if (if_version != smu->smc_driver_if_version) {
@@ -264,7 +264,7 @@ int smu_v11_0_check_fw_version(struct smu_context *smu)
"smu fw program = %d, version = 0x%08x (%d.%d.%d)\n",
smu->smc_driver_if_version, if_version,
smu_program, smu_version, smu_major, smu_minor, 
smu_debug);
-   dev_warn(smu->adev->dev, "SMU driver if version not matched\n");
+   dev_info(smu->adev->dev, "SMU driver if version not matched\n");
}
 
return ret;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c
index 56a02bc60cee..c788aa7a99a9 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/smu_v12_0.c
@@ -93,7 +93,7 @@ int smu_v12_0_check_fw_version(struct smu_context *smu)
 * to be backward compatible.
 * 2. New fw usually brings some optimizations. But that's visible
 * only on the paired driver.
-* Considering above, we just leave user a warning message instead
+* Considering above, we just leave user a verbal message instead
 * of halt driver loading.
 */
if (if_version != smu->smc_driver_if_version) {
@@ -101,7 +101,7 @@ int smu_v12_0_check_fw_version(struct smu_context *smu)
"smu fw program = %d, smu fw version = 0x%08x 
(%d.%d.%d)\n",
smu->smc_driver_if_version, if_version,
smu_program, smu_version, smu_major, smu_minor, 
smu_debug);
-   dev_warn(smu->adev->dev, "SMU driver if version not matched\n");
+   dev_info(smu->adev->dev, "SMU driver if version not matched\n");
}
 
return ret;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
index 78945e79dbee..25f336829840 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
@@ -311,7 +311,7 @@ int smu_v13_0_check_fw_version(struct smu_context *smu)
 * to be backward compatible.
 * 2. New fw usually brings some optimizations. But that's visible
 * only on the paired driver.
-* Considering above, we just leave user a warning message instead
+* Considering above, we just leave user a verbal message instead
 * of halt driver loading.
 */
if (if_version != smu->smc_driver_if_version) {
@@ -319,7 +319,7 @@ int smu_v13_0_check_fw_version(struct smu_context *smu)
 "smu fw program = %d, smu fw version = 0x%08x 
(%d.%d.%d)\n",
 smu->smc_driver_if_version, if_version,
 smu_program, smu_version, smu_major, smu_minor, 
smu_debug);
-   dev_warn(adev->dev, "SMU driver if version not matched\n");
+   dev_info(adev->dev, "SMU driver if version not matched\n");
}
 
return ret;
-- 
2.25.1



[PATCH 1/2] drm/amd/pm: correct the baco state setting for ArmD3 scenario

2023-02-20 Thread Evan Quan
The check for baco support relies on the correct baco state.

Signed-off-by: Evan Quan 
Change-Id: I9dd15958c213eb83abdb8b7d858eff0d2d364b4e
---
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 21 +++
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
index f4927ada5c07..1984b01514bd 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
@@ -2217,10 +2217,23 @@ int smu_v13_0_gfx_ulv_control(struct smu_context *smu,
 int smu_v13_0_baco_set_armd3_sequence(struct smu_context *smu,
  enum smu_baco_seq baco_seq)
 {
-   return smu_cmn_send_smc_msg_with_param(smu,
-  SMU_MSG_ArmD3,
-  baco_seq,
-  NULL);
+   struct smu_baco_context *smu_baco = >smu_baco;
+   int ret;
+
+   ret = smu_cmn_send_smc_msg_with_param(smu,
+ SMU_MSG_ArmD3,
+ baco_seq,
+ NULL);
+   if (ret)
+   return ret;
+
+   if ((baco_seq == BACO_SEQ_BAMACO) ||
+   (baco_seq == BACO_SEQ_BACO))
+   smu_baco->state = SMU_BACO_STATE_ENTER;
+   else
+   smu_baco->state = SMU_BACO_STATE_EXIT;
+
+   return 0;
 }
 
 bool smu_v13_0_baco_is_support(struct smu_context *smu)
-- 
2.34.1



[PATCH 2/2] drm/amd/pm: no pptable resetup on runpm exiting

2023-02-20 Thread Evan Quan
It is assumed the pptable used before runpm is same as
the one used afterwards. Thus, we can reuse the stored
copy and do not need to resetup the pptable again.

Signed-off-by: Evan Quan 
Change-Id: Ib6d61f8e8cb58df45d9e24725b0672239b3ff653
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index ff806a2e804f..bb25f14f0733 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1220,10 +1220,17 @@ static int smu_smc_hw_setup(struct smu_context *smu)
return ret;
}
 
-   ret = smu_setup_pptable(smu);
-   if (ret) {
-   dev_err(adev->dev, "Failed to setup pptable!\n");
-   return ret;
+   /*
+* It is assumed the pptable used before runpm is same as
+* the one used afterwards. Thus, we can reuse the stored
+* copy and do not need to resetup the pptable again.
+*/
+   if (!adev->in_runpm) {
+   ret = smu_setup_pptable(smu);
+   if (ret) {
+   dev_err(adev->dev, "Failed to setup pptable!\n");
+   return ret;
+   }
}
 
/* smu_dump_pptable(smu); */
-- 
2.34.1