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

[Leo Liu]

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 */