Nevermind I moved the locking into amdgpu_pm.c and that did the trick.

Attached is a patch that contains all the changes.  If you guys want to give it 
a quick once-through I can then start splitting it up per Alex's comments.


Tom


________________________________
From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> on behalf of StDenis, Tom 
<tom.stde...@amd.com>
Sent: Thursday, July 28, 2016 07:10
To: Zhu, Rex; Alex Deucher
Cc: amd-gfx list
Subject: Re: [PATCH 4/4] drm/amd/powerplay: Prevent UVD powerdown before init


Quick question, how am I meant to get access to pm.mutex from powerplay?


I need a lock I can see around the SMU calls and in the amdgpu side (for 
userspace locking).


Tom


________________________________
From: Zhu, Rex
Sent: Thursday, July 28, 2016 03:43
To: Alex Deucher; Tom St Denis
Cc: StDenis, Tom; amd-gfx list
Subject: RE: [PATCH 4/4] drm/amd/powerplay: Prevent UVD powerdown before init


From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Alex 
Deucher
Sent: Thursday, July 28, 2016 1:46 PM
To: Tom St Denis
Cc: StDenis, Tom; amd-gfx list
Subject: Re: [PATCH 4/4] drm/amd/powerplay: Prevent UVD powerdown before init

On Tue, Jul 26, 2016 at 11:38 AM, Tom St Denis <tstdeni...@gmail.com> wrote:
> Because of the ip_blocks init order powerplay would power down the UVD
> block before UVD start is called.  This results in a VCPU hang.
>
> This patch prevents power down before UVD is initialized.
>
> Also correct the power up order so clocking is set after power is
> ungated.
>
> With this applied comparable clock/power behaviour to powerplay=0 with
> DPM is observed.
>
> Signed-off-by: Tom St Denis <tom.stde...@amd.com>

This patch needs to be split into several patches and reworked a bit.
Also, don't include amdgpu.h in powerplay.  We have cgs for access to registers 
and data from adev, etc.  The idea is to minimize the dependencies between 
components.  We shouldn't be accessing adev directly in powerplay.  A couple 
more comments inline.


Rex:  I also think so.
1. We can move
+                       WREG32(mmUVD_POWER_STATUS,
+                               UVD_POWER_STATUS__UVD_PG_EN_MASK |
+                               UVD_POWER_STATUS__UVD_PG_MODE_MASK);
+               else
+                       WREG32(mmUVD_POWER_STATUS,
+                               UVD_POWER_STATUS__UVD_PG_EN_MASK);
to uvd_v6_0_start.  no need to visit adev in powerplay and dpm.  And uvd test 
also can pass.

2.  for the lock, we can just use pm.mutex.

3.  please also delete enable_clock_power_gatings_tasks in resume_action_chain 
in a separate patch for powerplay.

4.  do we need to add cg_state, pg_state?



