Re: [PATCH] drm/radeon/kms: make r600-NI blit suspend code common

[Ilija Hadzic <ihadzic@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, 14 Oct 2011 alexdeucher@xxxxxxxxx wrote:

> From: Alex Deucher <alexander.deucher@xxxxxxx>
>
> r600-NI shared the same blit suspend code.  Clean it up
> and make it a shared function.
>
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: Ilija Hadzic <ihadzic@xxxxxxxxxxxxxxxxxxxxxx>

Thanks, this one slipped my eye in my cleanup. Correctness should be 
obvious, so as much as my review is worth
Reviewed-by: Ilija Hadzic <ihadzic@xxxxxxxxxxxxxxxxxxxxxx>


> ---
> drivers/gpu/drm/radeon/evergreen.c |   10 +---------
> drivers/gpu/drm/radeon/ni.c        |   10 +---------
> drivers/gpu/drm/radeon/r600.c      |   26 ++++++++++++++++----------
> drivers/gpu/drm/radeon/radeon.h    |    2 ++
> drivers/gpu/drm/radeon/rv770.c     |   12 ++----------
> 5 files changed, 22 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
> index 80c4ee3..5c515f2 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -3107,21 +3107,13 @@ int evergreen_resume(struct radeon_device *rdev)
>
> int evergreen_suspend(struct radeon_device *rdev)
> {
> -	int r;
> -
> 	/* FIXME: we should wait for ring to be empty */
> 	r700_cp_stop(rdev);
> 	rdev->cp.ready = false;
> 	evergreen_irq_suspend(rdev);
> 	radeon_wb_disable(rdev);
> 	evergreen_pcie_gart_disable(rdev);
> -
> -	/* unpin shaders bo */
> -	r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
> -	if (likely(r == 0)) {
> -		radeon_bo_unpin(rdev->r600_blit.shader_obj);
> -		radeon_bo_unreserve(rdev->r600_blit.shader_obj);
> -	}
> +	r600_blit_suspend(rdev);
>
> 	return 0;
> }
> diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
> index cc0e911..e560da5 100644
> --- a/drivers/gpu/drm/radeon/ni.c
> +++ b/drivers/gpu/drm/radeon/ni.c
> @@ -1423,21 +1423,13 @@ int cayman_resume(struct radeon_device *rdev)
>
> int cayman_suspend(struct radeon_device *rdev)
> {
> -	int r;
> -
> 	/* FIXME: we should wait for ring to be empty */
> 	cayman_cp_enable(rdev, false);
> 	rdev->cp.ready = false;
> 	evergreen_irq_suspend(rdev);
> 	radeon_wb_disable(rdev);
> 	cayman_pcie_gart_disable(rdev);
> -
> -	/* unpin shaders bo */
> -	r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
> -	if (likely(r == 0)) {
> -		radeon_bo_unpin(rdev->r600_blit.shader_obj);
> -		radeon_bo_unreserve(rdev->r600_blit.shader_obj);
> -	}
> +	r600_blit_suspend(rdev);
>
> 	return 0;
> }
> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> index c2f0dbe..f7f693a 100644
> --- a/drivers/gpu/drm/radeon/r600.c
> +++ b/drivers/gpu/drm/radeon/r600.c
> @@ -2375,6 +2375,20 @@ int r600_copy_blit(struct radeon_device *rdev,
> 	return 0;
> }
>
> +void r600_blit_suspend(struct radeon_device *rdev)
> +{
> +	int r;
> +
> +	/* unpin shaders bo */
> +	if (rdev->r600_blit.shader_obj) {
> +		r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
> +		if (!r) {
> +			radeon_bo_unpin(rdev->r600_blit.shader_obj);
> +			radeon_bo_unreserve(rdev->r600_blit.shader_obj);
> +		}
> +	}
> +}
> +
> int r600_set_surface_reg(struct radeon_device *rdev, int reg,
> 			 uint32_t tiling_flags, uint32_t pitch,
> 			 uint32_t offset, uint32_t obj_size)
> @@ -2494,8 +2508,6 @@ int r600_resume(struct radeon_device *rdev)
>
> int r600_suspend(struct radeon_device *rdev)
> {
> -	int r;
> -
> 	r600_audio_fini(rdev);
> 	/* FIXME: we should wait for ring to be empty */
> 	r600_cp_stop(rdev);
> @@ -2503,14 +2515,8 @@ int r600_suspend(struct radeon_device *rdev)
> 	r600_irq_suspend(rdev);
> 	radeon_wb_disable(rdev);
> 	r600_pcie_gart_disable(rdev);
> -	/* unpin shaders bo */
> -	if (rdev->r600_blit.shader_obj) {
> -		r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
> -		if (!r) {
> -			radeon_bo_unpin(rdev->r600_blit.shader_obj);
> -			radeon_bo_unreserve(rdev->r600_blit.shader_obj);
> -		}
> -	}
> +	r600_blit_suspend(rdev);
> +
> 	return 0;
> }
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 7b0cb74..93768f5 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -555,6 +555,8 @@ struct r600_blit {
> 	struct radeon_ib *vb_ib;
> };
>
> +void r600_blit_suspend(struct radeon_device *rdev);
> +
> int radeon_ib_get(struct radeon_device *rdev, struct radeon_ib **ib);
> void radeon_ib_free(struct radeon_device *rdev, struct radeon_ib **ib);
> int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib);
> diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
> index b13c2ee..4901837 100644
> --- a/drivers/gpu/drm/radeon/rv770.c
> +++ b/drivers/gpu/drm/radeon/rv770.c
> @@ -1184,8 +1184,6 @@ int rv770_resume(struct radeon_device *rdev)
>
> int rv770_suspend(struct radeon_device *rdev)
> {
> -	int r;
> -
> 	r600_audio_fini(rdev);
> 	/* FIXME: we should wait for ring to be empty */
> 	r700_cp_stop(rdev);
> @@ -1193,14 +1191,8 @@ int rv770_suspend(struct radeon_device *rdev)
> 	r600_irq_suspend(rdev);
> 	radeon_wb_disable(rdev);
> 	rv770_pcie_gart_disable(rdev);
> -	/* unpin shaders bo */
> -	if (rdev->r600_blit.shader_obj) {
> -		r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
> -		if (likely(r == 0)) {
> -			radeon_bo_unpin(rdev->r600_blit.shader_obj);
> -			radeon_bo_unreserve(rdev->r600_blit.shader_obj);
> -		}
> -	}
> +	r600_blit_suspend(rdev);
> +
> 	return 0;
> }
>
> --
> 1.7.1.1
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel