From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michel =?ISO-8859-1?Q?D=E4nzer?= <michel@daenzer.net>
Subject: Re: [PATCH 16/16] drm/radeon: implement ring saving on reset
Date: Mon, 09 Jul 2012 17:06:45 +0200
Message-ID: <1341842820.12576.272.camel@thor.local>
References: <1341830523-30320-1-git-send-email-deathsimple@vodafone.de>
	<1341830523-30320-17-git-send-email-deathsimple@vodafone.de>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org>
Received: from mail.gna.ch (darkcity.gna.ch [195.226.6.51])
	by gabe.freedesktop.org (Postfix) with ESMTP id 0DD6F9E771
	for <dri-devel@lists.freedesktop.org>;
	Mon,  9 Jul 2012 08:06:59 -0700 (PDT)
Resent-Message-Id: <E1SoFXx-0006qw-M8@thor>
In-Reply-To: <1341830523-30320-17-git-send-email-deathsimple@vodafone.de>
Resent-To: Christian =?ISO-8859-1?Q?K=F6nig?= <deathsimple@vodafone.de>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
To: Christian =?ISO-8859-1?Q?K=F6nig?= <deathsimple@vodafone.de>
Cc: dri-devel@lists.freedesktop.org
List-Id: dri-devel@lists.freedesktop.org

On Mon, 2012-07-09 at 12:42 +0200, Christian K=F6nig wrote: =

> Try to save whatever is on the rings when
> we encounter an lockup.
> =

> Signed-off-by: Christian K=F6nig <deathsimple@vodafone.de>
[...] =

> @@ -1005,20 +1010,43 @@ int radeon_gpu_reset(struct radeon_device *rdev)
>  	resched =3D ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
>  	radeon_suspend(rdev);
>  =

> +	for (i =3D 0; i < RADEON_NUM_RINGS; ++i) {
> +		ring_sizes[i] =3D radeon_ring_backup(rdev, &rdev->ring[i],
> +						   &ring_data[i]);
> +		if (ring_sizes[i]) {
> +			saved =3D true;
> +			dev_info(rdev->dev, "Saved %d dwords of commands "
> +				 "on ring %d.\n", ring_sizes[i], i);
> +		}
> +	}
> +
> +retry:
>  	r =3D radeon_asic_reset(rdev);
>  	if (!r) {
> -		dev_info(rdev->dev, "GPU reset succeed\n");
> +		dev_info(rdev->dev, "GPU reset succeed trying to resume\n");

Could fix the spelling of 'succeeded' while you're at it. :)


> 		radeon_resume(rdev);
> +	}
>  =

> -		r =3D radeon_ib_ring_tests(rdev);
> -		if (r)
> -			DRM_ERROR("ib ring test failed (%d).\n", r);
> +	radeon_restore_bios_scratch_regs(rdev);
> +	drm_helper_resume_force_mode(rdev->ddev);
> +
> +	if (!r) {
> +		for (i =3D 0; i < RADEON_NUM_RINGS; ++i) {
> +			radeon_ring_restore(rdev, &rdev->ring[i],
> +					    ring_sizes[i], ring_data[i]);
> +		}

If radeon_asic_reset fails, this leaks the memory referenced by
ring_data, doesn't it?


Also, the added functions aren't documented as mandated by the rules
Alex proposed.


-- =

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