<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">Am 11.09.2018 um 17:29 schrieb James Zhu: </div> <blockquote type="cite" cite="mid:62baa235-e113-3635-6060-f5ffcc14e967@amd.com"> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <div class="moz-cite-prefix">On 2018-09-11 11:17 AM, Christian König wrote: </div> <blockquote type="cite" cite="mid:68b30f71-86eb-89a0-0f8d-fcfb18e89cc2@gmail.com"> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <div class="moz-cite-prefix">Am 11.09.2018 um 17:06 schrieb James Zhu: </div> <blockquote type="cite" cite="mid:d358b2be-26d1-627a-7105-5d90685d0f3b@amd.com"> <div class="moz-cite-prefix">On 2018-09-11 10:44 AM, Alex Deucher wrote: </div> <blockquote type="cite" cite="mid:CADnq5_P5Oj_Fdxhv2n8ZxDTm1mV_NQo6M1kmgGzLV0fHhhR8sA@mail.gmail.com"> <pre wrap="">On Mon, Sep 10, 2018 at 4:34 PM James Zhu <a class="moz-txt-link-rfc2396E" href="mailto:jzhums@gmail.com" moz-do-not-send="true"><jzhums@gmail.com></a> wrote: </pre> <blockquote type="cite"> <pre wrap="">Signed-off-by: James Zhu <a class="moz-txt-link-rfc2396E" href="mailto:James.Zhu@amd.com" moz-do-not-send="true"><James.Zhu@amd.com></a> When VCN PG state is unchanged, it is unnecessary to reset power gate state again. </pre> </blockquote> <pre wrap="">Don't you need to initialize and store the PG state somewhere? You are just using a local variable here. Alex </pre> </blockquote> Hi Alex, I used static for this local state variable(cur_state) with initialization state AMD_PG_STATE_GATE. </blockquote> That won't work correctly. A "static" variable is global, but the power state is per device. Regards, Christian. </blockquote> 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) </blockquote> Still an absolute no-go for kernel coding. VCN will soon be used for dGPUs as well. Please fix and reiterate, Christian. <blockquote type="cite" cite="mid:62baa235-e113-3635-6060-f5ffcc14e967@amd.com"> Best Regards! James Zhu <blockquote type="cite" cite="mid:68b30f71-86eb-89a0-0f8d-fcfb18e89cc2@gmail.com"> <blockquote type="cite" cite="mid:d358b2be-26d1-627a-7105-5d90685d0f3b@amd.com"> 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 <blockquote type="cite" cite="mid:CADnq5_P5Oj_Fdxhv2n8ZxDTm1mV_NQo6M1kmgGzLV0fHhhR8sA@mail.gmail.com"> <blockquote type="cite"> <pre wrap="">--- 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 <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org" moz-do-not-send="true">amd-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" moz-do-not-send="true">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </pre> </blockquote> </blockquote> <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ amd-gfx mailing list <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org" moz-do-not-send="true">amd-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" moz-do-not-send="true">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </pre> </blockquote> </blockquote> <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ amd-gfx mailing list <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </pre> </blockquote> </body> </html>