[PATCH 1/4] tests/amdgpu: expand write/copy tests to compute

[PATCH 1/4] tests/amdgpu: expand write/copy tests to compute

[Alex Deucher]

Uses same packets as gfx.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 tests/amdgpu/basic_tests.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
index 599c090..05ab145 100644
--- a/tests/amdgpu/basic_tests.c
+++ b/tests/amdgpu/basic_tests.c
@@ -813,7 +813,8 @@ static void amdgpu_command_submission_write_linear_helper(unsigned ip_type)
 			pm4[i++] = sdma_write_length;
 			while(j++ < sdma_write_length)
 				pm4[i++] = 0xdeadbeaf;
-		} else if (ip_type == AMDGPU_HW_IP_GFX) {
+		} else if ((ip_type == AMDGPU_HW_IP_GFX) ||
+			   (ip_type == AMDGPU_HW_IP_COMPUTE)) {
 			pm4[i++] = PACKET3(PACKET3_WRITE_DATA, 2 + sdma_write_length);
 			pm4[i++] = WRITE_DATA_DST_SEL(5) | WR_CONFIRM;
 			pm4[i++] = 0xfffffffc & bo_mc;
@@ -911,7 +912,8 @@ static void amdgpu_command_submission_const_fill_helper(unsigned ip_type)
 			pm4[i++] = (0xffffffff00000000 & bo_mc) >> 32;
 			pm4[i++] = 0xdeadbeaf;
 			pm4[i++] = sdma_write_length;
-		} else if (ip_type == AMDGPU_HW_IP_GFX) {
+		} else if ((ip_type == AMDGPU_HW_IP_GFX) ||
+			   (ip_type == AMDGPU_HW_IP_COMPUTE)) {
 			pm4[i++] = PACKET3(PACKET3_DMA_DATA, 5);
 			pm4[i++] = PACKET3_DMA_DATA_ENGINE(0) |
 				PACKET3_DMA_DATA_DST_SEL(0) |
@@ -1030,7 +1032,8 @@ static void amdgpu_command_submission_copy_linear_helper(unsigned ip_type)
 				pm4[i++] = (0xffffffff00000000 & bo1_mc) >> 32;
 				pm4[i++] = 0xffffffff & bo2_mc;
 				pm4[i++] = (0xffffffff00000000 & bo2_mc) >> 32;
-			} else if (ip_type == AMDGPU_HW_IP_GFX) {
+			} else if ((ip_type == AMDGPU_HW_IP_GFX) ||
+				   (ip_type == AMDGPU_HW_IP_COMPUTE)) {
 				pm4[i++] = PACKET3(PACKET3_DMA_DATA, 5);
 				pm4[i++] = PACKET3_DMA_DATA_ENGINE(0) |
 					PACKET3_DMA_DATA_DST_SEL(0) |
-- 
2.5.5

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

[PATCH 2/4] tests/amdgpu: expand write/copy tests to compute

[Alex Deucher]

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 tests/amdgpu/basic_tests.c | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
index 05ab145..e512bda 100644
--- a/tests/amdgpu/basic_tests.c
+++ b/tests/amdgpu/basic_tests.c
@@ -445,17 +445,17 @@ static void amdgpu_command_submission_gfx_shared_ib(void)
 	CU_ASSERT_EQUAL(r, 0);
 }
 
-static void amdgpu_command_submission_cp_write_data(void)
+static void amdgpu_command_submission_gfx_cp_write_data(void)
 {
 	amdgpu_command_submission_write_linear_helper(AMDGPU_HW_IP_GFX);
 }
 
-static void amdgpu_command_submission_cp_const_fill(void)
+static void amdgpu_command_submission_gfx_cp_const_fill(void)
 {
 	amdgpu_command_submission_const_fill_helper(AMDGPU_HW_IP_GFX);
 }
 
-static void amdgpu_command_submission_cp_copy_data(void)
+static void amdgpu_command_submission_gfx_cp_copy_data(void)
 {
 	amdgpu_command_submission_copy_linear_helper(AMDGPU_HW_IP_GFX);
 }
@@ -463,11 +463,11 @@ static void amdgpu_command_submission_cp_copy_data(void)
 static void amdgpu_command_submission_gfx(void)
 {
 	/* write data using the CP */
-	amdgpu_command_submission_cp_write_data();
+	amdgpu_command_submission_gfx_cp_write_data();
 	/* const fill using the CP */
-	amdgpu_command_submission_cp_const_fill();
+	amdgpu_command_submission_gfx_cp_const_fill();
 	/* copy data using the CP */
-	amdgpu_command_submission_cp_copy_data();
+	amdgpu_command_submission_gfx_cp_copy_data();
 	/* separate IB buffers for multi-IB submission */
 	amdgpu_command_submission_gfx_separate_ibs();
 	/* shared IB buffer for multi-IB submission */
@@ -602,7 +602,7 @@ static void amdgpu_semaphore_test(void)
 	CU_ASSERT_EQUAL(r, 0);
 }
 
-static void amdgpu_command_submission_compute(void)
+static void amdgpu_command_submission_compute_nop(void)
 {
 	amdgpu_context_handle context_handle;
 	amdgpu_bo_handle ib_result_handle;
@@ -673,6 +673,33 @@ static void amdgpu_command_submission_compute(void)
 	CU_ASSERT_EQUAL(r, 0);
 }
 
+static void amdgpu_command_submission_compute_cp_write_data(void)
+{
+	amdgpu_command_submission_write_linear_helper(AMDGPU_HW_IP_COMPUTE);
+}
+
+static void amdgpu_command_submission_compute_cp_const_fill(void)
+{
+	amdgpu_command_submission_const_fill_helper(AMDGPU_HW_IP_COMPUTE);
+}
+
+static void amdgpu_command_submission_compute_cp_copy_data(void)
+{
+	amdgpu_command_submission_copy_linear_helper(AMDGPU_HW_IP_COMPUTE);
+}
+
+static void amdgpu_command_submission_compute(void)
+{
+	/* write data using the CP */
+	amdgpu_command_submission_compute_cp_write_data();
+	/* const fill using the CP */
+	amdgpu_command_submission_compute_cp_const_fill();
+	/* copy data using the CP */
+	amdgpu_command_submission_compute_cp_copy_data();
+	/* nop test */
+	amdgpu_command_submission_compute_nop();
+}
+
 /*
  * caller need create/release:
  * pm4_src, resources, ib_info, and ibs_request
-- 
2.5.5

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

[PATCH 3/4] tests/amdgpu: add interface to adapt firmware requirement

[Alex Deucher]

From: Leo Liu <leo.liu@amd.com>

Signed-off-by: Leo Liu <leo.liu@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 tests/amdgpu/cs_tests.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
index dfbf5af..c6930c0 100644
--- a/tests/amdgpu/cs_tests.c
+++ b/tests/amdgpu/cs_tests.c
@@ -266,9 +266,12 @@ static void amdgpu_cs_uvd_decode(void)
 	r = amdgpu_bo_cpu_map(buf_handle, (void **)&ptr);
 	CU_ASSERT_EQUAL(r, 0);
 
-	memcpy(ptr, uvd_decode_msg, sizeof(uvd_decode_msg));
-	if (family_id >= AMDGPU_FAMILY_VI)
+	memcpy(ptr, uvd_decode_msg, sizeof(uvd_create_msg));
+	if (family_id >= AMDGPU_FAMILY_VI) {
 		ptr[0x10] = 7;
+		ptr[0x98] = 0xb0;
+		ptr[0x99] = 0x1;
+	}
 
 	ptr += 4*1024;
 	memset(ptr, 0, 4*1024);
-- 
2.5.5

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

[PATCH 4/4] tests/amdgpu: adapt to new polaris10/11 uvd fw

[Alex Deucher]

From: Sonny Jiang <sonny.jiang@amd.com>

Signed-off-by: Sonny Jiang <sonny.jiang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 tests/amdgpu/cs_tests.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
index c6930c0..a01ee48 100644
--- a/tests/amdgpu/cs_tests.c
+++ b/tests/amdgpu/cs_tests.c
@@ -43,6 +43,8 @@ static amdgpu_device_handle device_handle;
 static uint32_t major_version;
 static uint32_t minor_version;
 static uint32_t family_id;
+static uint32_t chip_rev;
+static uint32_t chip_id;
 
 static amdgpu_context_handle context_handle;
 static amdgpu_bo_handle ib_handle;
@@ -78,6 +80,9 @@ int suite_cs_tests_init(void)
 		return CUE_SINIT_FAILED;
 
 	family_id = device_handle->info.family_id;
+	/* VI asic POLARIS10/11 have specific external_rev_id */
+	chip_rev = device_handle->info.chip_rev;
+	chip_id = device_handle->info.chip_external_rev;
 
 	r = amdgpu_cs_ctx_create(device_handle, &context_handle);
 	if (r)
@@ -200,8 +205,17 @@ static void amdgpu_cs_uvd_create(void)
 	CU_ASSERT_EQUAL(r, 0);
 
 	memcpy(msg, uvd_create_msg, sizeof(uvd_create_msg));
-	if (family_id >= AMDGPU_FAMILY_VI)
+	if (family_id >= AMDGPU_FAMILY_VI) {
 		((uint8_t*)msg)[0x10] = 7;
+		/* chip polaris 10/11 */
+		if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A) {
+			/* dpb size */
+			((uint8_t*)msg)[0x28] = 0x00;
+			((uint8_t*)msg)[0x29] = 0x94;
+			((uint8_t*)msg)[0x2A] = 0x6B;
+			((uint8_t*)msg)[0x2B] = 0x00;
+		}
+	}
 
 	r = amdgpu_bo_cpu_unmap(buf_handle);
 	CU_ASSERT_EQUAL(r, 0);
@@ -230,8 +244,8 @@ static void amdgpu_cs_uvd_create(void)
 
 static void amdgpu_cs_uvd_decode(void)
 {
-	const unsigned dpb_size = 15923584, dt_size = 737280;
-	uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, dt_addr, it_addr;
+	const unsigned dpb_size = 15923584, ctx_size = 5287680, dt_size = 737280;
+	uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, ctx_addr, dt_addr, it_addr;
 	struct amdgpu_bo_alloc_request req = {0};
 	amdgpu_bo_handle buf_handle;
 	amdgpu_va_handle va_handle;
@@ -269,8 +283,21 @@ static void amdgpu_cs_uvd_decode(void)
 	memcpy(ptr, uvd_decode_msg, sizeof(uvd_create_msg));
 	if (family_id >= AMDGPU_FAMILY_VI) {
 		ptr[0x10] = 7;
-		ptr[0x98] = 0xb0;
-		ptr[0x99] = 0x1;
+		ptr[0x98] = 0x00;
+		ptr[0x99] = 0x02;
+		/* chip polaris10/11 */
+		if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A) {
+			/*dpb size */
+			ptr[0x24] = 0x00;
+			ptr[0x25] = 0x94;
+			ptr[0x26] = 0x6B;
+			ptr[0x27] = 0x00;
+			/*ctx size */
+			ptr[0x2C] = 0x00;
+			ptr[0x2D] = 0xAF;
+			ptr[0x2E] = 0x50;
+			ptr[0x2F] = 0x00;
+		}
 	}
 
 	ptr += 4*1024;
@@ -301,6 +328,12 @@ static void amdgpu_cs_uvd_decode(void)
 	} else
 		bs_addr = fb_addr + 4*1024;
 	dpb_addr = ALIGN(bs_addr + sizeof(uvd_bitstream), 4*1024);
+
+	if ((family_id >= AMDGPU_FAMILY_VI) &&
+		(chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A)) {
+		ctx_addr = ALIGN(dpb_addr + 0x006B9400, 4*1024);
+	}
+
 	dt_addr = ALIGN(dpb_addr + dpb_size, 4*1024);
 
 	i = 0;
@@ -309,8 +342,11 @@ static void amdgpu_cs_uvd_decode(void)
 	uvd_cmd(dt_addr, 0x2, &i);
 	uvd_cmd(fb_addr, 0x3, &i);
 	uvd_cmd(bs_addr, 0x100, &i);
-	if (family_id >= AMDGPU_FAMILY_VI)
+	if (family_id >= AMDGPU_FAMILY_VI) {
 		uvd_cmd(it_addr, 0x204, &i);
+		if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A)
+			uvd_cmd(ctx_addr, 0x206, &i);
+}
 	ib_cpu[i++] = 0x3BC6;
 	ib_cpu[i++] = 0x1;
 	for (; i % 16; ++i)
-- 
2.5.5

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

Re: [PATCH 1/4] tests/amdgpu: expand write/copy tests to compute

[Christian König]

Am 13.05.2016 um 18:48 schrieb Alex Deucher:
> Uses same packets as gfx.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

For the series Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   tests/amdgpu/basic_tests.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
> index 599c090..05ab145 100644
> --- a/tests/amdgpu/basic_tests.c
> +++ b/tests/amdgpu/basic_tests.c
> @@ -813,7 +813,8 @@ static void amdgpu_command_submission_write_linear_helper(unsigned ip_type)
>   			pm4[i++] = sdma_write_length;
>   			while(j++ < sdma_write_length)
>   				pm4[i++] = 0xdeadbeaf;
> -		} else if (ip_type == AMDGPU_HW_IP_GFX) {
> +		} else if ((ip_type == AMDGPU_HW_IP_GFX) ||
> +			   (ip_type == AMDGPU_HW_IP_COMPUTE)) {
>   			pm4[i++] = PACKET3(PACKET3_WRITE_DATA, 2 + sdma_write_length);
>   			pm4[i++] = WRITE_DATA_DST_SEL(5) | WR_CONFIRM;
>   			pm4[i++] = 0xfffffffc & bo_mc;
> @@ -911,7 +912,8 @@ static void amdgpu_command_submission_const_fill_helper(unsigned ip_type)
>   			pm4[i++] = (0xffffffff00000000 & bo_mc) >> 32;
>   			pm4[i++] = 0xdeadbeaf;
>   			pm4[i++] = sdma_write_length;
> -		} else if (ip_type == AMDGPU_HW_IP_GFX) {
> +		} else if ((ip_type == AMDGPU_HW_IP_GFX) ||
> +			   (ip_type == AMDGPU_HW_IP_COMPUTE)) {
>   			pm4[i++] = PACKET3(PACKET3_DMA_DATA, 5);
>   			pm4[i++] = PACKET3_DMA_DATA_ENGINE(0) |
>   				PACKET3_DMA_DATA_DST_SEL(0) |
> @@ -1030,7 +1032,8 @@ static void amdgpu_command_submission_copy_linear_helper(unsigned ip_type)
>   				pm4[i++] = (0xffffffff00000000 & bo1_mc) >> 32;
>   				pm4[i++] = 0xffffffff & bo2_mc;
>   				pm4[i++] = (0xffffffff00000000 & bo2_mc) >> 32;
> -			} else if (ip_type == AMDGPU_HW_IP_GFX) {
> +			} else if ((ip_type == AMDGPU_HW_IP_GFX) ||
> +				   (ip_type == AMDGPU_HW_IP_COMPUTE)) {
>   				pm4[i++] = PACKET3(PACKET3_DMA_DATA, 5);
>   				pm4[i++] = PACKET3_DMA_DATA_ENGINE(0) |
>   					PACKET3_DMA_DATA_DST_SEL(0) |

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

Re: [PATCH 4/4] tests/amdgpu: adapt to new polaris10/11 uvd fw

[Emil Velikov]

Hi all,

On 13 May 2016 at 17:48, Alex Deucher <alexdeucher@gmail.com> wrote:
> From: Sonny Jiang <sonny.jiang@amd.com>
>
> Signed-off-by: Sonny Jiang <sonny.jiang@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  tests/amdgpu/cs_tests.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
> index c6930c0..a01ee48 100644
> --- a/tests/amdgpu/cs_tests.c
> +++ b/tests/amdgpu/cs_tests.c
> @@ -43,6 +43,8 @@ static amdgpu_device_handle device_handle;
>  static uint32_t major_version;
>  static uint32_t minor_version;
>  static uint32_t family_id;
> +static uint32_t chip_rev;
> +static uint32_t chip_id;
>
>  static amdgpu_context_handle context_handle;
>  static amdgpu_bo_handle ib_handle;
> @@ -78,6 +80,9 @@ int suite_cs_tests_init(void)
>                 return CUE_SINIT_FAILED;
>
>         family_id = device_handle->info.family_id;
> +       /* VI asic POLARIS10/11 have specific external_rev_id */
> +       chip_rev = device_handle->info.chip_rev;
> +       chip_id = device_handle->info.chip_external_rev;
>
>         r = amdgpu_cs_ctx_create(device_handle, &context_handle);
>         if (r)
> @@ -200,8 +205,17 @@ static void amdgpu_cs_uvd_create(void)
>         CU_ASSERT_EQUAL(r, 0);
>
>         memcpy(msg, uvd_create_msg, sizeof(uvd_create_msg));
> -       if (family_id >= AMDGPU_FAMILY_VI)
> +       if (family_id >= AMDGPU_FAMILY_VI) {
>                 ((uint8_t*)msg)[0x10] = 7;
> +               /* chip polaris 10/11 */
> +               if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A) {
> +                       /* dpb size */
> +                       ((uint8_t*)msg)[0x28] = 0x00;
> +                       ((uint8_t*)msg)[0x29] = 0x94;
> +                       ((uint8_t*)msg)[0x2A] = 0x6B;
> +                       ((uint8_t*)msg)[0x2B] = 0x00;
I realise that many of the UVD stuff is 'top secret', although one
should really try and give symbolic names for magic numbers. With them
it's be easier and less error prone when/if the above value changes.

> +               }
> +       }
>
>         r = amdgpu_bo_cpu_unmap(buf_handle);
>         CU_ASSERT_EQUAL(r, 0);
> @@ -230,8 +244,8 @@ static void amdgpu_cs_uvd_create(void)
>
>  static void amdgpu_cs_uvd_decode(void)
>  {
> -       const unsigned dpb_size = 15923584, dt_size = 737280;
> -       uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, dt_addr, it_addr;
> +       const unsigned dpb_size = 15923584, ctx_size = 5287680, dt_size = 737280;
> +       uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, ctx_addr, dt_addr, it_addr;
>         struct amdgpu_bo_alloc_request req = {0};
>         amdgpu_bo_handle buf_handle;
>         amdgpu_va_handle va_handle;
> @@ -269,8 +283,21 @@ static void amdgpu_cs_uvd_decode(void)
>         memcpy(ptr, uvd_decode_msg, sizeof(uvd_create_msg));
>         if (family_id >= AMDGPU_FAMILY_VI) {
>                 ptr[0x10] = 7;
> -               ptr[0x98] = 0xb0;
> -               ptr[0x99] = 0x1;
> +               ptr[0x98] = 0x00;
> +               ptr[0x99] = 0x02;
> +               /* chip polaris10/11 */
> +               if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A) {
> +                       /*dpb size */
> +                       ptr[0x24] = 0x00;
> +                       ptr[0x25] = 0x94;
> +                       ptr[0x26] = 0x6B;
> +                       ptr[0x27] = 0x00;
Based on the const dpb_size a few lines above... this value is
incorrect. So either the comment is off, or one/both of the values ?


> +                       /*ctx size */
> +                       ptr[0x2C] = 0x00;
> +                       ptr[0x2D] = 0xAF;
> +                       ptr[0x2E] = 0x50;
> +                       ptr[0x2F] = 0x00;
> +               }
While this one does match ctx_size above, one should really set a
macro for these magic values and use them throughout. Also considering
that there's three almost identical places where this happens perhaps
it's better to have a common helper ?

Regards,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] tests/amdgpu: adapt to new polaris10/11 uvd fw

[Christian König]

Am 14.05.2016 um 16:19 schrieb Emil Velikov:
> Hi all,
>
> On 13 May 2016 at 17:48, Alex Deucher <alexdeucher@gmail.com> wrote:
>> From: Sonny Jiang <sonny.jiang@amd.com>
>>
>> Signed-off-by: Sonny Jiang <sonny.jiang@amd.com>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>   tests/amdgpu/cs_tests.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
>>   1 file changed, 42 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
>> index c6930c0..a01ee48 100644
>> --- a/tests/amdgpu/cs_tests.c
>> +++ b/tests/amdgpu/cs_tests.c
>> @@ -43,6 +43,8 @@ static amdgpu_device_handle device_handle;
>>   static uint32_t major_version;
>>   static uint32_t minor_version;
>>   static uint32_t family_id;
>> +static uint32_t chip_rev;
>> +static uint32_t chip_id;
>>
>>   static amdgpu_context_handle context_handle;
>>   static amdgpu_bo_handle ib_handle;
>> @@ -78,6 +80,9 @@ int suite_cs_tests_init(void)
>>                  return CUE_SINIT_FAILED;
>>
>>          family_id = device_handle->info.family_id;
>> +       /* VI asic POLARIS10/11 have specific external_rev_id */
>> +       chip_rev = device_handle->info.chip_rev;
>> +       chip_id = device_handle->info.chip_external_rev;
>>
>>          r = amdgpu_cs_ctx_create(device_handle, &context_handle);
>>          if (r)
>> @@ -200,8 +205,17 @@ static void amdgpu_cs_uvd_create(void)
>>          CU_ASSERT_EQUAL(r, 0);
>>
>>          memcpy(msg, uvd_create_msg, sizeof(uvd_create_msg));
>> -       if (family_id >= AMDGPU_FAMILY_VI)
>> +       if (family_id >= AMDGPU_FAMILY_VI) {
>>                  ((uint8_t*)msg)[0x10] = 7;
>> +               /* chip polaris 10/11 */
>> +               if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A) {
>> +                       /* dpb size */
>> +                       ((uint8_t*)msg)[0x28] = 0x00;
>> +                       ((uint8_t*)msg)[0x29] = 0x94;
>> +                       ((uint8_t*)msg)[0x2A] = 0x6B;
>> +                       ((uint8_t*)msg)[0x2B] = 0x00;
> I realise that many of the UVD stuff is 'top secret', although one
> should really try and give symbolic names for magic numbers. With them
> it's be easier and less error prone when/if the above value changes.

Actually we have exposed mostly everything in the UVD headers in mesa 
and those binary values here are just captured example streams.

Saying so I would also prefer that we don't patch the messages on the 
fly as necessary, but rather have a full copy for each chipset family 
they differ. On the other hand it's just the unit tests.

Regards,
Christian.

>
>> +               }
>> +       }
>>
>>          r = amdgpu_bo_cpu_unmap(buf_handle);
>>          CU_ASSERT_EQUAL(r, 0);
>> @@ -230,8 +244,8 @@ static void amdgpu_cs_uvd_create(void)
>>
>>   static void amdgpu_cs_uvd_decode(void)
>>   {
>> -       const unsigned dpb_size = 15923584, dt_size = 737280;
>> -       uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, dt_addr, it_addr;
>> +       const unsigned dpb_size = 15923584, ctx_size = 5287680, dt_size = 737280;
>> +       uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, ctx_addr, dt_addr, it_addr;
>>          struct amdgpu_bo_alloc_request req = {0};
>>          amdgpu_bo_handle buf_handle;
>>          amdgpu_va_handle va_handle;
>> @@ -269,8 +283,21 @@ static void amdgpu_cs_uvd_decode(void)
>>          memcpy(ptr, uvd_decode_msg, sizeof(uvd_create_msg));
>>          if (family_id >= AMDGPU_FAMILY_VI) {
>>                  ptr[0x10] = 7;
>> -               ptr[0x98] = 0xb0;
>> -               ptr[0x99] = 0x1;
>> +               ptr[0x98] = 0x00;
>> +               ptr[0x99] = 0x02;
>> +               /* chip polaris10/11 */
>> +               if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A) {
>> +                       /*dpb size */
>> +                       ptr[0x24] = 0x00;
>> +                       ptr[0x25] = 0x94;
>> +                       ptr[0x26] = 0x6B;
>> +                       ptr[0x27] = 0x00;
> Based on the const dpb_size a few lines above... this value is
> incorrect. So either the comment is off, or one/both of the values ?
>
>
>> +                       /*ctx size */
>> +                       ptr[0x2C] = 0x00;
>> +                       ptr[0x2D] = 0xAF;
>> +                       ptr[0x2E] = 0x50;
>> +                       ptr[0x2F] = 0x00;
>> +               }
> While this one does match ctx_size above, one should really set a
> macro for these magic values and use them throughout. Also considering
> that there's three almost identical places where this happens perhaps
> it's better to have a common helper ?
>
> Regards,
> Emil
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 4/4] tests/amdgpu: adapt to new polaris10/11 uvd fw

[Emil Velikov]

On 14 May 2016 at 19:29, Christian König <deathsimple@vodafone.de> wrote:
> Am 14.05.2016 um 16:19 schrieb Emil Velikov:
>>
>> Hi all,
>>
>> On 13 May 2016 at 17:48, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>
>>> From: Sonny Jiang <sonny.jiang@amd.com>
>>>
>>> Signed-off-by: Sonny Jiang <sonny.jiang@amd.com>
>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>   tests/amdgpu/cs_tests.c | 48
>>> ++++++++++++++++++++++++++++++++++++++++++------
>>>   1 file changed, 42 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
>>> index c6930c0..a01ee48 100644
>>> --- a/tests/amdgpu/cs_tests.c
>>> +++ b/tests/amdgpu/cs_tests.c
>>> @@ -43,6 +43,8 @@ static amdgpu_device_handle device_handle;
>>>   static uint32_t major_version;
>>>   static uint32_t minor_version;
>>>   static uint32_t family_id;
>>> +static uint32_t chip_rev;
>>> +static uint32_t chip_id;
>>>
>>>   static amdgpu_context_handle context_handle;
>>>   static amdgpu_bo_handle ib_handle;
>>> @@ -78,6 +80,9 @@ int suite_cs_tests_init(void)
>>>                  return CUE_SINIT_FAILED;
>>>
>>>          family_id = device_handle->info.family_id;
>>> +       /* VI asic POLARIS10/11 have specific external_rev_id */
>>> +       chip_rev = device_handle->info.chip_rev;
>>> +       chip_id = device_handle->info.chip_external_rev;
>>>
>>>          r = amdgpu_cs_ctx_create(device_handle, &context_handle);
>>>          if (r)
>>> @@ -200,8 +205,17 @@ static void amdgpu_cs_uvd_create(void)
>>>          CU_ASSERT_EQUAL(r, 0);
>>>
>>>          memcpy(msg, uvd_create_msg, sizeof(uvd_create_msg));
>>> -       if (family_id >= AMDGPU_FAMILY_VI)
>>> +       if (family_id >= AMDGPU_FAMILY_VI) {
>>>                  ((uint8_t*)msg)[0x10] = 7;
>>> +               /* chip polaris 10/11 */
>>> +               if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A)
>>> {
>>> +                       /* dpb size */
>>> +                       ((uint8_t*)msg)[0x28] = 0x00;
>>> +                       ((uint8_t*)msg)[0x29] = 0x94;
>>> +                       ((uint8_t*)msg)[0x2A] = 0x6B;
>>> +                       ((uint8_t*)msg)[0x2B] = 0x00;
>>
>> I realise that many of the UVD stuff is 'top secret', although one
>> should really try and give symbolic names for magic numbers. With them
>> it's be easier and less error prone when/if the above value changes.
>
>
> Actually we have exposed mostly everything in the UVD headers in mesa and
> those binary values here are just captured example streams.
>
> Saying so I would also prefer that we don't patch the messages on the fly as
> necessary, but rather have a full copy for each chipset family they differ.
> On the other hand it's just the unit tests.
>
Just saw copy copy/paste job so I'd suggested unification. At the end
it's up-to you guys to decide for/against and (if so) do it.

Do you have any information on the dpb_size comment below ?

> Regards,
> Christian.
>
>>
>>> +               }
>>> +       }
>>>
>>>          r = amdgpu_bo_cpu_unmap(buf_handle);
>>>          CU_ASSERT_EQUAL(r, 0);
>>> @@ -230,8 +244,8 @@ static void amdgpu_cs_uvd_create(void)
>>>
>>>   static void amdgpu_cs_uvd_decode(void)
>>>   {
>>> -       const unsigned dpb_size = 15923584, dt_size = 737280;
>>> -       uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, dt_addr, it_addr;
>>> +       const unsigned dpb_size = 15923584, ctx_size = 5287680, dt_size =
>>> 737280;
>>> +       uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, ctx_addr, dt_addr,
>>> it_addr;
>>>          struct amdgpu_bo_alloc_request req = {0};
>>>          amdgpu_bo_handle buf_handle;
>>>          amdgpu_va_handle va_handle;
>>> @@ -269,8 +283,21 @@ static void amdgpu_cs_uvd_decode(void)
>>>          memcpy(ptr, uvd_decode_msg, sizeof(uvd_create_msg));
>>>          if (family_id >= AMDGPU_FAMILY_VI) {
>>>                  ptr[0x10] = 7;
>>> -               ptr[0x98] = 0xb0;
>>> -               ptr[0x99] = 0x1;
>>> +               ptr[0x98] = 0x00;
>>> +               ptr[0x99] = 0x02;
>>> +               /* chip polaris10/11 */
>>> +               if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A)
>>> {
>>> +                       /*dpb size */
>>> +                       ptr[0x24] = 0x00;
>>> +                       ptr[0x25] = 0x94;
>>> +                       ptr[0x26] = 0x6B;
>>> +                       ptr[0x27] = 0x00;
>>
>> Based on the const dpb_size a few lines above... this value is
>> incorrect. So either the comment is off, or one/both of the values ?
>>
Namely this one ?

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] tests/amdgpu: adapt to new polaris10/11 uvd fw

[Jiang, Sonny]

Hi Emil,

These values are just for this clip using on this test.
We have given method in both kernel and mesa codes on how to calculate dpb size and context size. I calculated them from the sample clip and used only for this test.

Regards,
Sonny
________________________________________
From: Emil Velikov <emil.l.velikov@gmail.com>
Sent: Sunday, May 15, 2016 2:16:41 PM
To: Christian König
Cc: Alex Deucher; Jiang, Sonny; Deucher, Alexander; ML dri-devel
Subject: Re: [PATCH 4/4] tests/amdgpu: adapt to new polaris10/11 uvd fw

On 14 May 2016 at 19:29, Christian König <deathsimple@vodafone.de> wrote:
> Am 14.05.2016 um 16:19 schrieb Emil Velikov:
>>
>> Hi all,
>>
>> On 13 May 2016 at 17:48, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>
>>> From: Sonny Jiang <sonny.jiang@amd.com>
>>>
>>> Signed-off-by: Sonny Jiang <sonny.jiang@amd.com>
>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>   tests/amdgpu/cs_tests.c | 48
>>> ++++++++++++++++++++++++++++++++++++++++++------
>>>   1 file changed, 42 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
>>> index c6930c0..a01ee48 100644
>>> --- a/tests/amdgpu/cs_tests.c
>>> +++ b/tests/amdgpu/cs_tests.c
>>> @@ -43,6 +43,8 @@ static amdgpu_device_handle device_handle;
>>>   static uint32_t major_version;
>>>   static uint32_t minor_version;
>>>   static uint32_t family_id;
>>> +static uint32_t chip_rev;
>>> +static uint32_t chip_id;
>>>
>>>   static amdgpu_context_handle context_handle;
>>>   static amdgpu_bo_handle ib_handle;
>>> @@ -78,6 +80,9 @@ int suite_cs_tests_init(void)
>>>                  return CUE_SINIT_FAILED;
>>>
>>>          family_id = device_handle->info.family_id;
>>> +       /* VI asic POLARIS10/11 have specific external_rev_id */
>>> +       chip_rev = device_handle->info.chip_rev;
>>> +       chip_id = device_handle->info.chip_external_rev;
>>>
>>>          r = amdgpu_cs_ctx_create(device_handle, &context_handle);
>>>          if (r)
>>> @@ -200,8 +205,17 @@ static void amdgpu_cs_uvd_create(void)
>>>          CU_ASSERT_EQUAL(r, 0);
>>>
>>>          memcpy(msg, uvd_create_msg, sizeof(uvd_create_msg));
>>> -       if (family_id >= AMDGPU_FAMILY_VI)
>>> +       if (family_id >= AMDGPU_FAMILY_VI) {
>>>                  ((uint8_t*)msg)[0x10] = 7;
>>> +               /* chip polaris 10/11 */
>>> +               if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A)
>>> {
>>> +                       /* dpb size */
>>> +                       ((uint8_t*)msg)[0x28] = 0x00;
>>> +                       ((uint8_t*)msg)[0x29] = 0x94;
>>> +                       ((uint8_t*)msg)[0x2A] = 0x6B;
>>> +                       ((uint8_t*)msg)[0x2B] = 0x00;
>>
>> I realise that many of the UVD stuff is 'top secret', although one
>> should really try and give symbolic names for magic numbers. With them
>> it's be easier and less error prone when/if the above value changes.
>
>
> Actually we have exposed mostly everything in the UVD headers in mesa and
> those binary values here are just captured example streams.
>
> Saying so I would also prefer that we don't patch the messages on the fly as
> necessary, but rather have a full copy for each chipset family they differ.
> On the other hand it's just the unit tests.
>
Just saw copy copy/paste job so I'd suggested unification. At the end
it's up-to you guys to decide for/against and (if so) do it.

Do you have any information on the dpb_size comment below ?

> Regards,
> Christian.
>
>>
>>> +               }
>>> +       }
>>>
>>>          r = amdgpu_bo_cpu_unmap(buf_handle);
>>>          CU_ASSERT_EQUAL(r, 0);
>>> @@ -230,8 +244,8 @@ static void amdgpu_cs_uvd_create(void)
>>>
>>>   static void amdgpu_cs_uvd_decode(void)
>>>   {
>>> -       const unsigned dpb_size = 15923584, dt_size = 737280;
>>> -       uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, dt_addr, it_addr;
>>> +       const unsigned dpb_size = 15923584, ctx_size = 5287680, dt_size =
>>> 737280;
>>> +       uint64_t msg_addr, fb_addr, bs_addr, dpb_addr, ctx_addr, dt_addr,
>>> it_addr;
>>>          struct amdgpu_bo_alloc_request req = {0};
>>>          amdgpu_bo_handle buf_handle;
>>>          amdgpu_va_handle va_handle;
>>> @@ -269,8 +283,21 @@ static void amdgpu_cs_uvd_decode(void)
>>>          memcpy(ptr, uvd_decode_msg, sizeof(uvd_create_msg));
>>>          if (family_id >= AMDGPU_FAMILY_VI) {
>>>                  ptr[0x10] = 7;
>>> -               ptr[0x98] = 0xb0;
>>> -               ptr[0x99] = 0x1;
>>> +               ptr[0x98] = 0x00;
>>> +               ptr[0x99] = 0x02;
>>> +               /* chip polaris10/11 */
>>> +               if (chip_id == chip_rev+0x50 || chip_id == chip_rev+0x5A)
>>> {
>>> +                       /*dpb size */
>>> +                       ptr[0x24] = 0x00;
>>> +                       ptr[0x25] = 0x94;
>>> +                       ptr[0x26] = 0x6B;
>>> +                       ptr[0x27] = 0x00;
>>
>> Based on the const dpb_size a few lines above... this value is
>> incorrect. So either the comment is off, or one/both of the values ?
>>
Namely this one ?

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel