* [PATCH] drm/amdkfd: fix add queue process context clear for hsa non-init cases
@ 2023-09-12 12:17 Jonathan Kim
2023-09-12 12:19 ` Kim, Jonathan
2023-09-13 0:35 ` Felix Kuehling
0 siblings, 2 replies; 6+ messages in thread
From: Jonathan Kim @ 2023-09-12 12:17 UTC (permalink / raw)
To: amd-gfx; +Cc: Ruili.Ji, Felix.Kuehling, Jonathan Kim, JinHuiEricHuang,
Shikai Guo
There are cases where HSA is not initialized when adding queues and
the ADD_QUEUE API should clear the MES process context instead of
SET_SHADER_DEBUGGER.
The only time ADD_QUEUE.skip_process_ctx_clear is required is for
debugger use cases and a debugged process is always runtime enabled
when adding a queue.
Tested-by: Shikai Guo <shikai.guo@amd.com>
Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 6d07a5dd2648..77159b03a422 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -227,8 +227,10 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
queue_input.tba_addr = qpd->tba_addr;
queue_input.tma_addr = qpd->tma_addr;
queue_input.trap_en = !kfd_dbg_has_cwsr_workaround(q->device);
- queue_input.skip_process_ctx_clear = qpd->pqm->process->debug_trap_enabled ||
- kfd_dbg_has_ttmps_always_setup(q->device);
+ queue_input.skip_process_ctx_clear =
+ qpd->pqm->process->runtime_info.runtime_state == DEBUG_RUNTIME_STATE_ENABLED &&
+ (qpd->pqm->process->debug_trap_enabled ||
+ kfd_dbg_has_ttmps_always_setup(q->device));
queue_type = convert_to_mes_queue_type(q->properties.type);
if (queue_type < 0) {
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdkfd: fix add queue process context clear for hsa non-init cases
2023-09-12 12:17 [PATCH] drm/amdkfd: fix add queue process context clear for hsa non-init cases Jonathan Kim
@ 2023-09-12 12:19 ` Kim, Jonathan
2023-09-13 0:35 ` Felix Kuehling
1 sibling, 0 replies; 6+ messages in thread
From: Kim, Jonathan @ 2023-09-12 12:19 UTC (permalink / raw)
To: amd-gfx@lists.freedesktop.org
Cc: Huang, JinHuiEric, Ji, Ruili, Kuehling, Felix, Guo, Shikai
[Public]
+ Eric
> -----Original Message-----
> From: Kim, Jonathan <Jonathan.Kim@amd.com>
> Sent: Tuesday, September 12, 2023 8:17 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Ji, Ruili <Ruili.Ji@amd.com>; Guo, Shikai <Shikai.Guo@amd.com>;
> JinHuiEricHuang@amd.com; Kuehling, Felix <Felix.Kuehling@amd.com>; Kim,
> Jonathan <Jonathan.Kim@amd.com>; Guo, Shikai <Shikai.Guo@amd.com>;
> Kim, Jonathan <Jonathan.Kim@amd.com>
> Subject: [PATCH] drm/amdkfd: fix add queue process context clear for hsa
> non-init cases
>
> There are cases where HSA is not initialized when adding queues and
> the ADD_QUEUE API should clear the MES process context instead of
> SET_SHADER_DEBUGGER.
>
> The only time ADD_QUEUE.skip_process_ctx_clear is required is for
> debugger use cases and a debugged process is always runtime enabled
> when adding a queue.
>
> Tested-by: Shikai Guo <shikai.guo@amd.com>
> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 6d07a5dd2648..77159b03a422 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -227,8 +227,10 @@ static int add_queue_mes(struct
> device_queue_manager *dqm, struct queue *q,
> queue_input.tba_addr = qpd->tba_addr;
> queue_input.tma_addr = qpd->tma_addr;
> queue_input.trap_en = !kfd_dbg_has_cwsr_workaround(q->device);
> - queue_input.skip_process_ctx_clear = qpd->pqm->process-
> >debug_trap_enabled ||
> -
> kfd_dbg_has_ttmps_always_setup(q->device);
> + queue_input.skip_process_ctx_clear =
> + qpd->pqm->process->runtime_info.runtime_state ==
> DEBUG_RUNTIME_STATE_ENABLED &&
> + (qpd->pqm->process-
> >debug_trap_enabled ||
> +
> kfd_dbg_has_ttmps_always_setup(q->device));
>
> queue_type = convert_to_mes_queue_type(q->properties.type);
> if (queue_type < 0) {
> --
> 2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdkfd: fix add queue process context clear for hsa non-init cases
2023-09-12 12:17 [PATCH] drm/amdkfd: fix add queue process context clear for hsa non-init cases Jonathan Kim
2023-09-12 12:19 ` Kim, Jonathan
@ 2023-09-13 0:35 ` Felix Kuehling
2023-09-13 0:53 ` Kim, Jonathan
1 sibling, 1 reply; 6+ messages in thread
From: Felix Kuehling @ 2023-09-13 0:35 UTC (permalink / raw)
To: Jonathan Kim, amd-gfx; +Cc: Ruili.Ji, JinHuiEricHuang, Shikai.Guo
On 2023-09-12 8:17, Jonathan Kim wrote:
> There are cases where HSA is not initialized when adding queues
This statement doesn't make sense to me. If HSA is not initialized, it
means user mode hasn't opened the KFD device. So it can't create queues.
What do you really mean here?
Regards,
Felix
> and
> the ADD_QUEUE API should clear the MES process context instead of
> SET_SHADER_DEBUGGER.
>
> The only time ADD_QUEUE.skip_process_ctx_clear is required is for
> debugger use cases and a debugged process is always runtime enabled
> when adding a queue.
>
> Tested-by: Shikai Guo <shikai.guo@amd.com>
> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 6d07a5dd2648..77159b03a422 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -227,8 +227,10 @@ static int add_queue_mes(struct device_queue_manager *dqm, struct queue *q,
> queue_input.tba_addr = qpd->tba_addr;
> queue_input.tma_addr = qpd->tma_addr;
> queue_input.trap_en = !kfd_dbg_has_cwsr_workaround(q->device);
> - queue_input.skip_process_ctx_clear = qpd->pqm->process->debug_trap_enabled ||
> - kfd_dbg_has_ttmps_always_setup(q->device);
> + queue_input.skip_process_ctx_clear =
> + qpd->pqm->process->runtime_info.runtime_state == DEBUG_RUNTIME_STATE_ENABLED &&
> + (qpd->pqm->process->debug_trap_enabled ||
> + kfd_dbg_has_ttmps_always_setup(q->device));
>
> queue_type = convert_to_mes_queue_type(q->properties.type);
> if (queue_type < 0) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdkfd: fix add queue process context clear for hsa non-init cases
2023-09-13 0:35 ` Felix Kuehling
@ 2023-09-13 0:53 ` Kim, Jonathan
2023-09-13 1:01 ` Felix Kuehling
0 siblings, 1 reply; 6+ messages in thread
From: Kim, Jonathan @ 2023-09-13 0:53 UTC (permalink / raw)
To: Kuehling, Felix, amd-gfx@lists.freedesktop.org
Cc: Ji, Ruili, JinHuiEricHuang@amd.com, Guo, Shikai
[Public]
> -----Original Message-----
> From: Kuehling, Felix <Felix.Kuehling@amd.com>
> Sent: Tuesday, September 12, 2023 8:36 PM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Ji, Ruili <Ruili.Ji@amd.com>; Guo, Shikai <Shikai.Guo@amd.com>;
> JinHuiEricHuang@amd.com
> Subject: Re: [PATCH] drm/amdkfd: fix add queue process context clear for hsa
> non-init cases
>
> On 2023-09-12 8:17, Jonathan Kim wrote:
> > There are cases where HSA is not initialized when adding queues
>
> This statement doesn't make sense to me. If HSA is not initialized, it
> means user mode hasn't opened the KFD device. So it can't create queues.
> What do you really mean here?
I meant the call to runtime enable e.g. KFD test can add a queue without runtime enable call.
Thanks,
Jon
>
> Regards,
> Felix
>
>
> > and
> > the ADD_QUEUE API should clear the MES process context instead of
> > SET_SHADER_DEBUGGER.
> >
> > The only time ADD_QUEUE.skip_process_ctx_clear is required is for
> > debugger use cases and a debugged process is always runtime enabled
> > when adding a queue.
> >
> > Tested-by: Shikai Guo <shikai.guo@amd.com>
> > Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > index 6d07a5dd2648..77159b03a422 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > @@ -227,8 +227,10 @@ static int add_queue_mes(struct
> device_queue_manager *dqm, struct queue *q,
> > queue_input.tba_addr = qpd->tba_addr;
> > queue_input.tma_addr = qpd->tma_addr;
> > queue_input.trap_en = !kfd_dbg_has_cwsr_workaround(q->device);
> > - queue_input.skip_process_ctx_clear = qpd->pqm->process-
> >debug_trap_enabled ||
> > -
> kfd_dbg_has_ttmps_always_setup(q->device);
> > + queue_input.skip_process_ctx_clear =
> > + qpd->pqm->process->runtime_info.runtime_state ==
> DEBUG_RUNTIME_STATE_ENABLED &&
> > + (qpd->pqm->process-
> >debug_trap_enabled ||
> > +
> kfd_dbg_has_ttmps_always_setup(q->device));
> >
> > queue_type = convert_to_mes_queue_type(q->properties.type);
> > if (queue_type < 0) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdkfd: fix add queue process context clear for hsa non-init cases
2023-09-13 0:53 ` Kim, Jonathan
@ 2023-09-13 1:01 ` Felix Kuehling
2023-09-13 1:40 ` Kim, Jonathan
0 siblings, 1 reply; 6+ messages in thread
From: Felix Kuehling @ 2023-09-13 1:01 UTC (permalink / raw)
To: Kim, Jonathan, amd-gfx@lists.freedesktop.org
Cc: Eric Huang, Ji, Ruili, Guo, Shikai
On 2023-09-12 20:53, Kim, Jonathan wrote:
> [Public]
>
>> -----Original Message-----
>> From: Kuehling, Felix <Felix.Kuehling@amd.com>
>> Sent: Tuesday, September 12, 2023 8:36 PM
>> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Ji, Ruili <Ruili.Ji@amd.com>; Guo, Shikai <Shikai.Guo@amd.com>;
>> JinHuiEricHuang@amd.com
>> Subject: Re: [PATCH] drm/amdkfd: fix add queue process context clear for hsa
>> non-init cases
>>
>> On 2023-09-12 8:17, Jonathan Kim wrote:
>>> There are cases where HSA is not initialized when adding queues
>> This statement doesn't make sense to me. If HSA is not initialized, it
>> means user mode hasn't opened the KFD device. So it can't create queues.
>> What do you really mean here?
> I meant the call to runtime enable e.g. KFD test can add a queue without runtime enable call.
OK, this can also happen when you run an older version of the HSA
runtime that doesn't support the ROCm debugger yet. Please update the
patch description accordingly.
Thanks,
Felix
>
> Thanks,
>
> Jon
>
>> Regards,
>> Felix
>>
>>
>>> and
>>> the ADD_QUEUE API should clear the MES process context instead of
>>> SET_SHADER_DEBUGGER.
>>>
>>> The only time ADD_QUEUE.skip_process_ctx_clear is required is for
>>> debugger use cases and a debugged process is always runtime enabled
>>> when adding a queue.
>>>
>>> Tested-by: Shikai Guo <shikai.guo@amd.com>
>>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> index 6d07a5dd2648..77159b03a422 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> @@ -227,8 +227,10 @@ static int add_queue_mes(struct
>> device_queue_manager *dqm, struct queue *q,
>>> queue_input.tba_addr = qpd->tba_addr;
>>> queue_input.tma_addr = qpd->tma_addr;
>>> queue_input.trap_en = !kfd_dbg_has_cwsr_workaround(q->device);
>>> - queue_input.skip_process_ctx_clear = qpd->pqm->process-
>>> debug_trap_enabled ||
>>> -
>> kfd_dbg_has_ttmps_always_setup(q->device);
>>> + queue_input.skip_process_ctx_clear =
>>> + qpd->pqm->process->runtime_info.runtime_state ==
>> DEBUG_RUNTIME_STATE_ENABLED &&
>>> + (qpd->pqm->process-
>>> debug_trap_enabled ||
>>> +
>> kfd_dbg_has_ttmps_always_setup(q->device));
>>> queue_type = convert_to_mes_queue_type(q->properties.type);
>>> if (queue_type < 0) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdkfd: fix add queue process context clear for hsa non-init cases
2023-09-13 1:01 ` Felix Kuehling
@ 2023-09-13 1:40 ` Kim, Jonathan
0 siblings, 0 replies; 6+ messages in thread
From: Kim, Jonathan @ 2023-09-13 1:40 UTC (permalink / raw)
To: Kuehling, Felix, amd-gfx@lists.freedesktop.org
Cc: Huang, JinHuiEric, Ji, Ruili, Guo, Shikai
[Public]
> -----Original Message-----
> From: Kuehling, Felix <Felix.Kuehling@amd.com>
> Sent: Tuesday, September 12, 2023 9:01 PM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Ji, Ruili <Ruili.Ji@amd.com>; Guo, Shikai <Shikai.Guo@amd.com>; Huang,
> JinHuiEric <JinHuiEric.Huang@amd.com>
> Subject: Re: [PATCH] drm/amdkfd: fix add queue process context clear for hsa
> non-init cases
>
> On 2023-09-12 20:53, Kim, Jonathan wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Kuehling, Felix <Felix.Kuehling@amd.com>
> >> Sent: Tuesday, September 12, 2023 8:36 PM
> >> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-
> gfx@lists.freedesktop.org
> >> Cc: Ji, Ruili <Ruili.Ji@amd.com>; Guo, Shikai <Shikai.Guo@amd.com>;
> >> JinHuiEricHuang@amd.com
> >> Subject: Re: [PATCH] drm/amdkfd: fix add queue process context clear for
> hsa
> >> non-init cases
> >>
> >> On 2023-09-12 8:17, Jonathan Kim wrote:
> >>> There are cases where HSA is not initialized when adding queues
> >> This statement doesn't make sense to me. If HSA is not initialized, it
> >> means user mode hasn't opened the KFD device. So it can't create queues.
> >> What do you really mean here?
> > I meant the call to runtime enable e.g. KFD test can add a queue without
> runtime enable call.
>
> OK, this can also happen when you run an older version of the HSA
> runtime that doesn't support the ROCm debugger yet. Please update the
> patch description accordingly.
Ok will do thanks. Sorry for the confusing description.
For some reason I forgot that HSA was umbrella term and doesn't only refer to the ROCr component.
Thanks,
Jon
>
> Thanks,
> Felix
>
>
> >
> > Thanks,
> >
> > Jon
> >
> >> Regards,
> >> Felix
> >>
> >>
> >>> and
> >>> the ADD_QUEUE API should clear the MES process context instead of
> >>> SET_SHADER_DEBUGGER.
> >>>
> >>> The only time ADD_QUEUE.skip_process_ctx_clear is required is for
> >>> debugger use cases and a debugged process is always runtime enabled
> >>> when adding a queue.
> >>>
> >>> Tested-by: Shikai Guo <shikai.guo@amd.com>
> >>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
> >>> ---
> >>> drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 ++++--
> >>> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >>> index 6d07a5dd2648..77159b03a422 100644
> >>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> >>> @@ -227,8 +227,10 @@ static int add_queue_mes(struct
> >> device_queue_manager *dqm, struct queue *q,
> >>> queue_input.tba_addr = qpd->tba_addr;
> >>> queue_input.tma_addr = qpd->tma_addr;
> >>> queue_input.trap_en = !kfd_dbg_has_cwsr_workaround(q->device);
> >>> - queue_input.skip_process_ctx_clear = qpd->pqm->process-
> >>> debug_trap_enabled ||
> >>> -
> >> kfd_dbg_has_ttmps_always_setup(q->device);
> >>> + queue_input.skip_process_ctx_clear =
> >>> + qpd->pqm->process->runtime_info.runtime_state ==
> >> DEBUG_RUNTIME_STATE_ENABLED &&
> >>> + (qpd->pqm->process-
> >>> debug_trap_enabled ||
> >>> +
> >> kfd_dbg_has_ttmps_always_setup(q->device));
> >>> queue_type = convert_to_mes_queue_type(q->properties.type);
> >>> if (queue_type < 0) {
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-13 1:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-12 12:17 [PATCH] drm/amdkfd: fix add queue process context clear for hsa non-init cases Jonathan Kim
2023-09-12 12:19 ` Kim, Jonathan
2023-09-13 0:35 ` Felix Kuehling
2023-09-13 0:53 ` Kim, Jonathan
2023-09-13 1:01 ` Felix Kuehling
2023-09-13 1:40 ` Kim, Jonathan
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.