Am 18.07.2016 um 16:07 schrieb Christian König:
> 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.
Typo that should have read "can't really review them".
Sorry for that,
Christian.
>
> 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
>
>
>
> _______________________________________________
> 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/3d75a6ed/attachment-0001.html>
