* Various tidy-ups for VCE/UVD
@ 2016-08-11 14:32 Tom St Denis
[not found] ` <20160811143253.24718-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Tom St Denis @ 2016-08-11 14:32 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Various cleanups using WREG32_FIELD as well as a bug fix in
the soft_reset check for VCEv3 which didn't hold the GRBM lock
around accessing GRBM_GFX_IDX.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3
[not found] ` <20160811143253.24718-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2016-08-11 14:32 ` Tom St Denis
[not found] ` <20160811143253.24718-2-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 14:32 ` [PATCH 2/4] drm/amd/amdgpu: add mutex in check_soft for " Tom St Denis
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Tom St Denis @ 2016-08-11 14:32 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/vce_v3_0.c | 145 ++++++++++------------------------
1 file changed, 43 insertions(+), 102 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
index 7e6bb45658f6..073cf9ed0674 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
@@ -110,22 +110,13 @@ static void vce_v3_0_ring_set_wptr(struct amdgpu_ring *ring)
static void vce_v3_0_override_vce_clock_gating(struct amdgpu_device *adev, bool override)
{
- u32 tmp, data;
-
- tmp = data = RREG32(mmVCE_RB_ARB_CTRL);
- if (override)
- data |= VCE_RB_ARB_CTRL__VCE_CGTT_OVERRIDE_MASK;
- else
- data &= ~VCE_RB_ARB_CTRL__VCE_CGTT_OVERRIDE_MASK;
-
- if (tmp != data)
- WREG32(mmVCE_RB_ARB_CTRL, data);
+ WREG32_FIELD(VCE_RB_ARB_CTRL, VCE_CGTT_OVERRIDE, override ? 1 : 0);
}
static void vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device *adev,
bool gated)
{
- u32 tmp, data;
+ u32 data;
/* Set Override to disable Clock Gating */
vce_v3_0_override_vce_clock_gating(adev, true);
@@ -136,65 +127,55 @@ static void vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device *adev,
fly as necessary.
*/
if (gated) {
- tmp = data = RREG32(mmVCE_CLOCK_GATING_B);
+ data = RREG32(mmVCE_CLOCK_GATING_B);
data |= 0x1ff;
data &= ~0xef0000;
- if (tmp != data)
- WREG32(mmVCE_CLOCK_GATING_B, data);
+ WREG32(mmVCE_CLOCK_GATING_B, data);
- tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING);
+ data = RREG32(mmVCE_UENC_CLOCK_GATING);
data |= 0x3ff000;
data &= ~0xffc00000;
- if (tmp != data)
- WREG32(mmVCE_UENC_CLOCK_GATING, data);
+ WREG32(mmVCE_UENC_CLOCK_GATING, data);
- tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
+ data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
data |= 0x2;
data &= ~0x00010000;
- if (tmp != data)
- WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
+ WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
- tmp = data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
+ data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
data |= 0x37f;
- if (tmp != data)
- WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
+ WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
- tmp = data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
+ data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
data |= VCE_UENC_DMA_DCLK_CTRL__WRDMCLK_FORCEON_MASK |
VCE_UENC_DMA_DCLK_CTRL__RDDMCLK_FORCEON_MASK |
VCE_UENC_DMA_DCLK_CTRL__REGCLK_FORCEON_MASK |
0x8;
- if (tmp != data)
- WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
+ WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
} else {
- tmp = data = RREG32(mmVCE_CLOCK_GATING_B);
+ data = RREG32(mmVCE_CLOCK_GATING_B);
data &= ~0x80010;
data |= 0xe70008;
- if (tmp != data)
- WREG32(mmVCE_CLOCK_GATING_B, data);
+ WREG32(mmVCE_CLOCK_GATING_B, data);
- tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING);
+ data = RREG32(mmVCE_UENC_CLOCK_GATING);
data |= 0xffc00000;
- if (tmp != data)
- WREG32(mmVCE_UENC_CLOCK_GATING, data);
+ WREG32(mmVCE_UENC_CLOCK_GATING, data);
- tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
+ data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
data |= 0x10000;
- if (tmp != data)
- WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
+ WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
- tmp = data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
+ data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
data &= ~0xffc00000;
- if (tmp != data)
- WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
+ WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
- tmp = data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
+ data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
data &= ~(VCE_UENC_DMA_DCLK_CTRL__WRDMCLK_FORCEON_MASK |
VCE_UENC_DMA_DCLK_CTRL__RDDMCLK_FORCEON_MASK |
VCE_UENC_DMA_DCLK_CTRL__REGCLK_FORCEON_MASK |
0x8);
- if (tmp != data)
- WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
+ WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
}
vce_v3_0_override_vce_clock_gating(adev, false);
}
@@ -213,12 +194,9 @@ static int vce_v3_0_firmware_loaded(struct amdgpu_device *adev)
}
DRM_ERROR("VCE not responding, trying to reset the ECPU!!!\n");
- WREG32_P(mmVCE_SOFT_RESET,
- VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
- ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
mdelay(10);
- WREG32_P(mmVCE_SOFT_RESET, 0,
- ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
mdelay(10);
}
@@ -256,34 +234,22 @@ static int vce_v3_0_start(struct amdgpu_device *adev)
if (adev->vce.harvest_config & (1 << idx))
continue;
- if (idx == 0)
- WREG32_P(mmGRBM_GFX_INDEX, 0,
- ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
- else
- WREG32_P(mmGRBM_GFX_INDEX,
- GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
- ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
-
+ WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, idx);
vce_v3_0_mc_resume(adev, idx);
-
- WREG32_P(mmVCE_STATUS, VCE_STATUS__JOB_BUSY_MASK,
- ~VCE_STATUS__JOB_BUSY_MASK);
+ WREG32_FIELD(VCE_STATUS, JOB_BUSY, 1);
if (adev->asic_type >= CHIP_STONEY)
WREG32_P(mmVCE_VCPU_CNTL, 1, ~0x200001);
else
- WREG32_P(mmVCE_VCPU_CNTL, VCE_VCPU_CNTL__CLK_EN_MASK,
- ~VCE_VCPU_CNTL__CLK_EN_MASK);
-
- WREG32_P(mmVCE_SOFT_RESET, 0,
- ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
+ WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
mdelay(100);
r = vce_v3_0_firmware_loaded(adev);
/* clear BUSY flag */
- WREG32_P(mmVCE_STATUS, 0, ~VCE_STATUS__JOB_BUSY_MASK);
+ WREG32_FIELD(VCE_STATUS, JOB_BUSY, 0);
if (r) {
DRM_ERROR("VCE not responding, giving up!!!\n");
@@ -292,7 +258,7 @@ static int vce_v3_0_start(struct amdgpu_device *adev)
}
}
- WREG32_P(mmGRBM_GFX_INDEX, 0, ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
+ WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
mutex_unlock(&adev->grbm_idx_mutex);
return 0;
@@ -307,33 +273,25 @@ static int vce_v3_0_stop(struct amdgpu_device *adev)
if (adev->vce.harvest_config & (1 << idx))
continue;
- if (idx == 0)
- WREG32_P(mmGRBM_GFX_INDEX, 0,
- ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
- else
- WREG32_P(mmGRBM_GFX_INDEX,
- GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
- ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
+ WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, idx);
if (adev->asic_type >= CHIP_STONEY)
WREG32_P(mmVCE_VCPU_CNTL, 0, ~0x200001);
else
- WREG32_P(mmVCE_VCPU_CNTL, 0,
- ~VCE_VCPU_CNTL__CLK_EN_MASK);
+ WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
+
/* hold on ECPU */
- WREG32_P(mmVCE_SOFT_RESET,
- VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
- ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
/* clear BUSY flag */
- WREG32_P(mmVCE_STATUS, 0, ~VCE_STATUS__JOB_BUSY_MASK);
+ WREG32_FIELD(VCE_STATUS, JOB_BUSY, 0);
/* Set Clock-Gating off */
if (adev->cg_flags & AMD_CG_SUPPORT_VCE_MGCG)
vce_v3_0_set_vce_sw_clock_gating(adev, false);
}
- WREG32_P(mmGRBM_GFX_INDEX, 0, ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
+ WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
mutex_unlock(&adev->grbm_idx_mutex);
return 0;
@@ -561,9 +519,7 @@ static void vce_v3_0_mc_resume(struct amdgpu_device *adev, int idx)
}
WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
-
- WREG32_P(mmVCE_SYS_INT_EN, VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
- ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
+ WREG32_FIELD(VCE_SYS_INT_EN, VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
}
static bool vce_v3_0_is_idle(void *handle)
@@ -599,7 +555,6 @@ static int vce_v3_0_check_soft_reset(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
u32 srbm_soft_reset = 0;
- u32 tmp;
/* According to VCE team , we should use VCE_STATUS instead
* SRBM_STATUS.VCE_BUSY bit for busy status checking.
@@ -614,23 +569,17 @@ static int vce_v3_0_check_soft_reset(void *handle)
*
* VCE team suggest use bit 3--bit 6 for busy status check
*/
- tmp = RREG32(mmGRBM_GFX_INDEX);
- tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
- WREG32(mmGRBM_GFX_INDEX, tmp);
+ WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
if (RREG32(mmVCE_STATUS) & AMDGPU_VCE_STATUS_BUSY_MASK) {
srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
}
- tmp = RREG32(mmGRBM_GFX_INDEX);
- tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX, 0x10);
- WREG32(mmGRBM_GFX_INDEX, tmp);
+ WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0x10);
if (RREG32(mmVCE_STATUS) & AMDGPU_VCE_STATUS_BUSY_MASK) {
srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
}
- tmp = RREG32(mmGRBM_GFX_INDEX);
- tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
- WREG32(mmGRBM_GFX_INDEX, tmp);
+ WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
if (srbm_soft_reset) {
adev->ip_block_status[AMD_IP_BLOCK_TYPE_VCE].hang = true;
@@ -718,9 +667,7 @@ static int vce_v3_0_process_interrupt(struct amdgpu_device *adev,
{
DRM_DEBUG("IH: VCE\n");
- WREG32_P(mmVCE_SYS_INT_STATUS,
- VCE_SYS_INT_STATUS__VCE_SYS_INT_TRAP_INTERRUPT_INT_MASK,
- ~VCE_SYS_INT_STATUS__VCE_SYS_INT_TRAP_INTERRUPT_INT_MASK);
+ WREG32_FIELD(VCE_SYS_INT_STATUS, VCE_SYS_INT_TRAP_INTERRUPT_INT, 1);
switch (entry->src_data) {
case 0:
@@ -767,13 +714,7 @@ static int vce_v3_0_set_clockgating_state(void *handle,
if (adev->vce.harvest_config & (1 << i))
continue;
- if (i == 0)
- WREG32_P(mmGRBM_GFX_INDEX, 0,
- ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
- else
- WREG32_P(mmGRBM_GFX_INDEX,
- GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
- ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
+ WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, i);
if (enable) {
/* initialize VCE_CLOCK_GATING_A: Clock ON/OFF delay */
@@ -792,7 +733,7 @@ static int vce_v3_0_set_clockgating_state(void *handle,
vce_v3_0_set_vce_sw_clock_gating(adev, enable);
}
- WREG32_P(mmGRBM_GFX_INDEX, 0, ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
+ WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
mutex_unlock(&adev->grbm_idx_mutex);
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] 17+ messages in thread
* [PATCH 2/4] drm/amd/amdgpu: add mutex in check_soft for VCE v3
[not found] ` <20160811143253.24718-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 14:32 ` [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3 Tom St Denis
@ 2016-08-11 14:32 ` Tom St Denis
[not found] ` <20160811143253.24718-3-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 14:32 ` [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup Tom St Denis
2016-08-11 14:32 ` [PATCH 4/4] drm/amd/amdgpu: UVD v6 " Tom St Denis
3 siblings, 1 reply; 17+ messages in thread
From: Tom St Denis @ 2016-08-11 14:32 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/vce_v3_0.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
index 073cf9ed0674..615b8b16ad04 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
@@ -569,6 +569,7 @@ static int vce_v3_0_check_soft_reset(void *handle)
*
* VCE team suggest use bit 3--bit 6 for busy status check
*/
+ mutex_lock(&adev->grbm_idx_mutex);
WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
if (RREG32(mmVCE_STATUS) & AMDGPU_VCE_STATUS_BUSY_MASK) {
srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
@@ -588,6 +589,7 @@ static int vce_v3_0_check_soft_reset(void *handle)
adev->ip_block_status[AMD_IP_BLOCK_TYPE_VCE].hang = false;
adev->vce.srbm_soft_reset = 0;
}
+ mutex_unlock(&adev->grbm_idx_mutex);
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] 17+ messages in thread
* [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
[not found] ` <20160811143253.24718-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 14:32 ` [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3 Tom St Denis
2016-08-11 14:32 ` [PATCH 2/4] drm/amd/amdgpu: add mutex in check_soft for " Tom St Denis
@ 2016-08-11 14:32 ` Tom St Denis
[not found] ` <20160811143253.24718-4-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 14:32 ` [PATCH 4/4] drm/amd/amdgpu: UVD v6 " Tom St Denis
3 siblings, 1 reply; 17+ messages in thread
From: Tom St Denis @ 2016-08-11 14:32 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/vce_v2_0.c | 39 +++++++++++++----------------------
1 file changed, 14 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
index 80a37a602181..21ba219e943b 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
@@ -127,15 +127,10 @@ static int vce_v2_0_start(struct amdgpu_device *adev)
WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
- WREG32_P(mmVCE_VCPU_CNTL, VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
-
- WREG32_P(mmVCE_SOFT_RESET,
- VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
- ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
-
+ WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
mdelay(100);
-
- WREG32_P(mmVCE_SOFT_RESET, 0, ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
for (i = 0; i < 10; ++i) {
uint32_t status;
@@ -150,10 +145,9 @@ static int vce_v2_0_start(struct amdgpu_device *adev)
break;
DRM_ERROR("VCE not responding, trying to reset the ECPU!!!\n");
- WREG32_P(mmVCE_SOFT_RESET, VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
- ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
mdelay(10);
- WREG32_P(mmVCE_SOFT_RESET, 0, ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
mdelay(10);
r = -1;
}
@@ -345,13 +339,13 @@ static void vce_v2_0_set_dyn_cg(struct amdgpu_device *adev, bool gated)
DRM_INFO("VCE is busy, Can't set clock gateing");
return;
}
- WREG32_P(mmVCE_VCPU_CNTL, 0, ~VCE_VCPU_CNTL__CLK_EN_MASK);
- WREG32_P(mmVCE_SOFT_RESET, VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK, ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
+ WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
mdelay(100);
WREG32(mmVCE_STATUS, 0);
} else {
- WREG32_P(mmVCE_VCPU_CNTL, VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
- WREG32_P(mmVCE_SOFT_RESET, VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK, ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
+ WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
+ WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
mdelay(100);
}
@@ -458,9 +452,7 @@ static void vce_v2_0_mc_resume(struct amdgpu_device *adev)
WREG32(mmVCE_VCPU_CACHE_SIZE2, size);
WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
-
- WREG32_P(mmVCE_SYS_INT_EN, VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
- ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
+ WREG32_FIELD(VCE_SYS_INT_EN, VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
vce_v2_0_init_cg(adev);
}
@@ -474,11 +466,11 @@ static bool vce_v2_0_is_idle(void *handle)
static int vce_v2_0_wait_for_idle(void *handle)
{
- unsigned i;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+ unsigned i;
for (i = 0; i < adev->usec_timeout; i++) {
- if (!(RREG32(mmSRBM_STATUS2) & SRBM_STATUS2__VCE_BUSY_MASK))
+ if (vce_v2_0_is_idle(handle))
return 0;
}
return -ETIMEDOUT;
@@ -488,8 +480,7 @@ static int vce_v2_0_soft_reset(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- WREG32_P(mmSRBM_SOFT_RESET, SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK,
- ~SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK);
+ WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_VCE, 1);
mdelay(5);
return vce_v2_0_start(adev);
@@ -516,10 +507,8 @@ static int vce_v2_0_process_interrupt(struct amdgpu_device *adev,
DRM_DEBUG("IH: VCE\n");
switch (entry->src_data) {
case 0:
- amdgpu_fence_process(&adev->vce.ring[0]);
- break;
case 1:
- amdgpu_fence_process(&adev->vce.ring[1]);
+ amdgpu_fence_process(&adev->vce.ring[entry->src_data]);
break;
default:
DRM_ERROR("Unhandled interrupt: %d %d\n",
--
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] 17+ messages in thread
* [PATCH 4/4] drm/amd/amdgpu: UVD v6 register cleanup
[not found] ` <20160811143253.24718-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
` (2 preceding siblings ...)
2016-08-11 14:32 ` [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup Tom St Denis
@ 2016-08-11 14:32 ` Tom St Denis
[not found] ` <20160811143253.24718-5-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
3 siblings, 1 reply; 17+ messages in thread
From: Tom St Denis @ 2016-08-11 14:32 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/uvd_v6_0.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index c11b97f8e376..7b7e82840c95 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -397,15 +397,13 @@ static int uvd_v6_0_start(struct amdgpu_device *adev)
uvd_v6_0_mc_resume(adev);
/* disable clock gating */
- tmp = RREG32(mmUVD_CGC_CTRL);
- tmp &= ~UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK;
- WREG32(mmUVD_CGC_CTRL, tmp);
+ WREG32_FIELD(UVD_CGC_CTRL, DYN_CLOCK_MODE, 0);
/* disable interupt */
- WREG32_P(mmUVD_MASTINT_EN, 0, ~UVD_MASTINT_EN__VCPU_EN_MASK);
+ WREG32_FIELD(UVD_MASTINT_EN, VCPU_EN, 0);
/* stall UMC and register bus before resetting VCPU */
- WREG32_P(mmUVD_LMI_CTRL2, UVD_LMI_CTRL2__STALL_ARB_UMC_MASK, ~UVD_LMI_CTRL2__STALL_ARB_UMC_MASK);
+ WREG32_FIELD(UVD_LMI_CTRL2, STALL_ARB_UMC, 1);
mdelay(1);
/* put LMI, VCPU, RBC etc... into reset */
@@ -421,7 +419,7 @@ static int uvd_v6_0_start(struct amdgpu_device *adev)
mdelay(5);
/* take UVD block out of reset */
- WREG32_P(mmSRBM_SOFT_RESET, 0, ~SRBM_SOFT_RESET__SOFT_RESET_UVD_MASK);
+ WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_UVD, 0);
mdelay(5);
/* initialize UVD memory controller */
@@ -456,7 +454,7 @@ static int uvd_v6_0_start(struct amdgpu_device *adev)
WREG32(mmUVD_VCPU_CNTL, UVD_VCPU_CNTL__CLK_EN_MASK);
/* enable UMC */
- WREG32_P(mmUVD_LMI_CTRL2, 0, ~UVD_LMI_CTRL2__STALL_ARB_UMC_MASK);
+ WREG32_FIELD(UVD_LMI_CTRL2, STALL_ARB_UMC, 0);
/* boot up the VCPU */
WREG32(mmUVD_SOFT_RESET, 0);
@@ -476,11 +474,9 @@ static int uvd_v6_0_start(struct amdgpu_device *adev)
break;
DRM_ERROR("UVD not responding, trying to reset the VCPU!!!\n");
- WREG32_P(mmUVD_SOFT_RESET, UVD_SOFT_RESET__VCPU_SOFT_RESET_MASK,
- ~UVD_SOFT_RESET__VCPU_SOFT_RESET_MASK);
+ WREG32_FIELD(UVD_SOFT_RESET, VCPU_SOFT_RESET, 1);
mdelay(10);
- WREG32_P(mmUVD_SOFT_RESET, 0,
- ~UVD_SOFT_RESET__VCPU_SOFT_RESET_MASK);
+ WREG32_FIELD(UVD_SOFT_RESET, VCPU_SOFT_RESET, 0);
mdelay(10);
r = -1;
}
@@ -497,15 +493,14 @@ static int uvd_v6_0_start(struct amdgpu_device *adev)
/* clear the bit 4 of UVD_STATUS */
WREG32_P(mmUVD_STATUS, 0, ~(2 << UVD_STATUS__VCPU_REPORT__SHIFT));
+ /* force RBC into idle state */
rb_bufsz = order_base_2(ring->ring_size);
- tmp = 0;
- tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_BUFSZ, rb_bufsz);
+ tmp = REG_SET_FIELD(0, UVD_RBC_RB_CNTL, RB_BUFSZ, rb_bufsz);
tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_BLKSZ, 1);
tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_NO_FETCH, 1);
tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_WPTR_POLL_EN, 0);
tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_NO_UPDATE, 1);
tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_RPTR_WR_EN, 1);
- /* force RBC into idle state */
WREG32(mmUVD_RBC_RB_CNTL, tmp);
/* set the write pointer delay */
@@ -526,7 +521,7 @@ static int uvd_v6_0_start(struct amdgpu_device *adev)
ring->wptr = RREG32(mmUVD_RBC_RB_RPTR);
WREG32(mmUVD_RBC_RB_WPTR, ring->wptr);
- WREG32_P(mmUVD_RBC_RB_CNTL, 0, ~UVD_RBC_RB_CNTL__RB_NO_FETCH_MASK);
+ WREG32_FIELD(UVD_RBC_RB_CNTL, RB_NO_FETCH, 0);
return 0;
}
@@ -743,7 +738,7 @@ static int uvd_v6_0_wait_for_idle(void *handle)
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
for (i = 0; i < adev->usec_timeout; i++) {
- if (!(RREG32(mmSRBM_STATUS) & SRBM_STATUS__UVD_BUSY_MASK))
+ if (uvd_v6_0_is_idle(handle))
return 0;
}
return -ETIMEDOUT;
--
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] 17+ messages in thread
* Re: [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3
[not found] ` <20160811143253.24718-2-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2016-08-11 14:42 ` Christian König
2016-08-11 15:41 ` Deucher, Alexander
1 sibling, 0 replies; 17+ messages in thread
From: Christian König @ 2016-08-11 14:42 UTC (permalink / raw)
To: Tom St Denis, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Tom St Denis
Am 11.08.2016 um 16:32 schrieb Tom St Denis:
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Regards,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 145 ++++++++++------------------------
> 1 file changed, 43 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index 7e6bb45658f6..073cf9ed0674 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -110,22 +110,13 @@ static void vce_v3_0_ring_set_wptr(struct amdgpu_ring *ring)
>
> static void vce_v3_0_override_vce_clock_gating(struct amdgpu_device *adev, bool override)
> {
> - u32 tmp, data;
> -
> - tmp = data = RREG32(mmVCE_RB_ARB_CTRL);
> - if (override)
> - data |= VCE_RB_ARB_CTRL__VCE_CGTT_OVERRIDE_MASK;
> - else
> - data &= ~VCE_RB_ARB_CTRL__VCE_CGTT_OVERRIDE_MASK;
> -
> - if (tmp != data)
> - WREG32(mmVCE_RB_ARB_CTRL, data);
> + WREG32_FIELD(VCE_RB_ARB_CTRL, VCE_CGTT_OVERRIDE, override ? 1 : 0);
> }
>
> static void vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device *adev,
> bool gated)
> {
> - u32 tmp, data;
> + u32 data;
>
> /* Set Override to disable Clock Gating */
> vce_v3_0_override_vce_clock_gating(adev, true);
> @@ -136,65 +127,55 @@ static void vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device *adev,
> fly as necessary.
> */
> if (gated) {
> - tmp = data = RREG32(mmVCE_CLOCK_GATING_B);
> + data = RREG32(mmVCE_CLOCK_GATING_B);
> data |= 0x1ff;
> data &= ~0xef0000;
> - if (tmp != data)
> - WREG32(mmVCE_CLOCK_GATING_B, data);
> + WREG32(mmVCE_CLOCK_GATING_B, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING);
> data |= 0x3ff000;
> data &= ~0xffc00000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> data |= 0x2;
> data &= ~0x00010000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
>
> - tmp = data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> data |= 0x37f;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
> + WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> + data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> data |= VCE_UENC_DMA_DCLK_CTRL__WRDMCLK_FORCEON_MASK |
> VCE_UENC_DMA_DCLK_CTRL__RDDMCLK_FORCEON_MASK |
> VCE_UENC_DMA_DCLK_CTRL__REGCLK_FORCEON_MASK |
> 0x8;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> + WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> } else {
> - tmp = data = RREG32(mmVCE_CLOCK_GATING_B);
> + data = RREG32(mmVCE_CLOCK_GATING_B);
> data &= ~0x80010;
> data |= 0xe70008;
> - if (tmp != data)
> - WREG32(mmVCE_CLOCK_GATING_B, data);
> + WREG32(mmVCE_CLOCK_GATING_B, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING);
> data |= 0xffc00000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> data |= 0x10000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
>
> - tmp = data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> data &= ~0xffc00000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
> + WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> + data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> data &= ~(VCE_UENC_DMA_DCLK_CTRL__WRDMCLK_FORCEON_MASK |
> VCE_UENC_DMA_DCLK_CTRL__RDDMCLK_FORCEON_MASK |
> VCE_UENC_DMA_DCLK_CTRL__REGCLK_FORCEON_MASK |
> 0x8);
> - if (tmp != data)
> - WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> + WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> }
> vce_v3_0_override_vce_clock_gating(adev, false);
> }
> @@ -213,12 +194,9 @@ static int vce_v3_0_firmware_loaded(struct amdgpu_device *adev)
> }
>
> DRM_ERROR("VCE not responding, trying to reset the ECPU!!!\n");
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(10);
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(10);
> }
>
> @@ -256,34 +234,22 @@ static int vce_v3_0_start(struct amdgpu_device *adev)
> if (adev->vce.harvest_config & (1 << idx))
> continue;
>
> - if (idx == 0)
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> - ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> - else
> - WREG32_P(mmGRBM_GFX_INDEX,
> - GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> - ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> -
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, idx);
> vce_v3_0_mc_resume(adev, idx);
> -
> - WREG32_P(mmVCE_STATUS, VCE_STATUS__JOB_BUSY_MASK,
> - ~VCE_STATUS__JOB_BUSY_MASK);
> + WREG32_FIELD(VCE_STATUS, JOB_BUSY, 1);
>
> if (adev->asic_type >= CHIP_STONEY)
> WREG32_P(mmVCE_VCPU_CNTL, 1, ~0x200001);
> else
> - WREG32_P(mmVCE_VCPU_CNTL, VCE_VCPU_CNTL__CLK_EN_MASK,
> - ~VCE_VCPU_CNTL__CLK_EN_MASK);
> -
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
>
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(100);
>
> r = vce_v3_0_firmware_loaded(adev);
>
> /* clear BUSY flag */
> - WREG32_P(mmVCE_STATUS, 0, ~VCE_STATUS__JOB_BUSY_MASK);
> + WREG32_FIELD(VCE_STATUS, JOB_BUSY, 0);
>
> if (r) {
> DRM_ERROR("VCE not responding, giving up!!!\n");
> @@ -292,7 +258,7 @@ static int vce_v3_0_start(struct amdgpu_device *adev)
> }
> }
>
> - WREG32_P(mmGRBM_GFX_INDEX, 0, ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
> @@ -307,33 +273,25 @@ static int vce_v3_0_stop(struct amdgpu_device *adev)
> if (adev->vce.harvest_config & (1 << idx))
> continue;
>
> - if (idx == 0)
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> - ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> - else
> - WREG32_P(mmGRBM_GFX_INDEX,
> - GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> - ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, idx);
>
> if (adev->asic_type >= CHIP_STONEY)
> WREG32_P(mmVCE_VCPU_CNTL, 0, ~0x200001);
> else
> - WREG32_P(mmVCE_VCPU_CNTL, 0,
> - ~VCE_VCPU_CNTL__CLK_EN_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
> +
> /* hold on ECPU */
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>
> /* clear BUSY flag */
> - WREG32_P(mmVCE_STATUS, 0, ~VCE_STATUS__JOB_BUSY_MASK);
> + WREG32_FIELD(VCE_STATUS, JOB_BUSY, 0);
>
> /* Set Clock-Gating off */
> if (adev->cg_flags & AMD_CG_SUPPORT_VCE_MGCG)
> vce_v3_0_set_vce_sw_clock_gating(adev, false);
> }
>
> - WREG32_P(mmGRBM_GFX_INDEX, 0, ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
> @@ -561,9 +519,7 @@ static void vce_v3_0_mc_resume(struct amdgpu_device *adev, int idx)
> }
>
> WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
> -
> - WREG32_P(mmVCE_SYS_INT_EN, VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
> - ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
> + WREG32_FIELD(VCE_SYS_INT_EN, VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
> }
>
> static bool vce_v3_0_is_idle(void *handle)
> @@ -599,7 +555,6 @@ static int vce_v3_0_check_soft_reset(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> u32 srbm_soft_reset = 0;
> - u32 tmp;
>
> /* According to VCE team , we should use VCE_STATUS instead
> * SRBM_STATUS.VCE_BUSY bit for busy status checking.
> @@ -614,23 +569,17 @@ static int vce_v3_0_check_soft_reset(void *handle)
> *
> * VCE team suggest use bit 3--bit 6 for busy status check
> */
> - tmp = RREG32(mmGRBM_GFX_INDEX);
> - tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
> - WREG32(mmGRBM_GFX_INDEX, tmp);
> + WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
> if (RREG32(mmVCE_STATUS) & AMDGPU_VCE_STATUS_BUSY_MASK) {
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
> }
> - tmp = RREG32(mmGRBM_GFX_INDEX);
> - tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX, 0x10);
> - WREG32(mmGRBM_GFX_INDEX, tmp);
> + WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0x10);
> if (RREG32(mmVCE_STATUS) & AMDGPU_VCE_STATUS_BUSY_MASK) {
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
> }
> - tmp = RREG32(mmGRBM_GFX_INDEX);
> - tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
> - WREG32(mmGRBM_GFX_INDEX, tmp);
> + WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
>
> if (srbm_soft_reset) {
> adev->ip_block_status[AMD_IP_BLOCK_TYPE_VCE].hang = true;
> @@ -718,9 +667,7 @@ static int vce_v3_0_process_interrupt(struct amdgpu_device *adev,
> {
> DRM_DEBUG("IH: VCE\n");
>
> - WREG32_P(mmVCE_SYS_INT_STATUS,
> - VCE_SYS_INT_STATUS__VCE_SYS_INT_TRAP_INTERRUPT_INT_MASK,
> - ~VCE_SYS_INT_STATUS__VCE_SYS_INT_TRAP_INTERRUPT_INT_MASK);
> + WREG32_FIELD(VCE_SYS_INT_STATUS, VCE_SYS_INT_TRAP_INTERRUPT_INT, 1);
>
> switch (entry->src_data) {
> case 0:
> @@ -767,13 +714,7 @@ static int vce_v3_0_set_clockgating_state(void *handle,
> if (adev->vce.harvest_config & (1 << i))
> continue;
>
> - if (i == 0)
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> - ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> - else
> - WREG32_P(mmGRBM_GFX_INDEX,
> - GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> - ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, i);
>
> if (enable) {
> /* initialize VCE_CLOCK_GATING_A: Clock ON/OFF delay */
> @@ -792,7 +733,7 @@ static int vce_v3_0_set_clockgating_state(void *handle,
> vce_v3_0_set_vce_sw_clock_gating(adev, enable);
> }
>
> - WREG32_P(mmGRBM_GFX_INDEX, 0, ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3
[not found] ` <20160811143253.24718-2-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 14:42 ` Christian König
@ 2016-08-11 15:41 ` Deucher, Alexander
[not found] ` <MWHPR12MB169403F24E091D033BEE21F6F71E0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
1 sibling, 1 reply; 17+ messages in thread
From: Deucher, Alexander @ 2016-08-11 15:41 UTC (permalink / raw)
To: 'Tom St Denis', amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: StDenis, Tom
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Thursday, August 11, 2016 10:33 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: StDenis, Tom
> Subject: [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
This potentially adds a bunch of unnecessary register writes which increases latency on VCE power up.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 145 ++++++++++-----------------
> -------
> 1 file changed, 43 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index 7e6bb45658f6..073cf9ed0674 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -110,22 +110,13 @@ static void vce_v3_0_ring_set_wptr(struct
> amdgpu_ring *ring)
>
> static void vce_v3_0_override_vce_clock_gating(struct amdgpu_device
> *adev, bool override)
> {
> - u32 tmp, data;
> -
> - tmp = data = RREG32(mmVCE_RB_ARB_CTRL);
> - if (override)
> - data |= VCE_RB_ARB_CTRL__VCE_CGTT_OVERRIDE_MASK;
> - else
> - data &=
> ~VCE_RB_ARB_CTRL__VCE_CGTT_OVERRIDE_MASK;
> -
> - if (tmp != data)
> - WREG32(mmVCE_RB_ARB_CTRL, data);
> + WREG32_FIELD(VCE_RB_ARB_CTRL, VCE_CGTT_OVERRIDE, override
> ? 1 : 0);
> }
>
> static void vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device
> *adev,
> bool gated)
> {
> - u32 tmp, data;
> + u32 data;
>
> /* Set Override to disable Clock Gating */
> vce_v3_0_override_vce_clock_gating(adev, true);
> @@ -136,65 +127,55 @@ static void
> vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device *adev,
> fly as necessary.
> */
> if (gated) {
> - tmp = data = RREG32(mmVCE_CLOCK_GATING_B);
> + data = RREG32(mmVCE_CLOCK_GATING_B);
> data |= 0x1ff;
> data &= ~0xef0000;
> - if (tmp != data)
> - WREG32(mmVCE_CLOCK_GATING_B, data);
> + WREG32(mmVCE_CLOCK_GATING_B, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING);
> data |= 0x3ff000;
> data &= ~0xffc00000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> data |= 0x2;
> data &= ~0x00010000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
>
> - tmp = data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> data |= 0x37f;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_REG_CLOCK_GATING,
> data);
> + WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> + data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> data |=
> VCE_UENC_DMA_DCLK_CTRL__WRDMCLK_FORCEON_MASK |
>
> VCE_UENC_DMA_DCLK_CTRL__RDDMCLK_FORCEON_MASK |
>
> VCE_UENC_DMA_DCLK_CTRL__REGCLK_FORCEON_MASK |
> 0x8;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> + WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> } else {
> - tmp = data = RREG32(mmVCE_CLOCK_GATING_B);
> + data = RREG32(mmVCE_CLOCK_GATING_B);
> data &= ~0x80010;
> data |= 0xe70008;
> - if (tmp != data)
> - WREG32(mmVCE_CLOCK_GATING_B, data);
> + WREG32(mmVCE_CLOCK_GATING_B, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING);
> data |= 0xffc00000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> data |= 0x10000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
>
> - tmp = data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> data &= ~0xffc00000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_REG_CLOCK_GATING,
> data);
> + WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> + data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> data &=
> ~(VCE_UENC_DMA_DCLK_CTRL__WRDMCLK_FORCEON_MASK |
>
> VCE_UENC_DMA_DCLK_CTRL__RDDMCLK_FORCEON_MASK |
>
> VCE_UENC_DMA_DCLK_CTRL__REGCLK_FORCEON_MASK |
> 0x8);
> - if (tmp != data)
> - WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> + WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> }
> vce_v3_0_override_vce_clock_gating(adev, false);
> }
> @@ -213,12 +194,9 @@ static int vce_v3_0_firmware_loaded(struct
> amdgpu_device *adev)
> }
>
> DRM_ERROR("VCE not responding, trying to reset the
> ECPU!!!\n");
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(10);
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(10);
> }
>
> @@ -256,34 +234,22 @@ static int vce_v3_0_start(struct amdgpu_device
> *adev)
> if (adev->vce.harvest_config & (1 << idx))
> continue;
>
> - if (idx == 0)
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> - else
> - WREG32_P(mmGRBM_GFX_INDEX,
> - GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> -
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, idx);
> vce_v3_0_mc_resume(adev, idx);
> -
> - WREG32_P(mmVCE_STATUS,
> VCE_STATUS__JOB_BUSY_MASK,
> - ~VCE_STATUS__JOB_BUSY_MASK);
> + WREG32_FIELD(VCE_STATUS, JOB_BUSY, 1);
>
> if (adev->asic_type >= CHIP_STONEY)
> WREG32_P(mmVCE_VCPU_CNTL, 1, ~0x200001);
> else
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK,
> - ~VCE_VCPU_CNTL__CLK_EN_MASK);
> -
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
>
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(100);
>
> r = vce_v3_0_firmware_loaded(adev);
>
> /* clear BUSY flag */
> - WREG32_P(mmVCE_STATUS, 0,
> ~VCE_STATUS__JOB_BUSY_MASK);
> + WREG32_FIELD(VCE_STATUS, JOB_BUSY, 0);
>
> if (r) {
> DRM_ERROR("VCE not responding, giving up!!!\n");
> @@ -292,7 +258,7 @@ static int vce_v3_0_start(struct amdgpu_device
> *adev)
> }
> }
>
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
> @@ -307,33 +273,25 @@ static int vce_v3_0_stop(struct amdgpu_device
> *adev)
> if (adev->vce.harvest_config & (1 << idx))
> continue;
>
> - if (idx == 0)
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> - else
> - WREG32_P(mmGRBM_GFX_INDEX,
> - GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, idx);
>
> if (adev->asic_type >= CHIP_STONEY)
> WREG32_P(mmVCE_VCPU_CNTL, 0, ~0x200001);
> else
> - WREG32_P(mmVCE_VCPU_CNTL, 0,
> - ~VCE_VCPU_CNTL__CLK_EN_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
> +
> /* hold on ECPU */
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>
> /* clear BUSY flag */
> - WREG32_P(mmVCE_STATUS, 0,
> ~VCE_STATUS__JOB_BUSY_MASK);
> + WREG32_FIELD(VCE_STATUS, JOB_BUSY, 0);
>
> /* Set Clock-Gating off */
> if (adev->cg_flags & AMD_CG_SUPPORT_VCE_MGCG)
> vce_v3_0_set_vce_sw_clock_gating(adev, false);
> }
>
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
> @@ -561,9 +519,7 @@ static void vce_v3_0_mc_resume(struct
> amdgpu_device *adev, int idx)
> }
>
> WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
> -
> - WREG32_P(mmVCE_SYS_INT_EN,
> VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
> -
> ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
> + WREG32_FIELD(VCE_SYS_INT_EN,
> VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
> }
>
> static bool vce_v3_0_is_idle(void *handle)
> @@ -599,7 +555,6 @@ static int vce_v3_0_check_soft_reset(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> u32 srbm_soft_reset = 0;
> - u32 tmp;
>
> /* According to VCE team , we should use VCE_STATUS instead
> * SRBM_STATUS.VCE_BUSY bit for busy status checking.
> @@ -614,23 +569,17 @@ static int vce_v3_0_check_soft_reset(void
> *handle)
> *
> * VCE team suggest use bit 3--bit 6 for busy status check
> */
> - tmp = RREG32(mmGRBM_GFX_INDEX);
> - tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX,
> 0);
> - WREG32(mmGRBM_GFX_INDEX, tmp);
> + WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
> if (RREG32(mmVCE_STATUS) &
> AMDGPU_VCE_STATUS_BUSY_MASK) {
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset,
> SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset,
> SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
> }
> - tmp = RREG32(mmGRBM_GFX_INDEX);
> - tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX,
> 0x10);
> - WREG32(mmGRBM_GFX_INDEX, tmp);
> + WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0x10);
> if (RREG32(mmVCE_STATUS) &
> AMDGPU_VCE_STATUS_BUSY_MASK) {
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset,
> SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset,
> SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
> }
> - tmp = RREG32(mmGRBM_GFX_INDEX);
> - tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX,
> 0);
> - WREG32(mmGRBM_GFX_INDEX, tmp);
> + WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
>
> if (srbm_soft_reset) {
> adev->ip_block_status[AMD_IP_BLOCK_TYPE_VCE].hang =
> true;
> @@ -718,9 +667,7 @@ static int vce_v3_0_process_interrupt(struct
> amdgpu_device *adev,
> {
> DRM_DEBUG("IH: VCE\n");
>
> - WREG32_P(mmVCE_SYS_INT_STATUS,
> -
> VCE_SYS_INT_STATUS__VCE_SYS_INT_TRAP_INTERRUPT_INT_MAS
> K,
> -
> ~VCE_SYS_INT_STATUS__VCE_SYS_INT_TRAP_INTERRUPT_INT_MA
> SK);
> + WREG32_FIELD(VCE_SYS_INT_STATUS,
> VCE_SYS_INT_TRAP_INTERRUPT_INT, 1);
>
> switch (entry->src_data) {
> case 0:
> @@ -767,13 +714,7 @@ static int vce_v3_0_set_clockgating_state(void
> *handle,
> if (adev->vce.harvest_config & (1 << i))
> continue;
>
> - if (i == 0)
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> - else
> - WREG32_P(mmGRBM_GFX_INDEX,
> -
> GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, i);
>
> if (enable) {
> /* initialize VCE_CLOCK_GATING_A: Clock ON/OFF
> delay */
> @@ -792,7 +733,7 @@ static int vce_v3_0_set_clockgating_state(void
> *handle,
> vce_v3_0_set_vce_sw_clock_gating(adev, enable);
> }
>
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
> --
> 2.9.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 2/4] drm/amd/amdgpu: add mutex in check_soft for VCE v3
[not found] ` <20160811143253.24718-3-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2016-08-11 15:42 ` Deucher, Alexander
0 siblings, 0 replies; 17+ messages in thread
From: Deucher, Alexander @ 2016-08-11 15:42 UTC (permalink / raw)
To: 'Tom St Denis', amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: StDenis, Tom
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Thursday, August 11, 2016 10:33 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: StDenis, Tom
> Subject: [PATCH 2/4] drm/amd/amdgpu: add mutex in check_soft for VCE v3
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index 073cf9ed0674..615b8b16ad04 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -569,6 +569,7 @@ static int vce_v3_0_check_soft_reset(void *handle)
> *
> * VCE team suggest use bit 3--bit 6 for busy status check
> */
> + mutex_lock(&adev->grbm_idx_mutex);
> WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
> if (RREG32(mmVCE_STATUS) &
> AMDGPU_VCE_STATUS_BUSY_MASK) {
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset,
> SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
> @@ -588,6 +589,7 @@ static int vce_v3_0_check_soft_reset(void *handle)
> adev->ip_block_status[AMD_IP_BLOCK_TYPE_VCE].hang =
> false;
> adev->vce.srbm_soft_reset = 0;
> }
> + mutex_unlock(&adev->grbm_idx_mutex);
> return 0;
> }
>
> --
> 2.9.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3
[not found] ` <MWHPR12MB169403F24E091D033BEE21F6F71E0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-08-11 15:43 ` StDenis, Tom
0 siblings, 0 replies; 17+ messages in thread
From: StDenis, Tom @ 2016-08-11 15:43 UTC (permalink / raw)
To: Deucher, Alexander, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 14837 bytes --]
Sorta. The switch from gate/ungate for instance is kinda mutually exclusive. The device won't be gated when calling gate or ungated when calling ungate. The other conditional writes are similar.
Tom
________________________________
From: Deucher, Alexander
Sent: Thursday, August 11, 2016 11:41
To: 'Tom St Denis'; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: StDenis, Tom
Subject: RE: [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf
> Of Tom St Denis
> Sent: Thursday, August 11, 2016 10:33 AM
> To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> Cc: StDenis, Tom
> Subject: [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3
>
> Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org>
This potentially adds a bunch of unnecessary register writes which increases latency on VCE power up.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 145 ++++++++++-----------------
> -------
> 1 file changed, 43 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index 7e6bb45658f6..073cf9ed0674 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -110,22 +110,13 @@ static void vce_v3_0_ring_set_wptr(struct
> amdgpu_ring *ring)
>
> static void vce_v3_0_override_vce_clock_gating(struct amdgpu_device
> *adev, bool override)
> {
> - u32 tmp, data;
> -
> - tmp = data = RREG32(mmVCE_RB_ARB_CTRL);
> - if (override)
> - data |= VCE_RB_ARB_CTRL__VCE_CGTT_OVERRIDE_MASK;
> - else
> - data &=
> ~VCE_RB_ARB_CTRL__VCE_CGTT_OVERRIDE_MASK;
> -
> - if (tmp != data)
> - WREG32(mmVCE_RB_ARB_CTRL, data);
> + WREG32_FIELD(VCE_RB_ARB_CTRL, VCE_CGTT_OVERRIDE, override
> ? 1 : 0);
> }
>
> static void vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device
> *adev,
> bool gated)
> {
> - u32 tmp, data;
> + u32 data;
>
> /* Set Override to disable Clock Gating */
> vce_v3_0_override_vce_clock_gating(adev, true);
> @@ -136,65 +127,55 @@ static void
> vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device *adev,
> fly as necessary.
> */
> if (gated) {
> - tmp = data = RREG32(mmVCE_CLOCK_GATING_B);
> + data = RREG32(mmVCE_CLOCK_GATING_B);
> data |= 0x1ff;
> data &= ~0xef0000;
> - if (tmp != data)
> - WREG32(mmVCE_CLOCK_GATING_B, data);
> + WREG32(mmVCE_CLOCK_GATING_B, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING);
> data |= 0x3ff000;
> data &= ~0xffc00000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> data |= 0x2;
> data &= ~0x00010000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
>
> - tmp = data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> data |= 0x37f;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_REG_CLOCK_GATING,
> data);
> + WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> + data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> data |=
> VCE_UENC_DMA_DCLK_CTRL__WRDMCLK_FORCEON_MASK |
>
> VCE_UENC_DMA_DCLK_CTRL__RDDMCLK_FORCEON_MASK |
>
> VCE_UENC_DMA_DCLK_CTRL__REGCLK_FORCEON_MASK |
> 0x8;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> + WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> } else {
> - tmp = data = RREG32(mmVCE_CLOCK_GATING_B);
> + data = RREG32(mmVCE_CLOCK_GATING_B);
> data &= ~0x80010;
> data |= 0xe70008;
> - if (tmp != data)
> - WREG32(mmVCE_CLOCK_GATING_B, data);
> + WREG32(mmVCE_CLOCK_GATING_B, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING);
> data |= 0xffc00000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> + data = RREG32(mmVCE_UENC_CLOCK_GATING_2);
> data |= 0x10000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
> + WREG32(mmVCE_UENC_CLOCK_GATING_2, data);
>
> - tmp = data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> + data = RREG32(mmVCE_UENC_REG_CLOCK_GATING);
> data &= ~0xffc00000;
> - if (tmp != data)
> - WREG32(mmVCE_UENC_REG_CLOCK_GATING,
> data);
> + WREG32(mmVCE_UENC_REG_CLOCK_GATING, data);
>
> - tmp = data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> + data = RREG32(mmVCE_UENC_DMA_DCLK_CTRL);
> data &=
> ~(VCE_UENC_DMA_DCLK_CTRL__WRDMCLK_FORCEON_MASK |
>
> VCE_UENC_DMA_DCLK_CTRL__RDDMCLK_FORCEON_MASK |
>
> VCE_UENC_DMA_DCLK_CTRL__REGCLK_FORCEON_MASK |
> 0x8);
> - if (tmp != data)
> - WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> + WREG32(mmVCE_UENC_DMA_DCLK_CTRL, data);
> }
> vce_v3_0_override_vce_clock_gating(adev, false);
> }
> @@ -213,12 +194,9 @@ static int vce_v3_0_firmware_loaded(struct
> amdgpu_device *adev)
> }
>
> DRM_ERROR("VCE not responding, trying to reset the
> ECPU!!!\n");
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(10);
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(10);
> }
>
> @@ -256,34 +234,22 @@ static int vce_v3_0_start(struct amdgpu_device
> *adev)
> if (adev->vce.harvest_config & (1 << idx))
> continue;
>
> - if (idx == 0)
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> - else
> - WREG32_P(mmGRBM_GFX_INDEX,
> - GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> -
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, idx);
> vce_v3_0_mc_resume(adev, idx);
> -
> - WREG32_P(mmVCE_STATUS,
> VCE_STATUS__JOB_BUSY_MASK,
> - ~VCE_STATUS__JOB_BUSY_MASK);
> + WREG32_FIELD(VCE_STATUS, JOB_BUSY, 1);
>
> if (adev->asic_type >= CHIP_STONEY)
> WREG32_P(mmVCE_VCPU_CNTL, 1, ~0x200001);
> else
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK,
> - ~VCE_VCPU_CNTL__CLK_EN_MASK);
> -
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
>
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(100);
>
> r = vce_v3_0_firmware_loaded(adev);
>
> /* clear BUSY flag */
> - WREG32_P(mmVCE_STATUS, 0,
> ~VCE_STATUS__JOB_BUSY_MASK);
> + WREG32_FIELD(VCE_STATUS, JOB_BUSY, 0);
>
> if (r) {
> DRM_ERROR("VCE not responding, giving up!!!\n");
> @@ -292,7 +258,7 @@ static int vce_v3_0_start(struct amdgpu_device
> *adev)
> }
> }
>
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
> @@ -307,33 +273,25 @@ static int vce_v3_0_stop(struct amdgpu_device
> *adev)
> if (adev->vce.harvest_config & (1 << idx))
> continue;
>
> - if (idx == 0)
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> - else
> - WREG32_P(mmGRBM_GFX_INDEX,
> - GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, idx);
>
> if (adev->asic_type >= CHIP_STONEY)
> WREG32_P(mmVCE_VCPU_CNTL, 0, ~0x200001);
> else
> - WREG32_P(mmVCE_VCPU_CNTL, 0,
> - ~VCE_VCPU_CNTL__CLK_EN_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
> +
> /* hold on ECPU */
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>
> /* clear BUSY flag */
> - WREG32_P(mmVCE_STATUS, 0,
> ~VCE_STATUS__JOB_BUSY_MASK);
> + WREG32_FIELD(VCE_STATUS, JOB_BUSY, 0);
>
> /* Set Clock-Gating off */
> if (adev->cg_flags & AMD_CG_SUPPORT_VCE_MGCG)
> vce_v3_0_set_vce_sw_clock_gating(adev, false);
> }
>
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
> @@ -561,9 +519,7 @@ static void vce_v3_0_mc_resume(struct
> amdgpu_device *adev, int idx)
> }
>
> WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
> -
> - WREG32_P(mmVCE_SYS_INT_EN,
> VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
> -
> ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
> + WREG32_FIELD(VCE_SYS_INT_EN,
> VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
> }
>
> static bool vce_v3_0_is_idle(void *handle)
> @@ -599,7 +555,6 @@ static int vce_v3_0_check_soft_reset(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> u32 srbm_soft_reset = 0;
> - u32 tmp;
>
> /* According to VCE team , we should use VCE_STATUS instead
> * SRBM_STATUS.VCE_BUSY bit for busy status checking.
> @@ -614,23 +569,17 @@ static int vce_v3_0_check_soft_reset(void
> *handle)
> *
> * VCE team suggest use bit 3--bit 6 for busy status check
> */
> - tmp = RREG32(mmGRBM_GFX_INDEX);
> - tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX,
> 0);
> - WREG32(mmGRBM_GFX_INDEX, tmp);
> + WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
> if (RREG32(mmVCE_STATUS) &
> AMDGPU_VCE_STATUS_BUSY_MASK) {
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset,
> SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset,
> SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
> }
> - tmp = RREG32(mmGRBM_GFX_INDEX);
> - tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX,
> 0x10);
> - WREG32(mmGRBM_GFX_INDEX, tmp);
> + WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0x10);
> if (RREG32(mmVCE_STATUS) &
> AMDGPU_VCE_STATUS_BUSY_MASK) {
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset,
> SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
> srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset,
> SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
> }
> - tmp = RREG32(mmGRBM_GFX_INDEX);
> - tmp = REG_SET_FIELD(tmp, GRBM_GFX_INDEX, INSTANCE_INDEX,
> 0);
> - WREG32(mmGRBM_GFX_INDEX, tmp);
> + WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
>
> if (srbm_soft_reset) {
> adev->ip_block_status[AMD_IP_BLOCK_TYPE_VCE].hang =
> true;
> @@ -718,9 +667,7 @@ static int vce_v3_0_process_interrupt(struct
> amdgpu_device *adev,
> {
> DRM_DEBUG("IH: VCE\n");
>
> - WREG32_P(mmVCE_SYS_INT_STATUS,
> -
> VCE_SYS_INT_STATUS__VCE_SYS_INT_TRAP_INTERRUPT_INT_MAS
> K,
> -
> ~VCE_SYS_INT_STATUS__VCE_SYS_INT_TRAP_INTERRUPT_INT_MA
> SK);
> + WREG32_FIELD(VCE_SYS_INT_STATUS,
> VCE_SYS_INT_TRAP_INTERRUPT_INT, 1);
>
> switch (entry->src_data) {
> case 0:
> @@ -767,13 +714,7 @@ static int vce_v3_0_set_clockgating_state(void
> *handle,
> if (adev->vce.harvest_config & (1 << i))
> continue;
>
> - if (i == 0)
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> - else
> - WREG32_P(mmGRBM_GFX_INDEX,
> -
> GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> -
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, i);
>
> if (enable) {
> /* initialize VCE_CLOCK_GATING_A: Clock ON/OFF
> delay */
> @@ -792,7 +733,7 @@ static int vce_v3_0_set_clockgating_state(void
> *handle,
> vce_v3_0_set_vce_sw_clock_gating(adev, enable);
> }
>
> - WREG32_P(mmGRBM_GFX_INDEX, 0,
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + WREG32_FIELD(GRBM_GFX_INDEX, VCE_INSTANCE, 0);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
> --
> 2.9.2
>
> _______________________________________________
> 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: 33042 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] 17+ messages in thread
* RE: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
[not found] ` <20160811143253.24718-4-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2016-08-11 15:46 ` Deucher, Alexander
[not found] ` <MWHPR12MB1694FE8258E0ADB4F32DC885F71E0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Deucher, Alexander @ 2016-08-11 15:46 UTC (permalink / raw)
To: 'Tom St Denis', amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: StDenis, Tom
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Thursday, August 11, 2016 10:33 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: StDenis, Tom
> Subject: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
A couple pieces of this patch could be split out as separate cleanups (see below). However, I'm not sure how much value there is in switching WREG32_P for WREG_FIELD other than code churn. They basically do the same thing.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 39 +++++++++++++-------------
> ---------
> 1 file changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> index 80a37a602181..21ba219e943b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> @@ -127,15 +127,10 @@ static int vce_v2_0_start(struct amdgpu_device
> *adev)
> WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
>
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> -
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> -
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> -
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
>
> for (i = 0; i < 10; ++i) {
> uint32_t status;
> @@ -150,10 +145,9 @@ static int vce_v2_0_start(struct amdgpu_device
> *adev)
> break;
>
> DRM_ERROR("VCE not responding, trying to reset the
> ECPU!!!\n");
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> -
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(10);
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(10);
> r = -1;
> }
> @@ -345,13 +339,13 @@ static void vce_v2_0_set_dyn_cg(struct
> amdgpu_device *adev, bool gated)
> DRM_INFO("VCE is busy, Can't set clock gateing");
> return;
> }
> - WREG32_P(mmVCE_VCPU_CNTL, 0,
> ~VCE_VCPU_CNTL__CLK_EN_MASK);
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> WREG32(mmVCE_STATUS, 0);
> } else {
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> }
>
> @@ -458,9 +452,7 @@ static void vce_v2_0_mc_resume(struct
> amdgpu_device *adev)
> WREG32(mmVCE_VCPU_CACHE_SIZE2, size);
>
> WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
> -
> - WREG32_P(mmVCE_SYS_INT_EN,
> VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
> -
> ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
> + WREG32_FIELD(VCE_SYS_INT_EN,
> VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
>
> vce_v2_0_init_cg(adev);
> }
> @@ -474,11 +466,11 @@ static bool vce_v2_0_is_idle(void *handle)
>
> static int vce_v2_0_wait_for_idle(void *handle)
> {
> - unsigned i;
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + unsigned i;
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if (!(RREG32(mmSRBM_STATUS2) &
> SRBM_STATUS2__VCE_BUSY_MASK))
> + if (vce_v2_0_is_idle(handle))
> return 0;
This could be split out as a separate patch.
> }
> return -ETIMEDOUT;
> @@ -488,8 +480,7 @@ static int vce_v2_0_soft_reset(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - WREG32_P(mmSRBM_SOFT_RESET,
> SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK,
> - ~SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK);
> + WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_VCE, 1);
> mdelay(5);
>
> return vce_v2_0_start(adev);
> @@ -516,10 +507,8 @@ static int vce_v2_0_process_interrupt(struct
> amdgpu_device *adev,
> DRM_DEBUG("IH: VCE\n");
> switch (entry->src_data) {
> case 0:
> - amdgpu_fence_process(&adev->vce.ring[0]);
> - break;
> case 1:
> - amdgpu_fence_process(&adev->vce.ring[1]);
> + amdgpu_fence_process(&adev->vce.ring[entry->src_data]);
> break;
> default:
> DRM_ERROR("Unhandled interrupt: %d %d\n",
This too.
> --
> 2.9.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
[not found] ` <MWHPR12MB1694FE8258E0ADB4F32DC885F71E0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-08-11 15:48 ` StDenis, Tom
[not found] ` <DM5PR12MB1132A35D1BAAB0027D5FE23AF71E0-2J9CzHegvk+UzrhdoDeimQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: StDenis, Tom @ 2016-08-11 15:48 UTC (permalink / raw)
To: Deucher, Alexander, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 6482 bytes --]
Just trying to make it easier to read.
WREG32_FIELD(foo, FIELD, 1);
Is easier to read than
WREG32_P(foo, FOO__FIELD_MASK, ~FOO__FIELD_MASK);
(also I already pushed them after getting a RB by Christian this morning so we might need to hold a different discussion).
I agree it's not really a functional change but making things a bit more uniform and easier to read helps maintenance. And I did find a bug in the process :-)
Tom
________________________________
From: Deucher, Alexander
Sent: Thursday, August 11, 2016 11:46
To: 'Tom St Denis'; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: StDenis, Tom
Subject: RE: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf
> Of Tom St Denis
> Sent: Thursday, August 11, 2016 10:33 AM
> To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> Cc: StDenis, Tom
> Subject: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>
> Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org>
A couple pieces of this patch could be split out as separate cleanups (see below). However, I'm not sure how much value there is in switching WREG32_P for WREG_FIELD other than code churn. They basically do the same thing.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 39 +++++++++++++-------------
> ---------
> 1 file changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> index 80a37a602181..21ba219e943b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> @@ -127,15 +127,10 @@ static int vce_v2_0_start(struct amdgpu_device
> *adev)
> WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
>
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> -
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> -
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> -
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
>
> for (i = 0; i < 10; ++i) {
> uint32_t status;
> @@ -150,10 +145,9 @@ static int vce_v2_0_start(struct amdgpu_device
> *adev)
> break;
>
> DRM_ERROR("VCE not responding, trying to reset the
> ECPU!!!\n");
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> -
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(10);
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(10);
> r = -1;
> }
> @@ -345,13 +339,13 @@ static void vce_v2_0_set_dyn_cg(struct
> amdgpu_device *adev, bool gated)
> DRM_INFO("VCE is busy, Can't set clock gateing");
> return;
> }
> - WREG32_P(mmVCE_VCPU_CNTL, 0,
> ~VCE_VCPU_CNTL__CLK_EN_MASK);
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> WREG32(mmVCE_STATUS, 0);
> } else {
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> }
>
> @@ -458,9 +452,7 @@ static void vce_v2_0_mc_resume(struct
> amdgpu_device *adev)
> WREG32(mmVCE_VCPU_CACHE_SIZE2, size);
>
> WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
> -
> - WREG32_P(mmVCE_SYS_INT_EN,
> VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
> -
> ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
> + WREG32_FIELD(VCE_SYS_INT_EN,
> VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
>
> vce_v2_0_init_cg(adev);
> }
> @@ -474,11 +466,11 @@ static bool vce_v2_0_is_idle(void *handle)
>
> static int vce_v2_0_wait_for_idle(void *handle)
> {
> - unsigned i;
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + unsigned i;
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if (!(RREG32(mmSRBM_STATUS2) &
> SRBM_STATUS2__VCE_BUSY_MASK))
> + if (vce_v2_0_is_idle(handle))
> return 0;
This could be split out as a separate patch.
> }
> return -ETIMEDOUT;
> @@ -488,8 +480,7 @@ static int vce_v2_0_soft_reset(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - WREG32_P(mmSRBM_SOFT_RESET,
> SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK,
> - ~SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK);
> + WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_VCE, 1);
> mdelay(5);
>
> return vce_v2_0_start(adev);
> @@ -516,10 +507,8 @@ static int vce_v2_0_process_interrupt(struct
> amdgpu_device *adev,
> DRM_DEBUG("IH: VCE\n");
> switch (entry->src_data) {
> case 0:
> - amdgpu_fence_process(&adev->vce.ring[0]);
> - break;
> case 1:
> - amdgpu_fence_process(&adev->vce.ring[1]);
> + amdgpu_fence_process(&adev->vce.ring[entry->src_data]);
> break;
> default:
> DRM_ERROR("Unhandled interrupt: %d %d\n",
This too.
> --
> 2.9.2
>
> _______________________________________________
> 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: 12544 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] 17+ messages in thread
* RE: [PATCH 4/4] drm/amd/amdgpu: UVD v6 register cleanup
[not found] ` <20160811143253.24718-5-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2016-08-11 15:51 ` Deucher, Alexander
0 siblings, 0 replies; 17+ messages in thread
From: Deucher, Alexander @ 2016-08-11 15:51 UTC (permalink / raw)
To: 'Tom St Denis', amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: StDenis, Tom
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Thursday, August 11, 2016 10:33 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: StDenis, Tom
> Subject: [PATCH 4/4] drm/amd/amdgpu: UVD v6 register cleanup
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
Same comment as the previous patch.
> ---
> drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 27 +++++++++++----------------
> 1 file changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index c11b97f8e376..7b7e82840c95 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -397,15 +397,13 @@ static int uvd_v6_0_start(struct amdgpu_device
> *adev)
> uvd_v6_0_mc_resume(adev);
>
> /* disable clock gating */
> - tmp = RREG32(mmUVD_CGC_CTRL);
> - tmp &= ~UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK;
> - WREG32(mmUVD_CGC_CTRL, tmp);
> + WREG32_FIELD(UVD_CGC_CTRL, DYN_CLOCK_MODE, 0);
>
> /* disable interupt */
> - WREG32_P(mmUVD_MASTINT_EN, 0,
> ~UVD_MASTINT_EN__VCPU_EN_MASK);
> + WREG32_FIELD(UVD_MASTINT_EN, VCPU_EN, 0);
>
> /* stall UMC and register bus before resetting VCPU */
> - WREG32_P(mmUVD_LMI_CTRL2,
> UVD_LMI_CTRL2__STALL_ARB_UMC_MASK,
> ~UVD_LMI_CTRL2__STALL_ARB_UMC_MASK);
> + WREG32_FIELD(UVD_LMI_CTRL2, STALL_ARB_UMC, 1);
> mdelay(1);
>
> /* put LMI, VCPU, RBC etc... into reset */
> @@ -421,7 +419,7 @@ static int uvd_v6_0_start(struct amdgpu_device
> *adev)
> mdelay(5);
>
> /* take UVD block out of reset */
> - WREG32_P(mmSRBM_SOFT_RESET, 0,
> ~SRBM_SOFT_RESET__SOFT_RESET_UVD_MASK);
> + WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_UVD, 0);
> mdelay(5);
>
> /* initialize UVD memory controller */
> @@ -456,7 +454,7 @@ static int uvd_v6_0_start(struct amdgpu_device
> *adev)
> WREG32(mmUVD_VCPU_CNTL,
> UVD_VCPU_CNTL__CLK_EN_MASK);
>
> /* enable UMC */
> - WREG32_P(mmUVD_LMI_CTRL2, 0,
> ~UVD_LMI_CTRL2__STALL_ARB_UMC_MASK);
> + WREG32_FIELD(UVD_LMI_CTRL2, STALL_ARB_UMC, 0);
>
> /* boot up the VCPU */
> WREG32(mmUVD_SOFT_RESET, 0);
> @@ -476,11 +474,9 @@ static int uvd_v6_0_start(struct amdgpu_device
> *adev)
> break;
>
> DRM_ERROR("UVD not responding, trying to reset the
> VCPU!!!\n");
> - WREG32_P(mmUVD_SOFT_RESET,
> UVD_SOFT_RESET__VCPU_SOFT_RESET_MASK,
> -
> ~UVD_SOFT_RESET__VCPU_SOFT_RESET_MASK);
> + WREG32_FIELD(UVD_SOFT_RESET, VCPU_SOFT_RESET, 1);
> mdelay(10);
> - WREG32_P(mmUVD_SOFT_RESET, 0,
> - ~UVD_SOFT_RESET__VCPU_SOFT_RESET_MASK);
> + WREG32_FIELD(UVD_SOFT_RESET, VCPU_SOFT_RESET, 0);
> mdelay(10);
> r = -1;
> }
> @@ -497,15 +493,14 @@ static int uvd_v6_0_start(struct amdgpu_device
> *adev)
> /* clear the bit 4 of UVD_STATUS */
> WREG32_P(mmUVD_STATUS, 0, ~(2 <<
> UVD_STATUS__VCPU_REPORT__SHIFT));
>
> + /* force RBC into idle state */
> rb_bufsz = order_base_2(ring->ring_size);
> - tmp = 0;
> - tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_BUFSZ,
> rb_bufsz);
> + tmp = REG_SET_FIELD(0, UVD_RBC_RB_CNTL, RB_BUFSZ, rb_bufsz);
> tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_BLKSZ, 1);
> tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_NO_FETCH, 1);
> tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL,
> RB_WPTR_POLL_EN, 0);
> tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_NO_UPDATE,
> 1);
> tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_RPTR_WR_EN,
> 1);
> - /* force RBC into idle state */
> WREG32(mmUVD_RBC_RB_CNTL, tmp);
>
> /* set the write pointer delay */
> @@ -526,7 +521,7 @@ static int uvd_v6_0_start(struct amdgpu_device
> *adev)
> ring->wptr = RREG32(mmUVD_RBC_RB_RPTR);
> WREG32(mmUVD_RBC_RB_WPTR, ring->wptr);
>
> - WREG32_P(mmUVD_RBC_RB_CNTL, 0,
> ~UVD_RBC_RB_CNTL__RB_NO_FETCH_MASK);
> + WREG32_FIELD(UVD_RBC_RB_CNTL, RB_NO_FETCH, 0);
>
> return 0;
> }
> @@ -743,7 +738,7 @@ static int uvd_v6_0_wait_for_idle(void *handle)
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if (!(RREG32(mmSRBM_STATUS) &
> SRBM_STATUS__UVD_BUSY_MASK))
> + if (uvd_v6_0_is_idle(handle))
> return 0;
This could be split out as a separate patch.
> }
> return -ETIMEDOUT;
> --
> 2.9.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
[not found] ` <DM5PR12MB1132A35D1BAAB0027D5FE23AF71E0-2J9CzHegvk+UzrhdoDeimQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-08-11 16:09 ` Deucher, Alexander
[not found] ` <MWHPR12MB1694A14EF7C0FAD8E46B5A97F71E0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Deucher, Alexander @ 2016-08-11 16:09 UTC (permalink / raw)
To: StDenis, Tom, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 7060 bytes --]
I guess I don't really have a particularly strong opinion either way. If others are ok with it, I'm fine with it.
Alex
From: StDenis, Tom
Sent: Thursday, August 11, 2016 11:48 AM
To: Deucher, Alexander; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
Just trying to make it easier to read.
WREG32_FIELD(foo, FIELD, 1);
Is easier to read than
WREG32_P(foo, FOO__FIELD_MASK, ~FOO__FIELD_MASK);
(also I already pushed them after getting a RB by Christian this morning so we might need to hold a different discussion).
I agree it's not really a functional change but making things a bit more uniform and easier to read helps maintenance. And I did find a bug in the process :-)
Tom
________________________________
From: Deucher, Alexander
Sent: Thursday, August 11, 2016 11:46
To: 'Tom St Denis'; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32mptlylMvRsHA@public.gmane.orgdesktop.org>
Cc: StDenis, Tom
Subject: RE: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf
> Of Tom St Denis
> Sent: Thursday, August 11, 2016 10:33 AM
> To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> Cc: StDenis, Tom
> Subject: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>
> Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org<mailto:tom.stdenis@amd.com>>
A couple pieces of this patch could be split out as separate cleanups (see below). However, I'm not sure how much value there is in switching WREG32_P for WREG_FIELD other than code churn. They basically do the same thing.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 39 +++++++++++++-------------
> ---------
> 1 file changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> index 80a37a602181..21ba219e943b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> @@ -127,15 +127,10 @@ static int vce_v2_0_start(struct amdgpu_device
> *adev)
> WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
>
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> -
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> -
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> -
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
>
> for (i = 0; i < 10; ++i) {
> uint32_t status;
> @@ -150,10 +145,9 @@ static int vce_v2_0_start(struct amdgpu_device
> *adev)
> break;
>
> DRM_ERROR("VCE not responding, trying to reset the
> ECPU!!!\n");
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> -
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(10);
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(10);
> r = -1;
> }
> @@ -345,13 +339,13 @@ static void vce_v2_0_set_dyn_cg(struct
> amdgpu_device *adev, bool gated)
> DRM_INFO("VCE is busy, Can't set clock gateing");
> return;
> }
> - WREG32_P(mmVCE_VCPU_CNTL, 0,
> ~VCE_VCPU_CNTL__CLK_EN_MASK);
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> WREG32(mmVCE_STATUS, 0);
> } else {
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> }
>
> @@ -458,9 +452,7 @@ static void vce_v2_0_mc_resume(struct
> amdgpu_device *adev)
> WREG32(mmVCE_VCPU_CACHE_SIZE2, size);
>
> WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
> -
> - WREG32_P(mmVCE_SYS_INT_EN,
> VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
> -
> ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
> + WREG32_FIELD(VCE_SYS_INT_EN,
> VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
>
> vce_v2_0_init_cg(adev);
> }
> @@ -474,11 +466,11 @@ static bool vce_v2_0_is_idle(void *handle)
>
> static int vce_v2_0_wait_for_idle(void *handle)
> {
> - unsigned i;
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + unsigned i;
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if (!(RREG32(mmSRBM_STATUS2) &
> SRBM_STATUS2__VCE_BUSY_MASK))
> + if (vce_v2_0_is_idle(handle))
> return 0;
This could be split out as a separate patch.
> }
> return -ETIMEDOUT;
> @@ -488,8 +480,7 @@ static int vce_v2_0_soft_reset(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - WREG32_P(mmSRBM_SOFT_RESET,
> SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK,
> - ~SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK);
> + WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_VCE, 1);
> mdelay(5);
>
> return vce_v2_0_start(adev);
> @@ -516,10 +507,8 @@ static int vce_v2_0_process_interrupt(struct
> amdgpu_device *adev,
> DRM_DEBUG("IH: VCE\n");
> switch (entry->src_data) {
> case 0:
> - amdgpu_fence_process(&adev->vce.ring[0]);
> - break;
> case 1:
> - amdgpu_fence_process(&adev->vce.ring[1]);
> + amdgpu_fence_process(&adev->vce.ring[entry->src_data]);
> break;
> default:
> DRM_ERROR("Unhandled interrupt: %d %d\n",
This too.
> --
> 2.9.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 18592 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] 17+ messages in thread
* Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
[not found] ` <MWHPR12MB1694A14EF7C0FAD8E46B5A97F71E0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-08-12 1:52 ` zhoucm1
[not found] ` <57AD2BC7.70709-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: zhoucm1 @ 2016-08-12 1:52 UTC (permalink / raw)
To: Deucher, Alexander, StDenis, Tom,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 7499 bytes --]
Agree with Tom.
On 2016年08月12日 00:09, Deucher, Alexander wrote:
>
> I guess I don't really have a particularly strong opinion either way.
> If others are ok with it, I'm fine with it.
>
> Alex
>
> *From:*StDenis, Tom
> *Sent:* Thursday, August 11, 2016 11:48 AM
> *To:* Deucher, Alexander; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>
> Just trying to make it easier to read.
>
> WREG32_FIELD(foo, FIELD, 1);
>
> Is easier to read than
>
> WREG32_P(foo, FOO__FIELD_MASK, ~FOO__FIELD_MASK);
>
> (also I already pushed them after getting a RB by Christian this
> morning so we might need to hold a different discussion).
>
> I agree it's not really a functional change but making things a bit
> more uniform and easier to read helps maintenance. And I did find a
> bug in the process :-)
>
> Tom
>
> ------------------------------------------------------------------------
>
> *From:*Deucher, Alexander
> *Sent:* Thursday, August 11, 2016 11:46
> *To:* 'Tom St Denis'; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> *Cc:* StDenis, Tom
> *Subject:* RE: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>
> > -----Original Message-----
> > From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf
> > Of Tom St Denis
> > Sent: Thursday, August 11, 2016 10:33 AM
> > To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> > Cc: StDenis, Tom
> > Subject: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
> >
> > Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org
> <mailto:tom.stdenis-5C7GfCeVMHo@public.gmane.org>>
>
> A couple pieces of this patch could be split out as separate cleanups
> (see below). However, I'm not sure how much value there is in
> switching WREG32_P for WREG_FIELD other than code churn. They
> basically do the same thing.
>
> Alex
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 39 +++++++++++++-------------
> > ---------
> > 1 file changed, 14 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> > b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> > index 80a37a602181..21ba219e943b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> > @@ -127,15 +127,10 @@ static int vce_v2_0_start(struct amdgpu_device
> > *adev)
> > WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> > WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
> >
> > - WREG32_P(mmVCE_VCPU_CNTL,
> > VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> > -
> > - WREG32_P(mmVCE_SOFT_RESET,
> > - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> > - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> > -
> > + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> > mdelay(100);
> > -
> > - WREG32_P(mmVCE_SOFT_RESET, 0,
> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> >
> > for (i = 0; i < 10; ++i) {
> > uint32_t status;
> > @@ -150,10 +145,9 @@ static int vce_v2_0_start(struct amdgpu_device
> > *adev)
> > break;
> >
> > DRM_ERROR("VCE not responding, trying to reset the
> > ECPU!!!\n");
> > - WREG32_P(mmVCE_SOFT_RESET,
> > VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> > -
> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> > mdelay(10);
> > - WREG32_P(mmVCE_SOFT_RESET, 0,
> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> > mdelay(10);
> > r = -1;
> > }
> > @@ -345,13 +339,13 @@ static void vce_v2_0_set_dyn_cg(struct
> > amdgpu_device *adev, bool gated)
> > DRM_INFO("VCE is busy, Can't set clock gateing");
> > return;
> > }
> > - WREG32_P(mmVCE_VCPU_CNTL, 0,
> > ~VCE_VCPU_CNTL__CLK_EN_MASK);
> > - WREG32_P(mmVCE_SOFT_RESET,
> > VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> > + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> > mdelay(100);
> > WREG32(mmVCE_STATUS, 0);
> > } else {
> > - WREG32_P(mmVCE_VCPU_CNTL,
> > VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> > - WREG32_P(mmVCE_SOFT_RESET,
> > VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> > + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> > mdelay(100);
> > }
> >
> > @@ -458,9 +452,7 @@ static void vce_v2_0_mc_resume(struct
> > amdgpu_device *adev)
> > WREG32(mmVCE_VCPU_CACHE_SIZE2, size);
> >
> > WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
> > -
> > - WREG32_P(mmVCE_SYS_INT_EN,
> > VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
> > -
> > ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
> > + WREG32_FIELD(VCE_SYS_INT_EN,
> > VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
> >
> > vce_v2_0_init_cg(adev);
> > }
> > @@ -474,11 +466,11 @@ static bool vce_v2_0_is_idle(void *handle)
> >
> > static int vce_v2_0_wait_for_idle(void *handle)
> > {
> > - unsigned i;
> > struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> > + unsigned i;
> >
> > for (i = 0; i < adev->usec_timeout; i++) {
> > - if (!(RREG32(mmSRBM_STATUS2) &
> > SRBM_STATUS2__VCE_BUSY_MASK))
> > + if (vce_v2_0_is_idle(handle))
> > return 0;
>
> This could be split out as a separate patch.
>
> > }
> > return -ETIMEDOUT;
> > @@ -488,8 +480,7 @@ static int vce_v2_0_soft_reset(void *handle)
> > {
> > struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> >
> > - WREG32_P(mmSRBM_SOFT_RESET,
> > SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK,
> > - ~SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK);
> > + WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_VCE, 1);
> > mdelay(5);
> >
> > return vce_v2_0_start(adev);
> > @@ -516,10 +507,8 @@ static int vce_v2_0_process_interrupt(struct
> > amdgpu_device *adev,
> > DRM_DEBUG("IH: VCE\n");
> > switch (entry->src_data) {
> > case 0:
> > - amdgpu_fence_process(&adev->vce.ring[0]);
> > - break;
> > case 1:
> > - amdgpu_fence_process(&adev->vce.ring[1]);
> > + amdgpu_fence_process(&adev->vce.ring[entry->src_data]);
> > break;
> > default:
> > DRM_ERROR("Unhandled interrupt: %d %d\n",
>
> This too.
>
> > --
> > 2.9.2
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
> _______________________________________________
> 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: 21362 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] 17+ messages in thread
* Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
[not found] ` <57AD2BC7.70709-5C7GfCeVMHo@public.gmane.org>
@ 2016-08-12 7:34 ` Christian König
2016-08-12 14:01 ` Deucher, Alexander
1 sibling, 0 replies; 17+ messages in thread
From: Christian König @ 2016-08-12 7:34 UTC (permalink / raw)
To: zhoucm1, Deucher, Alexander, StDenis, Tom,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 8144 bytes --]
Dito, using WREG32_FIELD is clearly easier to read.
If the additional register writes are really an issue we could adjust
the macro to skip those if the value didn't changed.
Regards,
Christian.
Am 12.08.2016 um 03:52 schrieb zhoucm1:
> Agree with Tom.
>
> On 2016年08月12日 00:09, Deucher, Alexander wrote:
>>
>> I guess I don't really have a particularly strong opinion either
>> way. If others are ok with it, I'm fine with it.
>>
>> Alex
>>
>> *From:*StDenis, Tom
>> *Sent:* Thursday, August 11, 2016 11:48 AM
>> *To:* Deucher, Alexander; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> *Subject:* Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>>
>> Just trying to make it easier to read.
>>
>> WREG32_FIELD(foo, FIELD, 1);
>>
>> Is easier to read than
>>
>> WREG32_P(foo, FOO__FIELD_MASK, ~FOO__FIELD_MASK);
>>
>> (also I already pushed them after getting a RB by Christian this
>> morning so we might need to hold a different discussion).
>>
>> I agree it's not really a functional change but making things a bit
>> more uniform and easier to read helps maintenance. And I did find a
>> bug in the process :-)
>>
>> Tom
>>
>> ------------------------------------------------------------------------
>>
>> *From:*Deucher, Alexander
>> *Sent:* Thursday, August 11, 2016 11:46
>> *To:* 'Tom St Denis'; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>> *Cc:* StDenis, Tom
>> *Subject:* RE: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>>
>> > -----Original Message-----
>> > From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf
>> > Of Tom St Denis
>> > Sent: Thursday, August 11, 2016 10:33 AM
>> > To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>> > Cc: StDenis, Tom
>> > Subject: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>> >
>> > Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org
>> <mailto:tom.stdenis-5C7GfCeVMHo@public.gmane.org>>
>>
>> A couple pieces of this patch could be split out as separate cleanups
>> (see below). However, I'm not sure how much value there is in
>> switching WREG32_P for WREG_FIELD other than code churn. They
>> basically do the same thing.
>>
>> Alex
>>
>> > ---
>> > drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 39 +++++++++++++-------------
>> > ---------
>> > 1 file changed, 14 insertions(+), 25 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
>> > b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
>> > index 80a37a602181..21ba219e943b 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
>> > @@ -127,15 +127,10 @@ static int vce_v2_0_start(struct amdgpu_device
>> > *adev)
>> > WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
>> > WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
>> >
>> > - WREG32_P(mmVCE_VCPU_CNTL,
>> > VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
>> > -
>> > - WREG32_P(mmVCE_SOFT_RESET,
>> > - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
>> > - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> > -
>> > + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
>> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>> > mdelay(100);
>> > -
>> > - WREG32_P(mmVCE_SOFT_RESET, 0,
>> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
>> >
>> > for (i = 0; i < 10; ++i) {
>> > uint32_t status;
>> > @@ -150,10 +145,9 @@ static int vce_v2_0_start(struct amdgpu_device
>> > *adev)
>> > break;
>> >
>> > DRM_ERROR("VCE not responding, trying to reset the
>> > ECPU!!!\n");
>> > - WREG32_P(mmVCE_SOFT_RESET,
>> > VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
>> > -
>> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>> > mdelay(10);
>> > - WREG32_P(mmVCE_SOFT_RESET, 0,
>> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
>> > mdelay(10);
>> > r = -1;
>> > }
>> > @@ -345,13 +339,13 @@ static void vce_v2_0_set_dyn_cg(struct
>> > amdgpu_device *adev, bool gated)
>> > DRM_INFO("VCE is busy, Can't set clock
>> gateing");
>> > return;
>> > }
>> > - WREG32_P(mmVCE_VCPU_CNTL, 0,
>> > ~VCE_VCPU_CNTL__CLK_EN_MASK);
>> > - WREG32_P(mmVCE_SOFT_RESET,
>> > VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
>> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> > + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
>> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>> > mdelay(100);
>> > WREG32(mmVCE_STATUS, 0);
>> > } else {
>> > - WREG32_P(mmVCE_VCPU_CNTL,
>> > VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
>> > - WREG32_P(mmVCE_SOFT_RESET,
>> > VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
>> > ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> > + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
>> > + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>> > mdelay(100);
>> > }
>> >
>> > @@ -458,9 +452,7 @@ static void vce_v2_0_mc_resume(struct
>> > amdgpu_device *adev)
>> > WREG32(mmVCE_VCPU_CACHE_SIZE2, size);
>> >
>> > WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
>> > -
>> > - WREG32_P(mmVCE_SYS_INT_EN,
>> > VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
>> > -
>> > ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
>> > + WREG32_FIELD(VCE_SYS_INT_EN,
>> > VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
>> >
>> > vce_v2_0_init_cg(adev);
>> > }
>> > @@ -474,11 +466,11 @@ static bool vce_v2_0_is_idle(void *handle)
>> >
>> > static int vce_v2_0_wait_for_idle(void *handle)
>> > {
>> > - unsigned i;
>> > struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> > + unsigned i;
>> >
>> > for (i = 0; i < adev->usec_timeout; i++) {
>> > - if (!(RREG32(mmSRBM_STATUS2) &
>> > SRBM_STATUS2__VCE_BUSY_MASK))
>> > + if (vce_v2_0_is_idle(handle))
>> > return 0;
>>
>> This could be split out as a separate patch.
>>
>> > }
>> > return -ETIMEDOUT;
>> > @@ -488,8 +480,7 @@ static int vce_v2_0_soft_reset(void *handle)
>> > {
>> > struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> >
>> > - WREG32_P(mmSRBM_SOFT_RESET,
>> > SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK,
>> > - ~SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK);
>> > + WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_VCE, 1);
>> > mdelay(5);
>> >
>> > return vce_v2_0_start(adev);
>> > @@ -516,10 +507,8 @@ static int vce_v2_0_process_interrupt(struct
>> > amdgpu_device *adev,
>> > DRM_DEBUG("IH: VCE\n");
>> > switch (entry->src_data) {
>> > case 0:
>> > - amdgpu_fence_process(&adev->vce.ring[0]);
>> > - break;
>> > case 1:
>> > - amdgpu_fence_process(&adev->vce.ring[1]);
>> > + amdgpu_fence_process(&adev->vce.ring[entry->src_data]);
>> > break;
>> > default:
>> > DRM_ERROR("Unhandled interrupt: %d %d\n",
>>
>> This too.
>>
>> > --
>> > 2.9.2
>> >
>> > _______________________________________________
>> > amd-gfx mailing list
>> > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
> _______________________________________________
> 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: 23144 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] 17+ messages in thread
* RE: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
[not found] ` <57AD2BC7.70709-5C7GfCeVMHo@public.gmane.org>
2016-08-12 7:34 ` Christian König
@ 2016-08-12 14:01 ` Deucher, Alexander
[not found] ` <MWHPR12MB1694519FD1F5113FBA3EC20CF71F0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
1 sibling, 1 reply; 17+ messages in thread
From: Deucher, Alexander @ 2016-08-12 14:01 UTC (permalink / raw)
To: Zhou, David(ChunMing),
StDenis, Tom, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 7469 bytes --]
In that case:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of zhoucm1
Sent: Thursday, August 11, 2016 9:52 PM
To: Deucher, Alexander; StDenis, Tom; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
Agree with Tom.
On 2016年08月12日 00:09, Deucher, Alexander wrote:
I guess I don't really have a particularly strong opinion either way. If others are ok with it, I'm fine with it.
Alex
From: StDenis, Tom
Sent: Thursday, August 11, 2016 11:48 AM
To: Deucher, Alexander; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
Just trying to make it easier to read.
WREG32_FIELD(foo, FIELD, 1);
Is easier to read than
WREG32_P(foo, FOO__FIELD_MASK, ~FOO__FIELD_MASK);
(also I already pushed them after getting a RB by Christian this morning so we might need to hold a different discussion).
I agree it's not really a functional change but making things a bit more uniform and easier to read helps maintenance. And I did find a bug in the process :-)
Tom
________________________________
From: Deucher, Alexander
Sent: Thursday, August 11, 2016 11:46
To: 'Tom St Denis'; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: StDenis, Tom
Subject: RE: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Thursday, August 11, 2016 10:33 AM
> To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
> Cc: StDenis, Tom
> Subject: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com<mailto:tom.stdenis@amd.com>>
A couple pieces of this patch could be split out as separate cleanups (see below). However, I'm not sure how much value there is in switching WREG32_P for WREG_FIELD other than code churn. They basically do the same thing.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 39 +++++++++++++-------------
> ---------
> 1 file changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> index 80a37a602181..21ba219e943b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> @@ -127,15 +127,10 @@ static int vce_v2_0_start(struct amdgpu_device
> *adev)
> WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
>
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> -
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> -
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> -
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
>
> for (i = 0; i < 10; ++i) {
> uint32_t status;
> @@ -150,10 +145,9 @@ static int vce_v2_0_start(struct amdgpu_device
> *adev)
> break;
>
> DRM_ERROR("VCE not responding, trying to reset the
> ECPU!!!\n");
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> -
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(10);
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
> mdelay(10);
> r = -1;
> }
> @@ -345,13 +339,13 @@ static void vce_v2_0_set_dyn_cg(struct
> amdgpu_device *adev, bool gated)
> DRM_INFO("VCE is busy, Can't set clock gateing");
> return;
> }
> - WREG32_P(mmVCE_VCPU_CNTL, 0,
> ~VCE_VCPU_CNTL__CLK_EN_MASK);
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> WREG32(mmVCE_STATUS, 0);
> } else {
> - WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
> - WREG32_P(mmVCE_SOFT_RESET,
> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
> mdelay(100);
> }
>
> @@ -458,9 +452,7 @@ static void vce_v2_0_mc_resume(struct
> amdgpu_device *adev)
> WREG32(mmVCE_VCPU_CACHE_SIZE2, size);
>
> WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
> -
> - WREG32_P(mmVCE_SYS_INT_EN,
> VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
> -
> ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
> + WREG32_FIELD(VCE_SYS_INT_EN,
> VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
>
> vce_v2_0_init_cg(adev);
> }
> @@ -474,11 +466,11 @@ static bool vce_v2_0_is_idle(void *handle)
>
> static int vce_v2_0_wait_for_idle(void *handle)
> {
> - unsigned i;
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + unsigned i;
>
> for (i = 0; i < adev->usec_timeout; i++) {
> - if (!(RREG32(mmSRBM_STATUS2) &
> SRBM_STATUS2__VCE_BUSY_MASK))
> + if (vce_v2_0_is_idle(handle))
> return 0;
This could be split out as a separate patch.
> }
> return -ETIMEDOUT;
> @@ -488,8 +480,7 @@ static int vce_v2_0_soft_reset(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - WREG32_P(mmSRBM_SOFT_RESET,
> SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK,
> - ~SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK);
> + WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_VCE, 1);
> mdelay(5);
>
> return vce_v2_0_start(adev);
> @@ -516,10 +507,8 @@ static int vce_v2_0_process_interrupt(struct
> amdgpu_device *adev,
> DRM_DEBUG("IH: VCE\n");
> switch (entry->src_data) {
> case 0:
> - amdgpu_fence_process(&adev->vce.ring[0]);
> - break;
> case 1:
> - amdgpu_fence_process(&adev->vce.ring[1]);
> + amdgpu_fence_process(&adev->vce.ring[entry->src_data]);
> break;
> default:
> DRM_ERROR("Unhandled interrupt: %d %d\n",
This too.
> --
> 2.9.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 21052 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] 17+ messages in thread
* Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
[not found] ` <MWHPR12MB1694519FD1F5113FBA3EC20CF71F0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-08-12 14:54 ` Alex Deucher
0 siblings, 0 replies; 17+ messages in thread
From: Alex Deucher @ 2016-08-12 14:54 UTC (permalink / raw)
To: Deucher, Alexander
Cc: StDenis, Tom, Zhou, David(ChunMing),
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Fri, Aug 12, 2016 at 10:01 AM, Deucher, Alexander
<Alexander.Deucher@amd.com> wrote:
> In that case:
For the series:
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
>
>
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of
> zhoucm1
> Sent: Thursday, August 11, 2016 9:52 PM
> To: Deucher, Alexander; StDenis, Tom; amd-gfx@lists.freedesktop.org
>
>
> Subject: Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>
>
>
> Agree with Tom.
>
> On 2016年08月12日 00:09, Deucher, Alexander wrote:
>
> I guess I don't really have a particularly strong opinion either way. If
> others are ok with it, I'm fine with it.
>
>
>
> Alex
>
>
>
> From: StDenis, Tom
> Sent: Thursday, August 11, 2016 11:48 AM
> To: Deucher, Alexander; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>
>
>
> Just trying to make it easier to read.
>
>
>
> WREG32_FIELD(foo, FIELD, 1);
>
>
>
> Is easier to read than
>
>
>
> WREG32_P(foo, FOO__FIELD_MASK, ~FOO__FIELD_MASK);
>
>
>
> (also I already pushed them after getting a RB by Christian this morning so
> we might need to hold a different discussion).
>
>
>
> I agree it's not really a functional change but making things a bit more
> uniform and easier to read helps maintenance. And I did find a bug in the
> process :-)
>
>
>
> Tom
>
>
>
> ________________________________
>
> From: Deucher, Alexander
> Sent: Thursday, August 11, 2016 11:46
> To: 'Tom St Denis'; amd-gfx@lists.freedesktop.org
> Cc: StDenis, Tom
> Subject: RE: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>
>
>
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
>> Of Tom St Denis
>> Sent: Thursday, August 11, 2016 10:33 AM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: StDenis, Tom
>> Subject: [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup
>>
>> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
>
> A couple pieces of this patch could be split out as separate cleanups (see
> below). However, I'm not sure how much value there is in switching WREG32_P
> for WREG_FIELD other than code churn. They basically do the same thing.
>
> Alex
>
>> ---
>> drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 39 +++++++++++++-------------
>> ---------
>> 1 file changed, 14 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
>> index 80a37a602181..21ba219e943b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
>> @@ -127,15 +127,10 @@ static int vce_v2_0_start(struct amdgpu_device
>> *adev)
>> WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
>> WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
>>
>> - WREG32_P(mmVCE_VCPU_CNTL,
>> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
>> -
>> - WREG32_P(mmVCE_SOFT_RESET,
>> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
>> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> -
>> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
>> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>> mdelay(100);
>> -
>> - WREG32_P(mmVCE_SOFT_RESET, 0,
>> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
>>
>> for (i = 0; i < 10; ++i) {
>> uint32_t status;
>> @@ -150,10 +145,9 @@ static int vce_v2_0_start(struct amdgpu_device
>> *adev)
>> break;
>>
>> DRM_ERROR("VCE not responding, trying to reset the
>> ECPU!!!\n");
>> - WREG32_P(mmVCE_SOFT_RESET,
>> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
>> -
>> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>> mdelay(10);
>> - WREG32_P(mmVCE_SOFT_RESET, 0,
>> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 0);
>> mdelay(10);
>> r = -1;
>> }
>> @@ -345,13 +339,13 @@ static void vce_v2_0_set_dyn_cg(struct
>> amdgpu_device *adev, bool gated)
>> DRM_INFO("VCE is busy, Can't set clock gateing");
>> return;
>> }
>> - WREG32_P(mmVCE_VCPU_CNTL, 0,
>> ~VCE_VCPU_CNTL__CLK_EN_MASK);
>> - WREG32_P(mmVCE_SOFT_RESET,
>> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
>> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 0);
>> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>> mdelay(100);
>> WREG32(mmVCE_STATUS, 0);
>> } else {
>> - WREG32_P(mmVCE_VCPU_CNTL,
>> VCE_VCPU_CNTL__CLK_EN_MASK, ~VCE_VCPU_CNTL__CLK_EN_MASK);
>> - WREG32_P(mmVCE_SOFT_RESET,
>> VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
>> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>> + WREG32_FIELD(VCE_VCPU_CNTL, CLK_EN, 1);
>> + WREG32_FIELD(VCE_SOFT_RESET, ECPU_SOFT_RESET, 1);
>> mdelay(100);
>> }
>>
>> @@ -458,9 +452,7 @@ static void vce_v2_0_mc_resume(struct
>> amdgpu_device *adev)
>> WREG32(mmVCE_VCPU_CACHE_SIZE2, size);
>>
>> WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
>> -
>> - WREG32_P(mmVCE_SYS_INT_EN,
>> VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK,
>> -
>> ~VCE_SYS_INT_EN__VCE_SYS_INT_TRAP_INTERRUPT_EN_MASK);
>> + WREG32_FIELD(VCE_SYS_INT_EN,
>> VCE_SYS_INT_TRAP_INTERRUPT_EN, 1);
>>
>> vce_v2_0_init_cg(adev);
>> }
>> @@ -474,11 +466,11 @@ static bool vce_v2_0_is_idle(void *handle)
>>
>> static int vce_v2_0_wait_for_idle(void *handle)
>> {
>> - unsigned i;
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> + unsigned i;
>>
>> for (i = 0; i < adev->usec_timeout; i++) {
>> - if (!(RREG32(mmSRBM_STATUS2) &
>> SRBM_STATUS2__VCE_BUSY_MASK))
>> + if (vce_v2_0_is_idle(handle))
>> return 0;
>
> This could be split out as a separate patch.
>
>> }
>> return -ETIMEDOUT;
>> @@ -488,8 +480,7 @@ static int vce_v2_0_soft_reset(void *handle)
>> {
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>
>> - WREG32_P(mmSRBM_SOFT_RESET,
>> SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK,
>> - ~SRBM_SOFT_RESET__SOFT_RESET_VCE_MASK);
>> + WREG32_FIELD(SRBM_SOFT_RESET, SOFT_RESET_VCE, 1);
>> mdelay(5);
>>
>> return vce_v2_0_start(adev);
>> @@ -516,10 +507,8 @@ static int vce_v2_0_process_interrupt(struct
>> amdgpu_device *adev,
>> DRM_DEBUG("IH: VCE\n");
>> switch (entry->src_data) {
>> case 0:
>> - amdgpu_fence_process(&adev->vce.ring[0]);
>> - break;
>> case 1:
>> - amdgpu_fence_process(&adev->vce.ring[1]);
>> + amdgpu_fence_process(&adev->vce.ring[entry->src_data]);
>> break;
>> default:
>> DRM_ERROR("Unhandled interrupt: %d %d\n",
>
> This too.
>
>> --
>> 2.9.2
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
>
> _______________________________________________
>
> amd-gfx mailing list
>
> amd-gfx@lists.freedesktop.org
>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-08-12 14:54 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-11 14:32 Various tidy-ups for VCE/UVD Tom St Denis
[not found] ` <20160811143253.24718-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 14:32 ` [PATCH 1/4] drm/amd/amdgpu: Cleanup register access in VCE v3 Tom St Denis
[not found] ` <20160811143253.24718-2-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 14:42 ` Christian König
2016-08-11 15:41 ` Deucher, Alexander
[not found] ` <MWHPR12MB169403F24E091D033BEE21F6F71E0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-08-11 15:43 ` StDenis, Tom
2016-08-11 14:32 ` [PATCH 2/4] drm/amd/amdgpu: add mutex in check_soft for " Tom St Denis
[not found] ` <20160811143253.24718-3-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 15:42 ` Deucher, Alexander
2016-08-11 14:32 ` [PATCH 3/4] drm/amd/amdgpu: VCE v2 register cleanup Tom St Denis
[not found] ` <20160811143253.24718-4-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 15:46 ` Deucher, Alexander
[not found] ` <MWHPR12MB1694FE8258E0ADB4F32DC885F71E0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-08-11 15:48 ` StDenis, Tom
[not found] ` <DM5PR12MB1132A35D1BAAB0027D5FE23AF71E0-2J9CzHegvk+UzrhdoDeimQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-08-11 16:09 ` Deucher, Alexander
[not found] ` <MWHPR12MB1694A14EF7C0FAD8E46B5A97F71E0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-08-12 1:52 ` zhoucm1
[not found] ` <57AD2BC7.70709-5C7GfCeVMHo@public.gmane.org>
2016-08-12 7:34 ` Christian König
2016-08-12 14:01 ` Deucher, Alexander
[not found] ` <MWHPR12MB1694519FD1F5113FBA3EC20CF71F0-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-08-12 14:54 ` Alex Deucher
2016-08-11 14:32 ` [PATCH 4/4] drm/amd/amdgpu: UVD v6 " Tom St Denis
[not found] ` <20160811143253.24718-5-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-08-11 15:51 ` Deucher, Alexander
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.