| Message ID | 20170911141732.16565-1-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Michał Winiarski (2017-09-11 15:17:31) > To create an upper bound on number of GuC workitems, we need to change > the way that requests are being submitted. Rather than submitting each > request as an individual workitem, we can do coalescing in a similar way > we're handlig execlist submission ports. We also need to stop pretending > that we're doing "lite-restore" in GuC submission (we would create a > workitem each time we hit this condition). This allows us to completely > remove the reservation, replacing it with a compile time check. > > v2: Also coalesce when replaying on reset (Daniele) ? You are using the pattern I expected, just calling i915_guc_submit() with the ports[] reset to count=0 to trigger the resubmit. > v3: Consistent wq_resv - per-request (Daniele) > v4: Squash removing wq_resv > > References: https://bugs.freedesktop.org/show_bug.cgi?id=101873 > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Jeff McGee <jeff.mcgee@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Oscar Mateo <oscar.mateo@intel.com> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> Looks good, very similar to the way I did the rq coalescing, but with the improvement of restricting it to ARRAY_SIZE(port[]) and so asserting that wq cannot overflow. If you get adventurous you can replace the spinlock with cmpxchg. Fancy having a go at a lockless guc_wq_item_append? Kill off the redundant stats for simplicity. -Chris
Hi Michał, [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on v4.13 next-20170914] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Micha-Winiarski/drm-i915-guc-Submit-GuC-workitems-containing-coalesced-requests/20170914-122212 base: git://anongit.freedesktop.org/drm-intel for-linux-next reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) include/linux/init.h:1: warning: no structured comments found include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_major' description in 'fsl_mc_device_id' include/linux/mod_devicetable.h:687: warning: Excess struct/union/enum/typedef member 'ver_minor' description in 'fsl_mc_device_id' kernel/sys.c:1: warning: no structured comments found include/linux/device.h:968: warning: No description found for parameter 'dma_ops' drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found include/linux/sync_file.h:51: warning: No description found for parameter 'flags' include/linux/iio/iio.h:603: warning: No description found for parameter 'trig_readonly' include/linux/iio/trigger.h:151: warning: No description found for parameter 'indio_dev' include/linux/iio/trigger.h:151: warning: No description found for parameter 'trig' include/linux/device.h:969: warning: No description found for parameter 'dma_ops' arch/s390/include/asm/cmb.h:1: warning: no structured comments found drivers/scsi/scsi_lib.c:1116: warning: No description found for parameter 'rq' drivers/scsi/constants.c:1: warning: no structured comments found include/linux/usb/gadget.h:230: warning: No description found for parameter 'claimed' include/linux/usb/gadget.h:230: warning: No description found for parameter 'enabled' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_altset_not_supp' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_stall_not_supp' include/linux/usb/gadget.h:412: warning: No description found for parameter 'quirk_zlp_not_supp' fs/inode.c:1666: warning: No description found for parameter 'rcu' include/linux/jbd2.h:443: warning: No description found for parameter 'i_transaction' include/linux/jbd2.h:443: warning: No description found for parameter 'i_next_transaction' include/linux/jbd2.h:443: warning: No description found for parameter 'i_list' include/linux/jbd2.h:443: warning: No description found for parameter 'i_vfs_inode' include/linux/jbd2.h:443: warning: No description found for parameter 'i_flags' include/linux/jbd2.h:497: warning: No description found for parameter 'h_rsv_handle' include/linux/jbd2.h:497: warning: No description found for parameter 'h_reserved' include/linux/jbd2.h:497: warning: No description found for parameter 'h_type' include/linux/jbd2.h:497: warning: No description found for parameter 'h_line_no' include/linux/jbd2.h:497: warning: No description found for parameter 'h_start_jiffies' include/linux/jbd2.h:497: warning: No description found for parameter 'h_requested_credits' include/linux/jbd2.h:497: warning: No description found for parameter 'saved_alloc_context' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chkpt_bhs' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_devname' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_average_commit_time' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_min_batch_time' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_max_batch_time' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_commit_callback' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_failed_commit' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_chksum_driver' include/linux/jbd2.h:1050: warning: No description found for parameter 'j_csum_seed' fs/jbd2/transaction.c:511: warning: No description found for parameter 'type' fs/jbd2/transaction.c:511: warning: No description found for parameter 'line_no' fs/jbd2/transaction.c:641: warning: No description found for parameter 'gfp_mask' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_pin' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_unpin' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_res_obj' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_get_sg_table' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_import_sg_table' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_vmap' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_vunmap' include/drm/drm_drv.h:594: warning: No description found for parameter 'gem_prime_mmap' include/drm/drm_mode_config.h:771: warning: No description found for parameter 'modifiers_property' include/drm/drm_mode_config.h:771: warning: Excess struct/union/enum/typedef member 'modifiers' description in 'drm_mode_config' include/drm/drm_plane.h:544: warning: No description found for parameter 'modifiers' include/drm/drm_plane.h:544: warning: No description found for parameter 'modifier_count' >> drivers/gpu/drm/i915/i915_guc_submission.c:531: warning: No description found for parameter 'engine' >> drivers/gpu/drm/i915/i915_guc_submission.c:531: warning: Excess function parameter 'rq' description in 'i915_guc_submit' drivers/gpu/drm/i915/i915_guc_submission.c:532: warning: No description found for parameter 'engine' drivers/gpu/drm/i915/i915_guc_submission.c:532: warning: Excess function parameter 'rq' description in 'i915_guc_submit' drivers/gpu/host1x/bus.c:50: warning: No description found for parameter 'driver' Documentation/doc-guide/sphinx.rst:121: ERROR: Unknown target name: "sphinx c domain". kernel/sched/fair.c:7584: WARNING: Inline emphasis start-string without end-string. kernel/time/timer.c:1200: ERROR: Unexpected indentation. kernel/time/timer.c:1202: ERROR: Unexpected indentation. kernel/time/timer.c:1203: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/wait.h:108: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/wait.h:111: ERROR: Unexpected indentation. include/linux/wait.h:113: WARNING: Block quote ends without a blank line; unexpected unindent. kernel/time/hrtimer.c:991: WARNING: Block quote ends without a blank line; unexpected unindent. kernel/signal.c:323: WARNING: Inline literal start-string without end-string. kernel/rcu/tree.c:3187: ERROR: Unexpected indentation. kernel/rcu/tree.c:3214: ERROR: Unexpected indentation. kernel/rcu/tree.c:3215: WARNING: Bullet list ends without a blank line; unexpected unindent. include/linux/iio/iio.h:219: ERROR: Unexpected indentation. include/linux/iio/iio.h:220: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/iio/iio.h:226: WARNING: Definition list ends without a blank line; unexpected unindent. drivers/iio/industrialio-core.c:633: ERROR: Unknown target name: "iio_val". drivers/iio/industrialio-core.c:640: ERROR: Unknown target name: "iio_val". drivers/ata/libata-core.c:5906: ERROR: Unknown target name: "hw". drivers/message/fusion/mptbase.c:5051: WARNING: Definition list ends without a blank line; unexpected unindent. drivers/tty/serial/serial_core.c:1897: WARNING: Definition list ends without a blank line; unexpected unindent. drivers/pci/pci.c:3470: ERROR: Unexpected indentation. include/linux/regulator/driver.h:271: ERROR: Unknown target name: "regulator_regmap_x_voltage". include/linux/spi/spi.h:373: ERROR: Unexpected indentation. drivers/w1/w1_io.c:196: WARNING: Definition list ends without a blank line; unexpected unindent. block/bio.c:404: ERROR: Unknown target name: "gfp". sound/soc/soc-core.c:2703: ERROR: Unknown target name: "snd_soc_daifmt". sound/core/jack.c:312: ERROR: Unknown target name: "snd_jack_btn". Documentation/virtual/kvm/vcpu-requests.rst:: WARNING: document isn't included in any toctree Documentation/dev-tools/kselftest.rst:15: WARNING: Could not lex literal_block as "c". Highlighting skipped. Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 133: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 164: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "~/.fonts.conf", line 193: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 43: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 56: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 69: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 82: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 96: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 109: Having multiple values in <test> isn't supported and may not work as expected Fontconfig warning: "/home/kbuild/.config/fontconfig/fonts.conf", line 122: Having multiple values in <test> isn't supported and may not work as expected vim +/engine +531 drivers/gpu/drm/i915/i915_guc_submission.c 10d2c3e28 Dave Gordon 2016-06-13 522 44a28b1d3 Dave Gordon 2015-08-12 523 /** e875144fe Michał Winiarski 2017-09-11 524 * i915_guc_submit() - Submit commands through GuC feda33ef0 Alex Dai 2015-10-19 525 * @rq: request associated with the commands 44a28b1d3 Dave Gordon 2015-08-12 526 * 7c2c270d2 Dave Gordon 2016-05-13 527 * The only error here arises if the doorbell hardware isn't functioning 7c2c270d2 Dave Gordon 2016-05-13 528 * as expected, which really shouln't happen. 44a28b1d3 Dave Gordon 2015-08-12 529 */ e875144fe Michał Winiarski 2017-09-11 530 static void i915_guc_submit(struct intel_engine_cs *engine) 44a28b1d3 Dave Gordon 2015-08-12 @531 { e875144fe Michał Winiarski 2017-09-11 532 struct drm_i915_private *dev_priv = engine->i915; e875144fe Michał Winiarski 2017-09-11 533 struct intel_guc *guc = &dev_priv->guc; 7c2c270d2 Dave Gordon 2016-05-13 534 struct i915_guc_client *client = guc->execbuf_client; e875144fe Michał Winiarski 2017-09-11 535 struct execlist_port *port = engine->execlist_port; e875144fe Michał Winiarski 2017-09-11 536 unsigned int engine_id = engine->id; e875144fe Michał Winiarski 2017-09-11 537 unsigned int n; 25afdf89a Chris Wilson 2017-03-02 538 unsigned long flags; 0a31afbc0 Dave Gordon 2016-05-13 539 int b_ret; 44a28b1d3 Dave Gordon 2015-08-12 540 e875144fe Michał Winiarski 2017-09-11 541 for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) { e875144fe Michał Winiarski 2017-09-11 542 struct drm_i915_gem_request *rq; e875144fe Michał Winiarski 2017-09-11 543 unsigned int count; e875144fe Michał Winiarski 2017-09-11 544 e875144fe Michał Winiarski 2017-09-11 545 rq = port_unpack(&port[n], &count); e875144fe Michał Winiarski 2017-09-11 546 if (rq && count == 0) { e875144fe Michał Winiarski 2017-09-11 547 port_set(&port[n], port_pack(rq, ++count)); e875144fe Michał Winiarski 2017-09-11 548 ed4596ea9 Akash Goel 2016-10-25 549 if (i915_vma_is_map_and_fenceable(rq->ring->vma)) ed4596ea9 Akash Goel 2016-10-25 550 POSTING_READ_FW(GUC_STATUS); ed4596ea9 Akash Goel 2016-10-25 551 25afdf89a Chris Wilson 2017-03-02 552 spin_lock_irqsave(&client->wq_lock, flags); 0c33518db Chris Wilson 2017-02-28 553 0c33518db Chris Wilson 2017-02-28 554 guc_wq_item_append(client, rq); 44a28b1d3 Dave Gordon 2015-08-12 555 b_ret = guc_ring_doorbell(client); 44a28b1d3 Dave Gordon 2015-08-12 556 397097b02 Alex Dai 2016-01-23 557 client->submissions[engine_id] += 1; 0a31afbc0 Dave Gordon 2016-05-13 558 25afdf89a Chris Wilson 2017-03-02 559 spin_unlock_irqrestore(&client->wq_lock, flags); 44a28b1d3 Dave Gordon 2015-08-12 560 } e875144fe Michał Winiarski 2017-09-11 561 } 34ba5a80f Chris Wilson 2016-11-29 562 } 34ba5a80f Chris Wilson 2016-11-29 563 :::::: The code at line 531 was first introduced by commit :::::: 44a28b1d36762499de6fd701fcce6814eefe31d7 drm/i915: Implementation of GuC submission client :::::: TO: Dave Gordon <david.s.gordon@intel.com> :::::: CC: Daniel Vetter <daniel.vetter@ffwll.ch> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 6338018f655d..f5fd00cfb3b0 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2450,8 +2450,6 @@ static void i915_guc_client_info(struct seq_file *m, seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n", client->wq_size, client->wq_offset, client->wq_tail); - seq_printf(m, "\tWork queue full: %u\n", client->no_wq_space); - for_each_engine(engine, dev_priv, id) { u64 submissions = client->submissions[id]; tot += submissions; diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 48a1e9349a2c..77ded6c64aca 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -406,63 +406,6 @@ static void guc_stage_desc_fini(struct intel_guc *guc, memset(desc, 0, sizeof(*desc)); } -/** - * i915_guc_wq_reserve() - reserve space in the GuC's workqueue - * @request: request associated with the commands - * - * Return: 0 if space is available - * -EAGAIN if space is not currently available - * - * This function must be called (and must return 0) before a request - * is submitted to the GuC via i915_guc_submit() below. Once a result - * of 0 has been returned, it must be balanced by a corresponding - * call to submit(). - * - * Reservation allows the caller to determine in advance that space - * will be available for the next submission before committing resources - * to it, and helps avoid late failures with complicated recovery paths. - */ -int i915_guc_wq_reserve(struct drm_i915_gem_request *request) -{ - const size_t wqi_size = sizeof(struct guc_wq_item); - struct i915_guc_client *client = request->i915->guc.execbuf_client; - struct guc_process_desc *desc = __get_process_desc(client); - u32 freespace; - int ret; - - spin_lock_irq(&client->wq_lock); - freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size); - freespace -= client->wq_rsvd; - if (likely(freespace >= wqi_size)) { - client->wq_rsvd += wqi_size; - ret = 0; - } else { - client->no_wq_space++; - ret = -EAGAIN; - } - spin_unlock_irq(&client->wq_lock); - - return ret; -} - -static void guc_client_update_wq_rsvd(struct i915_guc_client *client, int size) -{ - unsigned long flags; - - spin_lock_irqsave(&client->wq_lock, flags); - client->wq_rsvd += size; - spin_unlock_irqrestore(&client->wq_lock, flags); -} - -void i915_guc_wq_unreserve(struct drm_i915_gem_request *request) -{ - const int wqi_size = sizeof(struct guc_wq_item); - struct i915_guc_client *client = request->i915->guc.execbuf_client; - - GEM_BUG_ON(READ_ONCE(client->wq_rsvd) < wqi_size); - guc_client_update_wq_rsvd(client, -wqi_size); -} - /* Construct a Work Item and append it to the GuC's Work Queue */ static void guc_wq_item_append(struct i915_guc_client *client, struct drm_i915_gem_request *rq) @@ -475,7 +418,7 @@ static void guc_wq_item_append(struct i915_guc_client *client, struct guc_wq_item *wqi; u32 freespace, tail, wq_off; - /* Free space is guaranteed, see i915_guc_wq_reserve() above */ + /* Free space is guaranteed */ freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size); GEM_BUG_ON(freespace < wqi_size); @@ -491,14 +434,12 @@ static void guc_wq_item_append(struct i915_guc_client *client, * workqueue buffer dw by dw. */ BUILD_BUG_ON(wqi_size != 16); - GEM_BUG_ON(client->wq_rsvd < wqi_size); /* postincrement WQ tail for next time */ wq_off = client->wq_tail; GEM_BUG_ON(wq_off & (wqi_size - 1)); client->wq_tail += wqi_size; client->wq_tail &= client->wq_size - 1; - client->wq_rsvd -= wqi_size; /* WQ starts from the page after doorbell / process_desc */ wqi = client->vaddr + wq_off + GUC_DB_SIZE; @@ -580,48 +521,44 @@ static int guc_ring_doorbell(struct i915_guc_client *client) } /** - * __i915_guc_submit() - Submit commands through GuC + * i915_guc_submit() - Submit commands through GuC * @rq: request associated with the commands * - * The caller must have already called i915_guc_wq_reserve() above with - * a result of 0 (success), guaranteeing that there is space in the work - * queue for the new request, so enqueuing the item cannot fail. - * - * Bad Things Will Happen if the caller violates this protocol e.g. calls - * submit() when _reserve() says there's no space, or calls _submit() - * a different number of times from (successful) calls to _reserve(). - * * The only error here arises if the doorbell hardware isn't functioning * as expected, which really shouln't happen. */ -static void __i915_guc_submit(struct drm_i915_gem_request *rq) +static void i915_guc_submit(struct intel_engine_cs *engine) { - struct drm_i915_private *dev_priv = rq->i915; - struct intel_engine_cs *engine = rq->engine; - unsigned int engine_id = engine->id; - struct intel_guc *guc = &rq->i915->guc; + struct drm_i915_private *dev_priv = engine->i915; + struct intel_guc *guc = &dev_priv->guc; struct i915_guc_client *client = guc->execbuf_client; + struct execlist_port *port = engine->execlist_port; + unsigned int engine_id = engine->id; + unsigned int n; unsigned long flags; int b_ret; - /* WA to flush out the pending GMADR writes to ring buffer. */ - if (i915_vma_is_map_and_fenceable(rq->ring->vma)) - POSTING_READ_FW(GUC_STATUS); + for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) { + struct drm_i915_gem_request *rq; + unsigned int count; - spin_lock_irqsave(&client->wq_lock, flags); + rq = port_unpack(&port[n], &count); + if (rq && count == 0) { + port_set(&port[n], port_pack(rq, ++count)); - guc_wq_item_append(client, rq); - b_ret = guc_ring_doorbell(client); + if (i915_vma_is_map_and_fenceable(rq->ring->vma)) + POSTING_READ_FW(GUC_STATUS); - client->submissions[engine_id] += 1; + spin_lock_irqsave(&client->wq_lock, flags); - spin_unlock_irqrestore(&client->wq_lock, flags); -} + guc_wq_item_append(client, rq); + b_ret = guc_ring_doorbell(client); -static void i915_guc_submit(struct drm_i915_gem_request *rq) -{ - __i915_gem_request_submit(rq); - __i915_guc_submit(rq); + client->submissions[engine_id] += 1; + + spin_unlock_irqrestore(&client->wq_lock, flags); + } + } } static void nested_enable_signaling(struct drm_i915_gem_request *rq) @@ -655,16 +592,19 @@ static void port_assign(struct execlist_port *port, if (port_isset(port)) i915_gem_request_put(port_request(port)); - port_set(port, i915_gem_request_get(rq)); + port_set(port, port_pack(i915_gem_request_get(rq), port_count(port))); nested_enable_signaling(rq); } -static bool i915_guc_dequeue(struct intel_engine_cs *engine) +static void i915_guc_dequeue(struct intel_engine_cs *engine) { struct execlist_port *port = engine->execlist_port; - struct drm_i915_gem_request *last = port_request(port); - struct rb_node *rb; + struct drm_i915_gem_request *last = NULL; bool submit = false; + struct rb_node *rb; + + if (port_isset(port)) + port++; spin_lock_irq(&engine->timeline->lock); rb = engine->execlist_first; @@ -689,7 +629,7 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine) INIT_LIST_HEAD(&rq->priotree.link); rq->priotree.priority = INT_MAX; - i915_guc_submit(rq); + __i915_gem_request_submit(rq); trace_i915_gem_request_in(rq, port_index(port, engine)); last = rq; submit = true; @@ -703,11 +643,11 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine) } done: engine->execlist_first = rb; - if (submit) + if (submit) { port_assign(port, last); + i915_guc_submit(engine); + } spin_unlock_irq(&engine->timeline->lock); - - return submit; } static void i915_guc_irq_handler(unsigned long data) @@ -715,24 +655,20 @@ static void i915_guc_irq_handler(unsigned long data) struct intel_engine_cs *engine = (struct intel_engine_cs *)data; struct execlist_port *port = engine->execlist_port; struct drm_i915_gem_request *rq; - bool submit; - do { - rq = port_request(&port[0]); - while (rq && i915_gem_request_completed(rq)) { - trace_i915_gem_request_out(rq); - i915_gem_request_put(rq); + rq = port_request(&port[0]); + while (rq && i915_gem_request_completed(rq)) { + trace_i915_gem_request_out(rq); + i915_gem_request_put(rq); - port[0] = port[1]; - memset(&port[1], 0, sizeof(port[1])); + port[0] = port[1]; + memset(&port[1], 0, sizeof(port[1])); - rq = port_request(&port[0]); - } + rq = port_request(&port[0]); + } - submit = false; - if (!port_count(&port[1])) - submit = i915_guc_dequeue(engine); - } while (submit); + if (!port_isset(&port[1])) + i915_guc_dequeue(engine); } /* @@ -1221,6 +1157,19 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv) enum intel_engine_id id; int err; + /* + * We're using GuC work items for submitting work through GuC. Since + * we're coalescing multiple requests from a single context into a + * single work item prior to assigning it to execlist_port, we can + * never have more work items than the total number of ports (for all + * engines). The GuC firmware is controlling the HEAD of work queue, + * and it is guaranteed that it will remove the work item from the + * queue before our request is completed. + */ + BUILD_BUG_ON(ARRAY_SIZE(engine->execlist_port) * + sizeof(struct guc_wq_item) * + I915_NUM_ENGINES > GUC_WQ_SIZE); + if (!client) { client = guc_client_alloc(dev_priv, INTEL_INFO(dev_priv)->ring_mask, @@ -1248,9 +1197,6 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv) guc_interrupts_capture(dev_priv); for_each_engine(engine, dev_priv, id) { - const int wqi_size = sizeof(struct guc_wq_item); - struct drm_i915_gem_request *rq; - /* The tasklet was initialised by execlists, and may be in * a state of flux (across a reset) and so we just want to * take over the callback without changing any other state @@ -1258,14 +1204,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv) */ engine->irq_tasklet.func = i915_guc_irq_handler; clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); - - /* Replay the current set of previously submitted requests */ - spin_lock_irq(&engine->timeline->lock); - list_for_each_entry(rq, &engine->timeline->requests, link) { - guc_client_update_wq_rsvd(client, wqi_size); - __i915_guc_submit(rq); - } - spin_unlock_irq(&engine->timeline->lock); + i915_guc_submit(engine); } return 0; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index d89e1b8e1cc5..5837b33f9705 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -914,27 +914,14 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request) */ request->reserved_space += EXECLISTS_REQUEST_SIZE; - if (i915.enable_guc_submission) { - /* - * Check that the GuC has space for the request before - * going any further, as the i915_add_request() call - * later on mustn't fail ... - */ - ret = i915_guc_wq_reserve(request); - if (ret) - goto err; - } - cs = intel_ring_begin(request, 0); - if (IS_ERR(cs)) { - ret = PTR_ERR(cs); - goto err_unreserve; - } + if (IS_ERR(cs)) + return PTR_ERR(cs); if (!ce->initialised) { ret = engine->init_context(request); if (ret) - goto err_unreserve; + return ret; ce->initialised = true; } @@ -948,12 +935,6 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request) request->reserved_space -= EXECLISTS_REQUEST_SIZE; return 0; - -err_unreserve: - if (i915.enable_guc_submission) - i915_guc_wq_unreserve(request); -err: - return ret; } /* diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 22ae52b17b0f..cfbc28e2396b 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -55,10 +55,6 @@ struct drm_i915_gem_request; * * We also keep a few statistics on failures. Ideally, these should all * be zero! - * no_wq_space: times that the submission pre-check found no space was - * available in the work queue (note, the queue is shared, - * not per-engine). It is OK for this to be nonzero, but - * it should not be huge! * b_fail: failed to ring the doorbell. This should never happen, unless * somehow the hardware misbehaves, or maybe if the GuC firmware * crashes? We probably need to reset the GPU to recover. @@ -83,8 +79,6 @@ struct i915_guc_client { uint32_t wq_offset; uint32_t wq_size; uint32_t wq_tail; - uint32_t wq_rsvd; - uint32_t no_wq_space; /* Per-engine counts of GuC submissions */ uint64_t submissions[I915_NUM_ENGINES]; @@ -250,8 +244,6 @@ u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv); /* i915_guc_submission.c */ int i915_guc_submission_init(struct drm_i915_private *dev_priv); int i915_guc_submission_enable(struct drm_i915_private *dev_priv); -int i915_guc_wq_reserve(struct drm_i915_gem_request *rq); -void i915_guc_wq_unreserve(struct drm_i915_gem_request *request); void i915_guc_submission_disable(struct drm_i915_private *dev_priv); void i915_guc_submission_fini(struct drm_i915_private *dev_priv); struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
To create an upper bound on number of GuC workitems, we need to change the way that requests are being submitted. Rather than submitting each request as an individual workitem, we can do coalescing in a similar way we're handlig execlist submission ports. We also need to stop pretending that we're doing "lite-restore" in GuC submission (we would create a workitem each time we hit this condition). This allows us to completely remove the reservation, replacing it with a compile time check. v2: Also coalesce when replaying on reset (Daniele) v3: Consistent wq_resv - per-request (Daniele) v4: Squash removing wq_resv References: https://bugs.freedesktop.org/show_bug.cgi?id=101873 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Jeff McGee <jeff.mcgee@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Oscar Mateo <oscar.mateo@intel.com> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 2 - drivers/gpu/drm/i915/i915_guc_submission.c | 179 ++++++++++------------------- drivers/gpu/drm/i915/intel_lrc.c | 25 +--- drivers/gpu/drm/i915/intel_uc.h | 8 -- 4 files changed, 62 insertions(+), 152 deletions(-)