[PATCH 1/3] Revert "drm/amd/powerplay: Enable/Disable NBPSTATE on On/OFF of UVD"

[PATCH 1/3] Revert "drm/amd/powerplay: Enable/Disable NBPSTATE on On/OFF of UVD"

[Guttula, Suresh]

From: "S, Shirish" <Shirish.S@amd.com>

This reverts commit dbd8299c32f6f413f6cfe322fe0308f3cfc577e8.

Reason for revert:
This patch sends  msg PPSMC_MSG_DisableLowMemoryPstate(0x002e)
in wrong of sequence to SMU which is before PPSMC_MSG_UVDPowerON (0x0008).
This leads to SMU failing to service the request as it is
dependent on UVD to be powered ON, since it accesses UVD
registers.

This msg should ideally be sent only when the UVD is about to decode
a 4k video.

Signed-off-by: Shirish S <shirish.s@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
index fef111d..53cf787 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
@@ -1228,17 +1228,14 @@ static int smu8_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
 
 static int smu8_dpm_powerdown_uvd(struct pp_hwmgr *hwmgr)
 {
-	if (PP_CAP(PHM_PlatformCaps_UVDPowerGating)) {
-		smu8_nbdpm_pstate_enable_disable(hwmgr, true, true);
+	if (PP_CAP(PHM_PlatformCaps_UVDPowerGating))
 		return smum_send_msg_to_smc(hwmgr, PPSMC_MSG_UVDPowerOFF);
-	}
 	return 0;
 }
 
 static int smu8_dpm_powerup_uvd(struct pp_hwmgr *hwmgr)
 {
 	if (PP_CAP(PHM_PlatformCaps_UVDPowerGating)) {
-		smu8_nbdpm_pstate_enable_disable(hwmgr, false, true);
 		return smum_send_msg_to_smc_with_parameter(
 			hwmgr,
 			PPSMC_MSG_UVDPowerON,
-- 
2.7.4

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

[PATCH 2/3] drm/amd/powerplay:add hwmgr callback to update nbpstate on Carrizo

[Guttula, Suresh]

This callback is used to access hwmgr function named as
cz_nbdpm_pstate_enable_disable.

Signed-off-by: suresh guttula <suresh.guttula@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c | 1 +
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h        | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
index 53cf787..553a203 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
@@ -1992,6 +1992,7 @@ static const struct pp_hwmgr_func smu8_hwmgr_funcs = {
 	.power_state_set = smu8_set_power_state_tasks,
 	.dynamic_state_management_disable = smu8_disable_dpm_tasks,
 	.notify_cac_buffer_info = smu8_notify_cac_buffer_info,
+	.update_nbdpm_pstate = smu8_nbdpm_pstate_enable_disable,
 	.get_thermal_temperature_range = smu8_get_thermal_temperature_range,
 };
 
diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
index 07d180ce..fb0f96f 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
@@ -317,6 +317,9 @@ struct pp_hwmgr_func {
 					uint32_t mc_addr_low,
 					uint32_t mc_addr_hi,
 					uint32_t size);
+	int (*update_nbdpm_pstate)(struct pp_hwmgr *hwmgr,
+					bool enable,
+					bool lock);
 	int (*get_thermal_temperature_range)(struct pp_hwmgr *hwmgr,
 					struct PP_TemperatureRange *range);
 	int (*get_power_profile_mode)(struct pp_hwmgr *hwmgr, char *buf);
-- 
2.7.4

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

[PATCH 3/3] drm/amd:Enable/Disable NBPSTATE on On/OFF of UVD

[Guttula, Suresh]

We observe black lines (underflow) on display when playing a
4K video with UVD. On Disabling Low memory P state this issue is
not seen.
In this patch ,disabling low memory P state only when video
size >= 4k.
Multiple runs of power measurement shows no imapct

Signed-off-by: suresh guttula <suresh.guttula@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c          | 17 +++++++++++++++++
 drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c |  4 +++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index e5a6db6..6902719 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -38,6 +38,7 @@
 #include "amdgpu_uvd.h"
 #include "cikd.h"
 #include "uvd/uvd_4_2_d.h"
+#include "hwmgr.h"
 
 /* 1 second timeout */
 #define UVD_IDLE_TIMEOUT	msecs_to_jiffies(1000)
@@ -78,6 +79,7 @@
 #define UVD_GPCOM_VCPU_DATA1	0x03c5
 #define UVD_NO_OP				0x03ff
 #define UVD_BASE_SI				0x3800
+#define WIDTH_4K                3840
 
 /**
  * amdgpu_uvd_cs_ctx - Command submission parser context
@@ -528,6 +530,21 @@ static int amdgpu_uvd_cs_msg_decode(struct amdgpu_device *adev, uint32_t *msg,
 	unsigned image_size, tmp, min_dpb_size, num_dpb_buffer;
 	unsigned min_ctx_size = ~0;
 
+	 //disable Low Memory PState for UVD(4k videos)
+	if (adev->asic_type == CHIP_STONEY && width >= WIDTH_4K) {
+		struct pp_hwmgr  *hwmgr;
+		struct pp_instance *pp_handle =
+			(struct pp_instance *)adev->powerplay.pp_handle;
+		if (pp_handle) {
+			hwmgr = pp_handle->hwmgr;
+			if (hwmgr && hwmgr->hwmgr_func &&
+				hwmgr->hwmgr_func->update_nbdpm_pstate)
+				hwmgr->hwmgr_func->update_nbdpm_pstate(hwmgr,
+									false,
+									true);
+		}
+	}
+
 	image_size = width * height;
 	image_size += image_size / 2;
 	image_size = ALIGN(image_size, 1024);
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
index 553a203..0bf56f7 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
@@ -1228,8 +1228,10 @@ static int smu8_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
 
 static int smu8_dpm_powerdown_uvd(struct pp_hwmgr *hwmgr)
 {
-	if (PP_CAP(PHM_PlatformCaps_UVDPowerGating))
+	if (PP_CAP(PHM_PlatformCaps_UVDPowerGating)) {
+		smu8_nbdpm_pstate_enable_disable(hwmgr, true, true);
 		return smum_send_msg_to_smc(hwmgr, PPSMC_MSG_UVDPowerOFF);
+	}
 	return 0;
 }
 
-- 
2.7.4

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

Re: [PATCH 3/3] drm/amd:Enable/Disable NBPSTATE on On/OFF of UVD

[Alex Deucher]

On Fri, Oct 26, 2018 at 9:46 AM Guttula, Suresh <Suresh.Guttula@amd.com> wrote:
>
> We observe black lines (underflow) on display when playing a
> 4K video with UVD. On Disabling Low memory P state this issue is
> not seen.
> In this patch ,disabling low memory P state only when video
> size >= 4k.
> Multiple runs of power measurement shows no imapct
>
> Signed-off-by: suresh guttula <suresh.guttula@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c          | 17 +++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c |  4 +++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index e5a6db6..6902719 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -38,6 +38,7 @@
>  #include "amdgpu_uvd.h"
>  #include "cikd.h"
>  #include "uvd/uvd_4_2_d.h"
> +#include "hwmgr.h"
>
>  /* 1 second timeout */
>  #define UVD_IDLE_TIMEOUT       msecs_to_jiffies(1000)
> @@ -78,6 +79,7 @@
>  #define UVD_GPCOM_VCPU_DATA1   0x03c5
>  #define UVD_NO_OP                              0x03ff
>  #define UVD_BASE_SI                            0x3800
> +#define WIDTH_4K                3840
>
>  /**
>   * amdgpu_uvd_cs_ctx - Command submission parser context
> @@ -528,6 +530,21 @@ static int amdgpu_uvd_cs_msg_decode(struct amdgpu_device *adev, uint32_t *msg,
>         unsigned image_size, tmp, min_dpb_size, num_dpb_buffer;
>         unsigned min_ctx_size = ~0;
>
> +        //disable Low Memory PState for UVD(4k videos)
> +       if (adev->asic_type == CHIP_STONEY && width >= WIDTH_4K) {
> +               struct pp_hwmgr  *hwmgr;
> +               struct pp_instance *pp_handle =
> +                       (struct pp_instance *)adev->powerplay.pp_handle;
> +               if (pp_handle) {
> +                       hwmgr = pp_handle->hwmgr;
> +                       if (hwmgr && hwmgr->hwmgr_func &&
> +                               hwmgr->hwmgr_func->update_nbdpm_pstate)
> +                               hwmgr->hwmgr_func->update_nbdpm_pstate(hwmgr,
> +                                                                       false,
> +                                                                       true);
> +               }
> +       }
> +

This should be added to amdgpu_dpm_enable_uvd() or
amdgpu_uvd_ring_begin_use() rather than in one of the parser
functions.

Alex

>         image_size = width * height;
>         image_size += image_size / 2;
>         image_size = ALIGN(image_size, 1024);
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
> index 553a203..0bf56f7 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
> @@ -1228,8 +1228,10 @@ static int smu8_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
>
>  static int smu8_dpm_powerdown_uvd(struct pp_hwmgr *hwmgr)
>  {
> -       if (PP_CAP(PHM_PlatformCaps_UVDPowerGating))
> +       if (PP_CAP(PHM_PlatformCaps_UVDPowerGating)) {
> +               smu8_nbdpm_pstate_enable_disable(hwmgr, true, true);
>                 return smum_send_msg_to_smc(hwmgr, PPSMC_MSG_UVDPowerOFF);
> +       }
>         return 0;
>  }
>
> --
> 2.7.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