On 05/30/2017 11:13 AM, Christian König wrote: > Am 30.05.2017 um 16:57 schrieb Deucher, Alexander: >>> -----Original Message----- >>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf >>> Of Leo Liu >>> Sent: Monday, May 29, 2017 2:22 PM >>> To: amd-gfx at lists.freedesktop.org >>> Cc: Liu, Leo >>> Subject: [PATCH] drm/amdgpu: Program ring for vce instance 1 at its own >>> register space >>> >>> when harvest part has only instance 1 available >>> >>> Signed-off-by: Leo Liu <leo.liu at amd.com> >>> --- >>> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 61 >>> +++++++++++++++++++++++++++++++---- >>> 1 file changed, 55 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >>> b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >>> index fb08193..77af395 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >>> @@ -77,13 +77,26 @@ static int vce_v3_0_set_clockgating_state(void >>> *handle, >>> static uint64_t vce_v3_0_ring_get_rptr(struct amdgpu_ring *ring) >>> { >>> struct amdgpu_device *adev = ring->adev; >>> + u32 v; >>> + >>> + if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) { >>> + mutex_lock(&adev->grbm_idx_mutex); >>> + WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(1)); >>> + } >>> >>> if (ring == &adev->vce.ring[0]) >>> - return RREG32(mmVCE_RB_RPTR); >>> + v = RREG32(mmVCE_RB_RPTR); >>> else if (ring == &adev->vce.ring[1]) >>> - return RREG32(mmVCE_RB_RPTR2); >>> + v = RREG32(mmVCE_RB_RPTR2); >>> else >>> - return RREG32(mmVCE_RB_RPTR3); >>> + v = RREG32(mmVCE_RB_RPTR3); >>> + >>> + if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) { >>> + WREG32(mmGRBM_GFX_INDEX, >>> mmGRBM_GFX_INDEX_DEFAULT); >>> + mutex_unlock(&adev->grbm_idx_mutex); >>> + } >>> + >>> + return v; >>> } >>> >>> /** >>> @@ -96,13 +109,26 @@ static uint64_t vce_v3_0_ring_get_rptr(struct >>> amdgpu_ring *ring) >>> static uint64_t vce_v3_0_ring_get_wptr(struct amdgpu_ring *ring) >>> { >>> struct amdgpu_device *adev = ring->adev; >>> + u32 v; >>> + >>> + if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) { >>> + mutex_lock(&adev->grbm_idx_mutex); >>> + WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(1)); >>> + } >>> >>> if (ring == &adev->vce.ring[0]) >>> - return RREG32(mmVCE_RB_WPTR); >>> + v = RREG32(mmVCE_RB_WPTR); >>> else if (ring == &adev->vce.ring[1]) >>> - return RREG32(mmVCE_RB_WPTR2); >>> + v = RREG32(mmVCE_RB_WPTR2); >>> else >>> - return RREG32(mmVCE_RB_WPTR3); >>> + v = RREG32(mmVCE_RB_WPTR3); >>> + >>> + if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) { >>> + WREG32(mmGRBM_GFX_INDEX, >>> mmGRBM_GFX_INDEX_DEFAULT); >>> + mutex_unlock(&adev->grbm_idx_mutex); >>> + } >>> + >>> + return v; >>> } >>> >>> /** >>> @@ -116,12 +142,22 @@ static void vce_v3_0_ring_set_wptr(struct >>> amdgpu_ring *ring) >>> { >>> struct amdgpu_device *adev = ring->adev; >>> >>> + if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) { >>> + mutex_lock(&adev->grbm_idx_mutex); >>> + WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(1)); >>> + } >>> + >>> if (ring == &adev->vce.ring[0]) >>> WREG32(mmVCE_RB_WPTR, lower_32_bits(ring->wptr)); >>> else if (ring == &adev->vce.ring[1]) >>> WREG32(mmVCE_RB_WPTR2, lower_32_bits(ring->wptr)); >>> else >>> WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr)); >>> + >>> + if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) { >>> + WREG32(mmGRBM_GFX_INDEX, >>> mmGRBM_GFX_INDEX_DEFAULT); >>> + mutex_unlock(&adev->grbm_idx_mutex); >>> + } >>> } >>> >>> static void vce_v3_0_override_vce_clock_gating(struct amdgpu_device >>> *adev, bool override) >>> @@ -231,6 +267,14 @@ static int vce_v3_0_start(struct amdgpu_device >>> *adev) >>> struct amdgpu_ring *ring; >>> int idx, r; >>> >>> + /* we need program ring buffer on instance 1 register space domain >>> + when only if instance 1 available, with two instances or >>> instance 0 >>> + we need only program instance 0 regsiter space domain for ring */ >> Please add this comment to the commit message as well. With that fixed: >> Reviewed-by: Alex Deucher <alexander.deucher at amd.com> >> Also, add CC: stable at vger.kernel.org >> >>> + if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) { >>> + mutex_lock(&adev->grbm_idx_mutex); >>> + WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(1)); >>> + } > > I would prefer if we always grab the mutex and program > mmGRBM_GFX_INDEX with the not harvested instance. > > This way we don't run into bad surprises any more when we only test on > boards where the second VCE instance is harvested. Okay. the v2 patch will send shortly. Leo > > Christian. > >>> + >>> ring = &adev->vce.ring[0]; >>> WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr)); >>> WREG32(mmVCE_RB_WPTR, lower_32_bits(ring->wptr)); >>> @@ -252,6 +296,11 @@ static int vce_v3_0_start(struct amdgpu_device >>> *adev) >>> WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr)); >>> WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4); >>> >>> + if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) { >>> + WREG32(mmGRBM_GFX_INDEX, >>> mmGRBM_GFX_INDEX_DEFAULT); >>> + mutex_unlock(&adev->grbm_idx_mutex); >>> + } >>> + >>> mutex_lock(&adev->grbm_idx_mutex); >>> for (idx = 0; idx < 2; ++idx) { >>> if (adev->vce.harvest_config & (1 << idx)) >>> -- >>> 2.9.3 >>> >>> _______________________________________________ >>> 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 > >
