[PATCH] drm/amdgpu:No action needs when VCN PG state is unchanged

[Christian König]

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.

>
> 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/fa2defa3/attachment.html>