All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: Nirmoy <nirmodas@amd.com>, Nirmoy Das <nirmoy.das@amd.com>,
	amd-gfx@lists.freedesktop.org
Cc: Alan.Harrison@amd.com, Alexander.Deucher@amd.com,
	Felix.Kuehling@amd.com, ray.huang@amd.com
Subject: Re: [RFC PATCH 2/2] drm/amdgpu: enable gfx wave limiting for high priority compute jobs
Date: Thu, 28 Jan 2021 17:14:05 +0100	[thread overview]
Message-ID: <a1d26d23-acf4-e76e-6f4d-fecc6bf4fa27@amd.com> (raw)
In-Reply-To: <b90dd6f2-919e-837b-f26f-73258ab899cb@amd.com>

Am 28.01.21 um 17:01 schrieb Nirmoy:
>
> On 1/28/21 4:25 PM, Christian König wrote:
>> Am 28.01.21 um 16:21 schrieb Nirmoy:
>>>
>>> On 1/28/21 3:49 PM, Christian König wrote:
>>>> Am 28.01.21 um 15:35 schrieb Nirmoy Das:
>>>>> Enable gfx wave limiting for gfx jobs before pushing high priority
>>>>> compute jobs so that high priority compute jobs gets more resources
>>>>> to finish early.
>>>>
>>>> The problem here is what happens if you have multiple high priority 
>>>> jobs running at the same time?
>>>
>>>
>>> AFAIU, in that case quantum duration will come into effect. The 
>>> queue arbiter will switch
>>>
>>> to next high priority active queue once quantum duration expires. 
>>> This should be similar to what
>>>
>>> we already have, multiple normal priority jobs sharing GPU resources 
>>> based on CP_HQD_QUANTUM.
>>>
>>> QUEUE_DURATION register value.
>>
>> Yeah, but when the first high priority job completes it will reset 
>> mmSPI_WCL_PIPE_PERCENT_GFX back to the default value.
>>
>> Have you considered that?
>
>
> Yes I need bit of clarity here. Isn't one frame(...pm4(wave_limit), 
> pm4(IBs), pm4(restore_wave_limit), ..)
>
> executes together as one unit? If that is the case then the next high 
> prio compute job will set the wave limit
>
> again and will be applied for its dispatch call.

Yeah, that is correct. But the problem is somewhere else.

>
>
> I guess that is not the case because you asked this question. Do you 
> think we should have only one high priority
>
> queue then?

Yes exactly that. IIRC we currently have 4 low priority and 4 high 
priority queues.

The problem is those 4 high priority queues. If we only use 1 then we 
won't run into this as far as I can see.

Regards,
Christian.

>
>
> I tried to test it by running two instances of same vulkan test 
> application. I can't trace
>
> two applications together using RGP. From the trace of one 
> application(along with other running together),
>
> I didn't see any throttling down of high priority compute job(yellow 
> bars).
>
>
> Let me know what do you think. I will work with Alan to change the 
> test application so that we can verify this
>
> using multiple high priority context.
>
>
> Regards,
>
> Nirmoy
>
>>
>> Thanks,
>> Christian.
>>
>>>
>>>
>>> Regards,
>>>
>>> Nirmoy
>>>
>>>
>>>>
>>>> Christian
>>>>
>>>>>
>>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 9 +++++++++
>>>>>   1 file changed, 9 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>>>> index 024d0a563a65..ee48989dfb4c 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>>>> @@ -195,6 +195,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring 
>>>>> *ring, unsigned num_ibs,
>>>>>       if ((ib->flags & AMDGPU_IB_FLAG_EMIT_MEM_SYNC) && 
>>>>> ring->funcs->emit_mem_sync)
>>>>>           ring->funcs->emit_mem_sync(ring);
>>>>>   +    if (ring->funcs->emit_wave_limit && job &&
>>>>> +        job->base.s_priority >= DRM_SCHED_PRIORITY_HIGH)
>>>>> +        ring->funcs->emit_wave_limit(ring, true);
>>>>> +
>>>>>       if (ring->funcs->insert_start)
>>>>>           ring->funcs->insert_start(ring);
>>>>>   @@ -295,6 +299,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring 
>>>>> *ring, unsigned num_ibs,
>>>>>       ring->current_ctx = fence_ctx;
>>>>>       if (vm && ring->funcs->emit_switch_buffer)
>>>>>           amdgpu_ring_emit_switch_buffer(ring);
>>>>> +
>>>>> +    if (ring->funcs->emit_wave_limit && job &&
>>>>> +        job->base.s_priority >= DRM_SCHED_PRIORITY_HIGH)
>>>>> +        ring->funcs->emit_wave_limit(ring, false);
>>>>> +
>>>>>       amdgpu_ring_commit(ring);
>>>>>       return 0;
>>>>>   }
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cnirmoy.das%40amd.com%7C67e903357ee247f9ceb008d8c3a0efdf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637474443287007930%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=EyouSUvaKjQIIWeKDilVra73iL1eb0rpnaUCDAIDvXA%3D&amp;reserved=0 
>>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cnirmoy.das%40amd.com%7C67e903357ee247f9ceb008d8c3a0efdf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637474443287007930%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=EyouSUvaKjQIIWeKDilVra73iL1eb0rpnaUCDAIDvXA%3D&amp;reserved=0 
>>>
>>

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

  reply	other threads:[~2021-01-28 16:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-28 14:35 [RFC PATCH v3 1/2] drm/amdgpu: add wave limit functionality for gfx8, 9 Nirmoy Das
2021-01-28 14:35 ` [RFC PATCH 2/2] drm/amdgpu: enable gfx wave limiting for high priority compute jobs Nirmoy Das
2021-01-28 14:49   ` Christian König
2021-01-28 15:21     ` Nirmoy
2021-01-28 15:25       ` Christian König
2021-01-28 16:01         ` Nirmoy
2021-01-28 16:14           ` Christian König [this message]
2021-01-28 16:54             ` Nirmoy
  -- strict thread matches above, loose matches on Subject: below --
2021-01-28 13:41 [RFC PATCH v2 1/2] drm/amdgpu: add wave limit functionality for gfx8, 9 Nirmoy Das
2021-01-28 13:41 ` [RFC PATCH 2/2] drm/amdgpu: enable gfx wave limiting for high priority compute jobs Nirmoy Das
2021-01-27 14:56 [RFC PATCH 1/2] drm/amdgpu: add wave limit functionality for gfx8,9 Nirmoy Das
2021-01-27 14:56 ` [RFC PATCH 2/2] drm/amdgpu: enable gfx wave limiting for high priority compute jobs Nirmoy Das

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a1d26d23-acf4-e76e-6f4d-fecc6bf4fa27@amd.com \
    --to=christian.koenig@amd.com \
    --cc=Alan.Harrison@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=nirmodas@amd.com \
    --cc=nirmoy.das@amd.com \
    --cc=ray.huang@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.