On Tue, Sep 11, 2018 at 12:20 PM James Zhu <jam...@amd.com> wrote:
>
> 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?
Well, it will may save some unnecessary programming in some cases, but
I think we track it pretty well already for manual PG.
Alex
>
> 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....@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 <jzh...@gmail.com> wrote:
>
> Signed-off-by: James Zhu <james....@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
