[PATCH 4/4] drm/amdgpu/vcn: use inst_idx relacing inst

[Leo Liu]

On 2020-01-21 3:58 p.m., James Zhu wrote:
> Actually RREG32_SOC15_DPG_MODE_2_0 has a bug inside to miss using inst 
> as inst_idx,

If there is a bug, you should clearly specify the bug in the commit 
message and fix it in a patch, and then if you think the code-name 
conversion need to be fixed, you should have that in a separated patch.

Leo


>
> So I want to clean up the header first to use inst_idx for 
> abbreviation of instance index.
>
> James
>
> On 2020-01-21 3:30 p.m., Leo Liu wrote:
>>
>> On 2020-01-21 12:50 p.m., James Zhu wrote:
>>>
>>> On 2020-01-21 12:43 p.m., Leo Liu wrote:
>>>>
>>>> On 2020-01-21 11:19 a.m., James Zhu wrote:
>>>>> Use inst_idx relacing inst in SOC15_DPG_MODE macro
>>>>>
>>>>> Signed-off-by: James Zhu <James.Zhu at amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 22 +++++++++++-----------
>>>>>   1 file changed, 11 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>> index 60fe3c4..98c1893 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>> @@ -65,23 +65,23 @@
>>>>>   /* 1 second timeout */
>>>>>   #define VCN_IDLE_TIMEOUT    msecs_to_jiffies(1000)
>>>>>   -#define RREG32_SOC15_DPG_MODE(ip, inst, reg, mask, 
>>>>> sram_sel)                 \
>>>>> -    ({    WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_MASK, 
>>>>> mask);             \
>>>>> -        WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_CTL,                 \
>>>>> +#define RREG32_SOC15_DPG_MODE(ip, inst_idx, reg, mask, 
>>>>> sram_sel)             \
>>>>> +    ({    WREG32_SOC15(ip, inst_idx, mmUVD_DPG_LMA_MASK, 
>>>>> mask);             \
>>>>> +        WREG32_SOC15(ip, inst_idx, 
>>>>> mmUVD_DPG_LMA_CTL,                 \
>>>>
>>>> I have only seen you are using inst_idx to replace inst, havn't 
>>>> you? this is not necessary, because we are using inst as the idx.
>>>
>>> Since we use inst for instance in struct amdgpu_vcn. To avoid 
>>> confusing, I create this patch,
>>
>> If only variable name changes, please drop these patch, we are using 
>> inst all the places.
>>
>> Leo
>>
>>
>>
>>
>>>
>>> James
>>>
>>>>
>>>> Leo
>>>>
>>>>
>>>>
>>>>> UVD_DPG_LMA_CTL__MASK_EN_MASK |                 \
>>>>> - ((adev->reg_offset[ip##_HWIP][inst][reg##_BASE_IDX] + reg)     \
>>>>> + ((adev->reg_offset[ip##_HWIP][inst_idx][reg##_BASE_IDX] + 
>>>>> reg)     \
>>>>>               << UVD_DPG_LMA_CTL__READ_WRITE_ADDR__SHIFT) 
>>>>> |             \
>>>>>               (sram_sel << 
>>>>> UVD_DPG_LMA_CTL__SRAM_SEL__SHIFT));         \
>>>>> -        RREG32_SOC15(ip, inst, 
>>>>> mmUVD_DPG_LMA_DATA);                 \
>>>>> +        RREG32_SOC15(ip, inst_idx, 
>>>>> mmUVD_DPG_LMA_DATA);             \
>>>>>       })
>>>>>   -#define WREG32_SOC15_DPG_MODE(ip, inst, reg, value, mask, 
>>>>> sram_sel)             \
>>>>> +#define WREG32_SOC15_DPG_MODE(ip, inst_idx, reg, value, mask, 
>>>>> sram_sel)         \
>>>>>       do {                                         \
>>>>> -        WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_DATA, 
>>>>> value);             \
>>>>> -        WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_MASK, 
>>>>> mask);             \
>>>>> -        WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_CTL,                 \
>>>>> +        WREG32_SOC15(ip, inst_idx, mmUVD_DPG_LMA_DATA, 
>>>>> value);             \
>>>>> +        WREG32_SOC15(ip, inst_idx, mmUVD_DPG_LMA_MASK, 
>>>>> mask);             \
>>>>> +        WREG32_SOC15(ip, inst_idx, 
>>>>> mmUVD_DPG_LMA_CTL,                 \
>>>>>               UVD_DPG_LMA_CTL__READ_WRITE_MASK | \
>>>>> - ((adev->reg_offset[ip##_HWIP][inst][reg##_BASE_IDX] + reg)     \
>>>>> + ((adev->reg_offset[ip##_HWIP][inst_idx][reg##_BASE_IDX] + 
>>>>> reg)     \
>>>>>               << UVD_DPG_LMA_CTL__READ_WRITE_ADDR__SHIFT) 
>>>>> |             \
>>>>>               (sram_sel << 
>>>>> UVD_DPG_LMA_CTL__SRAM_SEL__SHIFT));         \
>>>>>       } while (0)
>>>>> @@ -111,7 +111,7 @@
>>>>>     #define RREG32_SOC15_DPG_MODE_2_0(inst_idx, offset, 
>>>>> mask_en)                     \
>>>>>       ({                                            \
>>>>> -        WREG32_SOC15(VCN, inst, 
>>>>> mmUVD_DPG_LMA_CTL,                     \
>>>>> +        WREG32_SOC15(VCN, inst_idx, 
>>>>> mmUVD_DPG_LMA_CTL,                     \
>>>>>               (0x0 << UVD_DPG_LMA_CTL__READ_WRITE__SHIFT 
>>>>> |                \
>>>>>               mask_en << UVD_DPG_LMA_CTL__MASK_EN__SHIFT 
>>>>> |                \
>>>>>               offset << 
>>>>> UVD_DPG_LMA_CTL__READ_WRITE_ADDR__SHIFT));            \