All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/powerplay: handle features disablement for baco reset in SMU FW
@ 2020-02-06  8:19 Evan Quan
  2020-02-06  8:35 ` Zhang, Hawking
  2020-02-06 14:00 ` Alex Deucher
  0 siblings, 2 replies; 5+ messages in thread
From: Evan Quan @ 2020-02-06  8:19 UTC (permalink / raw)
  To: amd-gfx; +Cc: Evan Quan, hawking.zhang

SMU FW will handle the features disablement for baco reset
on Arcturus.

Change-Id: Ifd87a09de2fca0c67c041afbd5e711973769119a
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 53 +++++++++++++++++-----
 1 file changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 9d1075823681..efd10e6fa9ef 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1467,24 +1467,39 @@ void smu_late_fini(void *handle)
 	smu_send_smc_msg(smu, SMU_MSG_PrepareMp1ForShutdown);
 }
 
-static int smu_suspend(void *handle)
+static int smu_disabled_dpms(struct smu_context *smu)
 {
-	int ret;
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-	struct smu_context *smu = &adev->smu;
-	bool baco_feature_is_enabled = false;
+	struct amdgpu_device *adev = smu->adev;
+	uint32_t smu_version;
+	int ret = 0;
 
-	if (!smu->pm_enabled)
-		return 0;
+	ret = smu_get_smc_version(smu, NULL, &smu_version);
+	if (ret) {
+		pr_err("Failed to get smu version.\n");
+		return ret;
+	}
 
-	if(!smu->is_apu)
-		baco_feature_is_enabled = smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT);
+	/*
+	 * For baco reset on Arcturus, this operation
+	 * (disable all smu feature) will be handled by SMU FW.
+	 */
+	if (adev->in_gpu_reset &&
+	    (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO) &&
+	    (adev->asic_type == CHIP_ARCTURUS && smu_version > 0x360e00))
+		return 0;
 
+	/* Disable all enabled SMU features */
 	ret = smu_system_features_control(smu, false);
-	if (ret)
+	if (ret) {
+		pr_err("Failed to disable smu features.\n");
 		return ret;
+	}
 
-	if (baco_feature_is_enabled) {
+	/* For baco reset, need to leave BACO feature enabled */
+	if (adev->in_gpu_reset &&
+	    (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO) &&
+	    !smu->is_apu &&
+	    smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)) {
 		ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
 		if (ret) {
 			pr_warn("set BACO feature enabled failed, return %d\n", ret);
@@ -1492,6 +1507,22 @@ static int smu_suspend(void *handle)
 		}
 	}
 
