Am 15.07.2016 um 10:59 schrieb Wang, Qingqing:
>
> +static int vce_v3_0_firmware_loaded(struct amdgpu_device *adev)
> +{
> + int i, j;
> +
> + for (i = 0; i < 10; ++i) {
> + uint32_t status;
>
>
> Please move the definition of status to the start of the function and
> give it an intial value.
>
NAK, that is exactly what we should *NOT* do here.
"status" is just a temporary variable for the register content and as
such should only be declared where needed.
Additional to that please stop initializing variables when that isn't
necessary, we are already getting patches to remove that cruft from all
over the code.
What you should clearly do on the other hand is sending the patches as
text, not HTML mail. We can really review them this way.
Regards,
Christian.
> With that fixed.
>
> Reviewed-by: Ken Wang <Qingqing.Wang at amd.com>
>
> ------------------------------------------------------------------------
> *å??件人:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org> 代表 Qu, Jim
> <Jim.Qu at amd.com>
> *å??é??æ?¶é?´:* 2016å¹´7æ??15æ?¥ 10:45:07
> *�件人:* amd-gfx at lists.freedesktop.org
> *主é¢?:* ç?å¤?: [v2] drm/amdgpu: S3 resume fail on Polaris10
>
> Hi:
>
>
> Patch has updated, Please review.
>
>
> Thanks
>
> JimQu
>
> ------------------------------------------------------------------------
> *å??件人:* jimqu <Jim.Qu at amd.com>
> *å??é??æ?¶é?´:* 2016å¹´7æ??15æ?¥ 10:33:56
> *�件人:* amd-gfx at lists.freedesktop.org
> *æ??é??:* Qu, Jim
> *主�:* [v2] drm/amdgpu: S3 resume fail on Polaris10
> Sometimes, driver can not return from fence waiting when doing VCE ring
> ib test. The issue is a asic special and random issue. so adjust VCE
> suspend
> and resume sequence.
>
> Change-Id: If9e2006521ff17e55c33b18b1500126b9e7f2874
> Signed-off-by: JimQu <Jim.Qu at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 143
> +++++++++++++++++++++++-----------
> 1 file changed, 97 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index 30e8099..885b625 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -43,6 +43,7 @@
> #define mmVCE_LMI_VCPU_CACHE_40BIT_BAR0 0x8616
> #define mmVCE_LMI_VCPU_CACHE_40BIT_BAR1 0x8617
> #define mmVCE_LMI_VCPU_CACHE_40BIT_BAR2 0x8618
> +#define VCE_STATUS_VCPU_REPORT_FW_LOADED_MASK 0x02
>
> #define VCE_V3_0_FW_SIZE (384 * 1024)
> #define VCE_V3_0_STACK_SIZE (64 * 1024)
> @@ -51,6 +52,7 @@
> static void vce_v3_0_mc_resume(struct amdgpu_device *adev, int idx);
> static void vce_v3_0_set_ring_funcs(struct amdgpu_device *adev);
> static void vce_v3_0_set_irq_funcs(struct amdgpu_device *adev);
> +static int vce_v3_0_wait_for_idle(void *handle);
>
> /**
> * vce_v3_0_ring_get_rptr - get read pointer
> @@ -205,6 +207,32 @@ static void
> vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device *adev,
> vce_v3_0_override_vce_clock_gating(adev, false);
> }
>
> +static int vce_v3_0_firmware_loaded(struct amdgpu_device *adev)
> +{
> + int i, j;
> +
> + for (i = 0; i < 10; ++i) {
> + uint32_t status;
> + for (j = 0; j < 100; ++j) {
> + status = RREG32(mmVCE_STATUS);
> + if (status &
> VCE_STATUS_VCPU_REPORT_FW_LOADED_MASK)
> + return 0;
> + mdelay(10);
> + }
> +
> + DRM_ERROR("VCE not responding, trying to reset the
> ECPU!!!\n");
> + WREG32_P(mmVCE_SOFT_RESET,
> + VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> + ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + mdelay(10);
> + WREG32_P(mmVCE_SOFT_RESET, 0,
> + ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> + mdelay(10);
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> /**
> * vce_v3_0_start - start VCE block
> *
> @@ -215,11 +243,24 @@ static void
> vce_v3_0_set_vce_sw_clock_gating(struct amdgpu_device *adev,
> static int vce_v3_0_start(struct amdgpu_device *adev)
> {
> struct amdgpu_ring *ring;
> - int idx, i, j, r;
> + int idx, r;
> +
> + ring = &adev->vce.ring[0];
> + WREG32(mmVCE_RB_RPTR, ring->wptr);
> + WREG32(mmVCE_RB_WPTR, ring->wptr);
> + WREG32(mmVCE_RB_BASE_LO, ring->gpu_addr);
> + WREG32(mmVCE_RB_BASE_HI, upper_32_bits(ring->gpu_addr));
> + WREG32(mmVCE_RB_SIZE, ring->ring_size / 4);
> +
> + ring = &adev->vce.ring[1];
> + WREG32(mmVCE_RB_RPTR2, ring->wptr);
> + WREG32(mmVCE_RB_WPTR2, ring->wptr);
> + WREG32(mmVCE_RB_BASE_LO2, ring->gpu_addr);
> + WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> + WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
>
> mutex_lock(&adev->grbm_idx_mutex);
> for (idx = 0; idx < 2; ++idx) {
> -
> if (adev->vce.harvest_config & (1 << idx))
> continue;
>
> @@ -233,48 +274,24 @@ static int vce_v3_0_start(struct amdgpu_device
> *adev)
>
> vce_v3_0_mc_resume(adev, idx);
>
> - /* set BUSY flag */
> - WREG32_P(mmVCE_STATUS, 1, ~1);
> + WREG32_P(mmVCE_STATUS, VCE_STATUS__JOB_BUSY_MASK,
> + ~VCE_STATUS__JOB_BUSY_MASK);
> +
> if (adev->asic_type >= CHIP_STONEY)
> WREG32_P(mmVCE_VCPU_CNTL, 1, ~0x200001);
> else
> WREG32_P(mmVCE_VCPU_CNTL,
> VCE_VCPU_CNTL__CLK_EN_MASK,
> ~VCE_VCPU_CNTL__CLK_EN_MASK);
>
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> -
> - mdelay(100);
> -
> WREG32_P(mmVCE_SOFT_RESET, 0,
> ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
>
> - for (i = 0; i < 10; ++i) {
> - uint32_t status;
> - for (j = 0; j < 100; ++j) {
> - status = RREG32(mmVCE_STATUS);
> - if (status & 2)
> - break;
> - mdelay(10);
> - }
> - r = 0;
> - if (status & 2)
> - break;
> -
> - DRM_ERROR("VCE not responding, trying to reset
> the ECPU!!!\n");
> - WREG32_P(mmVCE_SOFT_RESET,
> - VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> - mdelay(10);
> - WREG32_P(mmVCE_SOFT_RESET, 0,
> - ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> - mdelay(10);
> - r = -1;
> - }
> + mdelay(100);
> +
> + r = vce_v3_0_firmware_loaded(adev);
>
> /* clear BUSY flag */
> - WREG32_P(mmVCE_STATUS, 0, ~1);
> + WREG32_P(mmVCE_STATUS, 0, ~VCE_STATUS__JOB_BUSY_MASK);
>
> /* Set Clock-Gating off */
> if (adev->cg_flags & AMD_CG_SUPPORT_VCE_MGCG)
> @@ -290,19 +307,46 @@ static int vce_v3_0_start(struct amdgpu_device
> *adev)
> WREG32_P(mmGRBM_GFX_INDEX, 0,
> ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> mutex_unlock(&adev->grbm_idx_mutex);
>
> - ring = &adev->vce.ring[0];
> - WREG32(mmVCE_RB_RPTR, ring->wptr);
> - WREG32(mmVCE_RB_WPTR, ring->wptr);
> - WREG32(mmVCE_RB_BASE_LO, ring->gpu_addr);
> - WREG32(mmVCE_RB_BASE_HI, upper_32_bits(ring->gpu_addr));
> - WREG32(mmVCE_RB_SIZE, ring->ring_size / 4);
> + return 0;
> +}
>
> - ring = &adev->vce.ring[1];
> - WREG32(mmVCE_RB_RPTR2, ring->wptr);
> - WREG32(mmVCE_RB_WPTR2, ring->wptr);
> - WREG32(mmVCE_RB_BASE_LO2, ring->gpu_addr);
> - WREG32(mmVCE_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> - WREG32(mmVCE_RB_SIZE2, ring->ring_size / 4);
> +static int vce_v3_0_stop(struct amdgpu_device *adev)
> +{
> + int idx;
> +
> + mutex_lock(&adev->grbm_idx_mutex);
> + for (idx = 0; idx < 2; ++idx) {
> + if (adev->vce.harvest_config & (1 << idx))
> + continue;
> +
> + if (idx == 0)
> + WREG32_P(mmGRBM_GFX_INDEX, 0,
> + ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + else
> + WREG32_P(mmGRBM_GFX_INDEX,
> + GRBM_GFX_INDEX__VCE_INSTANCE_MASK,
> + ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> +
> + if (adev->asic_type >= CHIP_STONEY)
> + WREG32_P(mmVCE_VCPU_CNTL, 0, ~0x200001);
> + else
> + WREG32_P(mmVCE_VCPU_CNTL, 0,
> + ~VCE_VCPU_CNTL__CLK_EN_MASK);
> + /* hold on ECPU */
> + WREG32_P(mmVCE_SOFT_RESET,
> + VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK,
> + ~VCE_SOFT_RESET__ECPU_SOFT_RESET_MASK);
> +
> + /* clear BUSY flag */
> + WREG32_P(mmVCE_STATUS, 0, ~VCE_STATUS__JOB_BUSY_MASK);
> +
> + /* Set Clock-Gating off */
> + if (adev->cg_flags & AMD_CG_SUPPORT_VCE_MGCG)
> + vce_v3_0_set_vce_sw_clock_gating(adev, false);
> + }
> +
> + WREG32_P(mmGRBM_GFX_INDEX, 0, ~GRBM_GFX_INDEX__VCE_INSTANCE_MASK);
> + mutex_unlock(&adev->grbm_idx_mutex);
>
> return 0;
> }
> @@ -441,7 +485,14 @@ static int vce_v3_0_hw_init(void *handle)
>
> static int vce_v3_0_hw_fini(void *handle)
> {
> - return 0;
> + int r;
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +
> + r = vce_v3_0_wait_for_idle(handle);
> + if (r)
> + return r;
> +
> + return vce_v3_0_stop(adev);
> }
>
> static int vce_v3_0_suspend(void *handle)
> --
> 1.9.1
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160718/ab4e56ca/attachment-0001.html>
