* Various cleanups for gfx_v8_0.c
@ 2016-08-09 14:27 Tom St Denis
[not found] ` <20160809142739.1401-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Tom St Denis @ 2016-08-09 14:27 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
These patches perform various cleanups in the GFX v8 driver.
Patch #1, performs various whitespace/error message cleanups.
Patch #2, performs various simplifications to routines to make them
easier to read (and reduce LOC).
Patch #3, introduces a WREG32_FIELD() macro which is then used to
reduce various patterns of setting fields in mmio registers down
to a single line.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] drm/amd/amdgpu: Correct whitespace in GFX v8
[not found] ` <20160809142739.1401-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2016-08-09 14:27 ` Tom St Denis
2016-08-09 14:27 ` [PATCH 2/3] drm/amd/amdgpu: Simplify various gfx v8 functions Tom St Denis
2016-08-09 14:27 ` [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8 Tom St Denis
2 siblings, 0 replies; 8+ messages in thread
From: Tom St Denis @ 2016-08-09 14:27 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Tom St Denis
Fix various whitespace issues in gfx v8 driver.
Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
---
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 41 ++++++++++++++---------------------
1 file changed, 16 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 4645181faa41..8aa6b54f4516 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -1236,10 +1236,9 @@ static void gfx_v8_0_rlc_fini(struct amdgpu_device *adev)
if (adev->gfx.rlc.clear_state_obj) {
r = amdgpu_bo_reserve(adev->gfx.rlc.clear_state_obj, false);
if (unlikely(r != 0))
- dev_warn(adev->dev, "(%d) reserve RLC c bo failed\n", r);
+ dev_warn(adev->dev, "(%d) reserve RLC cbs bo failed\n", r);
amdgpu_bo_unpin(adev->gfx.rlc.clear_state_obj);
amdgpu_bo_unreserve(adev->gfx.rlc.clear_state_obj);
-
amdgpu_bo_unref(&adev->gfx.rlc.clear_state_obj);
adev->gfx.rlc.clear_state_obj = NULL;
}
@@ -1251,7 +1250,6 @@ static void gfx_v8_0_rlc_fini(struct amdgpu_device *adev)
dev_warn(adev->dev, "(%d) reserve RLC cp table bo failed\n", r);
amdgpu_bo_unpin(adev->gfx.rlc.cp_table_obj);
amdgpu_bo_unreserve(adev->gfx.rlc.cp_table_obj);
-
amdgpu_bo_unref(&adev->gfx.rlc.cp_table_obj);
adev->gfx.rlc.cp_table_obj = NULL;
}
@@ -1293,14 +1291,14 @@ static int gfx_v8_0_rlc_init(struct amdgpu_device *adev)
&adev->gfx.rlc.clear_state_gpu_addr);
if (r) {
amdgpu_bo_unreserve(adev->gfx.rlc.clear_state_obj);
- dev_warn(adev->dev, "(%d) pin RLC c bo failed\n", r);
+ dev_warn(adev->dev, "(%d) pin RLC cbs bo failed\n", r);
gfx_v8_0_rlc_fini(adev);
return r;
}
r = amdgpu_bo_kmap(adev->gfx.rlc.clear_state_obj, (void **)&adev->gfx.rlc.cs_ptr);
if (r) {
- dev_warn(adev->dev, "(%d) map RLC c bo failed\n", r);
+ dev_warn(adev->dev, "(%d) map RLC cbs bo failed\n", r);
gfx_v8_0_rlc_fini(adev);
return r;
}
@@ -1335,7 +1333,7 @@ static int gfx_v8_0_rlc_init(struct amdgpu_device *adev)
&adev->gfx.rlc.cp_table_gpu_addr);
if (r) {
amdgpu_bo_unreserve(adev->gfx.rlc.cp_table_obj);
- dev_warn(adev->dev, "(%d) pin RLC cp_table bo failed\n", r);
+ dev_warn(adev->dev, "(%d) pin RLC cp table bo failed\n", r);
return r;
}
r = amdgpu_bo_kmap(adev->gfx.rlc.cp_table_obj, (void **)&adev->gfx.rlc.cp_table_ptr);
@@ -1348,7 +1346,6 @@ static int gfx_v8_0_rlc_init(struct amdgpu_device *adev)
amdgpu_bo_kunmap(adev->gfx.rlc.cp_table_obj);
amdgpu_bo_unreserve(adev->gfx.rlc.cp_table_obj);
-
}
return 0;
@@ -1364,7 +1361,6 @@ static void gfx_v8_0_mec_fini(struct amdgpu_device *adev)
dev_warn(adev->dev, "(%d) reserve HPD EOP bo failed\n", r);
amdgpu_bo_unpin(adev->gfx.mec.hpd_eop_obj);
amdgpu_bo_unreserve(adev->gfx.mec.hpd_eop_obj);
-
amdgpu_bo_unref(&adev->gfx.mec.hpd_eop_obj);
adev->gfx.mec.hpd_eop_obj = NULL;
}
@@ -2127,9 +2123,7 @@ static int gfx_v8_0_sw_fini(void *handle)
amdgpu_ring_fini(&adev->gfx.compute_ring[i]);
gfx_v8_0_mec_fini(adev);
-
gfx_v8_0_rlc_fini(adev);
-
gfx_v8_0_free_microcode(adev);
return 0;
@@ -3585,7 +3579,6 @@ static void gfx_v8_0_gpu_init(struct amdgpu_device *adev)
WREG32(mmDMIF_ADDR_CALC, adev->gfx.config.gb_addr_config);
gfx_v8_0_tiling_mode_table_init(adev);
-
gfx_v8_0_setup_rb(adev);
gfx_v8_0_get_cu_info(adev);
@@ -3998,14 +3991,13 @@ static int gfx_v8_0_rlc_resume(struct amdgpu_device *adev)
/* disable CG */
WREG32(mmRLC_CGCG_CGLS_CTRL, 0);
if (adev->asic_type == CHIP_POLARIS11 ||
- adev->asic_type == CHIP_POLARIS10)
+ adev->asic_type == CHIP_POLARIS10)
WREG32(mmRLC_CGCG_CGLS_CTRL_3D, 0);
/* disable PG */
WREG32(mmRLC_PG_CNTL, 0);
gfx_v8_0_rlc_reset(adev);
-
gfx_v8_0_init_pg(adev);
if (!adev->pp_enabled) {
@@ -4980,7 +4972,6 @@ static int gfx_v8_0_hw_init(void *handle)
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
gfx_v8_0_init_golden_registers(adev);
-
gfx_v8_0_gpu_init(adev);
r = gfx_v8_0_rlc_resume(adev);
@@ -5552,15 +5543,15 @@ static void gfx_v8_0_send_serdes_cmd(struct amdgpu_device *adev,
data = RREG32(mmRLC_SERDES_WR_CTRL);
if (adev->asic_type == CHIP_STONEY)
- data &= ~(RLC_SERDES_WR_CTRL__WRITE_COMMAND_MASK |
- RLC_SERDES_WR_CTRL__READ_COMMAND_MASK |
- RLC_SERDES_WR_CTRL__P1_SELECT_MASK |
- RLC_SERDES_WR_CTRL__P2_SELECT_MASK |
- RLC_SERDES_WR_CTRL__RDDATA_RESET_MASK |
- RLC_SERDES_WR_CTRL__POWER_DOWN_MASK |
- RLC_SERDES_WR_CTRL__POWER_UP_MASK |
- RLC_SERDES_WR_CTRL__SHORT_FORMAT_MASK |
- RLC_SERDES_WR_CTRL__SRBM_OVERRIDE_MASK);
+ data &= ~(RLC_SERDES_WR_CTRL__WRITE_COMMAND_MASK |
+ RLC_SERDES_WR_CTRL__READ_COMMAND_MASK |
+ RLC_SERDES_WR_CTRL__P1_SELECT_MASK |
+ RLC_SERDES_WR_CTRL__P2_SELECT_MASK |
+ RLC_SERDES_WR_CTRL__RDDATA_RESET_MASK |
+ RLC_SERDES_WR_CTRL__POWER_DOWN_MASK |
+ RLC_SERDES_WR_CTRL__POWER_UP_MASK |
+ RLC_SERDES_WR_CTRL__SHORT_FORMAT_MASK |
+ RLC_SERDES_WR_CTRL__SRBM_OVERRIDE_MASK);
else
data &= ~(RLC_SERDES_WR_CTRL__WRITE_COMMAND_MASK |
RLC_SERDES_WR_CTRL__READ_COMMAND_MASK |
@@ -6093,9 +6084,9 @@ static void gfx_v8_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
amdgpu_ring_write(ring,
#ifdef __BIG_ENDIAN
- (2 << 0) |
+ (2 << 0) |
#endif
- (ib->gpu_addr & 0xFFFFFFFC));
+ (ib->gpu_addr & 0xFFFFFFFC));
amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr) & 0xFFFF);
amdgpu_ring_write(ring, control);
}
--
2.9.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] drm/amd/amdgpu: Simplify various gfx v8 functions
[not found] ` <20160809142739.1401-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-09 14:27 ` [PATCH 1/3] drm/amd/amdgpu: Correct whitespace in GFX v8 Tom St Denis
@ 2016-08-09 14:27 ` Tom St Denis
2016-08-09 14:27 ` [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8 Tom St Denis
2 siblings, 0 replies; 8+ messages in thread
From: Tom St Denis @ 2016-08-09 14:27 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Tom St Denis
Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
---
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 61 ++++++++++++-----------------------
1 file changed, 20 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 8aa6b54f4516..5f91a834aed2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -3459,19 +3459,16 @@ static void gfx_v8_0_select_se_sh(struct amdgpu_device *adev,
else
data = REG_SET_FIELD(0, GRBM_GFX_INDEX, INSTANCE_INDEX, instance);
- if ((se_num == 0xffffffff) && (sh_num == 0xffffffff)) {
- data = REG_SET_FIELD(data, GRBM_GFX_INDEX, SH_BROADCAST_WRITES, 1);
- data = REG_SET_FIELD(data, GRBM_GFX_INDEX, SE_BROADCAST_WRITES, 1);
- } else if (se_num == 0xffffffff) {
- data = REG_SET_FIELD(data, GRBM_GFX_INDEX, SH_INDEX, sh_num);
+ if (se_num == 0xffffffff)
data = REG_SET_FIELD(data, GRBM_GFX_INDEX, SE_BROADCAST_WRITES, 1);
- } else if (sh_num == 0xffffffff) {
- data = REG_SET_FIELD(data, GRBM_GFX_INDEX, SH_BROADCAST_WRITES, 1);
+ else
data = REG_SET_FIELD(data, GRBM_GFX_INDEX, SE_INDEX, se_num);
- } else {
+
+ if (sh_num == 0xffffffff)
+ data = REG_SET_FIELD(data, GRBM_GFX_INDEX, SH_BROADCAST_WRITES, 1);
+ else
data = REG_SET_FIELD(data, GRBM_GFX_INDEX, SH_INDEX, sh_num);
- data = REG_SET_FIELD(data, GRBM_GFX_INDEX, SE_INDEX, se_num);
- }
+
WREG32(mmGRBM_GFX_INDEX, data);
}
@@ -3484,11 +3481,10 @@ static u32 gfx_v8_0_get_rb_active_bitmap(struct amdgpu_device *adev)
{
u32 data, mask;
- data = RREG32(mmCC_RB_BACKEND_DISABLE);
- data |= RREG32(mmGC_USER_RB_BACKEND_DISABLE);
+ data = RREG32(mmCC_RB_BACKEND_DISABLE) |
+ RREG32(mmGC_USER_RB_BACKEND_DISABLE);
- data &= CC_RB_BACKEND_DISABLE__BACKEND_DISABLE_MASK;
- data >>= GC_USER_RB_BACKEND_DISABLE__BACKEND_DISABLE__SHIFT;
+ data = REG_GET_FIELD(data, GC_USER_RB_BACKEND_DISABLE, BACKEND_DISABLE);
mask = gfx_v8_0_create_bitmask(adev->gfx.config.max_backends_per_se /
adev->gfx.config.max_sh_per_se);
@@ -4292,12 +4288,10 @@ static int gfx_v8_0_cp_gfx_resume(struct amdgpu_device *adev)
gfx_v8_0_cp_gfx_start(adev);
ring->ready = true;
r = amdgpu_ring_test_ring(ring);
- if (r) {
+ if (r)
ring->ready = false;
- return r;
- }
- return 0;
+ return r;
}
static void gfx_v8_0_cp_compute_enable(struct amdgpu_device *adev, bool enable)
@@ -4979,8 +4973,6 @@ static int gfx_v8_0_hw_init(void *handle)
return r;
r = gfx_v8_0_cp_resume(adev);
- if (r)
- return r;
return r;
}
@@ -5028,15 +5020,12 @@ static bool gfx_v8_0_is_idle(void *handle)
static int gfx_v8_0_wait_for_idle(void *handle)
{
unsigned i;
- u32 tmp;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
for (i = 0; i < adev->usec_timeout; i++) {
- /* read MC_STATUS */
- tmp = RREG32(mmGRBM_STATUS) & GRBM_STATUS__GUI_ACTIVE_MASK;
-
- if (!REG_GET_FIELD(tmp, GRBM_STATUS, GUI_ACTIVE))
+ if (gfx_v8_0_is_idle(handle))
return 0;
+
udelay(1);
}
return -ETIMEDOUT;
@@ -5967,25 +5956,18 @@ static int gfx_v8_0_set_clockgating_state(void *handle,
static u32 gfx_v8_0_ring_get_rptr_gfx(struct amdgpu_ring *ring)
{
- u32 rptr;
-
- rptr = ring->adev->wb.wb[ring->rptr_offs];
-
- return rptr;
+ return ring->adev->wb.wb[ring->rptr_offs];
}
static u32 gfx_v8_0_ring_get_wptr_gfx(struct amdgpu_ring *ring)
{
struct amdgpu_device *adev = ring->adev;
- u32 wptr;
if (ring->use_doorbell)
/* XXX check if swapping is necessary on BE */
- wptr = ring->adev->wb.wb[ring->wptr_offs];
+ return ring->adev->wb.wb[ring->wptr_offs];
else
- wptr = RREG32(mmCP_RB0_WPTR);
-
- return wptr;
+ return RREG32(mmCP_RB0_WPTR);
}
static void gfx_v8_0_ring_set_wptr_gfx(struct amdgpu_ring *ring)
@@ -6581,15 +6563,12 @@ static u32 gfx_v8_0_get_cu_active_bitmap(struct amdgpu_device *adev)
{
u32 data, mask;
- data = RREG32(mmCC_GC_SHADER_ARRAY_CONFIG);
- data |= RREG32(mmGC_USER_SHADER_ARRAY_CONFIG);
-
- data &= CC_GC_SHADER_ARRAY_CONFIG__INACTIVE_CUS_MASK;
- data >>= CC_GC_SHADER_ARRAY_CONFIG__INACTIVE_CUS__SHIFT;
+ data = RREG32(mmCC_GC_SHADER_ARRAY_CONFIG) |
+ RREG32(mmGC_USER_SHADER_ARRAY_CONFIG);
mask = gfx_v8_0_create_bitmask(adev->gfx.config.max_cu_per_sh);
- return (~data) & mask;
+ return ~REG_GET_FIELD(data, CC_GC_SHADER_ARRAY_CONFIG, INACTIVE_CUS) & mask;
}
static void gfx_v8_0_get_cu_info(struct amdgpu_device *adev)
--
2.9.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8
[not found] ` <20160809142739.1401-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-09 14:27 ` [PATCH 1/3] drm/amd/amdgpu: Correct whitespace in GFX v8 Tom St Denis
2016-08-09 14:27 ` [PATCH 2/3] drm/amd/amdgpu: Simplify various gfx v8 functions Tom St Denis
@ 2016-08-09 14:27 ` Tom St Denis
[not found] ` <20160809142739.1401-4-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2 siblings, 1 reply; 8+ messages in thread
From: Tom St Denis @ 2016-08-09 14:27 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Tom St Denis
This patch introduces a new macro WREG32_FIELD which is used
to write to a register with a new value in a field. It's designed
to replace the pattern:
tmp = RREG32(mmFoo);
tmp &= ~REG__FIELD_MASK;
tmp |= new_value << REG__FIELD__SHIFT;
WREG32(mmFoo, tmp)
with:
WREG32_FIELD(Foo, FIELD, new_value);
Unlike WREG32_P() it understands offsets/masks and doesn't
require the caller to shift the value (or mask properly).
It's applied where suitable in the gfx_v8_0.c driver to start
with.
Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 275 ++++++----------------------------
2 files changed, 48 insertions(+), 230 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index c309eaf468e9..f23eb38eb3aa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -2218,6 +2218,9 @@ bool amdgpu_device_has_dal_support(struct amdgpu_device *adev);
#define REG_GET_FIELD(value, reg, field) \
(((value) & REG_FIELD_MASK(reg, field)) >> REG_FIELD_SHIFT(reg, field))
+#define WREG32_FIELD(reg, field, val) \
+ WREG32(mm##reg, (RREG32(mm##reg) & ~REG_FIELD_MASK(reg, field)) | (val) << REG_FIELD_SHIFT(reg, field))
+
/*
* BIOS helpers.
*/
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 5f91a834aed2..6e01392facef 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -3566,10 +3566,7 @@ static void gfx_v8_0_gpu_init(struct amdgpu_device *adev)
u32 tmp;
int i;
- tmp = RREG32(mmGRBM_CNTL);
- tmp = REG_SET_FIELD(tmp, GRBM_CNTL, READ_TIMEOUT, 0xff);
- WREG32(mmGRBM_CNTL, tmp);
-
+ WREG32_FIELD(GRBM_CNTL, READ_TIMEOUT, 0xFF);
WREG32(mmGB_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
WREG32(mmHDP_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
WREG32(mmDMIF_ADDR_CALC, adev->gfx.config.gb_addr_config);
@@ -3758,9 +3755,7 @@ static int gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
sizeof(indirect_start_offsets)/sizeof(int));
/* save and restore list */
- temp = RREG32(mmRLC_SRM_CNTL);
- temp |= RLC_SRM_CNTL__AUTO_INCR_ADDR_MASK;
- WREG32(mmRLC_SRM_CNTL, temp);
+ WREG32_FIELD(RLC_SRM_CNTL, AUTO_INCR_ADDR, 1);
WREG32(mmRLC_SRM_ARAM_ADDR, 0);
for (i = 0; i < adev->gfx.rlc.reg_list_size_bytes >> 2; i++)
@@ -3797,11 +3792,7 @@ static int gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
static void gfx_v8_0_enable_save_restore_machine(struct amdgpu_device *adev)
{
- uint32_t data;
-
- data = RREG32(mmRLC_SRM_CNTL);
- data |= RLC_SRM_CNTL__SRM_ENABLE_MASK;
- WREG32(mmRLC_SRM_CNTL, data);
+ WREG32_FIELD(RLC_SRM_CNTL, SRM_ENABLE, 1);
}
static void gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
@@ -3811,75 +3802,34 @@ static void gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
if (adev->pg_flags & (AMD_PG_SUPPORT_GFX_PG |
AMD_PG_SUPPORT_GFX_SMG |
AMD_PG_SUPPORT_GFX_DMG)) {
- data = RREG32(mmCP_RB_WPTR_POLL_CNTL);
- data &= ~CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT_MASK;
- data |= (0x60 << CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT__SHIFT);
- WREG32(mmCP_RB_WPTR_POLL_CNTL, data);
-
- data = 0;
- data |= (0x10 << RLC_PG_DELAY__POWER_UP_DELAY__SHIFT);
- data |= (0x10 << RLC_PG_DELAY__POWER_DOWN_DELAY__SHIFT);
- data |= (0x10 << RLC_PG_DELAY__CMD_PROPAGATE_DELAY__SHIFT);
- data |= (0x10 << RLC_PG_DELAY__MEM_SLEEP_DELAY__SHIFT);
- WREG32(mmRLC_PG_DELAY, data);
+ WREG32_FIELD(CP_RB_WPTR_POLL_CNTL, IDLE_POLL_COUNT, 0x60);
- data = RREG32(mmRLC_PG_DELAY_2);
- data &= ~RLC_PG_DELAY_2__SERDES_CMD_DELAY_MASK;
- data |= (0x3 << RLC_PG_DELAY_2__SERDES_CMD_DELAY__SHIFT);
- WREG32(mmRLC_PG_DELAY_2, data);
+ data = REG_SET_FIELD(0, RLC_PG_DELAY, POWER_UP_DELAY, 0x10);
+ data = REG_SET_FIELD(data, RLC_PG_DELAY, POWER_DOWN_DELAY, 0x10);
+ data = REG_SET_FIELD(data, RLC_PG_DELAY, CMD_PROPAGATE_DELAY, 0x10);
+ data = REG_SET_FIELD(data, RLC_PG_DELAY, MEM_SLEEP_DELAY, 0x10);
+ WREG32(mmRLC_PG_DELAY, data);
- data = RREG32(mmRLC_AUTO_PG_CTRL);
- data &= ~RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD_MASK;
- data |= (0x55f0 << RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD__SHIFT);
- WREG32(mmRLC_AUTO_PG_CTRL, data);
+ WREG32_FIELD(RLC_PG_DELAY_2, SERDES_CMD_DELAY, 0x3);
+ WREG32_FIELD(RLC_AUTO_PG_CTRL, GRBM_REG_SAVE_GFX_IDLE_THRESHOLD, 0x55f0);
}
}
static void cz_enable_sck_slow_down_on_power_up(struct amdgpu_device *adev,
bool enable)
{
- u32 data, orig;
-
- orig = data = RREG32(mmRLC_PG_CNTL);
-
- if (enable)
- data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
- else
- data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
-
- if (orig != data)
- WREG32(mmRLC_PG_CNTL, data);
+ WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PU_ENABLE, enable ? 1 : 0);
}
static void cz_enable_sck_slow_down_on_power_down(struct amdgpu_device *adev,
bool enable)
{
- u32 data, orig;
-
- orig = data = RREG32(mmRLC_PG_CNTL);
-
- if (enable)
- data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
- else
- data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
-
- if (orig != data)
- WREG32(mmRLC_PG_CNTL, data);
+ WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PD_ENABLE, enable ? 1 : 0);
}
static void cz_enable_cp_power_gating(struct amdgpu_device *adev, bool enable)
{
- u32 data, orig;
-
- orig = data = RREG32(mmRLC_PG_CNTL);
-
- if (enable)
- data &= ~RLC_PG_CNTL__CP_PG_DISABLE_MASK;
- else
- data |= RLC_PG_CNTL__CP_PG_DISABLE_MASK;
-
- if (orig != data)
- WREG32(mmRLC_PG_CNTL, data);
+ WREG32_FIELD(RLC_PG_CNTL, CP_PG_DISABLE, enable ? 1 : 0);
}
static void gfx_v8_0_init_pg(struct amdgpu_device *adev)
@@ -3918,34 +3868,24 @@ static void gfx_v8_0_init_pg(struct amdgpu_device *adev)
void gfx_v8_0_rlc_stop(struct amdgpu_device *adev)
{
- u32 tmp = RREG32(mmRLC_CNTL);
-
- tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 0);
- WREG32(mmRLC_CNTL, tmp);
+ WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 0);
gfx_v8_0_enable_gui_idle_interrupt(adev, false);
-
gfx_v8_0_wait_for_rlc_serdes(adev);
}
static void gfx_v8_0_rlc_reset(struct amdgpu_device *adev)
{
- u32 tmp = RREG32(mmGRBM_SOFT_RESET);
-
- tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
- WREG32(mmGRBM_SOFT_RESET, tmp);
+ WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
udelay(50);
- tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
- WREG32(mmGRBM_SOFT_RESET, tmp);
+
+ WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
udelay(50);
}
static void gfx_v8_0_rlc_start(struct amdgpu_device *adev)
{
- u32 tmp = RREG32(mmRLC_CNTL);
-
- tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 1);
- WREG32(mmRLC_CNTL, tmp);
+ WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 1);
/* carrizo do enable cp interrupt after cp inited */
if (!(adev->flags & AMD_IS_APU))
@@ -5371,8 +5311,6 @@ static int gfx_v8_0_late_init(void *handle)
static void gfx_v8_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *adev,
bool enable)
{
- uint32_t data, temp;
-
if (adev->asic_type == CHIP_POLARIS11)
/* Send msg to SMU via Powerplay */
amdgpu_set_powergating_state(adev,
@@ -5380,83 +5318,35 @@ static void gfx_v8_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *ade
enable ?
AMD_PG_STATE_GATE : AMD_PG_STATE_UNGATE);
- temp = data = RREG32(mmRLC_PG_CNTL);
- /* Enable static MGPG */
- if (enable)
- data |= RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
- else
- data &= ~RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
-
- if (temp != data)
- WREG32(mmRLC_PG_CNTL, data);
+ WREG32_FIELD(RLC_PG_CNTL, STATIC_PER_CU_PG_ENABLE, enable ? 1 : 0);
}
static void gfx_v8_0_enable_gfx_dynamic_mg_power_gating(struct amdgpu_device *adev,
bool enable)
{
- uint32_t data, temp;
-
- temp = data = RREG32(mmRLC_PG_CNTL);
- /* Enable dynamic MGPG */
- if (enable)
- data |= RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
- else
- data &= ~RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
-
- if (temp != data)
- WREG32(mmRLC_PG_CNTL, data);
+ WREG32_FIELD(RLC_PG_CNTL, DYN_PER_CU_PG_ENABLE, enable ? 1 : 0);
}
static void polaris11_enable_gfx_quick_mg_power_gating(struct amdgpu_device *adev,
bool enable)
{
- uint32_t data, temp;
-
- temp = data = RREG32(mmRLC_PG_CNTL);
- /* Enable quick PG */
- if (enable)
- data |= RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
- else
- data &= ~RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
-
- if (temp != data)
- WREG32(mmRLC_PG_CNTL, data);
+ WREG32_FIELD(RLC_PG_CNTL, QUICK_PG_ENABLE, enable ? 1 : 0);
}
static void cz_enable_gfx_cg_power_gating(struct amdgpu_device *adev,
bool enable)
{
- u32 data, orig;
-
- orig = data = RREG32(mmRLC_PG_CNTL);
-
- if (enable)
- data |= RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
- else
- data &= ~RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
-
- if (orig != data)
- WREG32(mmRLC_PG_CNTL, data);
+ WREG32_FIELD(RLC_PG_CNTL, GFX_POWER_GATING_ENABLE, enable ? 1 : 0);
}
static void cz_enable_gfx_pipeline_power_gating(struct amdgpu_device *adev,
bool enable)
{
- u32 data, orig;
-
- orig = data = RREG32(mmRLC_PG_CNTL);
-
- if (enable)
- data |= RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
- else
- data &= ~RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
-
- if (orig != data)
- WREG32(mmRLC_PG_CNTL, data);
+ WREG32_FIELD(RLC_PG_CNTL, GFX_PIPELINE_PG_ENABLE, enable ? 1 : 0);
/* Read any GFX register to wake up GFX. */
if (!enable)
- data = RREG32(mmDB_RENDER_CONTROL);
+ RREG32(mmDB_RENDER_CONTROL);
}
static void cz_update_gfx_cg_power_gating(struct amdgpu_device *adev,
@@ -5563,10 +5453,10 @@ static void gfx_v8_0_send_serdes_cmd(struct amdgpu_device *adev,
#define MSG_ENTER_RLC_SAFE_MODE 1
#define MSG_EXIT_RLC_SAFE_MODE 0
-
-#define RLC_GPR_REG2__REQ_MASK 0x00000001
-#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
-#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
+#define RLC_GPR_REG2__REQ_MASK 0x00000001
+#define RLC_GPR_REG2__REQ__SHIFT 0
+#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
+#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
static void cz_enter_rlc_safe_mode(struct amdgpu_device *adev)
{
@@ -5596,7 +5486,7 @@ static void cz_enter_rlc_safe_mode(struct amdgpu_device *adev)
}
for (i = 0; i < adev->usec_timeout; i++) {
- if ((RREG32(mmRLC_GPR_REG2) & RLC_GPR_REG2__REQ_MASK) == 0)
+ if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2), RLC_GPR_REG2, REQ))
break;
udelay(1);
}
@@ -5624,7 +5514,7 @@ static void cz_exit_rlc_safe_mode(struct amdgpu_device *adev)
}
for (i = 0; i < adev->usec_timeout; i++) {
- if ((RREG32(mmRLC_GPR_REG2) & RLC_GPR_REG2__REQ_MASK) == 0)
+ if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2), RLC_GPR_REG2, REQ))
break;
udelay(1);
}
@@ -5656,7 +5546,7 @@ static void iceland_enter_rlc_safe_mode(struct amdgpu_device *adev)
}
for (i = 0; i < adev->usec_timeout; i++) {
- if ((RREG32(mmRLC_SAFE_MODE) & RLC_SAFE_MODE__CMD_MASK) == 0)
+ if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD))
break;
udelay(1);
}
@@ -5683,7 +5573,7 @@ static void iceland_exit_rlc_safe_mode(struct amdgpu_device *adev)
}
for (i = 0; i < adev->usec_timeout; i++) {
- if ((RREG32(mmRLC_SAFE_MODE) & RLC_SAFE_MODE__CMD_MASK) == 0)
+ if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD))
break;
udelay(1);
}
@@ -5724,21 +5614,12 @@ static void gfx_v8_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
/* It is disabled by HW by default */
if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGCG)) {
if (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGLS) {
- if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS) {
+ if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS)
/* 1 - RLC memory Light sleep */
- temp = data = RREG32(mmRLC_MEM_SLP_CNTL);
- data |= RLC_MEM_SLP_CNTL__RLC_MEM_LS_EN_MASK;
- if (temp != data)
- WREG32(mmRLC_MEM_SLP_CNTL, data);
- }
+ WREG32_FIELD(RLC_MEM_SLP_CNTL, RLC_MEM_LS_EN, 1);
- if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS) {
- /* 2 - CP memory Light sleep */
- temp = data = RREG32(mmCP_MEM_SLP_CNTL);
- data |= CP_MEM_SLP_CNTL__CP_MEM_LS_EN_MASK;
- if (temp != data)
- WREG32(mmCP_MEM_SLP_CNTL, data);
- }
+ if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS)
+ WREG32_FIELD(CP_MEM_SLP_CNTL, CP_MEM_LS_EN, 1);
}
/* 3 - RLC_CGTT_MGCG_OVERRIDE */
@@ -6213,33 +6094,14 @@ static void gfx_v8_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
enum amdgpu_interrupt_state state)
{
- u32 cp_int_cntl;
-
- switch (state) {
- case AMDGPU_IRQ_STATE_DISABLE:
- cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
- cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
- TIME_STAMP_INT_ENABLE, 0);
- WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
- break;
- case AMDGPU_IRQ_STATE_ENABLE:
- cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
- cp_int_cntl =
- REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
- TIME_STAMP_INT_ENABLE, 1);
- WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
- break;
- default:
- break;
- }
+ WREG32_FIELD(CP_INT_CNTL_RING0, TIME_STAMP_INT_ENABLE,
+ state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
}
static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
int me, int pipe,
enum amdgpu_interrupt_state state)
{
- u32 mec_int_cntl, mec_int_cntl_reg;
-
/*
* amdgpu controls only pipe 0 of MEC1. That's why this function only
* handles the setting of interrupts for this specific pipe. All other
@@ -6249,7 +6111,6 @@ static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
if (me == 1) {
switch (pipe) {
case 0:
- mec_int_cntl_reg = mmCP_ME1_PIPE0_INT_CNTL;
break;
default:
DRM_DEBUG("invalid pipe %d\n", pipe);
@@ -6260,22 +6121,8 @@ static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
return;
}
- switch (state) {
- case AMDGPU_IRQ_STATE_DISABLE:
- mec_int_cntl = RREG32(mec_int_cntl_reg);
- mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
- TIME_STAMP_INT_ENABLE, 0);
- WREG32(mec_int_cntl_reg, mec_int_cntl);
- break;
- case AMDGPU_IRQ_STATE_ENABLE:
- mec_int_cntl = RREG32(mec_int_cntl_reg);
- mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
- TIME_STAMP_INT_ENABLE, 1);
- WREG32(mec_int_cntl_reg, mec_int_cntl);
- break;
- default:
- break;
- }
+ WREG32_FIELD(CP_ME1_PIPE0_INT_CNTL, TIME_STAMP_INT_ENABLE,
+ state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
}
static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev,
@@ -6283,24 +6130,8 @@ static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev,
unsigned type,
enum amdgpu_interrupt_state state)
{
- u32 cp_int_cntl;
-
- switch (state) {
- case AMDGPU_IRQ_STATE_DISABLE:
- cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
- cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
- PRIV_REG_INT_ENABLE, 0);
- WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
- break;
- case AMDGPU_IRQ_STATE_ENABLE:
- cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
- cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
- PRIV_REG_INT_ENABLE, 1);
- WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
- break;
- default:
- break;
- }
+ WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_REG_INT_ENABLE,
+ state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
return 0;
}
@@ -6310,24 +6141,8 @@ static int gfx_v8_0_set_priv_inst_fault_state(struct amdgpu_device *adev,
unsigned type,
enum amdgpu_interrupt_state state)
{
- u32 cp_int_cntl;
-
- switch (state) {
- case AMDGPU_IRQ_STATE_DISABLE:
- cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
- cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
- PRIV_INSTR_INT_ENABLE, 0);
- WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
- break;
- case AMDGPU_IRQ_STATE_ENABLE:
- cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
- cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
- PRIV_INSTR_INT_ENABLE, 1);
- WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
- break;
- default:
- break;
- }
+ WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_INSTR_INT_ENABLE,
+ state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
return 0;
}
--
2.9.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8
[not found] ` <20160809142739.1401-4-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2016-08-09 14:56 ` Christian König
[not found] ` <dc95cc72-5e8e-6c5e-7c87-979d838652e7-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Christian König @ 2016-08-09 14:56 UTC (permalink / raw)
To: Tom St Denis, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Tom St Denis
Am 09.08.2016 um 16:27 schrieb Tom St Denis:
> This patch introduces a new macro WREG32_FIELD which is used
> to write to a register with a new value in a field. It's designed
> to replace the pattern:
>
> tmp = RREG32(mmFoo);
> tmp &= ~REG__FIELD_MASK;
> tmp |= new_value << REG__FIELD__SHIFT;
> WREG32(mmFoo, tmp)
>
> with:
>
> WREG32_FIELD(Foo, FIELD, new_value);
>
> Unlike WREG32_P() it understands offsets/masks and doesn't
> require the caller to shift the value (or mask properly).
>
> It's applied where suitable in the gfx_v8_0.c driver to start
> with.
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 275 ++++++----------------------------
> 2 files changed, 48 insertions(+), 230 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index c309eaf468e9..f23eb38eb3aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -2218,6 +2218,9 @@ bool amdgpu_device_has_dal_support(struct amdgpu_device *adev);
> #define REG_GET_FIELD(value, reg, field) \
> (((value) & REG_FIELD_MASK(reg, field)) >> REG_FIELD_SHIFT(reg, field))
>
> +#define WREG32_FIELD(reg, field, val) \
> + WREG32(mm##reg, (RREG32(mm##reg) & ~REG_FIELD_MASK(reg, field)) | (val) << REG_FIELD_SHIFT(reg, field))
Couldn't you use WREG32_P here to implement the new macro?
Christian.
> +
> /*
> * BIOS helpers.
> */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 5f91a834aed2..6e01392facef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -3566,10 +3566,7 @@ static void gfx_v8_0_gpu_init(struct amdgpu_device *adev)
> u32 tmp;
> int i;
>
> - tmp = RREG32(mmGRBM_CNTL);
> - tmp = REG_SET_FIELD(tmp, GRBM_CNTL, READ_TIMEOUT, 0xff);
> - WREG32(mmGRBM_CNTL, tmp);
> -
> + WREG32_FIELD(GRBM_CNTL, READ_TIMEOUT, 0xFF);
> WREG32(mmGB_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
> WREG32(mmHDP_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
> WREG32(mmDMIF_ADDR_CALC, adev->gfx.config.gb_addr_config);
> @@ -3758,9 +3755,7 @@ static int gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
> sizeof(indirect_start_offsets)/sizeof(int));
>
> /* save and restore list */
> - temp = RREG32(mmRLC_SRM_CNTL);
> - temp |= RLC_SRM_CNTL__AUTO_INCR_ADDR_MASK;
> - WREG32(mmRLC_SRM_CNTL, temp);
> + WREG32_FIELD(RLC_SRM_CNTL, AUTO_INCR_ADDR, 1);
>
> WREG32(mmRLC_SRM_ARAM_ADDR, 0);
> for (i = 0; i < adev->gfx.rlc.reg_list_size_bytes >> 2; i++)
> @@ -3797,11 +3792,7 @@ static int gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
>
> static void gfx_v8_0_enable_save_restore_machine(struct amdgpu_device *adev)
> {
> - uint32_t data;
> -
> - data = RREG32(mmRLC_SRM_CNTL);
> - data |= RLC_SRM_CNTL__SRM_ENABLE_MASK;
> - WREG32(mmRLC_SRM_CNTL, data);
> + WREG32_FIELD(RLC_SRM_CNTL, SRM_ENABLE, 1);
> }
>
> static void gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
> @@ -3811,75 +3802,34 @@ static void gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
> if (adev->pg_flags & (AMD_PG_SUPPORT_GFX_PG |
> AMD_PG_SUPPORT_GFX_SMG |
> AMD_PG_SUPPORT_GFX_DMG)) {
> - data = RREG32(mmCP_RB_WPTR_POLL_CNTL);
> - data &= ~CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT_MASK;
> - data |= (0x60 << CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT__SHIFT);
> - WREG32(mmCP_RB_WPTR_POLL_CNTL, data);
> -
> - data = 0;
> - data |= (0x10 << RLC_PG_DELAY__POWER_UP_DELAY__SHIFT);
> - data |= (0x10 << RLC_PG_DELAY__POWER_DOWN_DELAY__SHIFT);
> - data |= (0x10 << RLC_PG_DELAY__CMD_PROPAGATE_DELAY__SHIFT);
> - data |= (0x10 << RLC_PG_DELAY__MEM_SLEEP_DELAY__SHIFT);
> - WREG32(mmRLC_PG_DELAY, data);
> + WREG32_FIELD(CP_RB_WPTR_POLL_CNTL, IDLE_POLL_COUNT, 0x60);
>
> - data = RREG32(mmRLC_PG_DELAY_2);
> - data &= ~RLC_PG_DELAY_2__SERDES_CMD_DELAY_MASK;
> - data |= (0x3 << RLC_PG_DELAY_2__SERDES_CMD_DELAY__SHIFT);
> - WREG32(mmRLC_PG_DELAY_2, data);
> + data = REG_SET_FIELD(0, RLC_PG_DELAY, POWER_UP_DELAY, 0x10);
> + data = REG_SET_FIELD(data, RLC_PG_DELAY, POWER_DOWN_DELAY, 0x10);
> + data = REG_SET_FIELD(data, RLC_PG_DELAY, CMD_PROPAGATE_DELAY, 0x10);
> + data = REG_SET_FIELD(data, RLC_PG_DELAY, MEM_SLEEP_DELAY, 0x10);
> + WREG32(mmRLC_PG_DELAY, data);
>
> - data = RREG32(mmRLC_AUTO_PG_CTRL);
> - data &= ~RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD_MASK;
> - data |= (0x55f0 << RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD__SHIFT);
> - WREG32(mmRLC_AUTO_PG_CTRL, data);
> + WREG32_FIELD(RLC_PG_DELAY_2, SERDES_CMD_DELAY, 0x3);
> + WREG32_FIELD(RLC_AUTO_PG_CTRL, GRBM_REG_SAVE_GFX_IDLE_THRESHOLD, 0x55f0);
> }
> }
>
> static void cz_enable_sck_slow_down_on_power_up(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PU_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_sck_slow_down_on_power_down(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PD_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_cp_power_gating(struct amdgpu_device *adev, bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data &= ~RLC_PG_CNTL__CP_PG_DISABLE_MASK;
> - else
> - data |= RLC_PG_CNTL__CP_PG_DISABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, CP_PG_DISABLE, enable ? 1 : 0);
> }
>
> static void gfx_v8_0_init_pg(struct amdgpu_device *adev)
> @@ -3918,34 +3868,24 @@ static void gfx_v8_0_init_pg(struct amdgpu_device *adev)
>
> void gfx_v8_0_rlc_stop(struct amdgpu_device *adev)
> {
> - u32 tmp = RREG32(mmRLC_CNTL);
> -
> - tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 0);
> - WREG32(mmRLC_CNTL, tmp);
> + WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 0);
>
> gfx_v8_0_enable_gui_idle_interrupt(adev, false);
> -
> gfx_v8_0_wait_for_rlc_serdes(adev);
> }
>
> static void gfx_v8_0_rlc_reset(struct amdgpu_device *adev)
> {
> - u32 tmp = RREG32(mmGRBM_SOFT_RESET);
> -
> - tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
> - WREG32(mmGRBM_SOFT_RESET, tmp);
> + WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
> udelay(50);
> - tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
> - WREG32(mmGRBM_SOFT_RESET, tmp);
> +
> + WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
> udelay(50);
> }
>
> static void gfx_v8_0_rlc_start(struct amdgpu_device *adev)
> {
> - u32 tmp = RREG32(mmRLC_CNTL);
> -
> - tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 1);
> - WREG32(mmRLC_CNTL, tmp);
> + WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 1);
>
> /* carrizo do enable cp interrupt after cp inited */
> if (!(adev->flags & AMD_IS_APU))
> @@ -5371,8 +5311,6 @@ static int gfx_v8_0_late_init(void *handle)
> static void gfx_v8_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - uint32_t data, temp;
> -
> if (adev->asic_type == CHIP_POLARIS11)
> /* Send msg to SMU via Powerplay */
> amdgpu_set_powergating_state(adev,
> @@ -5380,83 +5318,35 @@ static void gfx_v8_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *ade
> enable ?
> AMD_PG_STATE_GATE : AMD_PG_STATE_UNGATE);
>
> - temp = data = RREG32(mmRLC_PG_CNTL);
> - /* Enable static MGPG */
> - if (enable)
> - data |= RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
> -
> - if (temp != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, STATIC_PER_CU_PG_ENABLE, enable ? 1 : 0);
> }
>
> static void gfx_v8_0_enable_gfx_dynamic_mg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - uint32_t data, temp;
> -
> - temp = data = RREG32(mmRLC_PG_CNTL);
> - /* Enable dynamic MGPG */
> - if (enable)
> - data |= RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
> -
> - if (temp != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, DYN_PER_CU_PG_ENABLE, enable ? 1 : 0);
> }
>
> static void polaris11_enable_gfx_quick_mg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - uint32_t data, temp;
> -
> - temp = data = RREG32(mmRLC_PG_CNTL);
> - /* Enable quick PG */
> - if (enable)
> - data |= RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
> -
> - if (temp != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, QUICK_PG_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_gfx_cg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, GFX_POWER_GATING_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_gfx_pipeline_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, GFX_PIPELINE_PG_ENABLE, enable ? 1 : 0);
>
> /* Read any GFX register to wake up GFX. */
> if (!enable)
> - data = RREG32(mmDB_RENDER_CONTROL);
> + RREG32(mmDB_RENDER_CONTROL);
> }
>
> static void cz_update_gfx_cg_power_gating(struct amdgpu_device *adev,
> @@ -5563,10 +5453,10 @@ static void gfx_v8_0_send_serdes_cmd(struct amdgpu_device *adev,
>
> #define MSG_ENTER_RLC_SAFE_MODE 1
> #define MSG_EXIT_RLC_SAFE_MODE 0
> -
> -#define RLC_GPR_REG2__REQ_MASK 0x00000001
> -#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
> -#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
> +#define RLC_GPR_REG2__REQ_MASK 0x00000001
> +#define RLC_GPR_REG2__REQ__SHIFT 0
> +#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
> +#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
>
> static void cz_enter_rlc_safe_mode(struct amdgpu_device *adev)
> {
> @@ -5596,7 +5486,7 @@ static void cz_enter_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_GPR_REG2) & RLC_GPR_REG2__REQ_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2), RLC_GPR_REG2, REQ))
> break;
> udelay(1);
> }
> @@ -5624,7 +5514,7 @@ static void cz_exit_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_GPR_REG2) & RLC_GPR_REG2__REQ_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2), RLC_GPR_REG2, REQ))
> break;
> udelay(1);
> }
> @@ -5656,7 +5546,7 @@ static void iceland_enter_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_SAFE_MODE) & RLC_SAFE_MODE__CMD_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD))
> break;
> udelay(1);
> }
> @@ -5683,7 +5573,7 @@ static void iceland_exit_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_SAFE_MODE) & RLC_SAFE_MODE__CMD_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD))
> break;
> udelay(1);
> }
> @@ -5724,21 +5614,12 @@ static void gfx_v8_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
> /* It is disabled by HW by default */
> if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGCG)) {
> if (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGLS) {
> - if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS) {
> + if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS)
> /* 1 - RLC memory Light sleep */
> - temp = data = RREG32(mmRLC_MEM_SLP_CNTL);
> - data |= RLC_MEM_SLP_CNTL__RLC_MEM_LS_EN_MASK;
> - if (temp != data)
> - WREG32(mmRLC_MEM_SLP_CNTL, data);
> - }
> + WREG32_FIELD(RLC_MEM_SLP_CNTL, RLC_MEM_LS_EN, 1);
>
> - if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS) {
> - /* 2 - CP memory Light sleep */
> - temp = data = RREG32(mmCP_MEM_SLP_CNTL);
> - data |= CP_MEM_SLP_CNTL__CP_MEM_LS_EN_MASK;
> - if (temp != data)
> - WREG32(mmCP_MEM_SLP_CNTL, data);
> - }
> + if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS)
> + WREG32_FIELD(CP_MEM_SLP_CNTL, CP_MEM_LS_EN, 1);
> }
>
> /* 3 - RLC_CGTT_MGCG_OVERRIDE */
> @@ -6213,33 +6094,14 @@ static void gfx_v8_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
> static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
> enum amdgpu_interrupt_state state)
> {
> - u32 cp_int_cntl;
> -
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - TIME_STAMP_INT_ENABLE, 0);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl =
> - REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - TIME_STAMP_INT_ENABLE, 1);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_INT_CNTL_RING0, TIME_STAMP_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> }
>
> static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> int me, int pipe,
> enum amdgpu_interrupt_state state)
> {
> - u32 mec_int_cntl, mec_int_cntl_reg;
> -
> /*
> * amdgpu controls only pipe 0 of MEC1. That's why this function only
> * handles the setting of interrupts for this specific pipe. All other
> @@ -6249,7 +6111,6 @@ static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> if (me == 1) {
> switch (pipe) {
> case 0:
> - mec_int_cntl_reg = mmCP_ME1_PIPE0_INT_CNTL;
> break;
> default:
> DRM_DEBUG("invalid pipe %d\n", pipe);
> @@ -6260,22 +6121,8 @@ static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> return;
> }
>
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - mec_int_cntl = RREG32(mec_int_cntl_reg);
> - mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
> - TIME_STAMP_INT_ENABLE, 0);
> - WREG32(mec_int_cntl_reg, mec_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - mec_int_cntl = RREG32(mec_int_cntl_reg);
> - mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
> - TIME_STAMP_INT_ENABLE, 1);
> - WREG32(mec_int_cntl_reg, mec_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_ME1_PIPE0_INT_CNTL, TIME_STAMP_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> }
>
> static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev,
> @@ -6283,24 +6130,8 @@ static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev,
> unsigned type,
> enum amdgpu_interrupt_state state)
> {
> - u32 cp_int_cntl;
> -
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_REG_INT_ENABLE, 0);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_REG_INT_ENABLE, 1);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_REG_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
>
> return 0;
> }
> @@ -6310,24 +6141,8 @@ static int gfx_v8_0_set_priv_inst_fault_state(struct amdgpu_device *adev,
> unsigned type,
> enum amdgpu_interrupt_state state)
> {
> - u32 cp_int_cntl;
> -
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_INSTR_INT_ENABLE, 0);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_INSTR_INT_ENABLE, 1);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_INSTR_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
>
> return 0;
> }
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8
[not found] ` <dc95cc72-5e8e-6c5e-7c87-979d838652e7-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2016-08-09 15:28 ` StDenis, Tom
[not found] ` <DM5PR12MB113283968678F567DACB84A6F71C0-2J9CzHegvk+UzrhdoDeimQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: StDenis, Tom @ 2016-08-09 15:28 UTC (permalink / raw)
To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 21979 bytes --]
Probably could but I felt that implementation was cleaner.
Tom
________________________________
From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
Sent: Tuesday, August 9, 2016 10:56
To: Tom St Denis; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: StDenis, Tom
Subject: Re: [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8
Am 09.08.2016 um 16:27 schrieb Tom St Denis:
> This patch introduces a new macro WREG32_FIELD which is used
> to write to a register with a new value in a field. It's designed
> to replace the pattern:
>
> tmp = RREG32(mmFoo);
> tmp &= ~REG__FIELD_MASK;
> tmp |= new_value << REG__FIELD__SHIFT;
> WREG32(mmFoo, tmp)
>
> with:
>
> WREG32_FIELD(Foo, FIELD, new_value);
>
> Unlike WREG32_P() it understands offsets/masks and doesn't
> require the caller to shift the value (or mask properly).
>
> It's applied where suitable in the gfx_v8_0.c driver to start
> with.
>
> Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 275 ++++++----------------------------
> 2 files changed, 48 insertions(+), 230 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index c309eaf468e9..f23eb38eb3aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -2218,6 +2218,9 @@ bool amdgpu_device_has_dal_support(struct amdgpu_device *adev);
> #define REG_GET_FIELD(value, reg, field) \
> (((value) & REG_FIELD_MASK(reg, field)) >> REG_FIELD_SHIFT(reg, field))
>
> +#define WREG32_FIELD(reg, field, val) \
> + WREG32(mm##reg, (RREG32(mm##reg) & ~REG_FIELD_MASK(reg, field)) | (val) << REG_FIELD_SHIFT(reg, field))
Couldn't you use WREG32_P here to implement the new macro?
Christian.
> +
> /*
> * BIOS helpers.
> */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 5f91a834aed2..6e01392facef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -3566,10 +3566,7 @@ static void gfx_v8_0_gpu_init(struct amdgpu_device *adev)
> u32 tmp;
> int i;
>
> - tmp = RREG32(mmGRBM_CNTL);
> - tmp = REG_SET_FIELD(tmp, GRBM_CNTL, READ_TIMEOUT, 0xff);
> - WREG32(mmGRBM_CNTL, tmp);
> -
> + WREG32_FIELD(GRBM_CNTL, READ_TIMEOUT, 0xFF);
> WREG32(mmGB_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
> WREG32(mmHDP_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
> WREG32(mmDMIF_ADDR_CALC, adev->gfx.config.gb_addr_config);
> @@ -3758,9 +3755,7 @@ static int gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
> sizeof(indirect_start_offsets)/sizeof(int));
>
> /* save and restore list */
> - temp = RREG32(mmRLC_SRM_CNTL);
> - temp |= RLC_SRM_CNTL__AUTO_INCR_ADDR_MASK;
> - WREG32(mmRLC_SRM_CNTL, temp);
> + WREG32_FIELD(RLC_SRM_CNTL, AUTO_INCR_ADDR, 1);
>
> WREG32(mmRLC_SRM_ARAM_ADDR, 0);
> for (i = 0; i < adev->gfx.rlc.reg_list_size_bytes >> 2; i++)
> @@ -3797,11 +3792,7 @@ static int gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
>
> static void gfx_v8_0_enable_save_restore_machine(struct amdgpu_device *adev)
> {
> - uint32_t data;
> -
> - data = RREG32(mmRLC_SRM_CNTL);
> - data |= RLC_SRM_CNTL__SRM_ENABLE_MASK;
> - WREG32(mmRLC_SRM_CNTL, data);
> + WREG32_FIELD(RLC_SRM_CNTL, SRM_ENABLE, 1);
> }
>
> static void gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
> @@ -3811,75 +3802,34 @@ static void gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
> if (adev->pg_flags & (AMD_PG_SUPPORT_GFX_PG |
> AMD_PG_SUPPORT_GFX_SMG |
> AMD_PG_SUPPORT_GFX_DMG)) {
> - data = RREG32(mmCP_RB_WPTR_POLL_CNTL);
> - data &= ~CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT_MASK;
> - data |= (0x60 << CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT__SHIFT);
> - WREG32(mmCP_RB_WPTR_POLL_CNTL, data);
> -
> - data = 0;
> - data |= (0x10 << RLC_PG_DELAY__POWER_UP_DELAY__SHIFT);
> - data |= (0x10 << RLC_PG_DELAY__POWER_DOWN_DELAY__SHIFT);
> - data |= (0x10 << RLC_PG_DELAY__CMD_PROPAGATE_DELAY__SHIFT);
> - data |= (0x10 << RLC_PG_DELAY__MEM_SLEEP_DELAY__SHIFT);
> - WREG32(mmRLC_PG_DELAY, data);
> + WREG32_FIELD(CP_RB_WPTR_POLL_CNTL, IDLE_POLL_COUNT, 0x60);
>
> - data = RREG32(mmRLC_PG_DELAY_2);
> - data &= ~RLC_PG_DELAY_2__SERDES_CMD_DELAY_MASK;
> - data |= (0x3 << RLC_PG_DELAY_2__SERDES_CMD_DELAY__SHIFT);
> - WREG32(mmRLC_PG_DELAY_2, data);
> + data = REG_SET_FIELD(0, RLC_PG_DELAY, POWER_UP_DELAY, 0x10);
> + data = REG_SET_FIELD(data, RLC_PG_DELAY, POWER_DOWN_DELAY, 0x10);
> + data = REG_SET_FIELD(data, RLC_PG_DELAY, CMD_PROPAGATE_DELAY, 0x10);
> + data = REG_SET_FIELD(data, RLC_PG_DELAY, MEM_SLEEP_DELAY, 0x10);
> + WREG32(mmRLC_PG_DELAY, data);
>
> - data = RREG32(mmRLC_AUTO_PG_CTRL);
> - data &= ~RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD_MASK;
> - data |= (0x55f0 << RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD__SHIFT);
> - WREG32(mmRLC_AUTO_PG_CTRL, data);
> + WREG32_FIELD(RLC_PG_DELAY_2, SERDES_CMD_DELAY, 0x3);
> + WREG32_FIELD(RLC_AUTO_PG_CTRL, GRBM_REG_SAVE_GFX_IDLE_THRESHOLD, 0x55f0);
> }
> }
>
> static void cz_enable_sck_slow_down_on_power_up(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PU_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_sck_slow_down_on_power_down(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PD_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_cp_power_gating(struct amdgpu_device *adev, bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data &= ~RLC_PG_CNTL__CP_PG_DISABLE_MASK;
> - else
> - data |= RLC_PG_CNTL__CP_PG_DISABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, CP_PG_DISABLE, enable ? 1 : 0);
> }
>
> static void gfx_v8_0_init_pg(struct amdgpu_device *adev)
> @@ -3918,34 +3868,24 @@ static void gfx_v8_0_init_pg(struct amdgpu_device *adev)
>
> void gfx_v8_0_rlc_stop(struct amdgpu_device *adev)
> {
> - u32 tmp = RREG32(mmRLC_CNTL);
> -
> - tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 0);
> - WREG32(mmRLC_CNTL, tmp);
> + WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 0);
>
> gfx_v8_0_enable_gui_idle_interrupt(adev, false);
> -
> gfx_v8_0_wait_for_rlc_serdes(adev);
> }
>
> static void gfx_v8_0_rlc_reset(struct amdgpu_device *adev)
> {
> - u32 tmp = RREG32(mmGRBM_SOFT_RESET);
> -
> - tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
> - WREG32(mmGRBM_SOFT_RESET, tmp);
> + WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
> udelay(50);
> - tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
> - WREG32(mmGRBM_SOFT_RESET, tmp);
> +
> + WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
> udelay(50);
> }
>
> static void gfx_v8_0_rlc_start(struct amdgpu_device *adev)
> {
> - u32 tmp = RREG32(mmRLC_CNTL);
> -
> - tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 1);
> - WREG32(mmRLC_CNTL, tmp);
> + WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 1);
>
> /* carrizo do enable cp interrupt after cp inited */
> if (!(adev->flags & AMD_IS_APU))
> @@ -5371,8 +5311,6 @@ static int gfx_v8_0_late_init(void *handle)
> static void gfx_v8_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - uint32_t data, temp;
> -
> if (adev->asic_type == CHIP_POLARIS11)
> /* Send msg to SMU via Powerplay */
> amdgpu_set_powergating_state(adev,
> @@ -5380,83 +5318,35 @@ static void gfx_v8_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *ade
> enable ?
> AMD_PG_STATE_GATE : AMD_PG_STATE_UNGATE);
>
> - temp = data = RREG32(mmRLC_PG_CNTL);
> - /* Enable static MGPG */
> - if (enable)
> - data |= RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
> -
> - if (temp != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, STATIC_PER_CU_PG_ENABLE, enable ? 1 : 0);
> }
>
> static void gfx_v8_0_enable_gfx_dynamic_mg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - uint32_t data, temp;
> -
> - temp = data = RREG32(mmRLC_PG_CNTL);
> - /* Enable dynamic MGPG */
> - if (enable)
> - data |= RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
> -
> - if (temp != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, DYN_PER_CU_PG_ENABLE, enable ? 1 : 0);
> }
>
> static void polaris11_enable_gfx_quick_mg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - uint32_t data, temp;
> -
> - temp = data = RREG32(mmRLC_PG_CNTL);
> - /* Enable quick PG */
> - if (enable)
> - data |= RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
> -
> - if (temp != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, QUICK_PG_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_gfx_cg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, GFX_POWER_GATING_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_gfx_pipeline_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, GFX_PIPELINE_PG_ENABLE, enable ? 1 : 0);
>
> /* Read any GFX register to wake up GFX. */
> if (!enable)
> - data = RREG32(mmDB_RENDER_CONTROL);
> + RREG32(mmDB_RENDER_CONTROL);
> }
>
> static void cz_update_gfx_cg_power_gating(struct amdgpu_device *adev,
> @@ -5563,10 +5453,10 @@ static void gfx_v8_0_send_serdes_cmd(struct amdgpu_device *adev,
>
> #define MSG_ENTER_RLC_SAFE_MODE 1
> #define MSG_EXIT_RLC_SAFE_MODE 0
> -
> -#define RLC_GPR_REG2__REQ_MASK 0x00000001
> -#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
> -#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
> +#define RLC_GPR_REG2__REQ_MASK 0x00000001
> +#define RLC_GPR_REG2__REQ__SHIFT 0
> +#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
> +#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
>
> static void cz_enter_rlc_safe_mode(struct amdgpu_device *adev)
> {
> @@ -5596,7 +5486,7 @@ static void cz_enter_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_GPR_REG2) & RLC_GPR_REG2__REQ_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2), RLC_GPR_REG2, REQ))
> break;
> udelay(1);
> }
> @@ -5624,7 +5514,7 @@ static void cz_exit_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_GPR_REG2) & RLC_GPR_REG2__REQ_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2), RLC_GPR_REG2, REQ))
> break;
> udelay(1);
> }
> @@ -5656,7 +5546,7 @@ static void iceland_enter_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_SAFE_MODE) & RLC_SAFE_MODE__CMD_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD))
> break;
> udelay(1);
> }
> @@ -5683,7 +5573,7 @@ static void iceland_exit_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_SAFE_MODE) & RLC_SAFE_MODE__CMD_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD))
> break;
> udelay(1);
> }
> @@ -5724,21 +5614,12 @@ static void gfx_v8_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
> /* It is disabled by HW by default */
> if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGCG)) {
> if (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGLS) {
> - if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS) {
> + if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS)
> /* 1 - RLC memory Light sleep */
> - temp = data = RREG32(mmRLC_MEM_SLP_CNTL);
> - data |= RLC_MEM_SLP_CNTL__RLC_MEM_LS_EN_MASK;
> - if (temp != data)
> - WREG32(mmRLC_MEM_SLP_CNTL, data);
> - }
> + WREG32_FIELD(RLC_MEM_SLP_CNTL, RLC_MEM_LS_EN, 1);
>
> - if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS) {
> - /* 2 - CP memory Light sleep */
> - temp = data = RREG32(mmCP_MEM_SLP_CNTL);
> - data |= CP_MEM_SLP_CNTL__CP_MEM_LS_EN_MASK;
> - if (temp != data)
> - WREG32(mmCP_MEM_SLP_CNTL, data);
> - }
> + if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS)
> + WREG32_FIELD(CP_MEM_SLP_CNTL, CP_MEM_LS_EN, 1);
> }
>
> /* 3 - RLC_CGTT_MGCG_OVERRIDE */
> @@ -6213,33 +6094,14 @@ static void gfx_v8_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
> static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
> enum amdgpu_interrupt_state state)
> {
> - u32 cp_int_cntl;
> -
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - TIME_STAMP_INT_ENABLE, 0);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl =
> - REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - TIME_STAMP_INT_ENABLE, 1);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_INT_CNTL_RING0, TIME_STAMP_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> }
>
> static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> int me, int pipe,
> enum amdgpu_interrupt_state state)
> {
> - u32 mec_int_cntl, mec_int_cntl_reg;
> -
> /*
> * amdgpu controls only pipe 0 of MEC1. That's why this function only
> * handles the setting of interrupts for this specific pipe. All other
> @@ -6249,7 +6111,6 @@ static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> if (me == 1) {
> switch (pipe) {
> case 0:
> - mec_int_cntl_reg = mmCP_ME1_PIPE0_INT_CNTL;
> break;
> default:
> DRM_DEBUG("invalid pipe %d\n", pipe);
> @@ -6260,22 +6121,8 @@ static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> return;
> }
>
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - mec_int_cntl = RREG32(mec_int_cntl_reg);
> - mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
> - TIME_STAMP_INT_ENABLE, 0);
> - WREG32(mec_int_cntl_reg, mec_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - mec_int_cntl = RREG32(mec_int_cntl_reg);
> - mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
> - TIME_STAMP_INT_ENABLE, 1);
> - WREG32(mec_int_cntl_reg, mec_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_ME1_PIPE0_INT_CNTL, TIME_STAMP_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> }
>
> static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev,
> @@ -6283,24 +6130,8 @@ static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev,
> unsigned type,
> enum amdgpu_interrupt_state state)
> {
> - u32 cp_int_cntl;
> -
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_REG_INT_ENABLE, 0);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_REG_INT_ENABLE, 1);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_REG_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
>
> return 0;
> }
> @@ -6310,24 +6141,8 @@ static int gfx_v8_0_set_priv_inst_fault_state(struct amdgpu_device *adev,
> unsigned type,
> enum amdgpu_interrupt_state state)
> {
> - u32 cp_int_cntl;
> -
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_INSTR_INT_ENABLE, 0);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_INSTR_INT_ENABLE, 1);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_INSTR_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
>
> return 0;
> }
[-- Attachment #1.2: Type: text/html, Size: 48561 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8
[not found] ` <DM5PR12MB113283968678F567DACB84A6F71C0-2J9CzHegvk+UzrhdoDeimQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-08-10 12:25 ` StDenis, Tom
[not found] ` <DM5PR12MB113243454107BC54C623786EF71D0-2J9CzHegvk+UzrhdoDeimQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: StDenis, Tom @ 2016-08-10 12:25 UTC (permalink / raw)
To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 22313 bytes --]
So is that a NAK to the series or are we good to go?
Cheers,
Tom
________________________________
From: StDenis, Tom
Sent: Tuesday, August 9, 2016 11:28
To: Christian König; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8
Probably could but I felt that implementation was cleaner.
Tom
________________________________
From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
Sent: Tuesday, August 9, 2016 10:56
To: Tom St Denis; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: StDenis, Tom
Subject: Re: [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8
Am 09.08.2016 um 16:27 schrieb Tom St Denis:
> This patch introduces a new macro WREG32_FIELD which is used
> to write to a register with a new value in a field. It's designed
> to replace the pattern:
>
> tmp = RREG32(mmFoo);
> tmp &= ~REG__FIELD_MASK;
> tmp |= new_value << REG__FIELD__SHIFT;
> WREG32(mmFoo, tmp)
>
> with:
>
> WREG32_FIELD(Foo, FIELD, new_value);
>
> Unlike WREG32_P() it understands offsets/masks and doesn't
> require the caller to shift the value (or mask properly).
>
> It's applied where suitable in the gfx_v8_0.c driver to start
> with.
>
> Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 275 ++++++----------------------------
> 2 files changed, 48 insertions(+), 230 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index c309eaf468e9..f23eb38eb3aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -2218,6 +2218,9 @@ bool amdgpu_device_has_dal_support(struct amdgpu_device *adev);
> #define REG_GET_FIELD(value, reg, field) \
> (((value) & REG_FIELD_MASK(reg, field)) >> REG_FIELD_SHIFT(reg, field))
>
> +#define WREG32_FIELD(reg, field, val) \
> + WREG32(mm##reg, (RREG32(mm##reg) & ~REG_FIELD_MASK(reg, field)) | (val) << REG_FIELD_SHIFT(reg, field))
Couldn't you use WREG32_P here to implement the new macro?
Christian.
> +
> /*
> * BIOS helpers.
> */
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 5f91a834aed2..6e01392facef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -3566,10 +3566,7 @@ static void gfx_v8_0_gpu_init(struct amdgpu_device *adev)
> u32 tmp;
> int i;
>
> - tmp = RREG32(mmGRBM_CNTL);
> - tmp = REG_SET_FIELD(tmp, GRBM_CNTL, READ_TIMEOUT, 0xff);
> - WREG32(mmGRBM_CNTL, tmp);
> -
> + WREG32_FIELD(GRBM_CNTL, READ_TIMEOUT, 0xFF);
> WREG32(mmGB_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
> WREG32(mmHDP_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
> WREG32(mmDMIF_ADDR_CALC, adev->gfx.config.gb_addr_config);
> @@ -3758,9 +3755,7 @@ static int gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
> sizeof(indirect_start_offsets)/sizeof(int));
>
> /* save and restore list */
> - temp = RREG32(mmRLC_SRM_CNTL);
> - temp |= RLC_SRM_CNTL__AUTO_INCR_ADDR_MASK;
> - WREG32(mmRLC_SRM_CNTL, temp);
> + WREG32_FIELD(RLC_SRM_CNTL, AUTO_INCR_ADDR, 1);
>
> WREG32(mmRLC_SRM_ARAM_ADDR, 0);
> for (i = 0; i < adev->gfx.rlc.reg_list_size_bytes >> 2; i++)
> @@ -3797,11 +3792,7 @@ static int gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
>
> static void gfx_v8_0_enable_save_restore_machine(struct amdgpu_device *adev)
> {
> - uint32_t data;
> -
> - data = RREG32(mmRLC_SRM_CNTL);
> - data |= RLC_SRM_CNTL__SRM_ENABLE_MASK;
> - WREG32(mmRLC_SRM_CNTL, data);
> + WREG32_FIELD(RLC_SRM_CNTL, SRM_ENABLE, 1);
> }
>
> static void gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
> @@ -3811,75 +3802,34 @@ static void gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
> if (adev->pg_flags & (AMD_PG_SUPPORT_GFX_PG |
> AMD_PG_SUPPORT_GFX_SMG |
> AMD_PG_SUPPORT_GFX_DMG)) {
> - data = RREG32(mmCP_RB_WPTR_POLL_CNTL);
> - data &= ~CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT_MASK;
> - data |= (0x60 << CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT__SHIFT);
> - WREG32(mmCP_RB_WPTR_POLL_CNTL, data);
> -
> - data = 0;
> - data |= (0x10 << RLC_PG_DELAY__POWER_UP_DELAY__SHIFT);
> - data |= (0x10 << RLC_PG_DELAY__POWER_DOWN_DELAY__SHIFT);
> - data |= (0x10 << RLC_PG_DELAY__CMD_PROPAGATE_DELAY__SHIFT);
> - data |= (0x10 << RLC_PG_DELAY__MEM_SLEEP_DELAY__SHIFT);
> - WREG32(mmRLC_PG_DELAY, data);
> + WREG32_FIELD(CP_RB_WPTR_POLL_CNTL, IDLE_POLL_COUNT, 0x60);
>
> - data = RREG32(mmRLC_PG_DELAY_2);
> - data &= ~RLC_PG_DELAY_2__SERDES_CMD_DELAY_MASK;
> - data |= (0x3 << RLC_PG_DELAY_2__SERDES_CMD_DELAY__SHIFT);
> - WREG32(mmRLC_PG_DELAY_2, data);
> + data = REG_SET_FIELD(0, RLC_PG_DELAY, POWER_UP_DELAY, 0x10);
> + data = REG_SET_FIELD(data, RLC_PG_DELAY, POWER_DOWN_DELAY, 0x10);
> + data = REG_SET_FIELD(data, RLC_PG_DELAY, CMD_PROPAGATE_DELAY, 0x10);
> + data = REG_SET_FIELD(data, RLC_PG_DELAY, MEM_SLEEP_DELAY, 0x10);
> + WREG32(mmRLC_PG_DELAY, data);
>
> - data = RREG32(mmRLC_AUTO_PG_CTRL);
> - data &= ~RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD_MASK;
> - data |= (0x55f0 << RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD__SHIFT);
> - WREG32(mmRLC_AUTO_PG_CTRL, data);
> + WREG32_FIELD(RLC_PG_DELAY_2, SERDES_CMD_DELAY, 0x3);
> + WREG32_FIELD(RLC_AUTO_PG_CTRL, GRBM_REG_SAVE_GFX_IDLE_THRESHOLD, 0x55f0);
> }
> }
>
> static void cz_enable_sck_slow_down_on_power_up(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PU_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_sck_slow_down_on_power_down(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PD_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_cp_power_gating(struct amdgpu_device *adev, bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data &= ~RLC_PG_CNTL__CP_PG_DISABLE_MASK;
> - else
> - data |= RLC_PG_CNTL__CP_PG_DISABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, CP_PG_DISABLE, enable ? 1 : 0);
> }
>
> static void gfx_v8_0_init_pg(struct amdgpu_device *adev)
> @@ -3918,34 +3868,24 @@ static void gfx_v8_0_init_pg(struct amdgpu_device *adev)
>
> void gfx_v8_0_rlc_stop(struct amdgpu_device *adev)
> {
> - u32 tmp = RREG32(mmRLC_CNTL);
> -
> - tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 0);
> - WREG32(mmRLC_CNTL, tmp);
> + WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 0);
>
> gfx_v8_0_enable_gui_idle_interrupt(adev, false);
> -
> gfx_v8_0_wait_for_rlc_serdes(adev);
> }
>
> static void gfx_v8_0_rlc_reset(struct amdgpu_device *adev)
> {
> - u32 tmp = RREG32(mmGRBM_SOFT_RESET);
> -
> - tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
> - WREG32(mmGRBM_SOFT_RESET, tmp);
> + WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
> udelay(50);
> - tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
> - WREG32(mmGRBM_SOFT_RESET, tmp);
> +
> + WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
> udelay(50);
> }
>
> static void gfx_v8_0_rlc_start(struct amdgpu_device *adev)
> {
> - u32 tmp = RREG32(mmRLC_CNTL);
> -
> - tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 1);
> - WREG32(mmRLC_CNTL, tmp);
> + WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 1);
>
> /* carrizo do enable cp interrupt after cp inited */
> if (!(adev->flags & AMD_IS_APU))
> @@ -5371,8 +5311,6 @@ static int gfx_v8_0_late_init(void *handle)
> static void gfx_v8_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - uint32_t data, temp;
> -
> if (adev->asic_type == CHIP_POLARIS11)
> /* Send msg to SMU via Powerplay */
> amdgpu_set_powergating_state(adev,
> @@ -5380,83 +5318,35 @@ static void gfx_v8_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *ade
> enable ?
> AMD_PG_STATE_GATE : AMD_PG_STATE_UNGATE);
>
> - temp = data = RREG32(mmRLC_PG_CNTL);
> - /* Enable static MGPG */
> - if (enable)
> - data |= RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
> -
> - if (temp != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, STATIC_PER_CU_PG_ENABLE, enable ? 1 : 0);
> }
>
> static void gfx_v8_0_enable_gfx_dynamic_mg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - uint32_t data, temp;
> -
> - temp = data = RREG32(mmRLC_PG_CNTL);
> - /* Enable dynamic MGPG */
> - if (enable)
> - data |= RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
> -
> - if (temp != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, DYN_PER_CU_PG_ENABLE, enable ? 1 : 0);
> }
>
> static void polaris11_enable_gfx_quick_mg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - uint32_t data, temp;
> -
> - temp = data = RREG32(mmRLC_PG_CNTL);
> - /* Enable quick PG */
> - if (enable)
> - data |= RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
> -
> - if (temp != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, QUICK_PG_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_gfx_cg_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, GFX_POWER_GATING_ENABLE, enable ? 1 : 0);
> }
>
> static void cz_enable_gfx_pipeline_power_gating(struct amdgpu_device *adev,
> bool enable)
> {
> - u32 data, orig;
> -
> - orig = data = RREG32(mmRLC_PG_CNTL);
> -
> - if (enable)
> - data |= RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
> - else
> - data &= ~RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
> -
> - if (orig != data)
> - WREG32(mmRLC_PG_CNTL, data);
> + WREG32_FIELD(RLC_PG_CNTL, GFX_PIPELINE_PG_ENABLE, enable ? 1 : 0);
>
> /* Read any GFX register to wake up GFX. */
> if (!enable)
> - data = RREG32(mmDB_RENDER_CONTROL);
> + RREG32(mmDB_RENDER_CONTROL);
> }
>
> static void cz_update_gfx_cg_power_gating(struct amdgpu_device *adev,
> @@ -5563,10 +5453,10 @@ static void gfx_v8_0_send_serdes_cmd(struct amdgpu_device *adev,
>
> #define MSG_ENTER_RLC_SAFE_MODE 1
> #define MSG_EXIT_RLC_SAFE_MODE 0
> -
> -#define RLC_GPR_REG2__REQ_MASK 0x00000001
> -#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
> -#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
> +#define RLC_GPR_REG2__REQ_MASK 0x00000001
> +#define RLC_GPR_REG2__REQ__SHIFT 0
> +#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
> +#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
>
> static void cz_enter_rlc_safe_mode(struct amdgpu_device *adev)
> {
> @@ -5596,7 +5486,7 @@ static void cz_enter_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_GPR_REG2) & RLC_GPR_REG2__REQ_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2), RLC_GPR_REG2, REQ))
> break;
> udelay(1);
> }
> @@ -5624,7 +5514,7 @@ static void cz_exit_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_GPR_REG2) & RLC_GPR_REG2__REQ_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2), RLC_GPR_REG2, REQ))
> break;
> udelay(1);
> }
> @@ -5656,7 +5546,7 @@ static void iceland_enter_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_SAFE_MODE) & RLC_SAFE_MODE__CMD_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD))
> break;
> udelay(1);
> }
> @@ -5683,7 +5573,7 @@ static void iceland_exit_rlc_safe_mode(struct amdgpu_device *adev)
> }
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if ((RREG32(mmRLC_SAFE_MODE) & RLC_SAFE_MODE__CMD_MASK) == 0)
> + if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD))
> break;
> udelay(1);
> }
> @@ -5724,21 +5614,12 @@ static void gfx_v8_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
> /* It is disabled by HW by default */
> if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGCG)) {
> if (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGLS) {
> - if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS) {
> + if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS)
> /* 1 - RLC memory Light sleep */
> - temp = data = RREG32(mmRLC_MEM_SLP_CNTL);
> - data |= RLC_MEM_SLP_CNTL__RLC_MEM_LS_EN_MASK;
> - if (temp != data)
> - WREG32(mmRLC_MEM_SLP_CNTL, data);
> - }
> + WREG32_FIELD(RLC_MEM_SLP_CNTL, RLC_MEM_LS_EN, 1);
>
> - if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS) {
> - /* 2 - CP memory Light sleep */
> - temp = data = RREG32(mmCP_MEM_SLP_CNTL);
> - data |= CP_MEM_SLP_CNTL__CP_MEM_LS_EN_MASK;
> - if (temp != data)
> - WREG32(mmCP_MEM_SLP_CNTL, data);
> - }
> + if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS)
> + WREG32_FIELD(CP_MEM_SLP_CNTL, CP_MEM_LS_EN, 1);
> }
>
> /* 3 - RLC_CGTT_MGCG_OVERRIDE */
> @@ -6213,33 +6094,14 @@ static void gfx_v8_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
> static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
> enum amdgpu_interrupt_state state)
> {
> - u32 cp_int_cntl;
> -
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - TIME_STAMP_INT_ENABLE, 0);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl =
> - REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - TIME_STAMP_INT_ENABLE, 1);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_INT_CNTL_RING0, TIME_STAMP_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> }
>
> static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> int me, int pipe,
> enum amdgpu_interrupt_state state)
> {
> - u32 mec_int_cntl, mec_int_cntl_reg;
> -
> /*
> * amdgpu controls only pipe 0 of MEC1. That's why this function only
> * handles the setting of interrupts for this specific pipe. All other
> @@ -6249,7 +6111,6 @@ static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> if (me == 1) {
> switch (pipe) {
> case 0:
> - mec_int_cntl_reg = mmCP_ME1_PIPE0_INT_CNTL;
> break;
> default:
> DRM_DEBUG("invalid pipe %d\n", pipe);
> @@ -6260,22 +6121,8 @@ static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> return;
> }
>
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - mec_int_cntl = RREG32(mec_int_cntl_reg);
> - mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
> - TIME_STAMP_INT_ENABLE, 0);
> - WREG32(mec_int_cntl_reg, mec_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - mec_int_cntl = RREG32(mec_int_cntl_reg);
> - mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
> - TIME_STAMP_INT_ENABLE, 1);
> - WREG32(mec_int_cntl_reg, mec_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_ME1_PIPE0_INT_CNTL, TIME_STAMP_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> }
>
> static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev,
> @@ -6283,24 +6130,8 @@ static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev,
> unsigned type,
> enum amdgpu_interrupt_state state)
> {
> - u32 cp_int_cntl;
> -
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_REG_INT_ENABLE, 0);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_REG_INT_ENABLE, 1);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_REG_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
>
> return 0;
> }
> @@ -6310,24 +6141,8 @@ static int gfx_v8_0_set_priv_inst_fault_state(struct amdgpu_device *adev,
> unsigned type,
> enum amdgpu_interrupt_state state)
> {
> - u32 cp_int_cntl;
> -
> - switch (state) {
> - case AMDGPU_IRQ_STATE_DISABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_INSTR_INT_ENABLE, 0);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - case AMDGPU_IRQ_STATE_ENABLE:
> - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> - cp_int_cntl = REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> - PRIV_INSTR_INT_ENABLE, 1);
> - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> - break;
> - default:
> - break;
> - }
> + WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_INSTR_INT_ENABLE,
> + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
>
> return 0;
> }
[-- Attachment #1.2: Type: text/html, Size: 49342 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8
[not found] ` <DM5PR12MB113243454107BC54C623786EF71D0-2J9CzHegvk+UzrhdoDeimQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-08-10 12:29 ` Christian König
0 siblings, 0 replies; 8+ messages in thread
From: Christian König @ 2016-08-10 12:29 UTC (permalink / raw)
To: StDenis, Tom, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 22686 bytes --]
No, not really. Just thought you could simplify this even more.
The whole set is Reviewed-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
Regards,
Christian.
Am 10.08.2016 um 14:25 schrieb StDenis, Tom:
>
> So is that a NAK to the series or are we good to go?
>
>
> Cheers,
>
> Tom
>
>
>
> ------------------------------------------------------------------------
> *From:* StDenis, Tom
> *Sent:* Tuesday, August 9, 2016 11:28
> *To:* Christian König; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield
> operations in gfx v8
>
> Probably could but I felt that implementation was cleaner.
>
>
> Tom
>
>
>
> ------------------------------------------------------------------------
> *From:* Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
> *Sent:* Tuesday, August 9, 2016 10:56
> *To:* Tom St Denis; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Cc:* StDenis, Tom
> *Subject:* Re: [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield
> operations in gfx v8
> Am 09.08.2016 um 16:27 schrieb Tom St Denis:
> > This patch introduces a new macro WREG32_FIELD which is used
> > to write to a register with a new value in a field. It's designed
> > to replace the pattern:
> >
> > tmp = RREG32(mmFoo);
> > tmp &= ~REG__FIELD_MASK;
> > tmp |= new_value << REG__FIELD__SHIFT;
> > WREG32(mmFoo, tmp)
> >
> > with:
> >
> > WREG32_FIELD(Foo, FIELD, new_value);
> >
> > Unlike WREG32_P() it understands offsets/masks and doesn't
> > require the caller to shift the value (or mask properly).
> >
> > It's applied where suitable in the gfx_v8_0.c driver to start
> > with.
> >
> > Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +
> > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 275
> ++++++----------------------------
> > 2 files changed, 48 insertions(+), 230 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index c309eaf468e9..f23eb38eb3aa 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -2218,6 +2218,9 @@ bool amdgpu_device_has_dal_support(struct
> amdgpu_device *adev);
> > #define REG_GET_FIELD(value, reg, field) \
> > (((value) & REG_FIELD_MASK(reg, field)) >>
> REG_FIELD_SHIFT(reg, field))
> >
> > +#define WREG32_FIELD(reg, field, val) \
> > + WREG32(mm##reg, (RREG32(mm##reg) & ~REG_FIELD_MASK(reg,
> field)) | (val) << REG_FIELD_SHIFT(reg, field))
>
> Couldn't you use WREG32_P here to implement the new macro?
>
> Christian.
>
> > +
> > /*
> > * BIOS helpers.
> > */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 5f91a834aed2..6e01392facef 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -3566,10 +3566,7 @@ static void gfx_v8_0_gpu_init(struct
> amdgpu_device *adev)
> > u32 tmp;
> > int i;
> >
> > - tmp = RREG32(mmGRBM_CNTL);
> > - tmp = REG_SET_FIELD(tmp, GRBM_CNTL, READ_TIMEOUT, 0xff);
> > - WREG32(mmGRBM_CNTL, tmp);
> > -
> > + WREG32_FIELD(GRBM_CNTL, READ_TIMEOUT, 0xFF);
> > WREG32(mmGB_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
> > WREG32(mmHDP_ADDR_CONFIG, adev->gfx.config.gb_addr_config);
> > WREG32(mmDMIF_ADDR_CALC, adev->gfx.config.gb_addr_config);
> > @@ -3758,9 +3755,7 @@ static int
> gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
> > sizeof(indirect_start_offsets)/sizeof(int));
> >
> > /* save and restore list */
> > - temp = RREG32(mmRLC_SRM_CNTL);
> > - temp |= RLC_SRM_CNTL__AUTO_INCR_ADDR_MASK;
> > - WREG32(mmRLC_SRM_CNTL, temp);
> > + WREG32_FIELD(RLC_SRM_CNTL, AUTO_INCR_ADDR, 1);
> >
> > WREG32(mmRLC_SRM_ARAM_ADDR, 0);
> > for (i = 0; i < adev->gfx.rlc.reg_list_size_bytes >> 2; i++)
> > @@ -3797,11 +3792,7 @@ static int
> gfx_v8_0_init_save_restore_list(struct amdgpu_device *adev)
> >
> > static void gfx_v8_0_enable_save_restore_machine(struct
> amdgpu_device *adev)
> > {
> > - uint32_t data;
> > -
> > - data = RREG32(mmRLC_SRM_CNTL);
> > - data |= RLC_SRM_CNTL__SRM_ENABLE_MASK;
> > - WREG32(mmRLC_SRM_CNTL, data);
> > + WREG32_FIELD(RLC_SRM_CNTL, SRM_ENABLE, 1);
> > }
> >
> > static void gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
> > @@ -3811,75 +3802,34 @@ static void
> gfx_v8_0_init_power_gating(struct amdgpu_device *adev)
> > if (adev->pg_flags & (AMD_PG_SUPPORT_GFX_PG |
> > AMD_PG_SUPPORT_GFX_SMG |
> > AMD_PG_SUPPORT_GFX_DMG)) {
> > - data = RREG32(mmCP_RB_WPTR_POLL_CNTL);
> > - data &= ~CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT_MASK;
> > - data |= (0x60 <<
> CP_RB_WPTR_POLL_CNTL__IDLE_POLL_COUNT__SHIFT);
> > - WREG32(mmCP_RB_WPTR_POLL_CNTL, data);
> > -
> > - data = 0;
> > - data |= (0x10 << RLC_PG_DELAY__POWER_UP_DELAY__SHIFT);
> > - data |= (0x10 << RLC_PG_DELAY__POWER_DOWN_DELAY__SHIFT);
> > - data |= (0x10 <<
> RLC_PG_DELAY__CMD_PROPAGATE_DELAY__SHIFT);
> > - data |= (0x10 << RLC_PG_DELAY__MEM_SLEEP_DELAY__SHIFT);
> > - WREG32(mmRLC_PG_DELAY, data);
> > + WREG32_FIELD(CP_RB_WPTR_POLL_CNTL, IDLE_POLL_COUNT, 0x60);
> >
> > - data = RREG32(mmRLC_PG_DELAY_2);
> > - data &= ~RLC_PG_DELAY_2__SERDES_CMD_DELAY_MASK;
> > - data |= (0x3 << RLC_PG_DELAY_2__SERDES_CMD_DELAY__SHIFT);
> > - WREG32(mmRLC_PG_DELAY_2, data);
> > + data = REG_SET_FIELD(0, RLC_PG_DELAY, POWER_UP_DELAY,
> 0x10);
> > + data = REG_SET_FIELD(data, RLC_PG_DELAY,
> POWER_DOWN_DELAY, 0x10);
> > + data = REG_SET_FIELD(data, RLC_PG_DELAY,
> CMD_PROPAGATE_DELAY, 0x10);
> > + data = REG_SET_FIELD(data, RLC_PG_DELAY,
> MEM_SLEEP_DELAY, 0x10);
> > + WREG32(mmRLC_PG_DELAY, data);
> >
> > - data = RREG32(mmRLC_AUTO_PG_CTRL);
> > - data &=
> ~RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD_MASK;
> > - data |= (0x55f0 <<
> RLC_AUTO_PG_CTRL__GRBM_REG_SAVE_GFX_IDLE_THRESHOLD__SHIFT);
> > - WREG32(mmRLC_AUTO_PG_CTRL, data);
> > + WREG32_FIELD(RLC_PG_DELAY_2, SERDES_CMD_DELAY, 0x3);
> > + WREG32_FIELD(RLC_AUTO_PG_CTRL,
> GRBM_REG_SAVE_GFX_IDLE_THRESHOLD, 0x55f0);
> > }
> > }
> >
> > static void cz_enable_sck_slow_down_on_power_up(struct
> amdgpu_device *adev,
> > bool enable)
> > {
> > - u32 data, orig;
> > -
> > - orig = data = RREG32(mmRLC_PG_CNTL);
> > -
> > - if (enable)
> > - data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
> > - else
> > - data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PU_ENABLE_MASK;
> > -
> > - if (orig != data)
> > - WREG32(mmRLC_PG_CNTL, data);
> > + WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PU_ENABLE,
> enable ? 1 : 0);
> > }
> >
> > static void cz_enable_sck_slow_down_on_power_down(struct
> amdgpu_device *adev,
> > bool enable)
> > {
> > - u32 data, orig;
> > -
> > - orig = data = RREG32(mmRLC_PG_CNTL);
> > -
> > - if (enable)
> > - data |= RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
> > - else
> > - data &= ~RLC_PG_CNTL__SMU_CLK_SLOWDOWN_ON_PD_ENABLE_MASK;
> > -
> > - if (orig != data)
> > - WREG32(mmRLC_PG_CNTL, data);
> > + WREG32_FIELD(RLC_PG_CNTL, SMU_CLK_SLOWDOWN_ON_PD_ENABLE,
> enable ? 1 : 0);
> > }
> >
> > static void cz_enable_cp_power_gating(struct amdgpu_device *adev,
> bool enable)
> > {
> > - u32 data, orig;
> > -
> > - orig = data = RREG32(mmRLC_PG_CNTL);
> > -
> > - if (enable)
> > - data &= ~RLC_PG_CNTL__CP_PG_DISABLE_MASK;
> > - else
> > - data |= RLC_PG_CNTL__CP_PG_DISABLE_MASK;
> > -
> > - if (orig != data)
> > - WREG32(mmRLC_PG_CNTL, data);
> > + WREG32_FIELD(RLC_PG_CNTL, CP_PG_DISABLE, enable ? 1 : 0);
> > }
> >
> > static void gfx_v8_0_init_pg(struct amdgpu_device *adev)
> > @@ -3918,34 +3868,24 @@ static void gfx_v8_0_init_pg(struct
> amdgpu_device *adev)
> >
> > void gfx_v8_0_rlc_stop(struct amdgpu_device *adev)
> > {
> > - u32 tmp = RREG32(mmRLC_CNTL);
> > -
> > - tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 0);
> > - WREG32(mmRLC_CNTL, tmp);
> > + WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 0);
> >
> > gfx_v8_0_enable_gui_idle_interrupt(adev, false);
> > -
> > gfx_v8_0_wait_for_rlc_serdes(adev);
> > }
> >
> > static void gfx_v8_0_rlc_reset(struct amdgpu_device *adev)
> > {
> > - u32 tmp = RREG32(mmGRBM_SOFT_RESET);
> > -
> > - tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
> > - WREG32(mmGRBM_SOFT_RESET, tmp);
> > + WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 1);
> > udelay(50);
> > - tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
> > - WREG32(mmGRBM_SOFT_RESET, tmp);
> > +
> > + WREG32_FIELD(GRBM_SOFT_RESET, SOFT_RESET_RLC, 0);
> > udelay(50);
> > }
> >
> > static void gfx_v8_0_rlc_start(struct amdgpu_device *adev)
> > {
> > - u32 tmp = RREG32(mmRLC_CNTL);
> > -
> > - tmp = REG_SET_FIELD(tmp, RLC_CNTL, RLC_ENABLE_F32, 1);
> > - WREG32(mmRLC_CNTL, tmp);
> > + WREG32_FIELD(RLC_CNTL, RLC_ENABLE_F32, 1);
> >
> > /* carrizo do enable cp interrupt after cp inited */
> > if (!(adev->flags & AMD_IS_APU))
> > @@ -5371,8 +5311,6 @@ static int gfx_v8_0_late_init(void *handle)
> > static void gfx_v8_0_enable_gfx_static_mg_power_gating(struct
> amdgpu_device *adev,
> > bool enable)
> > {
> > - uint32_t data, temp;
> > -
> > if (adev->asic_type == CHIP_POLARIS11)
> > /* Send msg to SMU via Powerplay */
> > amdgpu_set_powergating_state(adev,
> > @@ -5380,83 +5318,35 @@ static void
> gfx_v8_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *ade
> > enable ?
> > AMD_PG_STATE_GATE : AMD_PG_STATE_UNGATE);
> >
> > - temp = data = RREG32(mmRLC_PG_CNTL);
> > - /* Enable static MGPG */
> > - if (enable)
> > - data |= RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
> > - else
> > - data &= ~RLC_PG_CNTL__STATIC_PER_CU_PG_ENABLE_MASK;
> > -
> > - if (temp != data)
> > - WREG32(mmRLC_PG_CNTL, data);
> > + WREG32_FIELD(RLC_PG_CNTL, STATIC_PER_CU_PG_ENABLE, enable ? 1
> : 0);
> > }
> >
> > static void gfx_v8_0_enable_gfx_dynamic_mg_power_gating(struct
> amdgpu_device *adev,
> > bool enable)
> > {
> > - uint32_t data, temp;
> > -
> > - temp = data = RREG32(mmRLC_PG_CNTL);
> > - /* Enable dynamic MGPG */
> > - if (enable)
> > - data |= RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
> > - else
> > - data &= ~RLC_PG_CNTL__DYN_PER_CU_PG_ENABLE_MASK;
> > -
> > - if (temp != data)
> > - WREG32(mmRLC_PG_CNTL, data);
> > + WREG32_FIELD(RLC_PG_CNTL, DYN_PER_CU_PG_ENABLE, enable ? 1 : 0);
> > }
> >
> > static void polaris11_enable_gfx_quick_mg_power_gating(struct
> amdgpu_device *adev,
> > bool enable)
> > {
> > - uint32_t data, temp;
> > -
> > - temp = data = RREG32(mmRLC_PG_CNTL);
> > - /* Enable quick PG */
> > - if (enable)
> > - data |= RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
> > - else
> > - data &= ~RLC_PG_CNTL__QUICK_PG_ENABLE_MASK;
> > -
> > - if (temp != data)
> > - WREG32(mmRLC_PG_CNTL, data);
> > + WREG32_FIELD(RLC_PG_CNTL, QUICK_PG_ENABLE, enable ? 1 : 0);
> > }
> >
> > static void cz_enable_gfx_cg_power_gating(struct amdgpu_device *adev,
> > bool enable)
> > {
> > - u32 data, orig;
> > -
> > - orig = data = RREG32(mmRLC_PG_CNTL);
> > -
> > - if (enable)
> > - data |= RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
> > - else
> > - data &= ~RLC_PG_CNTL__GFX_POWER_GATING_ENABLE_MASK;
> > -
> > - if (orig != data)
> > - WREG32(mmRLC_PG_CNTL, data);
> > + WREG32_FIELD(RLC_PG_CNTL, GFX_POWER_GATING_ENABLE, enable ? 1
> : 0);
> > }
> >
> > static void cz_enable_gfx_pipeline_power_gating(struct
> amdgpu_device *adev,
> > bool enable)
> > {
> > - u32 data, orig;
> > -
> > - orig = data = RREG32(mmRLC_PG_CNTL);
> > -
> > - if (enable)
> > - data |= RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
> > - else
> > - data &= ~RLC_PG_CNTL__GFX_PIPELINE_PG_ENABLE_MASK;
> > -
> > - if (orig != data)
> > - WREG32(mmRLC_PG_CNTL, data);
> > + WREG32_FIELD(RLC_PG_CNTL, GFX_PIPELINE_PG_ENABLE, enable ? 1 : 0);
> >
> > /* Read any GFX register to wake up GFX. */
> > if (!enable)
> > - data = RREG32(mmDB_RENDER_CONTROL);
> > + RREG32(mmDB_RENDER_CONTROL);
> > }
> >
> > static void cz_update_gfx_cg_power_gating(struct amdgpu_device *adev,
> > @@ -5563,10 +5453,10 @@ static void gfx_v8_0_send_serdes_cmd(struct
> amdgpu_device *adev,
> >
> > #define MSG_ENTER_RLC_SAFE_MODE 1
> > #define MSG_EXIT_RLC_SAFE_MODE 0
> > -
> > -#define RLC_GPR_REG2__REQ_MASK 0x00000001
> > -#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
> > -#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
> > +#define RLC_GPR_REG2__REQ_MASK 0x00000001
> > +#define RLC_GPR_REG2__REQ__SHIFT 0
> > +#define RLC_GPR_REG2__MESSAGE__SHIFT 0x00000001
> > +#define RLC_GPR_REG2__MESSAGE_MASK 0x0000001e
> >
> > static void cz_enter_rlc_safe_mode(struct amdgpu_device *adev)
> > {
> > @@ -5596,7 +5486,7 @@ static void cz_enter_rlc_safe_mode(struct
> amdgpu_device *adev)
> > }
> >
> > for (i = 0; i < adev->usec_timeout; i++) {
> > - if ((RREG32(mmRLC_GPR_REG2) &
> RLC_GPR_REG2__REQ_MASK) == 0)
> > + if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2),
> RLC_GPR_REG2, REQ))
> > break;
> > udelay(1);
> > }
> > @@ -5624,7 +5514,7 @@ static void cz_exit_rlc_safe_mode(struct
> amdgpu_device *adev)
> > }
> >
> > for (i = 0; i < adev->usec_timeout; i++) {
> > - if ((RREG32(mmRLC_GPR_REG2) & RLC_GPR_REG2__REQ_MASK)
> == 0)
> > + if (!REG_GET_FIELD(RREG32(mmRLC_GPR_REG2),
> RLC_GPR_REG2, REQ))
> > break;
> > udelay(1);
> > }
> > @@ -5656,7 +5546,7 @@ static void iceland_enter_rlc_safe_mode(struct
> amdgpu_device *adev)
> > }
> >
> > for (i = 0; i < adev->usec_timeout; i++) {
> > - if ((RREG32(mmRLC_SAFE_MODE) &
> RLC_SAFE_MODE__CMD_MASK) == 0)
> > + if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE),
> RLC_SAFE_MODE, CMD))
> > break;
> > udelay(1);
> > }
> > @@ -5683,7 +5573,7 @@ static void iceland_exit_rlc_safe_mode(struct
> amdgpu_device *adev)
> > }
> >
> > for (i = 0; i < adev->usec_timeout; i++) {
> > - if ((RREG32(mmRLC_SAFE_MODE) &
> RLC_SAFE_MODE__CMD_MASK) == 0)
> > + if (!REG_GET_FIELD(RREG32(mmRLC_SAFE_MODE),
> RLC_SAFE_MODE, CMD))
> > break;
> > udelay(1);
> > }
> > @@ -5724,21 +5614,12 @@ static void
> gfx_v8_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
> > /* It is disabled by HW by default */
> > if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGCG)) {
> > if (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGLS) {
> > - if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS) {
> > + if (adev->cg_flags & AMD_CG_SUPPORT_GFX_RLC_LS)
> > /* 1 - RLC memory Light sleep */
> > - temp = data = RREG32(mmRLC_MEM_SLP_CNTL);
> > - data |=
> RLC_MEM_SLP_CNTL__RLC_MEM_LS_EN_MASK;
> > - if (temp != data)
> > - WREG32(mmRLC_MEM_SLP_CNTL, data);
> > - }
> > + WREG32_FIELD(RLC_MEM_SLP_CNTL, RLC_MEM_LS_EN, 1);
> >
> > - if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS) {
> > - /* 2 - CP memory Light sleep */
> > - temp = data = RREG32(mmCP_MEM_SLP_CNTL);
> > - data |=
> CP_MEM_SLP_CNTL__CP_MEM_LS_EN_MASK;
> > - if (temp != data)
> > - WREG32(mmCP_MEM_SLP_CNTL, data);
> > - }
> > + if (adev->cg_flags & AMD_CG_SUPPORT_GFX_CP_LS)
> > + WREG32_FIELD(CP_MEM_SLP_CNTL, CP_MEM_LS_EN, 1);
> > }
> >
> > /* 3 - RLC_CGTT_MGCG_OVERRIDE */
> > @@ -6213,33 +6094,14 @@ static void
> gfx_v8_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
> > static void gfx_v8_0_set_gfx_eop_interrupt_state(struct
> amdgpu_device *adev,
> > enum amdgpu_interrupt_state state)
> > {
> > - u32 cp_int_cntl;
> > -
> > - switch (state) {
> > - case AMDGPU_IRQ_STATE_DISABLE:
> > - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> > - cp_int_cntl = REG_SET_FIELD(cp_int_cntl,
> CP_INT_CNTL_RING0,
> > - TIME_STAMP_INT_ENABLE, 0);
> > - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> > - break;
> > - case AMDGPU_IRQ_STATE_ENABLE:
> > - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> > - cp_int_cntl =
> > - REG_SET_FIELD(cp_int_cntl, CP_INT_CNTL_RING0,
> > - TIME_STAMP_INT_ENABLE, 1);
> > - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> > - break;
> > - default:
> > - break;
> > - }
> > + WREG32_FIELD(CP_INT_CNTL_RING0, TIME_STAMP_INT_ENABLE,
> > + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> > }
> >
> > static void gfx_v8_0_set_compute_eop_interrupt_state(struct
> amdgpu_device *adev,
> > int me, int pipe,
> > enum amdgpu_interrupt_state state)
> > {
> > - u32 mec_int_cntl, mec_int_cntl_reg;
> > -
> > /*
> > * amdgpu controls only pipe 0 of MEC1. That's why this
> function only
> > * handles the setting of interrupts for this specific pipe.
> All other
> > @@ -6249,7 +6111,6 @@ static void
> gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> > if (me == 1) {
> > switch (pipe) {
> > case 0:
> > - mec_int_cntl_reg = mmCP_ME1_PIPE0_INT_CNTL;
> > break;
> > default:
> > DRM_DEBUG("invalid pipe %d\n", pipe);
> > @@ -6260,22 +6121,8 @@ static void
> gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev,
> > return;
> > }
> >
> > - switch (state) {
> > - case AMDGPU_IRQ_STATE_DISABLE:
> > - mec_int_cntl = RREG32(mec_int_cntl_reg);
> > - mec_int_cntl = REG_SET_FIELD(mec_int_cntl,
> CP_ME1_PIPE0_INT_CNTL,
> > - TIME_STAMP_INT_ENABLE, 0);
> > - WREG32(mec_int_cntl_reg, mec_int_cntl);
> > - break;
> > - case AMDGPU_IRQ_STATE_ENABLE:
> > - mec_int_cntl = RREG32(mec_int_cntl_reg);
> > - mec_int_cntl = REG_SET_FIELD(mec_int_cntl,
> CP_ME1_PIPE0_INT_CNTL,
> > - TIME_STAMP_INT_ENABLE, 1);
> > - WREG32(mec_int_cntl_reg, mec_int_cntl);
> > - break;
> > - default:
> > - break;
> > - }
> > + WREG32_FIELD(CP_ME1_PIPE0_INT_CNTL, TIME_STAMP_INT_ENABLE,
> > + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> > }
> >
> > static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device
> *adev,
> > @@ -6283,24 +6130,8 @@ static int
> gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev,
> > unsigned type,
> > enum amdgpu_interrupt_state state)
> > {
> > - u32 cp_int_cntl;
> > -
> > - switch (state) {
> > - case AMDGPU_IRQ_STATE_DISABLE:
> > - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> > - cp_int_cntl = REG_SET_FIELD(cp_int_cntl,
> CP_INT_CNTL_RING0,
> > - PRIV_REG_INT_ENABLE, 0);
> > - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> > - break;
> > - case AMDGPU_IRQ_STATE_ENABLE:
> > - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> > - cp_int_cntl = REG_SET_FIELD(cp_int_cntl,
> CP_INT_CNTL_RING0,
> > - PRIV_REG_INT_ENABLE, 1);
> > - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> > - break;
> > - default:
> > - break;
> > - }
> > + WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_REG_INT_ENABLE,
> > + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> >
> > return 0;
> > }
> > @@ -6310,24 +6141,8 @@ static int
> gfx_v8_0_set_priv_inst_fault_state(struct amdgpu_device *adev,
> > unsigned type,
> > enum amdgpu_interrupt_state state)
> > {
> > - u32 cp_int_cntl;
> > -
> > - switch (state) {
> > - case AMDGPU_IRQ_STATE_DISABLE:
> > - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> > - cp_int_cntl = REG_SET_FIELD(cp_int_cntl,
> CP_INT_CNTL_RING0,
> > - PRIV_INSTR_INT_ENABLE, 0);
> > - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> > - break;
> > - case AMDGPU_IRQ_STATE_ENABLE:
> > - cp_int_cntl = RREG32(mmCP_INT_CNTL_RING0);
> > - cp_int_cntl = REG_SET_FIELD(cp_int_cntl,
> CP_INT_CNTL_RING0,
> > - PRIV_INSTR_INT_ENABLE, 1);
> > - WREG32(mmCP_INT_CNTL_RING0, cp_int_cntl);
> > - break;
> > - default:
> > - break;
> > - }
> > + WREG32_FIELD(CP_INT_CNTL_RING0, PRIV_INSTR_INT_ENABLE,
> > + state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1);
> >
> > return 0;
> > }
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 50492 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-10 12:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09 14:27 Various cleanups for gfx_v8_0.c Tom St Denis
[not found] ` <20160809142739.1401-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-09 14:27 ` [PATCH 1/3] drm/amd/amdgpu: Correct whitespace in GFX v8 Tom St Denis
2016-08-09 14:27 ` [PATCH 2/3] drm/amd/amdgpu: Simplify various gfx v8 functions Tom St Denis
2016-08-09 14:27 ` [PATCH 3/3] drm/amd/amdgpu: Simplify bitfield operations in gfx v8 Tom St Denis
[not found] ` <20160809142739.1401-4-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-09 14:56 ` Christian König
[not found] ` <dc95cc72-5e8e-6c5e-7c87-979d838652e7-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2016-08-09 15:28 ` StDenis, Tom
[not found] ` <DM5PR12MB113283968678F567DACB84A6F71C0-2J9CzHegvk+UzrhdoDeimQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-08-10 12:25 ` StDenis, Tom
[not found] ` <DM5PR12MB113243454107BC54C623786EF71D0-2J9CzHegvk+UzrhdoDeimQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-08-10 12:29 ` Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.