[PATCH 1/3] drm/amdgpu: enable IH ring 1 and ring 2 v3

Kuehling, Felix Felix.Kuehling at amd.com
Sun Jan 13 17:13:56 UTC 2019


The series is Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken at gmail.com> 
Sent: Friday, January 11, 2019 7:02 AM
To: amd-gfx at lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher at amd.com>; Kuehling, Felix <Felix.Kuehling at amd.com>
Subject: Re: [PATCH 1/3] drm/amdgpu: enable IH ring 1 and ring 2 v3

Am 09.01.19 um 14:12 schrieb Christian König:
> The entries are ignored for now, but it at least stops crashing the 
> hardware when somebody tries to push something to the other IH rings.
>
> v2: limit ring size, add TODO comment
> v3: only program rings if they are actually allocated
>
> Signed-off-by: Christian König <christian.koenig at amd.com>

Ping Alex & Felix any more comments on this series?

Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h |   4 +-
>   drivers/gpu/drm/amd/amdgpu/vega10_ih.c  | 143 ++++++++++++++++++++----
>   2 files changed, 121 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> index f6ce171cb8aa..7e06fa64321a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> @@ -87,8 +87,8 @@ struct amdgpu_irq {
>   	/* status, etc. */
>   	bool				msi_enabled; /* msi enabled */
>   
> -	/* interrupt ring */
> -	struct amdgpu_ih_ring		ih;
> +	/* interrupt rings */
> +	struct amdgpu_ih_ring		ih, ih1, ih2;
>   	const struct amdgpu_ih_funcs	*ih_funcs;
>   
>   	/* gen irq stuff */
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c 
> b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> index 562701939d3e..eea5530d2961 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> @@ -50,6 +50,22 @@ static void vega10_ih_enable_interrupts(struct amdgpu_device *adev)
>   	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, ENABLE_INTR, 1);
>   	WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL, ih_rb_cntl);
>   	adev->irq.ih.enabled = true;
> +
> +	if (adev->irq.ih1.ring_size) {
> +		ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1);
> +		ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1,
> +					   RB_ENABLE, 1);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl);
> +		adev->irq.ih1.enabled = true;
> +	}
> +
> +	if (adev->irq.ih2.ring_size) {
> +		ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2);
> +		ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2,
> +					   RB_ENABLE, 1);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl);
> +		adev->irq.ih2.enabled = true;
> +	}
>   }
>   
>   /**
> @@ -71,6 +87,53 @@ static void vega10_ih_disable_interrupts(struct amdgpu_device *adev)
>   	WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR, 0);
>   	adev->irq.ih.enabled = false;
>   	adev->irq.ih.rptr = 0;
> +
> +	if (adev->irq.ih1.ring_size) {
> +		ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1);
> +		ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1,
> +					   RB_ENABLE, 0);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl);
> +		/* set rptr, wptr to 0 */
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING1, 0);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR_RING1, 0);
> +		adev->irq.ih1.enabled = false;
> +		adev->irq.ih1.rptr = 0;
> +	}
> +
> +	if (adev->irq.ih2.ring_size) {
> +		ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2);
> +		ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2,
> +					   RB_ENABLE, 0);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl);
> +		/* set rptr, wptr to 0 */
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING2, 0);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR_RING2, 0);
> +		adev->irq.ih2.enabled = false;
> +		adev->irq.ih2.rptr = 0;
> +	}
> +}
> +
> +static uint32_t vega10_ih_rb_cntl(struct amdgpu_ih_ring *ih, uint32_t 
> +ih_rb_cntl) {
> +	int rb_bufsz = order_base_2(ih->ring_size / 4);
> +
> +	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL,
> +				   MC_SPACE, ih->use_bus_addr ? 1 : 4);
> +	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL,
> +				   WPTR_OVERFLOW_CLEAR, 1);
> +	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL,
> +				   WPTR_OVERFLOW_ENABLE, 1);
> +	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, RB_SIZE, rb_bufsz);
> +	/* Ring Buffer write pointer writeback. If enabled, IH_RB_WPTR register
> +	 * value is written to memory
> +	 */
> +	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL,
> +				   WPTR_WRITEBACK_ENABLE, 1);
> +	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_SNOOP, 1);
> +	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_RO, 0);
> +	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_VMID, 0);
> +
> +	return ih_rb_cntl;
>   }
>   
>   /**
> @@ -86,9 +149,8 @@ static void vega10_ih_disable_interrupts(struct amdgpu_device *adev)
>    */
>   static int vega10_ih_irq_init(struct amdgpu_device *adev)
>   {
> -	struct amdgpu_ih_ring *ih = &adev->irq.ih;
> +	struct amdgpu_ih_ring *ih;
>   	int ret = 0;
> -	int rb_bufsz;
>   	u32 ih_rb_cntl, ih_doorbell_rtpr;
>   	u32 tmp;
>   
> @@ -97,26 +159,15 @@ static int vega10_ih_irq_init(struct 
> amdgpu_device *adev)
>   
>   	adev->nbio_funcs->ih_control(adev);
>   
> -	ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL);
> +	ih = &adev->irq.ih;
>   	/* Ring Buffer base. [39:8] of 40-bit address of the beginning of the ring buffer*/
> -	WREG32_SOC15(OSSSYS, 0, mmIH_RB_BASE, adev->irq.ih.gpu_addr >> 8);
> -	WREG32_SOC15(OSSSYS, 0, mmIH_RB_BASE_HI,
> -		     (adev->irq.ih.gpu_addr >> 40) & 0xff);
> -	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_SPACE,
> -				   ih->use_bus_addr ? 1 : 4);
> -	rb_bufsz = order_base_2(adev->irq.ih.ring_size / 4);
> -	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, WPTR_OVERFLOW_CLEAR, 1);
> -	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, WPTR_OVERFLOW_ENABLE, 1);
> -	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, RB_SIZE, rb_bufsz);
> -	/* Ring Buffer write pointer writeback. If enabled, IH_RB_WPTR register value is written to memory */
> -	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, WPTR_WRITEBACK_ENABLE, 1);
> -	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_SNOOP, 1);
> -	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_RO, 0);
> -	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, MC_VMID, 0);
> -
> -	if (adev->irq.msi_enabled)
> -		ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, RPTR_REARM, 1);
> +	WREG32_SOC15(OSSSYS, 0, mmIH_RB_BASE, ih->gpu_addr >> 8);
> +	WREG32_SOC15(OSSSYS, 0, mmIH_RB_BASE_HI, (ih->gpu_addr >> 40) & 
> +0xff);
>   
> +	ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL);
> +	ih_rb_cntl = vega10_ih_rb_cntl(ih, ih_rb_cntl);
> +	ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL, RPTR_REARM,
> +				   !!adev->irq.msi_enabled);
>   	WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL, ih_rb_cntl);
>   
>   	/* set the writeback address whether it's enabled or not */ @@ 
> -131,18 +182,51 @@ static int vega10_ih_irq_init(struct amdgpu_device 
> *adev)
>   
>   	ih_doorbell_rtpr = RREG32_SOC15(OSSSYS, 0, mmIH_DOORBELL_RPTR);
>   	if (adev->irq.ih.use_doorbell) {
> -		ih_doorbell_rtpr = REG_SET_FIELD(ih_doorbell_rtpr, IH_DOORBELL_RPTR,
> -						 OFFSET, adev->irq.ih.doorbell_index);
> -		ih_doorbell_rtpr = REG_SET_FIELD(ih_doorbell_rtpr, IH_DOORBELL_RPTR,
> +		ih_doorbell_rtpr = REG_SET_FIELD(ih_doorbell_rtpr,
> +						 IH_DOORBELL_RPTR, OFFSET,
> +						 adev->irq.ih.doorbell_index);
> +		ih_doorbell_rtpr = REG_SET_FIELD(ih_doorbell_rtpr,
> +						 IH_DOORBELL_RPTR,
>   						 ENABLE, 1);
>   	} else {
> -		ih_doorbell_rtpr = REG_SET_FIELD(ih_doorbell_rtpr, IH_DOORBELL_RPTR,
> +		ih_doorbell_rtpr = REG_SET_FIELD(ih_doorbell_rtpr,
> +						 IH_DOORBELL_RPTR,
>   						 ENABLE, 0);
>   	}
>   	WREG32_SOC15(OSSSYS, 0, mmIH_DOORBELL_RPTR, ih_doorbell_rtpr);
>   	adev->nbio_funcs->ih_doorbell_range(adev, adev->irq.ih.use_doorbell,
>   					    adev->irq.ih.doorbell_index);
>   
> +	ih = &adev->irq.ih1;
> +	if (ih->ring_size) {
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_BASE_RING1, ih->gpu_addr >> 8);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_BASE_HI_RING1,
> +			     (ih->gpu_addr >> 40) & 0xff);
> +
> +		ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1);
> +		ih_rb_cntl = vega10_ih_rb_cntl(ih, ih_rb_cntl);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl);
> +
> +		/* set rptr, wptr to 0 */
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING1, 0);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR_RING1, 0);
> +	}
> +
> +	ih = &adev->irq.ih2;
> +	if (ih->ring_size) {
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_BASE_RING2, ih->gpu_addr >> 8);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_BASE_HI_RING2,
> +			     (ih->gpu_addr >> 40) & 0xff);
> +
> +		ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1);
> +		ih_rb_cntl = vega10_ih_rb_cntl(ih, ih_rb_cntl);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl);
> +
> +		/* set rptr, wptr to 0 */
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR_RING2, 0);
> +		WREG32_SOC15(OSSSYS, 0, mmIH_RB_WPTR_RING2, 0);
> +	}
> +
>   	tmp = RREG32_SOC15(OSSSYS, 0, mmIH_STORM_CLIENT_LIST_CNTL);
>   	tmp = REG_SET_FIELD(tmp, IH_STORM_CLIENT_LIST_CNTL,
>   			    CLIENT18_IS_STORM_CLIENT, 1); @@ -299,6 +383,15 @@ static int 
> vega10_ih_sw_init(void *handle)
>   	if (r)
>   		return r;
>   
> +	r = amdgpu_ih_ring_init(adev, &adev->irq.ih1, PAGE_SIZE, true);
> +	if (r)
> +		return r;
> +
> +	r = amdgpu_ih_ring_init(adev, &adev->irq.ih2, PAGE_SIZE, true);
> +	if (r)
> +		return r;
> +
> +	/* TODO add doorbell for IH1 & IH2 as well */
>   	adev->irq.ih.use_doorbell = true;
>   	adev->irq.ih.doorbell_index = adev->doorbell_index.ih << 1;
>   
> @@ -312,6 +405,8 @@ static int vega10_ih_sw_fini(void *handle)
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
>   	amdgpu_irq_fini(adev);
> +	amdgpu_ih_ring_fini(adev, &adev->irq.ih2);
> +	amdgpu_ih_ring_fini(adev, &adev->irq.ih1);
>   	amdgpu_ih_ring_fini(adev, &adev->irq.ih);
>   
>   	return 0;



More information about the amd-gfx mailing list