[PATCH 2/5] drm/amdgpu/vcn:set vcn encode ring priority level

[Christian König]

Am 26.08.21 um 13:32 schrieb Sharma, Shashank:
> Hi Satyajit,
>
> On 8/26/2021 12:43 PM, Satyajit Sahu wrote:
>> There are multiple rings available in VCN encode. Map each ring
>> to different priority.
>>
>> Signed-off-by: Satyajit Sahu <satyajit.sahu at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 14 ++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  9 +++++++++
>>   2 files changed, 23 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> index 6780df0fb265..ce40e7a3ce05 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> @@ -951,3 +951,17 @@ int amdgpu_vcn_enc_ring_test_ib(struct 
>> amdgpu_ring *ring, long timeout)
>>         return r;
>>   }
>> +
>> +enum vcn_enc_ring_priority amdgpu_vcn_get_enc_ring_prio(int index)
>> +{
>> +    switch(index) {
>> +    case 0:
>
> As discussed in the previous patches, its far better to have MACROS or 
> enums instead of having 0/1/2 cases. As a matter of fact, we can 
> always call it RING_0 RING_1 and so on.

I strongly disagree. Adding macros or enums just to have names for the 
numbered rings doesn't gives you any advantage at all. That's just extra 
loc.

We could use the ring pointers to identify a ring instead, but using the 
switch here which is then used inside the init loop is perfectly fine.

Regards,
Christian.

>
>
> If this is being done just for the traditional reasons, we can have a 
> separate patch to replace it across the driver as well.
>
> - Shashank
>
>
>> +        return AMDGPU_VCN_ENC_PRIO_NORMAL;
>> +    case 1:
>> +        return AMDGPU_VCN_ENC_PRIO_HIGH;
>> +    case 2:
>> +        return AMDGPU_VCN_ENC_PRIO_VERY_HIGH;
>> +    default:
>> +        return AMDGPU_VCN_ENC_PRIO_NORMAL;
>> +    }
>> +}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> index d74c62b49795..938ee73dfbfc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> @@ -290,6 +290,13 @@ enum vcn_ring_type {
>>       VCN_UNIFIED_RING,
>>   };
>>   +enum vcn_enc_ring_priority {
>> +    AMDGPU_VCN_ENC_PRIO_NORMAL = 1,
>> +    AMDGPU_VCN_ENC_PRIO_HIGH,
>> +    AMDGPU_VCN_ENC_PRIO_VERY_HIGH,
>> +    AMDGPU_VCN_ENC_PRIO_MAX
>> +};
>> +
>>   int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
>>   int amdgpu_vcn_sw_fini(struct amdgpu_device *adev);
>>   int amdgpu_vcn_suspend(struct amdgpu_device *adev);
>> @@ -308,4 +315,6 @@ int amdgpu_vcn_dec_sw_ring_test_ib(struct 
>> amdgpu_ring *ring, long timeout);
>>   int amdgpu_vcn_enc_ring_test_ring(struct amdgpu_ring *ring);
>>   int amdgpu_vcn_enc_ring_test_ib(struct amdgpu_ring *ring, long 
>> timeout);
>>   +enum vcn_enc_ring_priority amdgpu_vcn_get_enc_ring_prio(int index);
>> +
>>   #endif
>>