radeon error on resume: "cp failed to get scratch reg" - anyone interested?

[Michel Dänzer]

On Don, 2012-09-20 at 08:54 +0200, Simon Kitching wrote: 
> On 19/09/12 11:12, Michel Dänzer wrote:
> > On Mon, 2012-09-17 at 15:06 +0200, Simon Kitching wrote:
> >> Hi,
> >>
> >> I've noticed that on resume from suspend, dmesg reports:
> >>
> >> [21895.997536] [drm] radeon: 1 quad pipes, 2 z pipes initialized.
> >> [21896.012072] [drm] PCIE GART of 512M enabled (table at
> >> 0x0000000000040000).
> >> [21896.012082] radeon 0000:01:00.0: WB enabled
> >> [21896.012085] radeon 0000:01:00.0: fence driver on ring 0 use gpu addr
> >> 0x0000000010000000 and cpu addr 0xffccb000
> >> [21896.012138] [drm] radeon: ring at 0x0000000010001000
> >> [21896.012157] [drm:r100_ring_test] *ERROR* radeon: cp failed to get
> >> scratch reg (-22).
> >> [21896.012158] [drm:r100_cp_init] *ERROR* radeon: cp isn't working (-22).
> >> [21896.012160] radeon 0000:01:00.0: failed initializing CP (-22).
> >>
> >> Graphics still seems to work fine though.
> >>
> >> Is this info of interest to anyone? I'm happy to do additional testing
> >> if that is useful.
> > Does the patch below fix the failure to get a scratch register?

[...]

> Sadly the patch does not fix the issue. However I agree that this patch 
> is a good thing - the original code does clearly leak scratch registers 
> in some failure conditions.
> 
> Maybe this patch could also add the DRM_ERROR calls for these error 
> cases from the r600 version into the r100 version?

Yeah, that's a good idea, done.

> Note that on failure of radeon_ib_schedule, the order of resource 
> releases *was*
>    radeon_scratch_free
>    radeon_ib_free
> and with this patch is:
>    radeon_ib_free
>    radeon_scratch_free
> This change looks correct to me, ie post-patch the frees are in reverse 
> order to original allocation.

Right, this is necessary for the new label exit structure to work.

> If you do submit this patch, please add "reviewed-by 
> skitching at vonos.net" or "tested-by" or whatever you think the 
> appropriate tag is.

I used the former, thanks for your (and Christian's) review!


> Thanks to this patch pointing me to the appropriate area, I have 
> actually found the real cause of the scratch-register leak, and will 
> post a candidate patch today.

Excellent, looking forward to it.


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