[PATCH 2/4] drm/amdgpu/vcn2.5: fix a bug for the 2nd vcn instance

[James Zhu]

It is not for DPG mode, but for SPG mode.

Just want to reuse the SPG mode instance harvest check here.

James

On 2020-01-21 3:20 p.m., Leo Liu wrote:
>
> On 2020-01-21 12:47 p.m., James Zhu wrote:
>>
>> On 2020-01-21 12:29 p.m., Leo Liu wrote:
>>>
>>> On 2020-01-21 11:19 a.m., James Zhu wrote:
>>>> Fix a bug for the 2nd vcn instance at start and stop.
>>>>
>>>> Signed-off-by: James Zhu <James.Zhu at amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 12 ++++++++----
>>>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>>>> index c351d1a..740a291 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>>>> @@ -891,8 +891,10 @@ static int vcn_v2_5_start(struct amdgpu_device 
>>>> *adev)
>>>>       for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>>>>           if (adev->vcn.harvest_config & (1 << i))
>>>>               continue;
>>>> -        if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG)
>>>> -            return vcn_v2_5_start_dpg_mode(adev, i, 
>>>> adev->vcn.indirect_sram);
>>>> +        if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
>>>> +            r = vcn_v2_5_start_dpg_mode(adev, i, 
>>>> adev->vcn.indirect_sram);
>>>> +            continue;
>>>> +        }
>>>
>>> "r" is not being considered, and after the loop, it will be going to 
>>> the code below, is it correct?
>> Since DPG mode start/stop always return 0. I have added code to 
>> return 0 below under DPG mode
>
> Then you should move the "return 0" here instead of adding two more 
> unnecessary lines.
>
>
>>>
>>>
>>>>           /* disable register anti-hang mechanism */
>>>>           WREG32_P(SOC15_REG_OFFSET(UVD, i, mmUVD_POWER_STATUS), 0,
>>>> @@ -903,6 +905,9 @@ static int vcn_v2_5_start(struct amdgpu_device 
>>>> *adev)
>>>>           WREG32_SOC15(UVD, i, mmUVD_STATUS, tmp);
>>>>       }
>>>>   +    if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG)
>>>> +        return 0;
>>>> +
>>>>       /*SW clock gating */
>>>>       vcn_v2_5_disable_clock_gating(adev);
>>>>   @@ -1294,10 +1299,9 @@ static int vcn_v2_5_stop(struct 
>>>> amdgpu_device *adev)
>>>>       for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>>>>           if (adev->vcn.harvest_config & (1 << i))
>>>>               continue;
>>>> -
>>>>           if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
>>>>               r = vcn_v2_5_stop_dpg_mode(adev, i);
>>>> -            goto power_off;
>>>> +            continue;
>>>>           }
>>>
>>> same problem as above, don't go through the code that isn't necessary.
>>
>> should be fine under DPG mode.
>
> It's about clean implementation. if not necessary, why do we need to 
> add them.
>
> Leo
>
>
>
>>
>> JAmes
>>
>>>
>>> Regards,
>>>
>>> Leo
>>>
>>>
>>>>             /* wait for vcn idle */