I'm a little curious about the failures test cases. Is it related to a specific ASIC? Using CPC_INT_CNTL seemed to be working well for me on polaris10 (I was getting terrible perf on pipes 1+ without the original patch). Regards, Andres On 2017-06-09 08:49 AM, Alex Deucher wrote: > The interrupt registers are not indexed. > > Fixes: 763a47b8e (drm/amdgpu: teach amdgpu how to enable interrupts for any pipe v3) > Signed-off-by: Alex Deucher <alexander.deucher at amd.com> > --- > drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 57 +++++++++++++++++++++++---------- > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 57 +++++++++++++++++++++++---------- > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 59 +++++++++++++++++++++++++---------- > 3 files changed, 124 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > index e30c7d0..fb0a94c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > @@ -5015,28 +5015,51 @@ static void gfx_v7_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev, > int me, int pipe, > enum amdgpu_interrupt_state state) > { > - /* Me 0 is for graphics and Me 2 is reserved for HW scheduling > - * So we should only really be configuring ME 1 i.e. MEC0 > + u32 mec_int_cntl, mec_int_cntl_reg; > + > + /* > + * amdgpu controls only the first MEC. That's why this function only > + * handles the setting of interrupts for this specific MEC. All other > + * pipes' interrupts are set by amdkfd. > */ > - if (me != 1) { > - DRM_ERROR("Ignoring request to enable interrupts for invalid me:%d\n", me); > - return; > - } > > - if (pipe >= adev->gfx.mec.num_pipe_per_mec) { > - DRM_ERROR("Ignoring request to enable interrupts for invalid " > - "me:%d pipe:%d\n", pipe, me); > + if (me == 1) { > + switch (pipe) { > + case 0: > + mec_int_cntl_reg = mmCP_ME1_PIPE0_INT_CNTL; > + break; > + case 1: > + mec_int_cntl_reg = mmCP_ME1_PIPE1_INT_CNTL; > + break; > + case 2: > + mec_int_cntl_reg = mmCP_ME1_PIPE2_INT_CNTL; > + break; > + case 3: > + mec_int_cntl_reg = mmCP_ME1_PIPE3_INT_CNTL; > + break; > + default: > + DRM_DEBUG("invalid pipe %d\n", pipe); > + return; > + } > + } else { > + DRM_DEBUG("invalid me %d\n", me); > return; > } > > - mutex_lock(&adev->srbm_mutex); > - cik_srbm_select(adev, me, pipe, 0, 0); > - > - WREG32_FIELD(CPC_INT_CNTL, TIME_STAMP_INT_ENABLE, > - state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1); > - > - cik_srbm_select(adev, 0, 0, 0, 0); > - mutex_unlock(&adev->srbm_mutex); > + switch (state) { > + case AMDGPU_IRQ_STATE_DISABLE: > + mec_int_cntl = RREG32(mec_int_cntl_reg); > + mec_int_cntl &= ~CP_INT_CNTL_RING0__TIME_STAMP_INT_ENABLE_MASK; > + WREG32(mec_int_cntl_reg, mec_int_cntl); > + break; > + case AMDGPU_IRQ_STATE_ENABLE: > + mec_int_cntl = RREG32(mec_int_cntl_reg); > + mec_int_cntl |= CP_INT_CNTL_RING0__TIME_STAMP_INT_ENABLE_MASK; > + WREG32(mec_int_cntl_reg, mec_int_cntl); > + break; > + default: > + break; > + } > } > > static int gfx_v7_0_set_priv_reg_fault_state(struct amdgpu_device *adev, > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > index 6e541af..1a75ab1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > @@ -6610,26 +6610,51 @@ static void gfx_v8_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev, > int me, int pipe, > enum amdgpu_interrupt_state state) > { > - /* Me 0 is reserved for graphics */ > - if (me < 1 || me > adev->gfx.mec.num_mec) { > - DRM_ERROR("Ignoring request to enable interrupts for invalid me:%d\n", me); > - return; > - } > + u32 mec_int_cntl, mec_int_cntl_reg; > > - if (pipe >= adev->gfx.mec.num_pipe_per_mec) { > - DRM_ERROR("Ignoring request to enable interrupts for invalid " > - "me:%d pipe:%d\n", pipe, me); > + /* > + * amdgpu controls only the first MEC. That's why this function only > + * handles the setting of interrupts for this specific MEC. All other > + * pipes' interrupts are set by amdkfd. > + */ > + > + if (me == 1) { > + switch (pipe) { > + case 0: > + mec_int_cntl_reg = mmCP_ME1_PIPE0_INT_CNTL; > + break; > + case 1: > + mec_int_cntl_reg = mmCP_ME1_PIPE1_INT_CNTL; > + break; > + case 2: > + mec_int_cntl_reg = mmCP_ME1_PIPE2_INT_CNTL; > + break; > + case 3: > + mec_int_cntl_reg = mmCP_ME1_PIPE3_INT_CNTL; > + break; > + default: > + DRM_DEBUG("invalid pipe %d\n", pipe); > + return; > + } > + } else { > + DRM_DEBUG("invalid me %d\n", me); > return; > } > > - mutex_lock(&adev->srbm_mutex); > - vi_srbm_select(adev, me, pipe, 0, 0); > - > - WREG32_FIELD(CPC_INT_CNTL, TIME_STAMP_INT_ENABLE, > - state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1); > - > - vi_srbm_select(adev, 0, 0, 0, 0); > - mutex_unlock(&adev->srbm_mutex); > + switch (state) { > + case AMDGPU_IRQ_STATE_DISABLE: > + mec_int_cntl = RREG32(mec_int_cntl_reg); > + mec_int_cntl &= ~CP_INT_CNTL_RING0__TIME_STAMP_INT_ENABLE_MASK; > + WREG32(mec_int_cntl_reg, mec_int_cntl); > + break; > + case AMDGPU_IRQ_STATE_ENABLE: > + mec_int_cntl = RREG32(mec_int_cntl_reg); > + mec_int_cntl |= CP_INT_CNTL_RING0__TIME_STAMP_INT_ENABLE_MASK; > + WREG32(mec_int_cntl_reg, mec_int_cntl); > + break; > + default: > + break; > + } > } > > static int gfx_v8_0_set_priv_reg_fault_state(struct amdgpu_device *adev, > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > index 375620a..e9dd2c1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > @@ -3982,26 +3982,53 @@ static void gfx_v9_0_set_compute_eop_interrupt_state(struct amdgpu_device *adev, > int me, int pipe, > enum amdgpu_interrupt_state state) > { > - /* Me 0 is reserved for graphics */ > - if (me < 1 || me > adev->gfx.mec.num_mec) { > - DRM_ERROR("Ignoring request to enable interrupts for invalid me:%d\n", me); > - return; > - } > + u32 mec_int_cntl, mec_int_cntl_reg; > > - if (pipe >= adev->gfx.mec.num_pipe_per_mec) { > - DRM_ERROR("Ignoring request to enable interrupts for invalid " > - "me:%d pipe:%d\n", pipe, me); > + /* > + * amdgpu controls only the first MEC. That's why this function only > + * handles the setting of interrupts for this specific MEC. All other > + * pipes' interrupts are set by amdkfd. > + */ > + > + if (me == 1) { > + switch (pipe) { > + case 0: > + mec_int_cntl_reg = SOC15_REG_OFFSET(GC, 0, mmCP_ME1_PIPE0_INT_CNTL); > + break; > + case 1: > + mec_int_cntl_reg = SOC15_REG_OFFSET(GC, 0, mmCP_ME1_PIPE1_INT_CNTL); > + break; > + case 2: > + mec_int_cntl_reg = SOC15_REG_OFFSET(GC, 0, mmCP_ME1_PIPE2_INT_CNTL); > + break; > + case 3: > + mec_int_cntl_reg = SOC15_REG_OFFSET(GC, 0, mmCP_ME1_PIPE3_INT_CNTL); > + break; > + default: > + DRM_DEBUG("invalid pipe %d\n", pipe); > + return; > + } > + } else { > + DRM_DEBUG("invalid me %d\n", me); > return; > } > > - mutex_lock(&adev->srbm_mutex); > - soc15_grbm_select(adev, me, pipe, 0, 0); > - > - WREG32_FIELD(CPC_INT_CNTL, TIME_STAMP_INT_ENABLE, > - state == AMDGPU_IRQ_STATE_DISABLE ? 0 : 1); > - > - soc15_grbm_select(adev, 0, 0, 0, 0); > - mutex_unlock(&adev->srbm_mutex); > + switch (state) { > + case AMDGPU_IRQ_STATE_DISABLE: > + mec_int_cntl = RREG32(mec_int_cntl_reg); > + mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL, > + TIME_STAMP_INT_ENABLE, 0); > + WREG32(mec_int_cntl_reg, mec_int_cntl); > + break; > + case AMDGPU_IRQ_STATE_ENABLE: > + mec_int_cntl = RREG32(mec_int_cntl_reg); > + mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL, > + TIME_STAMP_INT_ENABLE, 1); > + WREG32(mec_int_cntl_reg, mec_int_cntl); > + break; > + default: > + break; > + } > } > > static int gfx_v9_0_set_priv_reg_fault_state(struct amdgpu_device *adev, >