[PATCH] drm/amdgpu: skip IB tests for KIQ in general

[PATCH] drm/amdgpu: skip IB tests for KIQ in general

[Shirish S]

From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>

[Why]
1. We never submit IBs to KIQ.
2. Ring test pass without KIQ's ring also.
3. By skipping we see an improvement of around 500ms
   in the amdgpu's resume time.

[How]
skip IB tests for KIQ ring type.

Signed-off-by: Shirish S <shirish.s@amd.com>
Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
---

This patch is a follow-up to the suggestion given by Alex,
while reviewing the patch: https://patchwork.freedesktop.org/patch/250912/

-Shirish S

 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 47817e0..b8963b7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -354,6 +354,14 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
 		if (!ring || !ring->ready)
 			continue;
 
+		/* skip IB tests for KIQ in general for the below reasons:
+		 * 1. We never submit IBs to the KIQ
+		 * 2. KIQ doesn't use the EOP interrupts,
+		 *    we use some other CP interrupt.
+		 */
+		if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
+			continue;
+
 		/* MM engine need more time */
 		if (ring->funcs->type == AMDGPU_RING_TYPE_UVD ||
 			ring->funcs->type == AMDGPU_RING_TYPE_VCE ||
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: skip IB tests for KIQ in general

[Alex Deucher]

On Wed, Oct 3, 2018 at 11:15 AM Shirish S <shirish.s@amd.com> wrote:
>
> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
>
> [Why]
> 1. We never submit IBs to KIQ.
> 2. Ring test pass without KIQ's ring also.
> 3. By skipping we see an improvement of around 500ms
>    in the amdgpu's resume time.
>
> [How]
> skip IB tests for KIQ ring type.
>
> Signed-off-by: Shirish S <shirish.s@amd.com>
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>

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


> ---
>
> This patch is a follow-up to the suggestion given by Alex,
> while reviewing the patch: https://patchwork.freedesktop.org/patch/250912/
>
> -Shirish S
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 47817e0..b8963b7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -354,6 +354,14 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
>                 if (!ring || !ring->ready)
>                         continue;
>
> +               /* skip IB tests for KIQ in general for the below reasons:
> +                * 1. We never submit IBs to the KIQ
> +                * 2. KIQ doesn't use the EOP interrupts,
> +                *    we use some other CP interrupt.
> +                */
> +               if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
> +                       continue;
> +
>                 /* MM engine need more time */
>                 if (ring->funcs->type == AMDGPU_RING_TYPE_UVD ||
>                         ring->funcs->type == AMDGPU_RING_TYPE_VCE ||
> --
> 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

Re: [PATCH] drm/amdgpu: skip IB tests for KIQ in general

[Christian König]

Am 03.10.2018 um 17:15 schrieb Shirish S:
> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
>
> [Why]
> 1. We never submit IBs to KIQ.
> 2. Ring test pass without KIQ's ring also.
> 3. By skipping we see an improvement of around 500ms
>     in the amdgpu's resume time.
>
> [How]
> skip IB tests for KIQ ring type.
>
> Signed-off-by: Shirish S <shirish.s@amd.com>
> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>

Well I'm not sure if that is a good idea or not.

On the one hand it is true that we never submit IBs to the KIQ, so 
testing that doesn't make much sense actually.

But on the other hand the 500ms delay during resume points out a problem 
with the KIQ, e.g. interrupts are not working correctly!

Question is now if we should ignore that problem because we never use 
interrupts on the KIQ?

If the answer is to keep it as it is we should remove the intterupt 
handling for the KIQ as well.

Otherwise I would say we should fix interrupts on the KIQ and then we 
also don't need this change any more.

Regards,
Christian.

> ---
>
> This patch is a follow-up to the suggestion given by Alex,
> while reviewing the patch: https://patchwork.freedesktop.org/patch/250912/
>
> -Shirish S
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 47817e0..b8963b7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -354,6 +354,14 @@ int amdgpu_ib_ring_tests(struct amdgpu_device *adev)
>   		if (!ring || !ring->ready)
>   			continue;
>   
> +		/* skip IB tests for KIQ in general for the below reasons:
> +		 * 1. We never submit IBs to the KIQ
> +		 * 2. KIQ doesn't use the EOP interrupts,
> +		 *    we use some other CP interrupt.
> +		 */
> +		if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
> +			continue;
> +
>   		/* MM engine need more time */
>   		if (ring->funcs->type == AMDGPU_RING_TYPE_UVD ||
>   			ring->funcs->type == AMDGPU_RING_TYPE_VCE ||

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

Re: [PATCH] drm/amdgpu: skip IB tests for KIQ in general

[S, Shirish]

On 10/4/2018 12:41 PM, Christian König wrote:
> Am 03.10.2018 um 17:15 schrieb Shirish S:
>> From: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
>>
>> [Why]
>> 1. We never submit IBs to KIQ.
>> 2. Ring test pass without KIQ's ring also.
>> 3. By skipping we see an improvement of around 500ms
>>     in the amdgpu's resume time.
>>
>> [How]
>> skip IB tests for KIQ ring type.
>>
>> Signed-off-by: Shirish S <shirish.s@amd.com>
>> Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma@amd.com>
>
> Well I'm not sure if that is a good idea or not.
>
> On the one hand it is true that we never submit IBs to the KIQ, so 
> testing that doesn't make much sense actually.
>
> But on the other hand the 500ms delay during resume points out a 
> problem with the KIQ, e.g. interrupts are not working correctly!
>
> Question is now if we should ignore that problem because we never use 
> interrupts on the KIQ?
>
Yes Christian,  that's the approach as no point fixing something we 
never use.
> If the answer is to keep it as it is we should remove the intterupt 
> handling for the KIQ as well.
>
I have sent a patch that shall remove interrupt handling for KIQ, please 
review.

Regards,
Shirish S
> Otherwise I would say we should fix interrupts on the KIQ and then we 
> also don't need this change any more.
>
> Regards,
> Christian.
>
>> ---
>>
>> This patch is a follow-up to the suggestion given by Alex,
>> while reviewing the patch: 
>> https://patchwork.freedesktop.org/patch/250912/
>>
>> -Shirish S
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 47817e0..b8963b7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -354,6 +354,14 @@ int amdgpu_ib_ring_tests(struct amdgpu_device 
>> *adev)
>>           if (!ring || !ring->ready)
>>               continue;
>>   +        /* skip IB tests for KIQ in general for the below reasons:
>> +         * 1. We never submit IBs to the KIQ
>> +         * 2. KIQ doesn't use the EOP interrupts,
>> +         *    we use some other CP interrupt.
>> +         */
>> +        if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
>> +            continue;
>> +
>>           /* MM engine need more time */
>>           if (ring->funcs->type == AMDGPU_RING_TYPE_UVD ||
>>               ring->funcs->type == AMDGPU_RING_TYPE_VCE ||
>

-- 
Regards,
Shirish S

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