<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div dir="auto">
<div>Hi James,<br>
<div class="gmail_extra"><br>
<div class="gmail_quote">Am 11.09.2018 17:44 schrieb "Zhu, James" <James.Zhu@amd.com>:<br type="attribution">
<blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<p><br>
</p>
<br>
<div>On 2018-09-11 11:36 AM, Christian König wrote:<br>
</div>
<blockquote>
<div>Am 11.09.2018 um 17:29 schrieb James Zhu:<br>
</div>
<blockquote>
<p><br>
</p>
<br>
<div>On 2018-09-11 11:17 AM, Christian König wrote:<br>
</div>
<blockquote>
<div>Am 11.09.2018 um 17:06 schrieb James Zhu:<br>
</div>
<blockquote>
<p><br>
</p>
<br>
<div>On 2018-09-11 10:44 AM, Alex Deucher wrote:<br>
</div>
<blockquote>
<pre>On Mon, Sep 10, 2018 at 4:34 PM James Zhu <a href="mailto:jzhums@gmail.com"><jzhums@gmail.com></a> wrote:
</pre>
<blockquote>
<pre>Signed-off-by: James Zhu <a href="mailto:James.Zhu@amd.com"><James.Zhu@amd.com></a>
When VCN PG state is unchanged, it is unnecessary to reset
power gate state again.
</pre>
</blockquote>
<pre>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,<br>
<br>
I used <b><i>static</i></b> for this local state variable(<b>cur_state</b>) with initialization state AMD_PG_STATE_GATE.<br>
</blockquote>
<br>
That won't work correctly. A "static" variable is global, but the power state is per device.<br>
<br>
Regards,<br>
Christian.<br>
<br>
</blockquote>
This "static" variable under local scope is mainly for VCN used only. It only tracks VCN's PG state.<br>
(currently VCN only have one hardware instance)<br>
</blockquote>
<br>
Still an absolute no-go for kernel coding. VCN will soon be used for dGPUs as well.<br>
<br>
Please fix and reiterate,<br>
Christian.<br>
</blockquote>
I see, then I will put this current state track into struct amdgpu_vcn.<br>
<br>
By the way, under multiple dPGU situation, I though per dPGU will create it's own driver instance.<br>
this static variable won't be shared cross driver instance. Am I right?<br>
</div>
</blockquote>
</div>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">No that is not correct.</div>
<div dir="auto"><br>
</div>
<div dir="auto">Static variables both on function as well as module level are shared between all amdgpu devices.</div>
<div dir="auto"><br>
</div>
<div dir="auto">Regards,</div>
<div dir="auto">Christian.</div>
<div dir="auto"><br>
</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
Thanks!<br>
James Zhu<br>
<blockquote>
<blockquote><br>
Best Regards!<br>
James Zhu<br>
<blockquote>
<blockquote>this variable's scope is only inside this function, but it's initialization is done
<br>
once at compile time and it's lifetime will last until the driver exit.<br>
<br>
Since it is only used inside this function, I didn't put it into struct amdgpu_vcn.<br>
<br>
Best Regards!<br>
James Zhu<br>
<blockquote>
<blockquote>
<pre>---
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 href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a>
</pre>
</blockquote>
</blockquote>
<br>
<br>
<fieldset></fieldset> <br>
<pre>_______________________________________________
amd-gfx mailing list
<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a>
</pre>
</blockquote>
<br>
</blockquote>
<br>
<br>
<fieldset></fieldset> <br>
<pre>_______________________________________________
amd-gfx mailing list
<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a>
</pre>
</blockquote>
<br>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</body>
</html>