On 2018-09-11 11:36 AM, Christian König wrote: > Am 11.09.2018 um 17:29 schrieb James Zhu: >> >> >> >> On 2018-09-11 11:17 AM, Christian König wrote: >>> Am 11.09.2018 um 17:06 schrieb James Zhu: >>>> >>>> >>>> >>>> On 2018-09-11 10:44 AM, Alex Deucher wrote: >>>>> On Mon, Sep 10, 2018 at 4:34 PM James Zhu<jzhums at gmail.com> wrote: >>>>>> Signed-off-by: James Zhu<James.Zhu at amd.com> >>>>>> >>>>>> When VCN PG state is unchanged, it is unnecessary to reset >>>>>> power gate state again. >>>>> Don't you need to initialize and store the PG state somewhere? You >>>>> are just using a local variable here. >>>>> >>>>> Alex >>>>> >>>> Hi Alex, >>>> >>>> I used */static/* for this local state variable(*cur_state*) with >>>> initialization state AMD_PG_STATE_GATE. >>> >>> That won't work correctly. A "static" variable is global, but the >>> power state is per device. >>> >>> Regards, >>> Christian. >>> >> This "static" variable under local scope is mainly for VCN used only. >> It only tracks VCN's PG state. >> (currently VCN only have one hardware instance) > > Still an absolute no-go for kernel coding. VCN will soon be used for > dGPUs as well. > > Please fix and reiterate, > Christian. I see, then I will put this current state track into struct amdgpu_vcn. By the way, under multiple dPGU situation, I though per dPGU will create it's own driver instance. this static variable won't be shared cross driver instance. Am I right? Thanks! James Zhu >> >> Best Regards! >> James Zhu >>>> this variable's scope is only inside this function, but it's >>>> initialization is done >>>> once at compile time and it's lifetime will last until the driver exit. >>>> >>>> Since it is only used inside this function, I didn't put it into >>>> struct amdgpu_vcn. >>>> >>>> Best Regards! >>>> James Zhu >>>>>> --- >>>>>> drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 13 +++++++++++-- >>>>>> 1 file changed, 11 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c >>>>>> index 2664bb2..86d98d2 100644 >>>>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c >>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c >>>>>> @@ -1633,12 +1633,21 @@ 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; >>>>>> + static enum amd_powergating_state cur_state = AMD_PG_STATE_GATE; >>>>>> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >>>>>> >>>>>> + if (state == 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) >>>>>> + 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 >>>> >>>> >>>> >>>> _______________________________________________ >>>> amd-gfx mailing list >>>> amd-gfx at lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>> >> >> >> >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180911/37d7b5a7/attachment-0001.html>