RE: [PATCH] drm/amdkfd: map sdma queues onto extended engines for navi2x

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[AMD Official Use Only]

> -----Original Message-----
> From: Kuehling, Felix <Felix.Kuehling@xxxxxxx>
> Sent: February 9, 2022 4:26 PM
> To: Kim, Jonathan <Jonathan.Kim@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] drm/amdkfd: map sdma queues onto extended engines for
> navi2x
>
>
> On 2022-02-09 11:11, Jonathan Kim wrote:
> > The hardware scheduler requires that all SDMA 5.2.x queues are put on
> > the RUN_LIST through the extended engines.
> >
> > Make extended engine unmap available as well.
> >
> > Signed-off-by: Jonathan Kim <jonathan.kim@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
> >   drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c       | 5 +++--
> >   drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c    | 8 +++++---
> >   drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c    | 3 ++-
> >   drivers/gpu/drm/amd/amdkfd/kfd_priv.h                 | 5 +++--
> >   5 files changed, 14 insertions(+), 9 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 7f6f1a842b0b..f12e32335eb3 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > @@ -1555,7 +1555,7 @@ static int unmap_queues_cpsch(struct
> device_queue_manager *dqm,
> >             return retval;
> >
> >     retval = pm_send_unmap_queue(&dqm->packet_mgr,
> KFD_QUEUE_TYPE_COMPUTE,
> > -                   filter, filter_param, reset, 0);
> > +                   filter, filter_param, reset, 0, false);
>
> Does this still work correctly? We currently rely on HWS unmapping SDMA
> queues when we request unmapping of compute queues. Is that still the case
> with extended queue selection in map_queues?

I wasn't aware of the implicit sdma unmap ...
That makes much more sense.

I followed up on the FW spec and apparently as long as extended_engine_select=0x1 (sdma0_sdma7),
a single call to unmap all queues or all dynamic queues will unmap both compute
queues mapped in legacy mode and sdma queues mapped in extended engine mode.

>
> How would the caller know to set this to "true"? For mapping, this detail is
> hidden in the packet-manager implementation. But for unmapping the caller
> needs to know? That doesn't make sense. But we could probably remove the
> SDMA filtering functionality from pm_send_unmap_queue completely. I don't
> see any calls where we try to unmap specific SDMA queues. Since we always
> have to replace the entire runlist anyway, there is not use case for it.

Agreed.
Aside from removing SDMA checks, maybe also pass the device itself through to pm_send_unmap_queue then?
Or could it be the SDMA ip version?
That way we can hide the check to toggle between extended_engine_select = 0x0 or 0x1 from the caller.

Thanks,

Jon

