From: "Marek Olšák" <maraeo@gmail.com>
To: Felix Kuehling <felix.kuehling@amd.com>
Cc: amd-gfx mailing list <amd-gfx@lists.freedesktop.org>,
Monk Liu <Monk.Liu@amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu: introduce a new parameter to configure how many KCQ we want(v5)
Date: Thu, 15 Oct 2020 11:05:07 -0400 [thread overview]
Message-ID: <CAAxE2A5JMHCxKKV+hSO7YL1Bt-M-9fuwcJKkSUSWyXxff9mezw@mail.gmail.com> (raw)
In-Reply-To: <904d9605-f061-d6a0-b72b-42b0d73bf11d@amd.com>
[-- Attachment #1.1: Type: text/plain, Size: 14905 bytes --]
Monk or perhaps Felix,
Do you by any chance know why the CS ioctl returns -EINVAL for all compute
submissions if num_kcq <= 4 and what could cause that?
If not, is there any way to disable mid-IB preemption for compute?
Thanks,
Marek
On Fri, Jul 31, 2020 at 9:53 AM Felix Kuehling <felix.kuehling@amd.com>
wrote:
> Am 2020-07-31 um 3:51 a.m. schrieb Monk Liu:
> > what:
> > the MQD's save and restore of KCQ (kernel compute queue)
> > cost lots of clocks during world switch which impacts a lot
> > to multi-VF performance
> >
> > how:
> > introduce a paramter to control the number of KCQ to avoid
> > performance drop if there is no kernel compute queue needed
> >
> > notes:
> > this paramter only affects gfx 8/9/10
> >
> > v2:
> > refine namings
> >
> > v3:
> > choose queues for each ring to that try best to cross pipes evenly.
> >
> > v4:
> > fix indentation
> > some cleanupsin the gfx_compute_queue_acquire()
> >
> > v5:
> > further fix on indentations
> > more cleanupsin gfx_compute_queue_acquire()
> >
> > TODO:
> > in the future we will let hypervisor driver to set this paramter
> > automatically thus no need for user to configure it through
> > modprobe in virtual machine
> >
> > Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>
> This patch is Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
>
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 49
> ++++++++++++------------------
> > drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 30 +++++++++---------
> > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 29 +++++++++---------
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 31 ++++++++++---------
> > 7 files changed, 76 insertions(+), 73 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index e97c088..de11136 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -201,6 +201,7 @@ extern int amdgpu_si_support;
> > #ifdef CONFIG_DRM_AMDGPU_CIK
> > extern int amdgpu_cik_support;
> > #endif
> > +extern int amdgpu_num_kcq;
> >
> > #define AMDGPU_VM_MAX_NUM_CTX 4096
> > #define AMDGPU_SG_THRESHOLD (256*1024*1024)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 62ecac9..cf445bab 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -1199,6 +1199,11 @@ static int amdgpu_device_check_arguments(struct
> amdgpu_device *adev)
> >
> > amdgpu_gmc_tmz_set(adev);
> >
> > + if (amdgpu_num_kcq > 8 || amdgpu_num_kcq < 0) {
> > + amdgpu_num_kcq = 8;
> > + dev_warn(adev->dev, "set kernel compute queue number to 8
> due to invalid paramter provided by user\n");
> > + }
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 6291f5f..b545c40 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -150,6 +150,7 @@ int amdgpu_noretry;
> > int amdgpu_force_asic_type = -1;
> > int amdgpu_tmz = 0;
> > int amdgpu_reset_method = -1; /* auto */
> > +int amdgpu_num_kcq = -1;
> >
> > struct amdgpu_mgpu_info mgpu_info = {
> > .mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
> > @@ -765,6 +766,9 @@ module_param_named(tmz, amdgpu_tmz, int, 0444);
> > MODULE_PARM_DESC(reset_method, "GPU reset method (-1 = auto (default),
> 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco)");
> > module_param_named(reset_method, amdgpu_reset_method, int, 0444);
> >
> > +MODULE_PARM_DESC(num_kcq, "number of kernel compute queue user want to
> setup (8 if set to greater than 8 or less than 0, only affect gfx 8+)");
> > +module_param_named(num_kcq, amdgpu_num_kcq, int, 0444);
> > +
> > static const struct pci_device_id pciidlist[] = {
> > #ifdef CONFIG_DRM_AMDGPU_SI
> > {0x1002, 0x6780, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TAHITI},
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > index 8eff017..0cd9de6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > @@ -202,40 +202,29 @@ bool
> amdgpu_gfx_is_high_priority_compute_queue(struct amdgpu_device *adev,
> >
> > void amdgpu_gfx_compute_queue_acquire(struct amdgpu_device *adev)
> > {
> > - int i, queue, pipe, mec;
> > + int i, queue, pipe;
> > bool multipipe_policy = amdgpu_gfx_is_multipipe_capable(adev);
> > -
> > - /* policy for amdgpu compute queue ownership */
> > - for (i = 0; i < AMDGPU_MAX_COMPUTE_QUEUES; ++i) {
> > - queue = i % adev->gfx.mec.num_queue_per_pipe;
> > - pipe = (i / adev->gfx.mec.num_queue_per_pipe)
> > - % adev->gfx.mec.num_pipe_per_mec;
> > - mec = (i / adev->gfx.mec.num_queue_per_pipe)
> > - / adev->gfx.mec.num_pipe_per_mec;
> > -
> > - /* we've run out of HW */
> > - if (mec >= adev->gfx.mec.num_mec)
> > - break;
> > -
> > - if (multipipe_policy) {
> > - /* policy: amdgpu owns the first two queues of the
> first MEC */
> > - if (mec == 0 && queue < 2)
> > - set_bit(i, adev->gfx.mec.queue_bitmap);
> > - } else {
> > - /* policy: amdgpu owns all queues in the first
> pipe */
> > - if (mec == 0 && pipe == 0)
> > - set_bit(i, adev->gfx.mec.queue_bitmap);
> > + int max_queues_per_mec = min(adev->gfx.mec.num_pipe_per_mec *
> > + adev->gfx.mec.num_queue_per_pipe,
> > + adev->gfx.num_compute_rings);
> > +
> > + if (multipipe_policy) {
> > + /* policy: make queues evenly cross all pipes on MEC1 only
> */
> > + for (i = 0; i < max_queues_per_mec; i++) {
> > + pipe = i % adev->gfx.mec.num_pipe_per_mec;
> > + queue = (i / adev->gfx.mec.num_pipe_per_mec) %
> > + adev->gfx.mec.num_queue_per_pipe;
> > +
> > + set_bit(pipe * adev->gfx.mec.num_queue_per_pipe +
> queue,
> > + adev->gfx.mec.queue_bitmap);
> > }
> > + } else {
> > + /* policy: amdgpu owns all queues in the given pipe */
> > + for (i = 0; i < max_queues_per_mec; ++i)
> > + set_bit(i, adev->gfx.mec.queue_bitmap);
> > }
> >
> > - /* update the number of active compute rings */
> > - adev->gfx.num_compute_rings =
> > - bitmap_weight(adev->gfx.mec.queue_bitmap,
> AMDGPU_MAX_COMPUTE_QUEUES);
> > -
> > - /* If you hit this case and edited the policy, you probably just
> > - * need to increase AMDGPU_MAX_COMPUTE_RINGS */
> > - if (WARN_ON(adev->gfx.num_compute_rings >
> AMDGPU_MAX_COMPUTE_RINGS))
> > - adev->gfx.num_compute_rings = AMDGPU_MAX_COMPUTE_RINGS;
> > + dev_dbg(adev->dev, "mec queue bitmap weight=%d\n",
> bitmap_weight(adev->gfx.mec.queue_bitmap, AMDGPU_MAX_COMPUTE_QUEUES));
> > }
> >
> > void amdgpu_gfx_graphics_queue_acquire(struct amdgpu_device *adev)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > index f571e25..4172bc8 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > @@ -4022,21 +4022,23 @@ static int gfx_v10_0_mec_init(struct
> amdgpu_device *adev)
> > amdgpu_gfx_compute_queue_acquire(adev);
> > mec_hpd_size = adev->gfx.num_compute_rings * GFX10_MEC_HPD_SIZE;
> >
> > - r = amdgpu_bo_create_reserved(adev, mec_hpd_size, PAGE_SIZE,
> > - AMDGPU_GEM_DOMAIN_GTT,
> > - &adev->gfx.mec.hpd_eop_obj,
> > - &adev->gfx.mec.hpd_eop_gpu_addr,
> > - (void **)&hpd);
> > - if (r) {
> > - dev_warn(adev->dev, "(%d) create HDP EOP bo failed\n", r);
> > - gfx_v10_0_mec_fini(adev);
> > - return r;
> > - }
> > + if (mec_hpd_size) {
> > + r = amdgpu_bo_create_reserved(adev, mec_hpd_size,
> PAGE_SIZE,
> > + AMDGPU_GEM_DOMAIN_GTT,
> > + &adev->gfx.mec.hpd_eop_obj,
> > +
> &adev->gfx.mec.hpd_eop_gpu_addr,
> > + (void **)&hpd);
> > + if (r) {
> > + dev_warn(adev->dev, "(%d) create HDP EOP bo
> failed\n", r);
> > + gfx_v10_0_mec_fini(adev);
> > + return r;
> > + }
> >
> > - memset(hpd, 0, mec_hpd_size);
> > + memset(hpd, 0, mec_hpd_size);
> >
> > - amdgpu_bo_kunmap(adev->gfx.mec.hpd_eop_obj);
> > - amdgpu_bo_unreserve(adev->gfx.mec.hpd_eop_obj);
> > + amdgpu_bo_kunmap(adev->gfx.mec.hpd_eop_obj);
> > + amdgpu_bo_unreserve(adev->gfx.mec.hpd_eop_obj);
> > + }
> >
> > if (adev->firmware.load_type == AMDGPU_FW_LOAD_DIRECT) {
> > mec_hdr = (const struct gfx_firmware_header_v1_0
> *)adev->gfx.mec_fw->data;
> > @@ -7162,7 +7164,7 @@ static int gfx_v10_0_early_init(void *handle)
> > break;
> > }
> >
> > - adev->gfx.num_compute_rings = AMDGPU_MAX_COMPUTE_RINGS;
> > + adev->gfx.num_compute_rings = amdgpu_num_kcq;
> >
> > gfx_v10_0_set_kiq_pm4_funcs(adev);
> > gfx_v10_0_set_ring_funcs(adev);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 8d72089..7df567a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -1343,21 +1343,22 @@ static int gfx_v8_0_mec_init(struct
> amdgpu_device *adev)
> > amdgpu_gfx_compute_queue_acquire(adev);
> >
> > mec_hpd_size = adev->gfx.num_compute_rings * GFX8_MEC_HPD_SIZE;
> > + if (mec_hpd_size) {
> > + r = amdgpu_bo_create_reserved(adev, mec_hpd_size,
> PAGE_SIZE,
> > + AMDGPU_GEM_DOMAIN_VRAM,
> > + &adev->gfx.mec.hpd_eop_obj,
> > +
> &adev->gfx.mec.hpd_eop_gpu_addr,
> > + (void **)&hpd);
> > + if (r) {
> > + dev_warn(adev->dev, "(%d) create HDP EOP bo
> failed\n", r);
> > + return r;
> > + }
> >
> > - r = amdgpu_bo_create_reserved(adev, mec_hpd_size, PAGE_SIZE,
> > - AMDGPU_GEM_DOMAIN_VRAM,
> > - &adev->gfx.mec.hpd_eop_obj,
> > - &adev->gfx.mec.hpd_eop_gpu_addr,
> > - (void **)&hpd);
> > - if (r) {
> > - dev_warn(adev->dev, "(%d) create HDP EOP bo failed\n", r);
> > - return r;
> > - }
> > -
> > - memset(hpd, 0, mec_hpd_size);
> > + memset(hpd, 0, mec_hpd_size);
> >
> > - amdgpu_bo_kunmap(adev->gfx.mec.hpd_eop_obj);
> > - amdgpu_bo_unreserve(adev->gfx.mec.hpd_eop_obj);
> > + amdgpu_bo_kunmap(adev->gfx.mec.hpd_eop_obj);
> > + amdgpu_bo_unreserve(adev->gfx.mec.hpd_eop_obj);
> > + }
> >
> > return 0;
> > }
> > @@ -5294,7 +5295,7 @@ static int gfx_v8_0_early_init(void *handle)
> > struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> >
> > adev->gfx.num_gfx_rings = GFX8_NUM_GFX_RINGS;
> > - adev->gfx.num_compute_rings = AMDGPU_MAX_COMPUTE_RINGS;
> > + adev->gfx.num_compute_rings = amdgpu_num_kcq;
> > adev->gfx.funcs = &gfx_v8_0_gfx_funcs;
> > gfx_v8_0_set_ring_funcs(adev);
> > gfx_v8_0_set_irq_funcs(adev);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index e4e751f..ef07e59 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -1938,22 +1938,23 @@ static int gfx_v9_0_mec_init(struct
> amdgpu_device *adev)
> > /* take ownership of the relevant compute queues */
> > amdgpu_gfx_compute_queue_acquire(adev);
> > mec_hpd_size = adev->gfx.num_compute_rings * GFX9_MEC_HPD_SIZE;
> > + if (mec_hpd_size) {
> > + r = amdgpu_bo_create_reserved(adev, mec_hpd_size,
> PAGE_SIZE,
> > + AMDGPU_GEM_DOMAIN_VRAM,
> > + &adev->gfx.mec.hpd_eop_obj,
> > +
> &adev->gfx.mec.hpd_eop_gpu_addr,
> > + (void **)&hpd);
> > + if (r) {
> > + dev_warn(adev->dev, "(%d) create HDP EOP bo
> failed\n", r);
> > + gfx_v9_0_mec_fini(adev);
> > + return r;
> > + }
> >
> > - r = amdgpu_bo_create_reserved(adev, mec_hpd_size, PAGE_SIZE,
> > - AMDGPU_GEM_DOMAIN_VRAM,
> > - &adev->gfx.mec.hpd_eop_obj,
> > - &adev->gfx.mec.hpd_eop_gpu_addr,
> > - (void **)&hpd);
> > - if (r) {
> > - dev_warn(adev->dev, "(%d) create HDP EOP bo failed\n", r);
> > - gfx_v9_0_mec_fini(adev);
> > - return r;
> > - }
> > -
> > - memset(hpd, 0, mec_hpd_size);
> > + memset(hpd, 0, mec_hpd_size);
> >
> > - amdgpu_bo_kunmap(adev->gfx.mec.hpd_eop_obj);
> > - amdgpu_bo_unreserve(adev->gfx.mec.hpd_eop_obj);
> > + amdgpu_bo_kunmap(adev->gfx.mec.hpd_eop_obj);
> > + amdgpu_bo_unreserve(adev->gfx.mec.hpd_eop_obj);
> > + }
> >
> > mec_hdr = (const struct gfx_firmware_header_v1_0
> *)adev->gfx.mec_fw->data;
> >
> > @@ -4625,7 +4626,7 @@ static int gfx_v9_0_early_init(void *handle)
> > adev->gfx.num_gfx_rings = 0;
> > else
> > adev->gfx.num_gfx_rings = GFX9_NUM_GFX_RINGS;
> > - adev->gfx.num_compute_rings = AMDGPU_MAX_COMPUTE_RINGS;
> > + adev->gfx.num_compute_rings = amdgpu_num_kcq;
> > gfx_v9_0_set_kiq_pm4_funcs(adev);
> > gfx_v9_0_set_ring_funcs(adev);
> > gfx_v9_0_set_irq_funcs(adev);
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
[-- Attachment #1.2: Type: text/html, Size: 19447 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2020-10-15 15:05 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-31 7:51 [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ Monk Liu
2020-07-31 7:51 ` [PATCH 2/2] drm/amdgpu: introduce a new parameter to configure how many KCQ we want(v5) Monk Liu
2020-07-31 13:53 ` Felix Kuehling
2020-10-15 15:05 ` Marek Olšák [this message]
2020-07-31 13:57 ` [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ Felix Kuehling
2020-08-03 1:54 ` Liu, Monk
2020-08-04 6:31 ` Liu, Monk
2020-08-04 8:01 ` Deng, Emily
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=CAAxE2A5JMHCxKKV+hSO7YL1Bt-M-9fuwcJKkSUSWyXxff9mezw@mail.gmail.com \
--to=maraeo@gmail.com \
--cc=Monk.Liu@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=felix.kuehling@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).