[PATCH v2 1/1] drm/amdgpu: detach ring priority from gfx priority

[Lazar, Lijo]

On 8/26/2021 4:21 PM, Das, Nirmoy wrote:
> 
> On 8/26/2021 12:48 PM, Christian König wrote:
>>
>>
>> Am 26.08.21 um 12:08 schrieb Nirmoy Das:
>>> Currently AMDGPU_RING_PRIO_MAX is redefinition of a
>>> max gfx hwip priority, this won't work well when we will
>>> have a hwip with different set of priorities than gfx.
>>> Also, HW ring priorities are different from ring priorities.
>>>
>>> Create a global enum for ring priority levels which each
>>> HWIP can use to define its own priority levels.
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das at amd.com>
>>> Reviewed-by: Lijo Lazar <lijo.lazar at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h  | 5 ++---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 +++++++--
>>>   2 files changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> index d43fe2ed8116..7f747a4291f3 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>>> @@ -43,9 +43,8 @@
>>>   #define AMDGPU_MAX_COMPUTE_QUEUES KGD_MAX_QUEUES
>>>
>>>   enum gfx_pipe_priority {
>>
>> While at it can you add an amdgpu_ prefix before the enum name?
>>
>> And if the enum isn't really used maybe even replace the enum with 
>> defines?
> 
> 
> Yes makes sense, I will resend with defines.
> 

Recommend against that so that ctx_to_ip priority returns a typed enum 
for that IP instead of something mapped randomly.

Thanks,
Lijo

>>
>> Thanks,
>> Christian.
>>
>>> -    AMDGPU_GFX_PIPE_PRIO_NORMAL = 1,
>>> -    AMDGPU_GFX_PIPE_PRIO_HIGH,
>>> -    AMDGPU_GFX_PIPE_PRIO_MAX
>>> +    AMDGPU_GFX_PIPE_PRIO_NORMAL = AMDGPU_RING_PRIO_1,
>>> +    AMDGPU_GFX_PIPE_PRIO_HIGH = AMDGPU_RING_PRIO_2
>>>   };
>>>
>>>   /* Argument for PPSMC_MSG_GpuChangeState */
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> index e713d31619fe..88d80eb3fea1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> @@ -36,8 +36,13 @@
>>>   #define AMDGPU_MAX_VCE_RINGS        3
>>>   #define AMDGPU_MAX_UVD_ENC_RINGS    2
>>>
>>> -#define AMDGPU_RING_PRIO_DEFAULT    1
>>> -#define AMDGPU_RING_PRIO_MAX        AMDGPU_GFX_PIPE_PRIO_MAX
>>> +enum amdgpu_ring_priority_level {
>>> +    AMDGPU_RING_PRIO_0,
>>> +    AMDGPU_RING_PRIO_1,
>>> +    AMDGPU_RING_PRIO_DEFAULT = 1,
>>> +    AMDGPU_RING_PRIO_2,
>>> +    AMDGPU_RING_PRIO_MAX
>>> +};
>>>
>>>   /* some special values for the owner field */
>>>   #define AMDGPU_FENCE_OWNER_UNDEFINED    ((void *)0ul)
>>> -- 
>>> 2.32.0
>>>
>>