Re: [PATCH 1/2] drm/radeon: compute ring fix hibernation (CI GPU family).

["Christian König" <deathsimple@vodafone.de>]

On 19.06.2015 00:31, j.glisse@gmail.com wrote:
> From: Jérôme Glisse <jglisse@redhat.com>
>
> In order for hibernation to reliably work we need to cleanup more
> thoroughly the compute ring. Hibernation is different from suspend
> resume as when we resume from hibernation the hardware is first
> fully initialize by regular kernel then freeze callback happens
> (which correspond to a suspend inside the radeon kernel driver)
> and turn off each of the block. It turns out we were not cleanly
> shutting down the compute ring. This patch fix that.
>
> Hibernation and suspend to ram were tested (several times) on :
> Bonaire
> Hawaii
> Mullins
> Kaveri
> Kabini
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> ---
>   drivers/gpu/drm/radeon/cik.c |   55 ++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 55 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index ba50f3c..d2576d4 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -4592,6 +4592,61 @@ static void cik_cp_compute_enable(struct radeon_device *rdev, bool enable)
>   	if (enable)
>   		WREG32(CP_MEC_CNTL, 0);
>   	else {
> +		u32 tmp;
> +		int idx, j;
> +
> +		/*
> +		 * To make hibernation reliable we need to clear compute ring
> +		 * configuration before halting the compute ring.
> +		 */
> +		mutex_lock(&rdev->srbm_mutex);
> +
> +		idx = CAYMAN_RING_TYPE_CP1_INDEX;
> +		cik_srbm_select(rdev, rdev->ring[idx].me,
> +				rdev->ring[idx].pipe,
> +				rdev->ring[idx].queue, 0);
> +		/* Disable wptr polling. */
> +		tmp = RREG32(CP_PQ_WPTR_POLL_CNTL);
> +		tmp &= ~WPTR_POLL_EN;
> +		WREG32(CP_PQ_WPTR_POLL_CNTL, tmp);
> +		/* Disable HQD. */
> +		if (RREG32(CP_HQD_ACTIVE) & 1) {
> +			WREG32(CP_HQD_DEQUEUE_REQUEST, 1);
> +			for (j = 0; j < rdev->usec_timeout; j++) {
> +				if (!(RREG32(CP_HQD_ACTIVE) & 1))
> +					break;
> +				udelay(1);
> +			}
> +			WREG32(CP_HQD_DEQUEUE_REQUEST, 0);
> +			WREG32(CP_HQD_PQ_RPTR, 0);
> +			WREG32(CP_HQD_PQ_WPTR, 0);
> +		}
> +		cik_srbm_select(rdev, 0, 0, 0, 0);

Could you factor out this part into a function which just takes a 
radeon_ring* argument?

Apart from that this patch and the other one looks good to me,
Christian.

> +
> +		idx = CAYMAN_RING_TYPE_CP2_INDEX;
> +		cik_srbm_select(rdev, rdev->ring[idx].me,
> +				rdev->ring[idx].pipe,
> +				rdev->ring[idx].queue, 0);
> +		/* Disable wptr polling. */
> +		tmp = RREG32(CP_PQ_WPTR_POLL_CNTL);
> +		tmp &= ~WPTR_POLL_EN;
> +		WREG32(CP_PQ_WPTR_POLL_CNTL, tmp);
> +		/* Disable HQD. */
> +		if (RREG32(CP_HQD_ACTIVE) & 1) {
> +			WREG32(CP_HQD_DEQUEUE_REQUEST, 1);
> +			for (j = 0; j < rdev->usec_timeout; j++) {
> +				if (!(RREG32(CP_HQD_ACTIVE) & 1))
> +					break;
> +				udelay(1);
> +			}
> +			WREG32(CP_HQD_DEQUEUE_REQUEST, 0);
> +			WREG32(CP_HQD_PQ_RPTR, 0);
> +			WREG32(CP_HQD_PQ_WPTR, 0);
> +		}
> +		cik_srbm_select(rdev, 0, 0, 0, 0);
> +
> +		mutex_unlock(&rdev->srbm_mutex);
> +
>   		WREG32(CP_MEC_CNTL, (MEC_ME1_HALT | MEC_ME2_HALT));
>   		rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX].ready = false;
>   		rdev->ring[CAYMAN_RING_TYPE_CP2_INDEX].ready = false;

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel