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

[James Zhu]

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>  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.
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?

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
>>>>>> 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/37d7b5a7/attachment-0001.html>