Re: [PATCH] drm/amdkfd: enable grace period for xcp instance

[Eric Huang <jinhuieric.huang@xxxxxxx>]

On 2023-07-11 14:38, Felix Kuehling wrote:
> On 2023-07-11 10:28, Eric Huang wrote:
> > Read/write grace period from/to first xcc instance of
> > xcp in kfd node.
> >
> > Signed-off-by: Eric Huang <jinhuieric.huang@xxxxxxx>
> > ---
> >   .../drm/amd/amdkfd/kfd_device_queue_manager.c | 21 ++++++++++++-------
> >   .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
> >   .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  8 ++++---
> >   3 files changed, 20 insertions(+), 11 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 31cac1fd0d58..9000c4b778fd 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > @@ -1619,10 +1619,14 @@ static int initialize_cpsch(struct 
> > device_queue_manager *dqm)
> >         init_sdma_bitmaps(dqm);
> >   -    if (dqm->dev->kfd2kgd->get_iq_wait_times)
> > +    if (dqm->dev->kfd2kgd->get_iq_wait_times) {
> > +        u32 first_inst = dqm->dev->xcp->id *
> > +                 dqm->dev->adev->gfx.num_xcc_per_xcp;
> > dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
> > -                    &dqm->wait_times,
> > -                    ffs(dqm->dev->xcc_mask) - 1);
> > +                    &dqm->wait_times[first_inst],
> > +                    first_inst);
> > +    }
> > +
> >       return 0;
> >   }
> >   @@ -1675,13 +1679,16 @@ static int start_cpsch(struct 
> > device_queue_manager *dqm)
> >                           grace_period);
> >           if (retval)
> >               pr_err("Setting grace timeout failed\n");
> > -        else if (dqm->dev->kfd2kgd->build_grace_period_packet_info)
> > +        else if (dqm->dev->kfd2kgd->build_grace_period_packet_info) {
> > +            u32 first_inst = dqm->dev->xcp->id *
> > + dqm->dev->adev->gfx.num_xcc_per_xcp;
> >               /* Update dqm->wait_times maintained in software */
> > dqm->dev->kfd2kgd->build_grace_period_packet_info(
> > -                    dqm->dev->adev, dqm->wait_times,
> > +                    dqm->dev->adev, dqm->wait_times[first_inst],
> >                       grace_period, &reg_offset,
> > -                    &dqm->wait_times,
> > -                    ffs(dqm->dev->xcc_mask) - 1);
> > +                    &dqm->wait_times[first_inst],
> > +                    first_inst);
> > +        }
> >       }
> >         dqm_unlock(dqm);
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h 
> > b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> > index 7dd4b177219d..45959c33b944 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> > @@ -262,7 +262,7 @@ struct device_queue_manager {
> >       /* used for GFX 9.4.3 only */
> >       uint32_t        current_logical_xcc_start;
> >   -    uint32_t        wait_times;
> > +    uint32_t        wait_times[MAX_XCP];
>
> Why do you need an array here, if it only saves the wait times in one 
> of the array entries [first_inst]?
That is my misunderstanding for XCP. Each DPM should be associated to 1 
XCP. I thought DPM has multiple XCPs.

Thanks,
Eric
> Regards,
>   Felix
>
>
> >         wait_queue_head_t    destroy_wait;
> >   };
> > 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 8fda16e6fee6..960404a6379b 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> > @@ -292,17 +292,19 @@ static int pm_set_grace_period_v9(struct 
> > packet_manager *pm,
> >       struct pm4_mec_write_data_mmio *packet;
> >       uint32_t reg_offset = 0;
> >       uint32_t reg_data = 0;
> > +    uint32_t first_inst = pm->dqm->dev->xcp->id *
> > + pm->dqm->dev->adev->gfx.num_xcc_per_xcp;
> > pm->dqm->dev->kfd2kgd->build_grace_period_packet_info(
> >               pm->dqm->dev->adev,
> > -            pm->dqm->wait_times,
> > +            pm->dqm->wait_times[first_inst],
> >               grace_period,
> >               &reg_offset,
> >               &reg_data,
> > -            0);
> > +            first_inst);
> >         if (grace_period == USE_DEFAULT_GRACE_PERIOD)
> > -        reg_data = pm->dqm->wait_times;
> > +        reg_data = pm->dqm->wait_times[first_inst];
> >         packet = (struct pm4_mec_write_data_mmio *)buffer;
> >       memset(buffer, 0, sizeof(struct pm4_mec_write_data_mmio));