[PATCH 1/4] drm/amdgpu: clear RB at ring init

[Liu, Monk]

> We should only call memset if amdgpu_bo_kmap succeeded. Same issue in patch 2.
[ml] agreed 

>There's something else about these two patches I'm a bit worried about:

>The GPU should only read data from ring buffers and IBs that we previously explicitly wrote there. I'm afraid these patches might just paper over bugs elsewhere, which might still bite us under different circumstances.

[ml] so do you mean we should use NOP to fulfill the ring buffer instead of 0 ?

-----Original Message-----
From: Michel Dänzer [mailto:michel at daenzer.net] 
Sent: Tuesday, June 07, 2016 3:34 PM
To: Alex Deucher <alexdeucher at gmail.com>
Cc: dri-devel at lists.freedesktop.org; Liu, Monk <Monk.Liu at amd.com>
Subject: Re: [PATCH 1/4] drm/amdgpu: clear RB at ring init

On 02.06.2016 07:27, Alex Deucher wrote:
> From: Monk Liu <Monk.Liu at amd.com>
> 
> This help fix reloading driver hang issue of SDMA ring.
> 
> Signed-off-by: Monk Liu <Monk.Liu at amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> Reviewed-by: Christian König <christian.koenig at amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 3b02272..a4b3f44 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -310,6 +310,9 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
>  		}
>  		r = amdgpu_bo_kmap(ring->ring_obj,
>  				       (void **)&ring->ring);
> +
> +		memset((void *)ring->ring, 0, ring->ring_size);
> +
>  		amdgpu_bo_unreserve(ring->ring_obj);
>  		if (r) {
>  			dev_err(adev->dev, "(%d) ring map failed\n", r);
> 

We should only call memset if amdgpu_bo_kmap succeeded. Same issue in patch 2.


There's something else about these two patches I'm a bit worried about:

The GPU should only read data from ring buffers and IBs that we previously explicitly wrote there. I'm afraid these patches might just paper over bugs elsewhere, which might still bite us under different circumstances.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer