All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Program ring for vce instance 1 at its own register space
@ 2017-05-29 18:21 Leo Liu
       [not found] ` <20170529182152.7451-1-leo.liu-5C7GfCeVMHo@public.gmane.org>
  2017-05-30 15:56   ` Leo Liu
  0 siblings, 2 replies; 9+ messages in thread
From: Leo Liu @ 2017-05-29 18:21 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Leo Liu

when harvest part has only instance 1 available

Signed-off-by: Leo Liu <leo.liu@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 */
+	if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+		mutex_lock(&adev->grbm_idx_mutex);
+		WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(1));
+	}
+
 	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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [PATCH] drm/amdgpu: Program ring for vce instance 1 at its own register space
       [not found] ` <20170529182152.7451-1-leo.liu-5C7GfCeVMHo@public.gmane.org>
@ 2017-05-30 14:57   ` Deucher, Alexander
       [not found]     ` <BN6PR12MB1652D8852FBB1EDD82FF2651F7F00-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Deucher, Alexander @ 2017-05-30 14:57 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Liu, Leo

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Leo Liu
> Sent: Monday, May 29, 2017 2:22 PM
> To: amd-gfx@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@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@amd.com>
Also, add CC: stable@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));
> +	}
> +
>  	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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: Program ring for vce instance 1 at its own register space
       [not found]     ` <BN6PR12MB1652D8852FBB1EDD82FF2651F7F00-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2017-05-30 15:13       ` Christian König
       [not found]         ` <d3fc8ec8-2167-126e-a9bd-cb98c6d0006c-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2017-05-30 15:13 UTC (permalink / raw)
  To: Deucher, Alexander, Liu, Leo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 30.05.2017 um 16:57 schrieb Deucher, Alexander:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
>> Of Leo Liu
>> Sent: Monday, May 29, 2017 2:22 PM
>> To: amd-gfx@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@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@amd.com>
> Also, add CC: stable@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.

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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: Program ring for vce instance 1 at its own register space
       [not found]         ` <d3fc8ec8-2167-126e-a9bd-cb98c6d0006c-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2017-05-30 15:55           ` Leo Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Leo Liu @ 2017-05-30 15:55 UTC (permalink / raw)
  To: Christian König, Deucher, Alexander,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW



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@lists.freedesktop.org] On Behalf
>>> Of Leo Liu
>>> Sent: Monday, May 29, 2017 2:22 PM
>>> To: amd-gfx@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@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@amd.com>
>> Also, add CC: stable@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@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] drm/amdgpu: Program ring for vce instance 1 at its register space
  2017-05-29 18:21 [PATCH] drm/amdgpu: Program ring for vce instance 1 at its own register space Leo Liu
@ 2017-05-30 15:56   ` Leo Liu
  2017-05-30 15:56   ` Leo Liu
  1 sibling, 0 replies; 9+ messages in thread
From: Leo Liu @ 2017-05-30 15:56 UTC (permalink / raw)
  To: amd-gfx; +Cc: Leo Liu, stable

We need program ring buffer on instance 1 register space domain,
when only if instance 1 available, with two instances or instance 0,
and we need only program instance 0 regsiter space domain for ring.

Signed-off-by: Leo Liu <leo.liu@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 95 +++++++++++++++++++++++++----------
 1 file changed, 68 insertions(+), 27 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..7e39e42 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,33 +267,38 @@ static int vce_v3_0_start(struct amdgpu_device *adev)
 	struct amdgpu_ring *ring;
 	int idx, r;
 
-	ring = &adev->vce.ring[0];
-	WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
-	WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
-	WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
-
-	ring = &adev->vce.ring[2];
-	WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
-	WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
-	WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
-	WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
-	WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
-
 	mutex_lock(&adev->grbm_idx_mutex);
 	for (idx = 0; idx < 2; ++idx) {
 		if (adev->vce.harvest_config & (1 << idx))
 			continue;
 
 		WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(idx));
+
+		/* Program instance 0 reg space for two instances or instance 0 case
+		program instance 1 reg space for only instance 1 available case */
+		if (idx != 1 || adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+			ring = &adev->vce.ring[0];
+			WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
+			WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
+			WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
+
+			ring = &adev->vce.ring[2];
+			WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
+			WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
+			WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
+			WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
+			WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
+		}
+
 		vce_v3_0_mc_resume(adev, idx);
 		WREG32_FIELD(VCE_STATUS, JOB_BUSY, 1);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] drm/amdgpu: Program ring for vce instance 1 at its register space
@ 2017-05-30 15:56   ` Leo Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Leo Liu @ 2017-05-30 15:56 UTC (permalink / raw)
  To: amd-gfx; +Cc: Leo Liu, stable

We need program ring buffer on instance 1 register space domain,
when only if instance 1 available, with two instances or instance 0,
and we need only program instance 0 regsiter space domain for ring.

Signed-off-by: Leo Liu <leo.liu@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 95 +++++++++++++++++++++++++----------
 1 file changed, 68 insertions(+), 27 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..7e39e42 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,33 +267,38 @@ static int vce_v3_0_start(struct amdgpu_device *adev)
 	struct amdgpu_ring *ring;
 	int idx, r;
 
-	ring = &adev->vce.ring[0];
-	WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
-	WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
-	WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
-
-	ring = &adev->vce.ring[2];
-	WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
-	WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
-	WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
-	WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
-	WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
-
 	mutex_lock(&adev->grbm_idx_mutex);
 	for (idx = 0; idx < 2; ++idx) {
 		if (adev->vce.harvest_config & (1 << idx))
 			continue;
 
 		WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(idx));
+
+		/* Program instance 0 reg space for two instances or instance 0 case
+		program instance 1 reg space for only instance 1 available case */
+		if (idx != 1 || adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+			ring = &adev->vce.ring[0];
+			WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
+			WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
+			WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
+
+			ring = &adev->vce.ring[2];
+			WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
+			WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
+			WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
+			WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
+			WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
+		}
+
 		vce_v3_0_mc_resume(adev, idx);
 		WREG32_FIELD(VCE_STATUS, JOB_BUSY, 1);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: Program ring for vce instance 1 at its register space
  2017-05-30 15:56   ` Leo Liu
  (?)
@ 2017-05-30 16:15   ` Christian König
  2017-05-30 16:27       ` Leo Liu
  -1 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2017-05-30 16:15 UTC (permalink / raw)
  To: Leo Liu, amd-gfx; +Cc: stable

Am 30.05.2017 um 17:56 schrieb Leo Liu:
> We need program ring buffer on instance 1 register space domain,
> when only if instance 1 available, with two instances or instance 0,
> and we need only program instance 0 regsiter space domain for ring.
>
> Signed-off-by: Leo Liu <leo.liu@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: stable@vger.kernel.org
> ---
>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 95 +++++++++++++++++++++++++----------
>   1 file changed, 68 insertions(+), 27 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..7e39e42 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));
> +	}

Did you missed my comment? I think we should always grab the mutex and 
program mmGRBM_GFX_INDEX.

Otherwise we silently rely on that it is correctly set when the function 
is called and have different code path for different harvested configs.

Apart from that the patch looks good to me.

Christian.

>   
>   	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,33 +267,38 @@ static int vce_v3_0_start(struct amdgpu_device *adev)
>   	struct amdgpu_ring *ring;
>   	int idx, r;
>   
> -	ring = &adev->vce.ring[0];
> -	WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
> -	WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
> -	WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
> -
> -	ring = &adev->vce.ring[2];
> -	WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
> -	WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
> -	WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
> -	WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
> -	WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
> -
>   	mutex_lock(&adev->grbm_idx_mutex);
>   	for (idx = 0; idx < 2; ++idx) {
>   		if (adev->vce.harvest_config & (1 << idx))
>   			continue;
>   
>   		WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(idx));
> +
> +		/* Program instance 0 reg space for two instances or instance 0 case
> +		program instance 1 reg space for only instance 1 available case */
> +		if (idx != 1 || adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
> +			ring = &adev->vce.ring[0];
> +			WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
> +			WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
> +			WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
> +
> +			ring = &adev->vce.ring[2];
> +			WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
> +			WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
> +			WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
> +			WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
> +			WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
> +		}
> +
>   		vce_v3_0_mc_resume(adev, idx);
>   		WREG32_FIELD(VCE_STATUS, JOB_BUSY, 1);
>   

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: Program ring for vce instance 1 at its register space
@ 2017-05-30 16:27       ` Leo Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Leo Liu @ 2017-05-30 16:27 UTC (permalink / raw)
  To: Christian König, amd-gfx; +Cc: stable



On 05/30/2017 12:15 PM, Christian König wrote:
> Am 30.05.2017 um 17:56 schrieb Leo Liu:
>> We need program ring buffer on instance 1 register space domain,
>> when only if instance 1 available, with two instances or instance 0,
>> and we need only program instance 0 regsiter space domain for ring.
>>
>> Signed-off-by: Leo Liu <leo.liu@amd.com>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> Cc: stable@vger.kernel.org
>> ---
>>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 95 
>> +++++++++++++++++++++++++----------
>>   1 file changed, 68 insertions(+), 27 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..7e39e42 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));
>> +    }
>
> Did you missed my comment? I think we should always grab the mutex and 
> program mmGRBM_GFX_INDEX.

No I haven't , I did move the programming the ring regs from "vce_start" 
for Instance 0, to under mutex and  mmGRBM_GFX_INDEX.

IMO, I don't think we need this here, because "mutex lock + 
mmGRBM_GFX_INDEX" and "mutex unlock+mmGRBM_GFX_DEFAULT" are always 
paired, So it should be always in default space when instance 0, and we 
do take care the case for instance 1 only.

I can follow your comment for here and other place in my patch for 
get/set w/rptr.

Thanks,
Leo

>
> Otherwise we silently rely on that it is correctly set when the 
> function is called and have different code path for different 
> harvested configs.
>
> Apart from that the patch looks good to me.
>
> Christian.
>
>>         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,33 +267,38 @@ static int vce_v3_0_start(struct amdgpu_device 
>> *adev)
>>       struct amdgpu_ring *ring;
>>       int idx, r;
>>   -    ring = &adev->vce.ring[0];
>> -    WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
>> -    WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
>> -    WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
>> -
>> -    ring = &adev->vce.ring[2];
>> -    WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
>> -    WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
>> -    WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
>> -    WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
>> -    WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
>> -
>>       mutex_lock(&adev->grbm_idx_mutex);
>>       for (idx = 0; idx < 2; ++idx) {
>>           if (adev->vce.harvest_config & (1 << idx))
>>               continue;
>>             WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(idx));
>> +
>> +        /* Program instance 0 reg space for two instances or 
>> instance 0 case
>> +        program instance 1 reg space for only instance 1 available 
>> case */
>> +        if (idx != 1 || adev->vce.harvest_config == 
>> AMDGPU_VCE_HARVEST_VCE0) {
>> +            ring = &adev->vce.ring[0];
>> +            WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
>> +            WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
>> +            WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
>> +
>> +            ring = &adev->vce.ring[2];
>> +            WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
>> +            WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
>> +            WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
>> +            WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
>> +            WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
>> +        }
>> +
>>           vce_v3_0_mc_resume(adev, idx);
>>           WREG32_FIELD(VCE_STATUS, JOB_BUSY, 1);
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: Program ring for vce instance 1 at its register space
@ 2017-05-30 16:27       ` Leo Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Leo Liu @ 2017-05-30 16:27 UTC (permalink / raw)
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: stable-u79uwXL29TY76Z2rM5mHXA



