amdgpu: Use the dynamic IP based offset for register access for SOC15

Christian König Thu, 30 Nov 2017 00:34:32 -0800

The reason to use define here is the original function didn't take the adev as 
parameter.

Yeah, I've seen that and this is exactly the reason why I want to avoidusing a macro here :)

If you don't want to change every caller then at least make the macroname capital, so that we can see in the code that this isn't a function.


Regards,
Christian.

Am 29.11.2017 um 21:38 schrieb Liu, Shaoyun:

Thanks for the review .  The reason to use define here is the original function 
didn't take the adev as parameter .  If I change it here , all the caller of 
this function need to be changed . If you think that's  proper way  I can 
change it.  But do you think it's  better to put it in another change so this 
patch  will looks much more cleaner .

Regards
Shaoyun.liu

-----Original Message-----
From: Christian König [mailto:[email protected]]
Sent: Wednesday, November 29, 2017 3:05 PM
To: Liu, Shaoyun; [email protected]
Subject: Re: [PATCH 2/5] drm/amdgpu: Use the dynamic IP based offset for 
register access for SOC15

Am 29.11.2017 um 20:09 schrieb Shaoyun Liu:

Change-Id: I29f33ee3b4bbd6737f3426385a9e8452fb528a67
Signed-off-by: Shaoyun Liu <[email protected]>
---
   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c    | 21 +++----------------
   drivers/gpu/drm/amd/amdgpu/soc15_common.h | 34 
++++++++-----------------------
   2 files changed, 11 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 4c55f21..e324c66 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -107,24 +107,9 @@
        SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_GB_ADDR_CONFIG_READ), 0x0018773f, 
0x00000002
   };

-static u32 sdma_v4_0_get_reg_offset(u32 instance, u32

internal_offset) -{
-       u32 base = 0;
-
-       switch (instance) {
-       case 0:
-               base = SDMA0_BASE.instance[0].segment[0];
-               break;
-       case 1:
-               base = SDMA1_BASE.instance[0].segment[0];
-               break;
-       default:
-               BUG();
-               break;
-       }
-
-       return base + internal_offset;
-}
+#define sdma_v4_0_get_reg_offset(inst, offset) ( 0 == inst ? \
+       (adev->reg_offset[SDMA0_HWIP][0][0] + offset) : \
+       (adev->reg_offset[SDMA1_HWIP][0][0] + offset))

Please keep that a function, not a define.

Apart from that looks really good to me.

Christian.

static void sdma_v4_0_init_golden_registers(struct amdgpu_device *adev)

   {
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15_common.h
b/drivers/gpu/drm/amd/amdgpu/soc15_common.h
index 7a8e4e28..62a6e21 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15_common.h
+++ b/drivers/gpu/drm/amd/amdgpu/soc15_common.h
@@ -54,42 +54,24 @@ struct nbio_pcie_index_data {

(ip##_BASE__INST##inst##_SEG4 + reg)))))#define WREG32_FIELD15(ip, idx, reg, field, val) \

-       WREG32(SOC15_REG_OFFSET(ip, idx, mm##reg), (RREG32(SOC15_REG_OFFSET(ip, idx, 
mm##reg)) & ~REG_FIELD_MASK(reg, field)) | (val) << REG_FIELD_SHIFT(reg, field))
+       WREG32(adev->reg_offset[ip##_HWIP][idx][mm##reg##_BASE_IDX] + mm##reg,  
     \
+       (RREG32(adev->reg_offset[ip##_HWIP][idx][mm##reg##_BASE_IDX] + mm##reg) 
     \
+       & ~REG_FIELD_MASK(reg, field)) | (val) << REG_FIELD_SHIFT(reg,
+field))

#define RREG32_SOC15(ip, inst, reg) \

-       RREG32( (0 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG0 + reg : \
-               (1 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG1 + reg : \
-               (2 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG2 + reg : \
-               (3 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG3 + reg : \
-               (ip##_BASE__INST##inst##_SEG4 + reg))))))
+       RREG32(adev->reg_offset[ip##_HWIP][inst][reg##_BASE_IDX] + reg)

#define RREG32_SOC15_OFFSET(ip, inst, reg, offset) \

-       RREG32( (0 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG0 + reg : \
-               (1 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG1 + reg : \
-               (2 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG2 + reg : \
-               (3 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG3 + reg : \
-               (ip##_BASE__INST##inst##_SEG4 + reg))))) + offset)
+       RREG32((adev->reg_offset[ip##_HWIP][inst][reg##_BASE_IDX] + reg) +
+offset)

#define WREG32_SOC15(ip, inst, reg, value) \

-       WREG32( (0 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG0 + reg : \
-               (1 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG1 + reg : \
-               (2 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG2 + reg : \
-               (3 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG3 + reg : \
-               (ip##_BASE__INST##inst##_SEG4 + reg))))), value)
+       WREG32((adev->reg_offset[ip##_HWIP][inst][reg##_BASE_IDX] + reg),
+value)

#define WREG32_SOC15_NO_KIQ(ip, inst, reg, value) \

-       WREG32_NO_KIQ( (0 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG0 + 
reg : \
-               (1 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG1 + reg : \
-               (2 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG2 + reg : \
-               (3 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG3 + reg : \
-               (ip##_BASE__INST##inst##_SEG4 + reg))))), value)
+       WREG32_NO_KIQ((adev->reg_offset[ip##_HWIP][inst][reg##_BASE_IDX] +
+reg), value)

#define WREG32_SOC15_OFFSET(ip, inst, reg, offset, value) \

-       WREG32( (0 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG0 + reg : \
-               (1 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG1 + reg : \
-               (2 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG2 + reg : \
-               (3 == reg##_BASE_IDX ? ip##_BASE__INST##inst##_SEG3 + reg : \
-               (ip##_BASE__INST##inst##_SEG4 + reg))))) + offset, value)
+       WREG32((adev->reg_offset[ip##_HWIP][inst][reg##_BASE_IDX] + reg) +
+offset, value)

#endif

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/5] drm/amdgpu: Use the dynamic IP based offset for register access for SOC15

Reply via email to