[PATCH 1/2] drm/amdgpu/gfx: fix MEC interrupt enablement for pipes != 0
Tom St Denis
tom.stdenis at amd.com
Fri Jun 9 13:19:28 UTC 2017
On 09/06/17 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;
> + }
Could use WREG32_FIELD() here to simplify these.
Cheers,
Tom
More information about the amd-gfx
mailing list