Re: [PATCH] drm/amdgpu: explicitly set mmGDS_VMID0_BASE to 0

From: "Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: "Zhu, James" <James.Zhu-5C7GfCeVMHo@public.gmane.org>,
	"amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org"
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Cc: "Shamis, Leonid" <Leonid.Shamis-5C7GfCeVMHo@public.gmane.org>
Subject: Re: [PATCH] drm/amdgpu: explicitly set mmGDS_VMID0_BASE to 0
Date: Wed, 12 Jun 2019 10:08:26 +0200	[thread overview]
Message-ID: <2eea4e60-194d-c7d6-4b29-2a7439f6edf3@gmail.com> (raw)
In-Reply-To: <1560187603-16238-1-git-send-email-James.Zhu-5C7GfCeVMHo@public.gmane.org>

Am 10.06.19 um 19:26 schrieb Zhu, James:
> Explicitly set mmGDS_VMID0_BASE to 0. Also update
> GDS_VMID0_BASE/_SIZE with direct register writes.
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 33 +++++++++++++++------------------
>   1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index ba36a28..78c79e9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -305,6 +305,7 @@ static int gfx_v9_0_get_cu_info(struct amdgpu_device *adev,
>   static uint64_t gfx_v9_0_get_gpu_clock_counter(struct amdgpu_device *adev);
>   static void gfx_v9_0_select_se_sh(struct amdgpu_device *adev, u32 se_num, u32 sh_num, u32 instance);
>   static void gfx_v9_0_ring_emit_de_meta(struct amdgpu_ring *ring);
> +static u64 gfx_v9_0_ring_get_rptr_compute(struct amdgpu_ring *ring);
>   
>   static void gfx_v9_0_init_golden_registers(struct amdgpu_device *adev)
>   {
> @@ -3630,25 +3631,20 @@ static const struct soc15_reg_entry sec_ded_counter_registers[] = {
>      { SOC15_REG_ENTRY(GC, 0, mmSQC_EDC_CNT3), 0, 4, 6},
>   };
>   
> -
>   static int gfx_v9_0_do_edc_gds_workarounds(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ring *ring = &adev->gfx.compute_ring[0];
> -	int r;
> +	int i, r;
>   
> -	r = amdgpu_ring_alloc(ring, 17);
> +	r = amdgpu_ring_alloc(ring, 7);
>   	if (r) {
>   		DRM_ERROR("amdgpu: GDS workarounds failed to lock ring %s (%d).\n",
>   			ring->name, r);
>   		return r;
>   	}
>   
> -	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
> -	amdgpu_ring_write(ring, WRITE_DATA_ENGINE_SEL(0) |
> -				WRITE_DATA_DST_SEL(0));
> -	amdgpu_ring_write(ring, SOC15_REG_OFFSET(GC, 0, mmGDS_VMID0_SIZE));
> -	amdgpu_ring_write(ring, 0);
> -	amdgpu_ring_write(ring, adev->gds.gds_size);
> +	WREG32_SOC15(GC, 0, mmGDS_VMID0_BASE, 0x00000000);
> +	WREG32_SOC15(GC, 0, mmGDS_VMID0_SIZE, adev->gds.gds_size);
>   
>   	amdgpu_ring_write(ring, PACKET3(PACKET3_DMA_DATA, 5));
>   	amdgpu_ring_write(ring, (PACKET3_DMA_DATA_CP_SYNC |
> @@ -3662,18 +3658,19 @@ static int gfx_v9_0_do_edc_gds_workarounds(struct amdgpu_device *adev)
>   	amdgpu_ring_write(ring, PACKET3_DMA_DATA_CMD_RAW_WAIT |
>   				adev->gds.gds_size);
>   
> -	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
> -	amdgpu_ring_write(ring, WRITE_DATA_ENGINE_SEL(0) |
> -				WRITE_DATA_DST_SEL(0));
> -	amdgpu_ring_write(ring, SOC15_REG_OFFSET(GC, 0, mmGDS_VMID0_SIZE));
> -	amdgpu_ring_write(ring, 0);
> -	amdgpu_ring_write(ring, 0x0);
> -
>   	amdgpu_ring_commit(ring);
>   
> -	return 0;
> -}
> +	for (i = 0; (i < adev->usec_timeout) &&
> +			(ring->wptr != gfx_v9_0_ring_get_rptr_compute(ring)); i++)

The indentation here looks wrong on first glance and you don't need the 
extra ().

Might be better to write this as for + if..break.

> +		DRM_UDELAY(1);
> +
> +	if (i >= adev->usec_timeout)
> +		r = -ETIMEDOUT;
> +
> +	WREG32_SOC15(GC, 0, mmGDS_VMID0_SIZE, 0x00000000);
>   
> +	return r;
> +}
>   
>   static int gfx_v9_0_do_edc_gpr_workarounds(struct amdgpu_device *adev)
>   {

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx