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@xxxxxxxxx>
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@xxxxxxxxxxx> wrote:
> Am 14.05.2016 um 16:19 schrieb Emil Velikov:
>>
>> Hi all,
>>
>> On 13 May 2016 at 17:48, Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
>>>
>>> From: Sonny Jiang <sonny.jiang@xxxxxxx>
>>>
>>> Signed-off-by: Sonny Jiang <sonny.jiang@xxxxxxx>
>>> Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
>>> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
