[PATCH v2] drm/amdgpu:No action when VCN PG state is unchanged

[Alex Deucher]

On Thu, Sep 20, 2018 at 11:27 AM James Zhu <jamesz at amd.com> wrote:
>
>
>
> On 2018-09-20 11:14 AM, Alex Deucher wrote:
> > On Thu, Sep 13, 2018 at 4:56 PM James Zhu <jzhums at gmail.com> wrote:
> >> When VCN PG state is unchanged, it is unnecessary to reset power
> >> gate state
> >>
> >> Signed-off-by: James Zhu <James.Zhu at amd.com>
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 +
> >>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   | 12 ++++++++++--
> >>   2 files changed, 11 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> >> index 0b0b863..d2219ab 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> >> @@ -69,6 +69,7 @@ struct amdgpu_vcn {
> >>          struct amdgpu_ring      ring_jpeg;
> >>          struct amdgpu_irq_src   irq;
> >>          unsigned                num_enc_rings;
> >> +       enum amd_powergating_state cur_state;
> > Does the default value (0) at init time properly reflect the default
> > powergating state?  If so,
> > Acked-by: Alex Deucher <alexander.deucher at amd.com>
> Yes, the below code shows it will be set to 0 during driver load stage.

Yes, I understand that.  Is 0 (AMD_PG_STATE_GATE) what we want as the
default though?  The first time the code runs are we going to do the
right thing or is the code going to return early?  IIRC, the hw
default is ungated.

Alex

>
> int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
> ....
>      adev = kzalloc(sizeof(struct amdgpu_device), GFP_KERNEL);
>
> struct amdgpu_device {
> ....
>      struct amdgpu_vcn        vcn;
>
> Best Regards!
> James zhu
> >>   };
> >>
> >>   int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> >> index 2664bb2..2cde0b4 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> >> @@ -1633,12 +1633,20 @@ static int vcn_v1_0_set_powergating_state(void *handle,
> >>           * revisit this when there is a cleaner line between
> >>           * the smc and the hw blocks
> >>           */
> >> +       int ret;
> >>          struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> >>
> >> +       if(state == adev->vcn.cur_state)
> >> +               return 0;
> >> +
> >>          if (state == AMD_PG_STATE_GATE)
> >> -               return vcn_v1_0_stop(adev);
> >> +               ret = vcn_v1_0_stop(adev);
> >>          else
> >> -               return vcn_v1_0_start(adev);
> >> +               ret = vcn_v1_0_start(adev);
> >> +
> >> +       if(!ret)
> >> +               adev->vcn.cur_state = state;
> >> +       return ret;
> >>   }
> >>
> >>   static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>