Hi Christian, Thanks! I add this check for VCN DPG/DPG PAUSE mode implementation. Do you think it is necessary to add for other IP blocks if they need or just for VCN only? Best Regards! James Zhu On 2018-09-11 12:14 PM, Koenig, Christian wrote: > Hi James, > > Am 11.09.2018 17:44 schrieb "Zhu, James" <James.Zhu at amd.com>: > > > > 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> <mailto:jzhums at gmail.com> wrote: > > Signed-off-by: James Zhu<James.Zhu at amd.com> <mailto: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? > > > No that is not correct. > > Static variables both on function as well as module level are shared > between all amdgpu devices. > > Regards, > Christian. > > > 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 > <mailto:amd-gfx at lists.freedesktop.org> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > > > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > <mailto:amd-gfx at lists.freedesktop.org> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > > > > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > <mailto: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/eccdb540/attachment.html>
