On 2022-05-05 16:04, Alex Deucher wrote:
> From: Likun Gao <Likun.Gao@xxxxxxx>
>
> Support memory power gating control for LSDMA.
>
> Signed-off-by: Likun Gao <Likun.Gao@xxxxxxx>
> Reviewed-by: Hawking Zhang <Hawking.Zhang@xxxxxxx>
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h | 1 +
> drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c | 16 +++++++++++++++-
> drivers/gpu/drm/amd/amdgpu/soc21.c | 12 +++++++++++-
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> index 9a29f18407b8..7ec7598e7dec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lsdma.h
> @@ -34,6 +34,7 @@ struct amdgpu_lsdma_funcs
> uint64_t dst_addr, uint64_t size);
> int (*fill_mem)(struct amdgpu_device *adev, uint64_t dst_addr,
> uint32_t data, uint64_t size);
> + void (*update_memory_power_gating)(struct amdgpu_device *adev, bool enable);
> };
>
> int amdgpu_lsdma_copy_mem(struct amdgpu_device *adev, uint64_t src_addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> index b4adb94a080b..1a285b531881 100644
> --- a/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
> @@ -101,7 +101,21 @@ static int lsdma_v6_0_fill_mem(struct amdgpu_device *adev,
> return ret;
> }
>
> +static void lsdma_v6_0_update_memory_power_gating(struct amdgpu_device *adev,
> + bool enable)
> +{
> + uint32_t tmp;
> +
> + tmp = RREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL);
> + tmp = REG_SET_FIELD(tmp, LSDMA_MEM_POWER_CTRL, MEM_POWER_CTRL_EN, 0);
> + WREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL, tmp);
> +
> + tmp = REG_SET_FIELD(tmp, LSDMA_MEM_POWER_CTRL, MEM_POWER_CTRL_EN, enable);
> + WREG32_SOC15(LSDMA, 0, regLSDMA_MEM_POWER_CTRL, tmp);
> +}
> +
Do we need to disable it first before updating?
What if we're updating it to the same value it currently is (on the first read)?
Shouldn't we just skip the subsequent writes?
> const struct amdgpu_lsdma_funcs lsdma_v6_0_funcs = {
> .copy_mem = lsdma_v6_0_copy_mem,
> - .fill_mem = lsdma_v6_0_fill_mem
> + .fill_mem = lsdma_v6_0_fill_mem,
> + .update_memory_power_gating = lsdma_v6_0_update_memory_power_gating
> };
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> index 741ed3ba84d6..3303e02f85d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> @@ -676,7 +676,17 @@ static int soc21_common_set_clockgating_state(void *handle,
> static int soc21_common_set_powergating_state(void *handle,
> enum amd_powergating_state state)
> {
> - /* TODO */
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + switch (adev->ip_versions[LSDMA_HWIP][0]) {
> + case IP_VERSION(6, 0, 0):
> + adev->lsdma.funcs->update_memory_power_gating(adev,
> + state == AMD_PG_STATE_GATE);
> + break;
> + default:
> + break;
> + }
> +
> return 0;
> }
>
Regards,
--
Luben