+	return ret;
+}
+
+static int smu_suspend(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	struct smu_context *smu = &adev->smu;
+	int ret;
+
+	if (!smu->pm_enabled)
+		return 0;
+
+	ret = smu_disabled_dpms(smu);
+	if (ret)
+		return ret;
+
 	smu->watermarks_bitmap &= ~(WATERMARKS_LOADED);
 
 	if (adev->asic_type >= CHIP_NAVI10 &&
-- 
2.25.0

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [PATCH] drm/amd/powerplay: handle features disablement for baco reset in SMU FW
  2020-02-06  8:19 [PATCH] drm/amd/powerplay: handle features disablement for baco reset in SMU FW Evan Quan
@ 2020-02-06  8:35 ` Zhang, Hawking
  2020-02-06 14:00 ` Alex Deucher
  1 sibling, 0 replies; 5+ messages in thread
From: Zhang, Hawking @ 2020-02-06  8:35 UTC (permalink / raw)
  To: Quan, Evan, amd-gfx

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>

Regards,
Hawking
-----Original Message-----
From: Quan, Evan <Evan.Quan@amd.com> 
Sent: Thursday, February 6, 2020 16:19
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Quan, Evan <Evan.Quan@amd.com>
Subject: [PATCH] drm/amd/powerplay: handle features disablement for baco reset in SMU FW

SMU FW will handle the features disablement for baco reset on Arcturus.

Change-Id: Ifd87a09de2fca0c67c041afbd5e711973769119a
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 53 +++++++++++++++++-----
 1 file changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 9d1075823681..efd10e6fa9ef 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1467,24 +1467,39 @@ void smu_late_fini(void *handle)
 	smu_send_smc_msg(smu, SMU_MSG_PrepareMp1ForShutdown);  }
 
-static int smu_suspend(void *handle)
+static int smu_disabled_dpms(struct smu_context *smu)
 {
-	int ret;
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-	struct smu_context *smu = &adev->smu;
-	bool baco_feature_is_enabled = false;
+	struct amdgpu_device *adev = smu->adev;
+	uint32_t smu_version;
+	int ret = 0;
 
-	if (!smu->pm_enabled)
-		return 0;
+	ret = smu_get_smc_version(smu, NULL, &smu_version);
+	if (ret) {
+		pr_err("Failed to get smu version.\n");
+		return ret;
+	}
 
-	if(!smu->is_apu)
-		baco_feature_is_enabled = smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT);
+	/*
+	 * For baco reset on Arcturus, this operation
+	 * (disable all smu feature) will be handled by SMU FW.
+	 */
+	if (adev->in_gpu_reset &&
+	    (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO) &&
+	    (adev->asic_type == CHIP_ARCTURUS && smu_version > 0x360e00))
+		return 0;
 
+	/* Disable all enabled SMU features */
 	ret = smu_system_features_control(smu, false);
-	if (ret)
+	if (ret) {
+		pr_err("Failed to disable smu features.\n");
 		return ret;
+	}
 
-	if (baco_feature_is_enabled) {
+	/* For baco reset, need to leave BACO feature enabled */
+	if (adev->in_gpu_reset &&
+	    (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO) &&
+	    !smu->is_apu &&
+	    smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)) {
 		ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
 		if (ret) {
 			pr_warn("set BACO feature enabled failed, return %d\n", ret); @@ -1492,6 +1507,22 @@ static int smu_suspend(void *handle)
 		}
 	}
 
+	return ret;
+}
+
+static int smu_suspend(void *handle)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	struct smu_context *smu = &adev->smu;
+	int ret;
+
+	if (!smu->pm_enabled)
+		return 0;
+
+	ret = smu_disabled_dpms(smu);
+	if (ret)
+		return ret;
+
 	smu->watermarks_bitmap &= ~(WATERMARKS_LOADED);
 
 	if (adev->asic_type >= CHIP_NAVI10 &&
--
2.25.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amd/powerplay: handle features disablement for baco reset in SMU FW
  2020-02-06  8:19 [PATCH] drm/amd/powerplay: handle features disablement for baco reset in SMU FW Evan Quan
  2020-02-06  8:35 ` Zhang, Hawking
@ 2020-02-06 14:00 ` Alex Deucher
  2020-02-07  8:28   ` Quan, Evan
  1 sibling, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2020-02-06 14:00 UTC (permalink / raw)
  To: Evan Quan; +Cc: amd-gfx list, Hawking Zhang

On Thu, Feb 6, 2020 at 3:19 AM Evan Quan <evan.quan@amd.com> wrote:
>
> SMU FW will handle the features disablement for baco reset
> on Arcturus.
>
> Change-Id: Ifd87a09de2fca0c67c041afbd5e711973769119a
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 53 +++++++++++++++++-----
>  1 file changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 9d1075823681..efd10e6fa9ef 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1467,24 +1467,39 @@ void smu_late_fini(void *handle)
>         smu_send_smc_msg(smu, SMU_MSG_PrepareMp1ForShutdown);
>  }
>
> -static int smu_suspend(void *handle)
> +static int smu_disabled_dpms(struct smu_context *smu)
>  {
> -       int ret;
> -       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> -       struct smu_context *smu = &adev->smu;
> -       bool baco_feature_is_enabled = false;
> +       struct amdgpu_device *adev = smu->adev;
> +       uint32_t smu_version;
> +       int ret = 0;
>
> -       if (!smu->pm_enabled)
> -               return 0;
> +       ret = smu_get_smc_version(smu, NULL, &smu_version);
> +       if (ret) {
> +               pr_err("Failed to get smu version.\n");
> +               return ret;
> +       }
>
> -       if(!smu->is_apu)
> -               baco_feature_is_enabled = smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT);
> +       /*
> +        * For baco reset on Arcturus, this operation
> +        * (disable all smu feature) will be handled by SMU FW.
> +        */
> +       if (adev->in_gpu_reset &&
> +           (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO) &&
> +           (adev->asic_type == CHIP_ARCTURUS && smu_version > 0x360e00))
> +               return 0;

Do we need the in_gpu_reset check here?  Is there ever a case where
would ever want to execute the rest of this?  What if we enable BACO
for power savings on arcturus?

>
> +       /* Disable all enabled SMU features */
>         ret = smu_system_features_control(smu, false);
> -       if (ret)
> +       if (ret) {
> +               pr_err("Failed to disable smu features.\n");
>                 return ret;
> +       }
>
> -       if (baco_feature_is_enabled) {
> +       /* For baco reset, need to leave BACO feature enabled */
> +       if (adev->in_gpu_reset &&
> +           (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO) &&
> +           !smu->is_apu &&
> +           smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)) {

This change will break BACO for power savings on navi1x.  I think we
can drop this hunk.

>                 ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);
>                 if (ret) {
>                         pr_warn("set BACO feature enabled failed, return %d\n", ret);
> @@ -1492,6 +1507,22 @@ static int smu_suspend(void *handle)
>                 }
>         }
>
> +       return ret;
> +}
> +
> +static int smu_suspend(void *handle)
> +{
> +       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +       struct smu_context *smu = &adev->smu;
> +       int ret;
> +
> +       if (!smu->pm_enabled)
> +               return 0;
> +
> +       ret = smu_disabled_dpms(smu);
> +       if (ret)
> +               return ret;
> +
>         smu->watermarks_bitmap &= ~(WATERMARKS_LOADED);
>
>         if (adev->asic_type >= CHIP_NAVI10 &&
> --
> 2.25.0
>
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] drm/amd/powerplay: handle features disablement for baco reset in SMU FW
  2020-02-06 14:00 ` Alex Deucher
@ 2020-02-07  8:28   ` Quan, Evan
  2020-02-07 14:38     ` Alex Deucher
  0 siblings, 1 reply; 5+ messages in thread
From: Quan, Evan @ 2020-02-07  8:28 UTC (permalink / raw)
  To: Alex Deucher; +Cc: amd-gfx list, Zhang, Hawking

>Do we need the in_gpu_reset check here?  Is there ever a case where would
>ever want to execute the rest of this?  What if we enable BACO for power
>savings on arcturus?
That is used to rule out the system suspend case.
But yes, it may cause some problem for the runtime suspend case(which uses baco for power saving).
How can we know whether it's a runtime suspend or just system suspend?

>-----Original Message-----
>From: Alex Deucher <alexdeucher@gmail.com>
>Sent: Thursday, February 6, 2020 10:01 PM
>To: Quan, Evan <Evan.Quan@amd.com>
>Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Zhang, Hawking
><Hawking.Zhang@amd.com>
>Subject: Re: [PATCH] drm/amd/powerplay: handle features disablement for
>baco reset in SMU FW
>
>On Thu, Feb 6, 2020 at 3:19 AM Evan Quan <evan.quan@amd.com> wrote:
>>
>> SMU FW will handle the features disablement for baco reset on
>> Arcturus.
>>
>> Change-Id: Ifd87a09de2fca0c67c041afbd5e711973769119a
>> Signed-off-by: Evan Quan <evan.quan@amd.com>
>> ---
>>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 53
>> +++++++++++++++++-----
>>  1 file changed, 42 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
>> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
>> index 9d1075823681..efd10e6fa9ef 100644
>> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
>> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
>> @@ -1467,24 +1467,39 @@ void smu_late_fini(void *handle)
>>         smu_send_smc_msg(smu, SMU_MSG_PrepareMp1ForShutdown);  }
>>
>> -static int smu_suspend(void *handle)
>> +static int smu_disabled_dpms(struct smu_context *smu)
>>  {
>> -       int ret;
>> -       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> -       struct smu_context *smu = &adev->smu;
>> -       bool baco_feature_is_enabled = false;
>> +       struct amdgpu_device *adev = smu->adev;
>> +       uint32_t smu_version;
>> +       int ret = 0;
>>
>> -       if (!smu->pm_enabled)
>> -               return 0;
>> +       ret = smu_get_smc_version(smu, NULL, &smu_version);
>> +       if (ret) {
>> +               pr_err("Failed to get smu version.\n");
>> +               return ret;
>> +       }
>>
>> -       if(!smu->is_apu)
>> -               baco_feature_is_enabled = smu_feature_is_enabled(smu,
>SMU_FEATURE_BACO_BIT);
>> +       /*
>> +        * For baco reset on Arcturus, this operation
>> +        * (disable all smu feature) will be handled by SMU FW.
>> +        */
>> +       if (adev->in_gpu_reset &&
>> +           (amdgpu_asic_reset_method(adev) ==
>AMD_RESET_METHOD_BACO) &&
>> +           (adev->asic_type == CHIP_ARCTURUS && smu_version > 0x360e00))
>> +               return 0;
>
>Do we need the in_gpu_reset check here?  Is there ever a case where would
>ever want to execute the rest of this?  What if we enable BACO for power
>savings on arcturus?
>
>>
>> +       /* Disable all enabled SMU features */
>>         ret = smu_system_features_control(smu, false);
>> -       if (ret)
>> +       if (ret) {
>> +               pr_err("Failed to disable smu features.\n");
>>                 return ret;
>> +       }
>>
>> -       if (baco_feature_is_enabled) {
>> +       /* For baco reset, need to leave BACO feature enabled */
>> +       if (adev->in_gpu_reset &&
>> +           (amdgpu_asic_reset_method(adev) ==
>AMD_RESET_METHOD_BACO) &&
>> +           !smu->is_apu &&
>> +           smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)) {
>
>This change will break BACO for power savings on navi1x.  I think we can drop
>this hunk.
>
>>                 ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT,
>true);
>>                 if (ret) {
>>                         pr_warn("set BACO feature enabled failed,
>> return %d\n", ret); @@ -1492,6 +1507,22 @@ static int smu_suspend(void
>*handle)
>>                 }
>>         }
>>
>> +       return ret;
>> +}
>> +
>> +static int smu_suspend(void *handle)
>> +{
>> +       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> +       struct smu_context *smu = &adev->smu;
>> +       int ret;
>> +
>> +       if (!smu->pm_enabled)
>> +               return 0;
>> +
>> +       ret = smu_disabled_dpms(smu);
>> +       if (ret)
>> +               return ret;
>> +
>>         smu->watermarks_bitmap &= ~(WATERMARKS_LOADED);
>>
>>         if (adev->asic_type >= CHIP_NAVI10 &&
>> --
>> 2.25.0
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
>> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
>gfx&amp;data=02%7C01%7Cev
>>
>an.quan%40amd.com%7Ce4212e8faa2849ebda5008d7ab0cfbfd%7C3dd8961fe
>4884e6
>>
>08e11a82d994e183d%7C0%7C0%7C637165944568797358&amp;sdata=M9jaswf
>JV%2Bq
>> KLZTxff%2Bju81y47M9WKT2iULEZjHBHcw%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amd/powerplay: handle features disablement for baco reset in SMU FW
  2020-02-07  8:28   ` Quan, Evan
@ 2020-02-07 14:38     ` Alex Deucher
  0 siblings, 0 replies; 5+ messages in thread
From: Alex Deucher @ 2020-02-07 14:38 UTC (permalink / raw)
  To: Quan, Evan; +Cc: amd-gfx list, Zhang, Hawking

On Fri, Feb 7, 2020 at 3:28 AM Quan, Evan <Evan.Quan@amd.com> wrote:
>
> >Do we need the in_gpu_reset check here?  Is there ever a case where would
> >ever want to execute the rest of this?  What if we enable BACO for power
> >savings on arcturus?
> That is used to rule out the system suspend case.
> But yes, it may cause some problem for the runtime suspend case(which uses baco for power saving).
> How can we know whether it's a runtime suspend or just system suspend?

At the moment, we haven't had a need to differentiate them, but we
could add a flag.  That said, I don't quite understand why BACO at
runtime is any different from BACO for reset.  They should both
require the same steps to enter and exit.  What is different about
system suspend?

Alex

>
> >-----Original Message-----
> >From: Alex Deucher <alexdeucher@gmail.com>
> >Sent: Thursday, February 6, 2020 10:01 PM
> >To: Quan, Evan <Evan.Quan@amd.com>
> >Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Zhang, Hawking
> ><Hawking.Zhang@amd.com>
> >Subject: Re: [PATCH] drm/amd/powerplay: handle features disablement for
> >baco reset in SMU FW
> >
> >On Thu, Feb 6, 2020 at 3:19 AM Evan Quan <evan.quan@amd.com> wrote:
> >>
> >> SMU FW will handle the features disablement for baco reset on
> >> Arcturus.
> >>
> >> Change-Id: Ifd87a09de2fca0c67c041afbd5e711973769119a
> >> Signed-off-by: Evan Quan <evan.quan@amd.com>
> >> ---
> >>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 53
> >> +++++++++++++++++-----
> >>  1 file changed, 42 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> >> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> >> index 9d1075823681..efd10e6fa9ef 100644
> >> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> >> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> >> @@ -1467,24 +1467,39 @@ void smu_late_fini(void *handle)
> >>         smu_send_smc_msg(smu, SMU_MSG_PrepareMp1ForShutdown);  }
> >>
> >> -static int smu_suspend(void *handle)
> >> +static int smu_disabled_dpms(struct smu_context *smu)
> >>  {
> >> -       int ret;
> >> -       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> >> -       struct smu_context *smu = &adev->smu;
> >> -       bool baco_feature_is_enabled = false;
> >> +       struct amdgpu_device *adev = smu->adev;
> >> +       uint32_t smu_version;
> >> +       int ret = 0;
> >>
> >> -       if (!smu->pm_enabled)
> >> -               return 0;
> >> +       ret = smu_get_smc_version(smu, NULL, &smu_version);
> >> +       if (ret) {
> >> +               pr_err("Failed to get smu version.\n");
> >> +               return ret;
> >> +       }
> >>
> >> -       if(!smu->is_apu)
> >> -               baco_feature_is_enabled = smu_feature_is_enabled(smu,
> >SMU_FEATURE_BACO_BIT);
> >> +       /*
> >> +        * For baco reset on Arcturus, this operation
> >> +        * (disable all smu feature) will be handled by SMU FW.
> >> +        */
> >> +       if (adev->in_gpu_reset &&
> >> +           (amdgpu_asic_reset_method(adev) ==
> >AMD_RESET_METHOD_BACO) &&
> >> +           (adev->asic_type == CHIP_ARCTURUS && smu_version > 0x360e00))
> >> +               return 0;
> >
> >Do we need the in_gpu_reset check here?  Is there ever a case where would
> >ever want to execute the rest of this?  What if we enable BACO for power
> >savings on arcturus?
> >
> >>
> >> +       /* Disable all enabled SMU features */
> >>         ret = smu_system_features_control(smu, false);
> >> -       if (ret)
> >> +       if (ret) {
> >> +               pr_err("Failed to disable smu features.\n");
> >>                 return ret;
> >> +       }
> >>
> >> -       if (baco_feature_is_enabled) {
> >> +       /* For baco reset, need to leave BACO feature enabled */
> >> +       if (adev->in_gpu_reset &&
> >> +           (amdgpu_asic_reset_method(adev) ==
> >AMD_RESET_METHOD_BACO) &&
> >> +           !smu->is_apu &&
> >> +           smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)) {
> >
> >This change will break BACO for power savings on navi1x.  I think we can drop
> >this hunk.
> >
> >>                 ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT,
> >true);
> >>                 if (ret) {
> >>                         pr_warn("set BACO feature enabled failed,
> >> return %d\n", ret); @@ -1492,6 +1507,22 @@ static int smu_suspend(void
> >*handle)
> >>                 }
> >>         }
> >>
> >> +       return ret;
> >> +}
> >> +
> >> +static int smu_suspend(void *handle)
> >> +{
> >> +       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> >> +       struct smu_context *smu = &adev->smu;
> >> +       int ret;
> >> +
> >> +       if (!smu->pm_enabled)
> >> +               return 0;
> >> +
> >> +       ret = smu_disabled_dpms(smu);
> >> +       if (ret)
> >> +               return ret;
> >> +
> >>         smu->watermarks_bitmap &= ~(WATERMARKS_LOADED);
> >>
> >>         if (adev->asic_type >= CHIP_NAVI10 &&
> >> --
> >> 2.25.0
> >>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> >> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> >gfx&amp;data=02%7C01%7Cev
> >>
> >an.quan%40amd.com%7Ce4212e8faa2849ebda5008d7ab0cfbfd%7C3dd8961fe
> >4884e6
> >>
> >08e11a82d994e183d%7C0%7C0%7C637165944568797358&amp;sdata=M9jaswf
> >JV%2Bq
> >> KLZTxff%2Bju81y47M9WKT2iULEZjHBHcw%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-02-07 14:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-06  8:19 [PATCH] drm/amd/powerplay: handle features disablement for baco reset in SMU FW Evan Quan
2020-02-06  8:35 ` Zhang, Hawking
2020-02-06 14:00 ` Alex Deucher
2020-02-07  8:28   ` Quan, Evan
2020-02-07 14:38     ` Alex Deucher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.