Re: [PATCH 7/8] drm/amdgpu: support memory power gating for lsdma

[Alex Deucher <alexdeucher@gmail.com>]

On Fri, May 6, 2022 at 1:10 AM Luben Tuikov <luben.tuikov@amd.com> wrote:
>
>
>
> On 2022-05-05 16:04, Alex Deucher wrote:
> > From: Likun Gao <Likun.Gao@amd.com>
> >
> > Support memory power gating control for LSDMA.
> >
> > Signed-off-by: Likun Gao <Likun.Gao@amd.com>
> > Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >  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?

I'll let Likun comment, but I suspect this sequence came from the hardware team.

Alex

>
> >  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