Bug (and probably, fix): UVD initialization / clock gating issue on kabini

[Nils Holland]

On Sun, Jan 22, 2017 at 08:26:10PM +0100, Nils Holland wrote:
> Hi folks,
> 
> while playing around with the amdgpu drm driver, I stumbled upon an
> issue. In fact, I have tracked it down to commit
> aa4747c00a2dd034c5fdf70ca73b1674ca15beb3 ("drm/amdgpu: refine uvd_4.2
> clock gate sequence.").
> 
> When I run the latest mainline git kernel, which contains this commit,
> on my system, I get the following in dmesg (notice the lines about UVD
> not responding at the end):
> [    4.870999] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [    5.891351] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [    6.911713] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [    7.932062] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [    8.952409] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [    9.974050] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [   10.995701] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [   12.017354] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [   13.039024] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [   14.060692] [drm:uvd_v4_2_start] *ERROR* UVD not responding, trying to reset the VCPU!!!
> [   14.082168] [drm:uvd_v4_2_start] *ERROR* UVD not responding, giving up!!!

Replying to myself here, as I believe I may have found a fix:

Leaving commit aa4747c00a2dd034c5fdf70ca73b1674ca15beb3 applied, and
then applying the following patch, which re-adds a little line that
the original patch removed, fixes the problem for me:


diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
index 96444e4d862a..350e7dab9b6d 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
@@ -273,6 +273,8 @@ static int uvd_v4_2_start(struct amdgpu_device *adev)
 
 	uvd_v4_2_mc_resume(adev);
 
+	WREG32(mmUVD_CGC_GATE, 0);
+
 	/* disable interupt */
 	WREG32_P(mmUVD_MASTINT_EN, 0, ~(1 << 1));

Hmm ... what does this do? Completely disable clock gating? And if so,
is this a sane thing to do at this point during UVD initialization?
The fact that it was done like that before aa4747c00a2dd034c5fdf70ca73b1674ca15beb3
might suggest that to be the case.

If the experts think I'm on the right track here I'll also re-submit
this change as a proper patch. I could probably have done so in the
right away and then ask for comments / review as part of the patch
submission instead of asking here in this message ... good question
if that would have been the better approach - I guess I'm still
learning all these things. ;-)

Greetings
Nils