On 05/30/2017 12:15 PM, Christian König wrote:
> Am 30.05.2017 um 17:56 schrieb Leo Liu:
>> We need program ring buffer on instance 1 register space domain,
>> when only if instance 1 available, with two instances or instance 0,
>> and we need only program instance 0 regsiter space domain for ring.
>>
>> Signed-off-by: Leo Liu <leo.liu@amd.com>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> Cc: stable@vger.kernel.org
>> ---
>>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 95 
>> +++++++++++++++++++++++++----------
>>   1 file changed, 68 insertions(+), 27 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..7e39e42 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));
>> +    }
>
> Did you missed my comment? I think we should always grab the mutex and 
> program mmGRBM_GFX_INDEX.

No I haven't , I did move the programming the ring regs from "vce_start" 
for Instance 0, to under mutex and  mmGRBM_GFX_INDEX.

IMO, I don't think we need this here, because "mutex lock + 
mmGRBM_GFX_INDEX" and "mutex unlock+mmGRBM_GFX_DEFAULT" are always 
paired, So it should be always in default space when instance 0, and we 
do take care the case for instance 1 only.

I can follow your comment for here and other place in my patch for 
get/set w/rptr.

Thanks,
Leo

>
> Otherwise we silently rely on that it is correctly set when the 
> function is called and have different code path for different 
> harvested configs.
>
> Apart from that the patch looks good to me.
>
> Christian.
>
>>         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,33 +267,38 @@ static int vce_v3_0_start(struct amdgpu_device 
>> *adev)
>>       struct amdgpu_ring *ring;
>>       int idx, r;
>>   -    ring = &adev->vce.ring[0];
>> -    WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
>> -    WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
>> -    WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
>> -
>> -    ring = &adev->vce.ring[2];
>> -    WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
>> -    WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
>> -    WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
>> -    WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
>> -    WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
>> -
>>       mutex_lock(&adev->grbm_idx_mutex);
>>       for (idx = 0; idx < 2; ++idx) {
>>           if (adev->vce.harvest_config & (1 << idx))
>>               continue;
>>             WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(idx));
>> +
>> +        /* Program instance 0 reg space for two instances or 
>> instance 0 case
>> +        program instance 1 reg space for only instance 1 available 
>> case */
>> +        if (idx != 1 || adev->vce.harvest_config == 
>> AMDGPU_VCE_HARVEST_VCE0) {
>> +            ring = &adev->vce.ring[0];
>> +            WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
>> +            WREG32(mmVCE_RB_WPTR, lower_32_bits(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, lower_32_bits(ring->wptr));
>> +            WREG32(mmVCE_RB_WPTR2, lower_32_bits(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);
>> +
>> +            ring = &adev->vce.ring[2];
>> +            WREG32(mmVCE_RB_RPTR3, lower_32_bits(ring->wptr));
>> +            WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
>> +            WREG32(mmVCE_RB_BASE_LO3, ring->gpu_addr);
>> +            WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
>> +            WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
>> +        }
>> +
>>           vce_v3_0_mc_resume(adev, idx);
>>           WREG32_FIELD(VCE_STATUS, JOB_BUSY, 1);
>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-05-30 16:27 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-29 18:21 [PATCH] drm/amdgpu: Program ring for vce instance 1 at its own register space Leo Liu
     [not found] ` <20170529182152.7451-1-leo.liu-5C7GfCeVMHo@public.gmane.org>
2017-05-30 14:57   ` Deucher, Alexander
     [not found]     ` <BN6PR12MB1652D8852FBB1EDD82FF2651F7F00-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-05-30 15:13       ` Christian König
     [not found]         ` <d3fc8ec8-2167-126e-a9bd-cb98c6d0006c-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-05-30 15:55           ` Leo Liu
2017-05-30 15:56 ` [PATCH] drm/amdgpu: Program ring for vce instance 1 at its " Leo Liu
2017-05-30 15:56   ` Leo Liu
2017-05-30 16:15   ` Christian König
2017-05-30 16:27     ` Leo Liu
2017-05-30 16:27       ` Leo Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.