Best Regards
Rex


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h                |  6 ++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c            |  5 -----
>  drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c              |  8 ++++---
>  drivers/gpu/drm/amd/amdgpu/vi.c                    | 12 ++++-------
>  .../drm/amd/powerplay/hwmgr/cz_clockpowergating.c  | 25 
> ++++++++++++++++++----
>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c     |  7 ++++++
>  6 files changed, 43 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index d0460ea2f85b..5616b16e6c0a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1692,6 +1692,7 @@ struct amdgpu_uvd {
>         uint32_t                srbm_soft_reset;
>         int                     cg_state, pg_state;
>         struct mutex            pg_lock;
> +       bool                    is_init;
>  };
>
>  /*
> @@ -2518,5 +2519,10 @@ int amdgpu_dm_display_resume(struct
> amdgpu_device *adev );  static inline int
> amdgpu_dm_display_resume(struct amdgpu_device *adev) { return 0; }
> #endif
>
> +struct amdgpu_cgs_device {
> +       struct cgs_device base;
> +       struct amdgpu_device *adev;
> +};
> +
>  #include "amdgpu_object.h"
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> index ee95e950a19b..d553e399a835 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> @@ -33,11 +33,6 @@
>  #include "atom.h"
>  #include "amdgpu_ucode.h"
>
> -struct amdgpu_cgs_device {
> -       struct cgs_device base;
> -       struct amdgpu_device *adev;
> -};
> -
>  #define CGS_FUNC_ADEV                                                  \
>         struct amdgpu_device *adev =                                    \
>                 ((struct amdgpu_cgs_device *)cgs_device)->adev diff
> --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 422d5300b92e..3b93327c5e25 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -389,9 +389,9 @@ static int uvd_v6_0_start(struct amdgpu_device *adev)
>         uint32_t mp_swap_cntl;
>         int i, j, r;
>
> -       /* is power gated? then we can't start (TODO: re-enable power) */
> -       if (adev->uvd.pg_state)
> -               return -EINVAL;
> +       /* is power gated? then we can't start but don't return an error */
> +       if (adev->uvd.is_init && adev->uvd.pg_state)
> +               return 0;
>
>         /* set CG state to -1 for unset */
>         adev->uvd.cg_state = -1;
> @@ -662,6 +662,8 @@ static int uvd_v6_0_ring_test_ring(struct amdgpu_ring 
> *ring)
>                           ring->idx, tmp);
>                 r = -EINVAL;
>         }
> +       if (!r)
> +               adev->uvd.is_init = true;
>         return r;
>  }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
> b/drivers/gpu/drm/amd/amdgpu/vi.c index 78fea940d120..f4fdde0641b0
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> @@ -1583,10 +1583,8 @@ static int vi_common_early_init(void *handle)
>                 if (adev->rev_id != 0x00) {
>                         adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
>                                 AMD_PG_SUPPORT_GFX_SMG |
> -                               AMD_PG_SUPPORT_GFX_PIPELINE;
> -                       /* powerplay UVD PG doesn't work yet */
> -                       if (!amdgpu_powerplay)
> -                               adev->pg_flags |= AMD_PG_SUPPORT_UVD;
> +                               AMD_PG_SUPPORT_GFX_PIPELINE |
> +                               AMD_PG_SUPPORT_UVD;

This should be a separate patch.

>                 }
>                 adev->external_rev_id = adev->rev_id + 0x1;
>                 break;
> @@ -1608,10 +1606,8 @@ static int vi_common_early_init(void *handle)
>                         AMD_CG_SUPPORT_SDMA_LS;
>                 adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
>                         AMD_PG_SUPPORT_GFX_SMG |
> -                       AMD_PG_SUPPORT_GFX_PIPELINE;
> -               /* powerplay UVD PG doesn't work yet */
> -               if (!amdgpu_powerplay)
> -                       adev->pg_flags |= AMD_PG_SUPPORT_UVD;
> +                       AMD_PG_SUPPORT_GFX_PIPELINE |
> +                       AMD_PG_SUPPORT_UVD;

Same with this.

>                 adev->external_rev_id = adev->rev_id + 0x1;
>                 break;
>         default:
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_clockpowergating.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_clockpowergating.c
> index 2da548f6337e..baa7366fad53 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_clockpowergating.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_clockpowergating.c
> @@ -21,9 +21,13 @@
>   *
>   */
>
> +#include "amdgpu.h"
>  #include "hwmgr.h"
>  #include "cz_clockpowergating.h"
>  #include "cz_ppsmc.h"
> +#include "cgs_linux.h"
> +#include "uvd/uvd_6_0_d.h"
> +#include "uvd/uvd_6_0_sh_mask.h"
>
>  /* PhyID -> Status Mapping in DDI_PHY_GEN_STATUS
>      0    GFX0L (3:0),                  (27:24),
> @@ -160,12 +164,24 @@ int cz_enable_disable_vce_dpm(struct pp_hwmgr
> *hwmgr, bool enable)  int cz_dpm_powergate_uvd(struct pp_hwmgr *hwmgr,
> bool bgate)  {
>         struct cz_hwmgr *cz_hwmgr = (struct cz_hwmgr
> *)(hwmgr->backend);
> +       struct amdgpu_cgs_device *cgs_dev = hwmgr->device;
> +       struct amdgpu_device *adev = cgs_dev->adev;
>
> -       if (cz_hwmgr->uvd_power_gated == bgate)
> +       if (!adev->uvd.is_init)
>                 return 0;
>
> +       mutex_lock(&adev->uvd.pg_lock);
> +
> +       if (cz_hwmgr->uvd_power_gated == bgate) {
> +               mutex_unlock(&adev->uvd.pg_lock);
> +               return 0;
> +       }
> +
> +       adev->uvd.pg_state = bgate;
>         cz_hwmgr->uvd_power_gated = bgate;
>
> +       WREG32(mmUVD_POWER_STATUS, UVD_POWER_STATUS__UVD_PG_EN_MASK);
> +

Could this be moved to the uvd 6 module?




>         if (bgate) {
>                 cgs_set_clockgating_state(hwmgr->device,
>                                                 AMD_IP_BLOCK_TYPE_UVD,
> @@ -177,14 +193,15 @@ int cz_dpm_powergate_uvd(struct pp_hwmgr *hwmgr, bool 
> bgate)
>                 cz_dpm_powerdown_uvd(hwmgr);
>         } else {
>                 cz_dpm_powerup_uvd(hwmgr);
> -               cgs_set_clockgating_state(hwmgr->device,
> -                                               AMD_IP_BLOCK_TYPE_UVD,
> -                                               AMD_PG_STATE_GATE);
>                 cgs_set_powergating_state(hwmgr->device,
>                                                 AMD_IP_BLOCK_TYPE_UVD,
>                                                 AMD_CG_STATE_UNGATE);
> +               cgs_set_clockgating_state(hwmgr->device,
> +                                               AMD_IP_BLOCK_TYPE_UVD,
> +                                               AMD_PG_STATE_GATE);

I think this is a standalone fix and should be split into a separate patch.

>                 cz_dpm_update_uvd_dpm(hwmgr, false);
>         }
> +       mutex_unlock(&adev->uvd.pg_lock);
>
>         return 0;
>  }
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> index 9bf622e123b6..bed0013674a1 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> @@ -39,6 +39,7 @@
>  #include "power_state.h"
>  #include "cz_clockpowergating.h"
>  #include "pp_debug.h"
> +#include "amdgpu.h"
>
>  #define ixSMUSVI_NB_CURRENTVID 0xD8230044  #define
> CURRENT_NB_VID_MASK 0xff000000 @@ -1356,6 +1357,12 @@ static int
> cz_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
>
>  int cz_dpm_powerdown_uvd(struct pp_hwmgr *hwmgr)  {
> +       struct amdgpu_cgs_device *cgs_dev = hwmgr->device;
> +       struct amdgpu_device *adev = cgs_dev->adev;
> +
> +       if (!adev->uvd.is_init)
> +               return 0;
> +
>         if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
>                                          PHM_PlatformCaps_UVDPowerGating))
>                 return smum_send_msg_to_smc(hwmgr->smumgr,
> --
> 2.9.2
>
> _______________________________________________
> 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
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index eab931a58d06..1fe8ef626407 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2335,22 +2335,26 @@ static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
 	struct amdgpu_device *adev = f->f_inode->i_private;
 	ssize_t result = 0;
 	int r;
-	bool use_bank;
+	bool pm_pg_lock, use_bank;
 	unsigned instance_bank, sh_bank, se_bank;
 
 	if (size & 0x3 || *pos & 0x3)
 		return -EINVAL;
 
+	/* are we reading registers for which a PG lock is necessary? */
+	pm_pg_lock = (*pos >> 23) & 1;
+
 	if (*pos & (1ULL << 62)) {
 		se_bank = (*pos >> 24) & 0x3FF;
 		sh_bank = (*pos >> 34) & 0x3FF;
 		instance_bank = (*pos >> 44) & 0x3FF;
 		use_bank = 1;
-		*pos &= 0xFFFFFF;
 	} else {
 		use_bank = 0;
 	}
 
+	*pos &= 0x3FFFF;
+
 	if (use_bank) {
 		if (sh_bank >= adev->gfx.config.max_sh_per_se ||
 		    se_bank >= adev->gfx.config.max_shader_engines)
@@ -2360,6 +2364,9 @@ static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
 					sh_bank, instance_bank);
 	}
 
+	if (pm_pg_lock)
+		mutex_lock(&adev->pm.mutex);
+
 	while (size) {
 		uint32_t value;
 
@@ -2385,6 +2392,9 @@ end:
 		mutex_unlock(&adev->grbm_idx_mutex);
 	}
 
+	if (pm_pg_lock)
+		mutex_unlock(&adev->pm.mutex);
+
 	return result;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 12ab58eca581..c4bb4ef8f2c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -1105,54 +1105,46 @@ force:
 
 void amdgpu_dpm_enable_uvd(struct amdgpu_device *adev, bool enable)
 {
-	if (adev->pp_enabled)
+	if (adev->pp_enabled || adev->pm.funcs->powergate_uvd) {
+		/* enable/disable UVD */
+		mutex_lock(&adev->pm.mutex);
 		amdgpu_dpm_powergate_uvd(adev, !enable);
-	else {
-		if (adev->pm.funcs->powergate_uvd) {
+		mutex_unlock(&adev->pm.mutex);
+	} else {
+		if (enable) {
 			mutex_lock(&adev->pm.mutex);
-			/* enable/disable UVD */
-			amdgpu_dpm_powergate_uvd(adev, !enable);
+			adev->pm.dpm.uvd_active = true;
+			adev->pm.dpm.state = POWER_STATE_TYPE_INTERNAL_UVD;
 			mutex_unlock(&adev->pm.mutex);
 		} else {
-			if (enable) {
-				mutex_lock(&adev->pm.mutex);
-				adev->pm.dpm.uvd_active = true;
-				adev->pm.dpm.state = POWER_STATE_TYPE_INTERNAL_UVD;
-				mutex_unlock(&adev->pm.mutex);
-			} else {
-				mutex_lock(&adev->pm.mutex);
-				adev->pm.dpm.uvd_active = false;
-				mutex_unlock(&adev->pm.mutex);
-			}
-			amdgpu_pm_compute_clocks(adev);
+			mutex_lock(&adev->pm.mutex);
+			adev->pm.dpm.uvd_active = false;
+			mutex_unlock(&adev->pm.mutex);
 		}
-
+		amdgpu_pm_compute_clocks(adev);
 	}
 }
 
 void amdgpu_dpm_enable_vce(struct amdgpu_device *adev, bool enable)
 {
-	if (adev->pp_enabled)
+	if (adev->pp_enabled || adev->pm.funcs->powergate_vce) {
+		/* enable/disable VCE */
+		mutex_lock(&adev->pm.mutex);
 		amdgpu_dpm_powergate_vce(adev, !enable);
-	else {
-		if (adev->pm.funcs->powergate_vce) {
+		mutex_unlock(&adev->pm.mutex);
+	} else {
+		if (enable) {
 			mutex_lock(&adev->pm.mutex);
-			amdgpu_dpm_powergate_vce(adev, !enable);
+			adev->pm.dpm.vce_active = true;
+			/* XXX select vce level based on ring/task */
+			adev->pm.dpm.vce_level = AMDGPU_VCE_LEVEL_AC_ALL;
 			mutex_unlock(&adev->pm.mutex);
 		} else {
-			if (enable) {
-				mutex_lock(&adev->pm.mutex);
-				adev->pm.dpm.vce_active = true;
-				/* XXX select vce level based on ring/task */
-				adev->pm.dpm.vce_level = AMDGPU_VCE_LEVEL_AC_ALL;
-				mutex_unlock(&adev->pm.mutex);
-			} else {
-				mutex_lock(&adev->pm.mutex);
-				adev->pm.dpm.vce_active = false;
-				mutex_unlock(&adev->pm.mutex);
-			}
-			amdgpu_pm_compute_clocks(adev);
+			mutex_lock(&adev->pm.mutex);
+			adev->pm.dpm.vce_active = false;
+			mutex_unlock(&adev->pm.mutex);
 		}
+		amdgpu_pm_compute_clocks(adev);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/cz_dpm.c b/drivers/gpu/drm/amd/amdgpu/cz_dpm.c
index 8ba07e79d4cb..301d0b98e607 100644
--- a/drivers/gpu/drm/amd/amdgpu/cz_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/cz_dpm.c
@@ -2108,29 +2108,58 @@ static void cz_dpm_powergate_uvd(struct amdgpu_device *adev, bool gate)
 			/* disable clockgating so we can properly shut down the block */
 			ret = amdgpu_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
 							    AMD_CG_STATE_UNGATE);
+			if (ret) {
+				DRM_ERROR("UVD DPM Power Gating failed to set clockgating state\n");
+				return;
+			}
+
 			/* shutdown the UVD block */
 			ret = amdgpu_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
 							    AMD_PG_STATE_GATE);
-			/* XXX: check for errors */
+
+			if (ret) {
+				DRM_ERROR("UVD DPM Power Gating failed to set powergating state\n");
+				return;
+			}
 		}
 		cz_update_uvd_dpm(adev, gate);
-		if (pi->caps_uvd_pg)
+		if (pi->caps_uvd_pg) {
 			/* power off the UVD block */
-			cz_send_msg_to_smc(adev, PPSMC_MSG_UVDPowerOFF);
+			ret = cz_send_msg_to_smc(adev, PPSMC_MSG_UVDPowerOFF);
+			if (ret) {
+				DRM_ERROR("UVD DPM Power Gating failed to send SMU PowerOFF message\n");
+				return;
+			}
+		}
 	} else {
 		if (pi->caps_uvd_pg) {
 			/* power on the UVD block */
 			if (pi->uvd_dynamic_pg)
-				cz_send_msg_to_smc_with_parameter(adev, PPSMC_MSG_UVDPowerON, 1);
+				ret = cz_send_msg_to_smc_with_parameter(adev, PPSMC_MSG_UVDPowerON, 1);
 			else
-				cz_send_msg_to_smc_with_parameter(adev, PPSMC_MSG_UVDPowerON, 0);
+				ret = cz_send_msg_to_smc_with_parameter(adev, PPSMC_MSG_UVDPowerON, 0);
+
+			if (ret) {
+				DRM_ERROR("UVD DPM Power Gating Failed to send SMU PowerON message\n");
+				return;
+			}
+
 			/* re-init the UVD block */
 			ret = amdgpu_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
 							    AMD_PG_STATE_UNGATE);
+
+			if (ret) {
+				DRM_ERROR("UVD DPM Power Gating Failed to set powergating state\n");
+				return;
+			}
+
 			/* enable clockgating. hw will dynamically gate/ungate clocks on the fly */
 			ret = amdgpu_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_UVD,
 							    AMD_CG_STATE_GATE);
-			/* XXX: check for errors */
+			if (ret) {
+				DRM_ERROR("UVD DPM Power Gating Failed to set clockgating state\n");
+				return;
+			}
 		}
 		cz_update_uvd_dpm(adev, gate);
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index 4fa50918e886..391457f1eafd 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -396,15 +396,10 @@ static int uvd_v6_0_start(struct amdgpu_device *adev)
 
 	uvd_v6_0_mc_resume(adev);
 
-	/* Set dynamic clock gating in S/W control mode */
-	if (adev->cg_flags & AMD_CG_SUPPORT_UVD_MGCG) {
-		uvd_v6_0_set_sw_clock_gating(adev);
-	} else {
-		/* disable clock gating */
-		uint32_t data = RREG32(mmUVD_CGC_CTRL);
-		data &= ~UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK;
-		WREG32(mmUVD_CGC_CTRL, data);
-	}
+	/* disable clock gating */
+	tmp = RREG32(mmUVD_CGC_CTRL);
+	tmp &= ~UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK;
+	WREG32(mmUVD_CGC_CTRL, tmp);
 
 	/* disable interupt */
 	WREG32_P(mmUVD_MASTINT_EN, 0, ~UVD_MASTINT_EN__VCPU_EN_MASK);
@@ -964,21 +959,15 @@ static int uvd_v6_0_set_clockgating_state(void *handle,
 					  enum amd_clockgating_state state)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-	bool enable = (state == AMD_CG_STATE_GATE) ? true : false;
-	static int curstate = -1;
-
-	if (adev->asic_type == CHIP_FIJI ||
-			adev->asic_type == CHIP_POLARIS10)
-		uvd_v6_set_bypass_mode(adev, enable);
 
 	if (!(adev->cg_flags & AMD_CG_SUPPORT_UVD_MGCG))
 		return 0;
 
-	if (curstate == state)
-		return 0;
+	if (adev->asic_type == CHIP_FIJI ||
+	    adev->asic_type == CHIP_POLARIS10)
+		uvd_v6_set_bypass_mode(adev, state == AMD_CG_STATE_GATE ? true : false);
 
-	curstate = state;
-	if (enable) {
+	if (state == AMD_CG_STATE_GATE) {
 		/* disable HW gating and enable Sw gating */
 		uvd_v6_0_set_sw_clock_gating(adev);
 	} else {
@@ -1008,6 +997,8 @@ static int uvd_v6_0_set_powergating_state(void *handle,
 	if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
 		return 0;
 
+	WREG32(mmUVD_POWER_STATUS, UVD_POWER_STATUS__UVD_PG_EN_MASK);
+
 	if (state == AMD_PG_STATE_GATE) {
 		uvd_v6_0_stop(adev);
 		return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
index e5b18ad42721..dd695201dccb 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
@@ -126,6 +126,7 @@ static void vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device *adev,
 					     bool gated)
 {
 	u32 tmp, data;
+
 	/* Set Override to disable Clock Gating */
 	vce_v3_0_override_vce_clock_gating(adev, true);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index 9ba64989f092..33bad99176fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -1578,7 +1578,15 @@ static int vi_common_early_init(void *handle)
 			AMD_CG_SUPPORT_HDP_LS |
 			AMD_CG_SUPPORT_SDMA_MGCG |
 			AMD_CG_SUPPORT_SDMA_LS;
+		/* rev0 hardware requires workarounds to support PG */
 		adev->pg_flags = 0;
+		if (adev->rev_id != 0x00) {
+			adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
+				AMD_PG_SUPPORT_GFX_SMG |
+				AMD_PG_SUPPORT_GFX_PIPELINE |
+				AMD_PG_SUPPORT_UVD |
+				AMD_PG_SUPPORT_VCE;
+		}
 		adev->external_rev_id = adev->rev_id + 0x1;
 		break;
 	case CHIP_STONEY:
@@ -1597,6 +1605,11 @@ static int vi_common_early_init(void *handle)
 			AMD_CG_SUPPORT_HDP_LS |
 			AMD_CG_SUPPORT_SDMA_MGCG |
 			AMD_CG_SUPPORT_SDMA_LS;
+		adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
+			AMD_PG_SUPPORT_GFX_SMG |
+			AMD_PG_SUPPORT_GFX_PIPELINE |
+			AMD_PG_SUPPORT_UVD |
+			AMD_PG_SUPPORT_VCE;
 		adev->external_rev_id = adev->rev_id + 0x1;
 		break;
 	default:
diff --git a/drivers/gpu/drm/amd/powerplay/eventmgr/eventactionchains.c b/drivers/gpu/drm/amd/powerplay/eventmgr/eventactionchains.c
index d6635cc4b0fc..635fc4b48184 100644
--- a/drivers/gpu/drm/amd/powerplay/eventmgr/eventactionchains.c
+++ b/drivers/gpu/drm/amd/powerplay/eventmgr/eventactionchains.c
@@ -30,7 +30,6 @@ static const pem_event_action * const initialize_event[] = {
 	system_config_tasks,
 	setup_asic_tasks,
 	enable_dynamic_state_management_tasks,
-	enable_clock_power_gatings_tasks,
 	get_2d_performance_state_tasks,
 	set_performance_state_tasks,
 	initialize_thermal_controller_tasks,
@@ -140,7 +139,6 @@ static const pem_event_action * const resume_event[] = {
 	setup_asic_tasks,
 	enable_stutter_mode_tasks, /*must do this in boot state and before SMC is started */
 	enable_dynamic_state_management_tasks,
-	enable_clock_power_gatings_tasks,
 	enable_disable_bapm_tasks,
 	initialize_thermal_controller_tasks,
 	get_2d_performance_state_tasks,
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_clockpowergating.c b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_clockpowergating.c
index 2da548f6337e..0e82e5bf062b 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_clockpowergating.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_clockpowergating.c
@@ -161,8 +161,9 @@ int cz_dpm_powergate_uvd(struct pp_hwmgr *hwmgr, bool bgate)
 {
 	struct cz_hwmgr *cz_hwmgr = (struct cz_hwmgr *)(hwmgr->backend);
 
-	if (cz_hwmgr->uvd_power_gated == bgate)
+	if (cz_hwmgr->uvd_power_gated == bgate) {
 		return 0;
+	}
 
 	cz_hwmgr->uvd_power_gated = bgate;
 
@@ -177,12 +178,12 @@ int cz_dpm_powergate_uvd(struct pp_hwmgr *hwmgr, bool bgate)
 		cz_dpm_powerdown_uvd(hwmgr);
 	} else {
 		cz_dpm_powerup_uvd(hwmgr);
-		cgs_set_clockgating_state(hwmgr->device,
-						AMD_IP_BLOCK_TYPE_UVD,
-						AMD_PG_STATE_GATE);
 		cgs_set_powergating_state(hwmgr->device,
 						AMD_IP_BLOCK_TYPE_UVD,
 						AMD_CG_STATE_UNGATE);
+		cgs_set_clockgating_state(hwmgr->device,
+						AMD_IP_BLOCK_TYPE_UVD,
+						AMD_PG_STATE_GATE);
 		cz_dpm_update_uvd_dpm(hwmgr, false);
 	}
 
@@ -211,14 +212,14 @@ int cz_dpm_powergate_vce(struct pp_hwmgr *hwmgr, bool bgate)
 			} else {
 				cz_dpm_powerup_vce(hwmgr);
 				cz_hwmgr->vce_power_gated = false;
-				cgs_set_clockgating_state(
-							hwmgr->device,
-							AMD_IP_BLOCK_TYPE_VCE,
-							AMD_PG_STATE_GATE);
 				cgs_set_powergating_state(
 							hwmgr->device,
 							AMD_IP_BLOCK_TYPE_VCE,
 							AMD_CG_STATE_UNGATE);
+				cgs_set_clockgating_state(
+							hwmgr->device,
+							AMD_IP_BLOCK_TYPE_VCE,
+							AMD_PG_STATE_GATE);
 				cz_dpm_update_vce_dpm(hwmgr);
 				cz_enable_disable_vce_dpm(hwmgr, true);
 				return 0;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to