On 8/26/2021 6:06 PM, Sharma, Shashank wrote:
On 8/26/2021 6:04 PM, Christian König wrote:
Am 26.08.21 um 14:31 schrieb Sharma, Shashank:
On 8/26/2021 5:54 PM, Christian König wrote:
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@xxxxxxx>
---
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.
Honestly, when I just see case '0', its a magic number for me, and
is making code less readable, harder for review, and even harder to
debug. RING_0 tells me that we are mapping a ring to a priority, and
clarifies the intention.
Well we should probably rename the variable then, e.g. like ring_idx
or just ring.
A switch on the variable named "ring" with a value of 0 has the same
meaning than RING_0, it's just not so much code to maintain.
Christian.
Perfect, sounds as good as anything.
- Shashank
I'll take care in v2.
regards,
Satyajit
- Shashank
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