>
> Regards,
>    Felix
>
>
> >     if (retval)
> >             return retval;
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> > index 1439420925a0..8694cfcd57d1 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> > @@ -371,7 +371,8 @@ int pm_send_query_status(struct packet_manager
> *pm, uint64_t fence_address,
> >   int pm_send_unmap_queue(struct packet_manager *pm, enum
> kfd_queue_type type,
> >                     enum kfd_unmap_queues_filter filter,
> >                     uint32_t filter_param, bool reset,
> > -                   unsigned int sdma_engine)
> > +                   unsigned int sdma_engine,
> > +                   bool is_sdma_ext)
> >   {
> >     uint32_t *buffer, size;
> >     int retval = 0;
> > @@ -387,7 +388,7 @@ int pm_send_unmap_queue(struct packet_manager
> *pm, enum kfd_queue_type type,
> >     }
> >
> >     retval = pm->pmf->unmap_queues(pm, buffer, type, filter, filter_param,
> > -                                  reset, sdma_engine);
> > +                                  reset, sdma_engine, is_sdma_ext);
> >     if (!retval)
> >             kq_submit_packet(pm->priv_queue);
> >     else
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > index 7ea3f671b325..08f736080b7e 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > @@ -183,6 +183,7 @@ static int pm_map_queues_v9(struct packet_manager
> *pm, uint32_t *buffer,
> >   {
> >     struct pm4_mes_map_queues *packet;
> >     bool use_static = is_static;
> > +   bool is_sdma_ext = q->device->adev->ip_versions[SDMA0_HWIP][0] >=
> > +IP_VERSION(5, 2, 0);
> >
> >     packet = (struct pm4_mes_map_queues *)buffer;
> >     memset(buffer, 0, sizeof(struct pm4_mes_map_queues)); @@ -214,7
> > +215,7 @@ static int pm_map_queues_v9(struct packet_manager *pm,
> uint32_t *buffer,
> >     case KFD_QUEUE_TYPE_SDMA:
> >     case KFD_QUEUE_TYPE_SDMA_XGMI:
> >             use_static = false; /* no static queues under SDMA */
> > -           if (q->properties.sdma_engine_id < 2)
> > +           if (q->properties.sdma_engine_id < 2 && !is_sdma_ext)
> >                     packet->bitfields2.engine_sel = q-
> >properties.sdma_engine_id +
> >                             engine_sel__mes_map_queues__sdma0_vi;
> >             else {
> > @@ -249,7 +250,8 @@ static int pm_unmap_queues_v9(struct
> packet_manager *pm, uint32_t *buffer,
> >                     enum kfd_queue_type type,
> >                     enum kfd_unmap_queues_filter filter,
> >                     uint32_t filter_param, bool reset,
> > -                   unsigned int sdma_engine)
> > +                   unsigned int sdma_engine,
> > +                   bool is_sdma_ext)
> >   {
> >     struct pm4_mes_unmap_queues *packet;
> >
> > @@ -268,7 +270,7 @@ static int pm_unmap_queues_v9(struct
> packet_manager *pm, uint32_t *buffer,
> >             break;
> >     case KFD_QUEUE_TYPE_SDMA:
> >     case KFD_QUEUE_TYPE_SDMA_XGMI:
> > -           if (sdma_engine < 2) {
> > +           if (sdma_engine < 2 && !is_sdma_ext) {
> >                     packet->bitfields2.extended_engine_sel =
> >
>       extended_engine_sel__mes_unmap_queues__legacy_engine_sel;
> >                     packet->bitfields2.engine_sel =
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
> > index 3c0658e32e93..a83aa94972e7 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
> > @@ -200,7 +200,8 @@ static int pm_unmap_queues_vi(struct
> packet_manager *pm, uint32_t *buffer,
> >                     enum kfd_queue_type type,
> >                     enum kfd_unmap_queues_filter filter,
> >                     uint32_t filter_param, bool reset,
> > -                   unsigned int sdma_engine)
> > +                   unsigned int sdma_engine,
> > +                   bool is_sdma_ext)
> >   {
> >     struct pm4_mes_unmap_queues *packet;
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > index b6790a637f5c..b157ba0216f0 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > @@ -1249,7 +1249,8 @@ struct packet_manager_funcs {
> >                     enum kfd_queue_type type,
> >                     enum kfd_unmap_queues_filter mode,
> >                     uint32_t filter_param, bool reset,
> > -                   unsigned int sdma_engine);
> > +                   unsigned int sdma_engine,
> > +                   bool is_sdma_ext);
> >     int (*query_status)(struct packet_manager *pm, uint32_t *buffer,
> >                     uint64_t fence_address, uint64_t fence_value);
> >     int (*release_mem)(uint64_t gpu_addr, uint32_t *buffer); @@ -1279,7
> > +1280,7 @@ int pm_send_query_status(struct packet_manager *pm, uint64_t
> fence_address,
> >   int pm_send_unmap_queue(struct packet_manager *pm, enum
> kfd_queue_type type,
> >                     enum kfd_unmap_queues_filter mode,
> >                     uint32_t filter_param, bool reset,
> > -                   unsigned int sdma_engine);
> > +                   unsigned int sdma_engine, bool is_sdma_ext);
> >
> >   void pm_release_ib(struct packet_manager *pm);
> >




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux