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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2018-09-21 10:05 AM, Zhu, Rex wrote:
> In my understand, when  dpg mode enabled,  we don't need to power up/down VCN via SMU and
> Also don't need to set vcn power gate/ungate state.
>
> Just need to enable the dpg mode.
>
> If not true, please correct me.
You are right.  Under DPG mode, vcn_v1_0_start is used to setup DPG mode 
instead of ungating power.
For code reuse purpose, the function/variable name may have different 
connotation under different mode.

James
>
> Best Regards
> Rex
>
>
>
>
>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
>> Christian König
>> Sent: Friday, September 21, 2018 8:53 PM
>> To: Zhu, James <James.Zhu at amd.com>; Alex Deucher
>> <alexdeucher at gmail.com>
>> Cc: James Zhu <jzhums at gmail.com>; Zhu, James <James.Zhu at amd.com>;
>> amd-gfx list <amd-gfx at lists.freedesktop.org>
>> Subject: Re: [PATCH v2] drm/amdgpu:No action when VCN PG state is
>> unchanged
>>
>> Am 21.09.2018 um 14:28 schrieb James Zhu:
>>>
>>> On 2018-09-20 01:54 PM, Christian König wrote:
>>>> Am 20.09.2018 um 18:24 schrieb James Zhu:
>>>>>
>>>>> On 2018-09-20 11:49 AM, Alex Deucher wrote:
>>>>>> On Thu, Sep 20, 2018 at 11:27 AM James Zhu <jamesz at amd.com>
>> wrote:
>>>>>>>
>>>>>>> On 2018-09-20 11:14 AM, Alex Deucher wrote:
>>>>>>>> On Thu, Sep 13, 2018 at 4:56 PM James Zhu <jzhums at gmail.com>
>> wrote:
>>>>>>>>> When VCN PG state is unchanged, it is unnecessary to reset power
>>>>>>>>> gate state
>>>>>>>>>
>>>>>>>>> Signed-off-by: James Zhu <James.Zhu at amd.com>
>>>>>>>>> ---
>>>>>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 +
>>>>>>>>>     drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   | 12 ++++++++++--
>>>>>>>>>     2 files changed, 11 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>>>>> index 0b0b863..d2219ab 100644
>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>>>>> @@ -69,6 +69,7 @@ struct amdgpu_vcn {
>>>>>>>>>            struct amdgpu_ring      ring_jpeg;
>>>>>>>>>            struct amdgpu_irq_src   irq;
>>>>>>>>>            unsigned                num_enc_rings;
>>>>>>>>> +       enum amd_powergating_state cur_state;
>>>>>>>> Does the default value (0) at init time properly reflect the
>>>>>>>> default powergating state?  If so,
>>>>>>>> Acked-by: Alex Deucher <alexander.deucher at amd.com>
>>>>>>> Yes, the below code shows it will be set to 0 during driver load
>>>>>>> stage.
>>>>>> Yes, I understand that.  Is 0 (AMD_PG_STATE_GATE) what we want as
>>>>>> the default though?  The first time the code runs are we going to
>>>>>> do the right thing or is the code going to return early?  IIRC, the
>>>>>> hw default is ungated.
>>>>> cur_state is used for tracking driver SW PG state, not HW  PG state.
>>>>> I though no matter what HW  PG state is after device powers up, when
>>>>> first vcn ring is scheduled to run,
>>>>> begin_use->set_powergating_state->vcn_v1_0_start->ungate
>>>>> power/clock->Boot_VCPU  will be tried.
>>>>>
>>>>> For DPG mode, the ungate power/clock , boot VCPU will not actually
>>>>> be activated during start setup stage, and only be activated during
>>>>> ring run stage.
>>>> Mhm, I wonder if it wouldn't be better to have that functionality one
>>>> layer up.
>>>>
>>>> E.g. in amdgpu_device_ip_set_powergating_state so that it applies to
>>>> all IP blocks in the same way.
>>> If necessary, I can add support for IP blocks later.
>> Yeah, agree. In general I'm not doing much with power management, so
>> can't judge if that is a good idea or not.
>>
>> Anyway feel free to add my Acked-by to the patch.
>>
>> Regards,
>> Christian.
>>
>>> James
>>>> But on the other hand the correct solution looks good to me as well,
>>>> so feel free to add my Acked-by as well.
>>>>
>>>> Christian.
>>>>
>>>>> James
>>>>>> Alex
>>>>>>> int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long
>>>>>>> flags)
>>>>>>> ....
>>>>>>>        adev = kzalloc(sizeof(struct amdgpu_device), GFP_KERNEL);
>>>>>>>
>>>>>>> struct amdgpu_device {
>>>>>>> ....
>>>>>>>        struct amdgpu_vcn        vcn;
>>>>>>>
>>>>>>> Best Regards!
>>>>>>> James zhu
>>>>>>>>>     };
>>>>>>>>>
>>>>>>>>>     int amdgpu_vcn_sw_init(struct amdgpu_device *adev); diff
>>>>>>>>> --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>>>>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>>>>>> index 2664bb2..2cde0b4 100644
>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>>>>>>> @@ -1633,12 +1633,20 @@ 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;
>>>>>>>>>            struct amdgpu_device *adev = (struct amdgpu_device
>>>>>>>>> *)handle;
>>>>>>>>>
>>>>>>>>> +       if(state == adev->vcn.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)
>>>>>>>>> +               adev->vcn.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



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux