[1/5] drm/amdgpu: fix insert nop for VCN decode ring

Submitted by Liu, Leo on May 17, 2018, 6:12 p.m.

Details

Message ID 20180517181218.6722-1-leo.liu@amd.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Liu, Leo May 17, 2018, 6:12 p.m.
NO_OP register should be writen to 0

Signed-off-by: Leo Liu <leo.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 0501746b..7fbbdb1 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -1048,14 +1048,17 @@  static int vcn_v1_0_process_interrupt(struct amdgpu_device *adev,
 	return 0;
 }
 
-static void vcn_v1_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
+static void vcn_v1_0_dec_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
 {
-	int i;
 	struct amdgpu_device *adev = ring->adev;
+	int i;
 
-	for (i = 0; i < count; i++)
-		amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_NO_OP), 0));
+	WARN_ON(ring->wptr % 2 || count % 2);
 
+	for (i = 0; i < count / 2; i++) {
+		amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_NO_OP), 0));
+		amdgpu_ring_write(ring, 0);
+	}
 }
 
 
@@ -1082,7 +1085,6 @@  static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
 static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
 	.type = AMDGPU_RING_TYPE_VCN_DEC,
 	.align_mask = 0xf,
-	.nop = PACKET0(0x81ff, 0),
 	.support_64bit_ptrs = false,
 	.vmhub = AMDGPU_MMHUB,
 	.get_rptr = vcn_v1_0_dec_ring_get_rptr,
@@ -1101,7 +1103,7 @@  static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
 	.emit_vm_flush = vcn_v1_0_dec_ring_emit_vm_flush,
 	.test_ring = amdgpu_vcn_dec_ring_test_ring,
 	.test_ib = amdgpu_vcn_dec_ring_test_ib,
-	.insert_nop = vcn_v1_0_ring_insert_nop,
+	.insert_nop = vcn_v1_0_dec_ring_insert_nop,
 	.insert_start = vcn_v1_0_dec_ring_insert_start,
 	.insert_end = vcn_v1_0_dec_ring_insert_end,
 	.pad_ib = amdgpu_ring_generic_pad_ib,

Comments

On Thu, May 17, 2018 at 2:12 PM, Leo Liu <leo.liu@amd.com> wrote:
> NO_OP register should be writen to 0
>
> Signed-off-by: Leo Liu <leo.liu@amd.com>

We discussed this when we first started working on amdgpu.  Does it
have to be 0?  This function is used for padding.  What happens if we
need an odd number of dwords of padding required?

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index 0501746b..7fbbdb1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1048,14 +1048,17 @@ static int vcn_v1_0_process_interrupt(struct amdgpu_device *adev,
>         return 0;
>  }
>
> -static void vcn_v1_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
> +static void vcn_v1_0_dec_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>  {
> -       int i;
>         struct amdgpu_device *adev = ring->adev;
> +       int i;
>
> -       for (i = 0; i < count; i++)
> -               amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_NO_OP), 0));
> +       WARN_ON(ring->wptr % 2 || count % 2);
>
> +       for (i = 0; i < count / 2; i++) {
> +               amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_NO_OP), 0));
> +               amdgpu_ring_write(ring, 0);
> +       }
>  }
>
>
> @@ -1082,7 +1085,6 @@ static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
>  static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>         .type = AMDGPU_RING_TYPE_VCN_DEC,
>         .align_mask = 0xf,
> -       .nop = PACKET0(0x81ff, 0),
>         .support_64bit_ptrs = false,
>         .vmhub = AMDGPU_MMHUB,
>         .get_rptr = vcn_v1_0_dec_ring_get_rptr,
> @@ -1101,7 +1103,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>         .emit_vm_flush = vcn_v1_0_dec_ring_emit_vm_flush,
>         .test_ring = amdgpu_vcn_dec_ring_test_ring,
>         .test_ib = amdgpu_vcn_dec_ring_test_ib,
> -       .insert_nop = vcn_v1_0_ring_insert_nop,
> +       .insert_nop = vcn_v1_0_dec_ring_insert_nop,
>         .insert_start = vcn_v1_0_dec_ring_insert_start,
>         .insert_end = vcn_v1_0_dec_ring_insert_end,
>         .pad_ib = amdgpu_ring_generic_pad_ib,
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On 05/17/2018 02:18 PM, Alex Deucher wrote:
> On Thu, May 17, 2018 at 2:12 PM, Leo Liu <leo.liu@amd.com> wrote:
>> NO_OP register should be writen to 0
>>
>> Signed-off-by: Leo Liu <leo.liu@amd.com>
> We discussed this when we first started working on amdgpu.  Does it
> have to be 0?
Yes. I confirmed with FW engineer that we need put 0 into noop.

>    This function is used for padding.  What happens if we
> need an odd number of dwords of padding required?
That should be never happening. should be always paired of PM4 packet 
for RBC to process.


Regards,
Leo

>
> Alex
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 14 ++++++++------
>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> index 0501746b..7fbbdb1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> @@ -1048,14 +1048,17 @@ static int vcn_v1_0_process_interrupt(struct amdgpu_device *adev,
>>          return 0;
>>   }
>>
>> -static void vcn_v1_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>> +static void vcn_v1_0_dec_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>>   {
>> -       int i;
>>          struct amdgpu_device *adev = ring->adev;
>> +       int i;
>>
>> -       for (i = 0; i < count; i++)
>> -               amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_NO_OP), 0));
>> +       WARN_ON(ring->wptr % 2 || count % 2);
>>
>> +       for (i = 0; i < count / 2; i++) {
>> +               amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_NO_OP), 0));
>> +               amdgpu_ring_write(ring, 0);
>> +       }
>>   }
>>
>>
>> @@ -1082,7 +1085,6 @@ static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
>>   static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>>          .type = AMDGPU_RING_TYPE_VCN_DEC,
>>          .align_mask = 0xf,
>> -       .nop = PACKET0(0x81ff, 0),
>>          .support_64bit_ptrs = false,
>>          .vmhub = AMDGPU_MMHUB,
>>          .get_rptr = vcn_v1_0_dec_ring_get_rptr,
>> @@ -1101,7 +1103,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>>          .emit_vm_flush = vcn_v1_0_dec_ring_emit_vm_flush,
>>          .test_ring = amdgpu_vcn_dec_ring_test_ring,
>>          .test_ib = amdgpu_vcn_dec_ring_test_ib,
>> -       .insert_nop = vcn_v1_0_ring_insert_nop,
>> +       .insert_nop = vcn_v1_0_dec_ring_insert_nop,
>>          .insert_start = vcn_v1_0_dec_ring_insert_start,
>>          .insert_end = vcn_v1_0_dec_ring_insert_end,
>>          .pad_ib = amdgpu_ring_generic_pad_ib,
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Thu, May 17, 2018 at 2:24 PM, Leo Liu <leo.liu@amd.com> wrote:
>
>
> On 05/17/2018 02:18 PM, Alex Deucher wrote:
>>
>> On Thu, May 17, 2018 at 2:12 PM, Leo Liu <leo.liu@amd.com> wrote:
>>>
>>> NO_OP register should be writen to 0
>>>
>>> Signed-off-by: Leo Liu <leo.liu@amd.com>
>>
>> We discussed this when we first started working on amdgpu.  Does it
>> have to be 0?
>
> Yes. I confirmed with FW engineer that we need put 0 into noop.
>
>>    This function is used for padding.  What happens if we
>> need an odd number of dwords of padding required?
>
> That should be never happening. should be always paired of PM4 packet for
> RBC to process.

Cool.  Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
>
> Regards,
> Leo
>
>
>>
>> Alex
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 14 ++++++++------
>>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>> index 0501746b..7fbbdb1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>> @@ -1048,14 +1048,17 @@ static int vcn_v1_0_process_interrupt(struct
>>> amdgpu_device *adev,
>>>          return 0;
>>>   }
>>>
>>> -static void vcn_v1_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t
>>> count)
>>> +static void vcn_v1_0_dec_ring_insert_nop(struct amdgpu_ring *ring,
>>> uint32_t count)
>>>   {
>>> -       int i;
>>>          struct amdgpu_device *adev = ring->adev;
>>> +       int i;
>>>
>>> -       for (i = 0; i < count; i++)
>>> -               amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0,
>>> mmUVD_NO_OP), 0));
>>> +       WARN_ON(ring->wptr % 2 || count % 2);
>>>
>>> +       for (i = 0; i < count / 2; i++) {
>>> +               amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0,
>>> mmUVD_NO_OP), 0));
>>> +               amdgpu_ring_write(ring, 0);
>>> +       }
>>>   }
>>>
>>>
>>> @@ -1082,7 +1085,6 @@ static const struct amd_ip_funcs vcn_v1_0_ip_funcs
>>> = {
>>>   static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>>>          .type = AMDGPU_RING_TYPE_VCN_DEC,
>>>          .align_mask = 0xf,
>>> -       .nop = PACKET0(0x81ff, 0),
>>>          .support_64bit_ptrs = false,
>>>          .vmhub = AMDGPU_MMHUB,
>>>          .get_rptr = vcn_v1_0_dec_ring_get_rptr,
>>> @@ -1101,7 +1103,7 @@ static const struct amdgpu_ring_funcs
>>> vcn_v1_0_dec_ring_vm_funcs = {
>>>          .emit_vm_flush = vcn_v1_0_dec_ring_emit_vm_flush,
>>>          .test_ring = amdgpu_vcn_dec_ring_test_ring,
>>>          .test_ib = amdgpu_vcn_dec_ring_test_ib,
>>> -       .insert_nop = vcn_v1_0_ring_insert_nop,
>>> +       .insert_nop = vcn_v1_0_dec_ring_insert_nop,
>>>          .insert_start = vcn_v1_0_dec_ring_insert_start,
>>>          .insert_end = vcn_v1_0_dec_ring_insert_end,
>>>          .pad_ib = amdgpu_ring_generic_pad_ib,
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
Am 17.05.2018 um 20:25 schrieb Alex Deucher:
> On Thu, May 17, 2018 at 2:24 PM, Leo Liu <leo.liu@amd.com> wrote:
>>
>> On 05/17/2018 02:18 PM, Alex Deucher wrote:
>>> On Thu, May 17, 2018 at 2:12 PM, Leo Liu <leo.liu@amd.com> wrote:
>>>> NO_OP register should be writen to 0
>>>>
>>>> Signed-off-by: Leo Liu <leo.liu@amd.com>
>>> We discussed this when we first started working on amdgpu.  Does it
>>> have to be 0?
>> Yes. I confirmed with FW engineer that we need put 0 into noop.
>>
>>>     This function is used for padding.  What happens if we
>>> need an odd number of dwords of padding required?
>> That should be never happening. should be always paired of PM4 packet for
>> RBC to process.

Good to know.

> Cool.  Series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com> as well.

>
>>
>> Regards,
>> Leo
>>
>>
>>> Alex
>>>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 14 ++++++++------
>>>>    1 file changed, 8 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>> index 0501746b..7fbbdb1 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>>>> @@ -1048,14 +1048,17 @@ static int vcn_v1_0_process_interrupt(struct
>>>> amdgpu_device *adev,
>>>>           return 0;
>>>>    }
>>>>
>>>> -static void vcn_v1_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t
>>>> count)
>>>> +static void vcn_v1_0_dec_ring_insert_nop(struct amdgpu_ring *ring,
>>>> uint32_t count)
>>>>    {
>>>> -       int i;
>>>>           struct amdgpu_device *adev = ring->adev;
>>>> +       int i;
>>>>
>>>> -       for (i = 0; i < count; i++)
>>>> -               amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0,
>>>> mmUVD_NO_OP), 0));
>>>> +       WARN_ON(ring->wptr % 2 || count % 2);
>>>>
>>>> +       for (i = 0; i < count / 2; i++) {
>>>> +               amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0,
>>>> mmUVD_NO_OP), 0));
>>>> +               amdgpu_ring_write(ring, 0);
>>>> +       }
>>>>    }
>>>>
>>>>
>>>> @@ -1082,7 +1085,6 @@ static const struct amd_ip_funcs vcn_v1_0_ip_funcs
>>>> = {
>>>>    static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>>>>           .type = AMDGPU_RING_TYPE_VCN_DEC,
>>>>           .align_mask = 0xf,
>>>> -       .nop = PACKET0(0x81ff, 0),
>>>>           .support_64bit_ptrs = false,
>>>>           .vmhub = AMDGPU_MMHUB,
>>>>           .get_rptr = vcn_v1_0_dec_ring_get_rptr,
>>>> @@ -1101,7 +1103,7 @@ static const struct amdgpu_ring_funcs
>>>> vcn_v1_0_dec_ring_vm_funcs = {
>>>>           .emit_vm_flush = vcn_v1_0_dec_ring_emit_vm_flush,
>>>>           .test_ring = amdgpu_vcn_dec_ring_test_ring,
>>>>           .test_ib = amdgpu_vcn_dec_ring_test_ib,
>>>> -       .insert_nop = vcn_v1_0_ring_insert_nop,
>>>> +       .insert_nop = vcn_v1_0_dec_ring_insert_nop,
>>>>           .insert_start = vcn_v1_0_dec_ring_insert_start,
>>>>           .insert_end = vcn_v1_0_dec_ring_insert_end,
>>>>           .pad_ib = amdgpu_ring_generic_pad_ib,
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> 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