[PATCH 1/2] drm/amdgpu/gfx: fix MEC interrupt enablement for pipes != 0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Friday, June 09, 2017 9:19 AM
> To: amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 1/2] drm/amdgpu/gfx: fix MEC interrupt enablement for
> pipes != 0
> 
> 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.

That doesn't work if you have variable register names as we do in this case.

Alex

> 
> Cheers,
> Tom
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux