On 2018-09-20 11:49 AM, Alex Deucher wrote: > 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. cur_state is used for tracking driver SW PG state, not HWÂ PG state. I though no matter what HWÂ PG state is after device powers up, when first vcn ring is scheduled to run, begin_use->set_powergating_state->vcn_v1_0_start->ungate power/clock->Boot_VCPUÂ will be tried. For DPG mode, the ungate power/clock , boot VCPU will not actually be activated during start setup stage, and only be activated during ring run stage. James > 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