Hi,
On 06/05/2021 22:13, Matthew Brost wrote:
Basic GuC submission support. This is the first bullet point in the
upstreaming plan covered in the following RFC [1].
At a very high level the GuC is a piece of firmware which sits between
the i915 and the GPU. It offloads some of the scheduling of contexts
from the i915 and programs the GPU to submit contexts. The i915
communicates with the GuC and the GuC communicates with the GPU.
May I ask what will GuC command submission do that execlist won't/can't
do? And what would be the impact on users? Even forgetting the troubled
history of GuC (instability, performance regression, poor level of user
support, 6+ years of trying to upstream it...), adding this much code
and doubling the amount of validation needed should come with a
rationale making it feel worth it... and I am not seeing here. Would you
mind providing the rationale behind this work?
GuC submission will be disabled by default on all current upstream
platforms behind a module parameter - enable_guc. A value of 3 will
enable submission and HuC loading via the GuC. GuC submission should
work on all gen11+ platforms assuming the GuC firmware is present.
What is the plan here when it comes to keeping support for execlist? I
am afraid that landing GuC support in Linux is the first step towards
killing the execlist, which would force users to use proprietary
firmwares that even most Intel engineers have little influence over.
Indeed, if "drm/i915/guc: Disable semaphores when using GuC scheduling"
which states "Disable semaphores when using GuC scheduling as semaphores
are broken in the current GuC firmware." is anything to go by, it means
that even Intel developers seem to prefer working around the GuC
firmware, rather than fixing it.
In the same vein, I have another concern related to the impact of GuC on
Linux's stable releases. Let's say that in 3 years, a new application
triggers a bug in command submission inside the firmware. Given that the
Linux community cannot patch the GuC firmware, how likely is it that
Intel would release a new GuC version? That would not be necessarily
such a big problem if newer versions of the GuC could easily be
backported to this potentially-decade-old Linux version, but given that
the GuC seems to have ABI-breaking changes on a monthly cadence (we are
at major version 60 *already*? :o), I would say that it is
highly-unlikely that it would not require potentially-extensive changes
to i915 to make it work, making the fix almost impossible to land in the
stable tree... Do you have a plan to mitigate this problem?
Patches like "drm/i915/guc: Disable bonding extension with GuC
submission" also make me twitch, as this means the two command
submission paths will not be functionally equivalent, and enabling GuC
could thus introduce a user-visible regression (one app used to work,
then stopped working). Could you add in the commit's message a proof
that this would not end up being a user regression (in which case, why
have this codepath to begin with?).
Finally, could you explain why IGT tests need to be modified to work the
GuC [1], and how much of the code in this series is covered by
existing/upcoming tests? I would expect a very solid set of tests to
minimize the maintenance burden, and enable users to reproduce potential
issues found in this new codepath (too many users run with enable_guc=3,
as can be seen on Google[2]).
Looking forward to reading up about your plan, and the commitments Intel
would put in place to make this feature something users should be
looking forward to rather than fear.
Thanks,
Martin
[2] https://www.google.com/search?q=enable_guc%3D3
This is a huge series and it is completely unrealistic to merge all of
these patches at once. Fortunately I believe we can break down the
series into different merges:
1. Merge Chris Wilson's patches. These have already been reviewed
upstream and I fully agree with these patches as a precursor to GuC
submission.
2. Update to GuC 60.1.2. These are largely Michal's patches.
3. Turn on GuC/HuC auto mode by default.
4. Additional patches needed to support GuC submission. This is any
patch not covered by 1-3 in the first 34 patches. e.g. 'Engine relative
MMIO'
5. GuC submission support. Patches number 35+. These all don't have to
merge at once though as we don't actually allow GuC submission until the
last patch of this series.
[1] https://patchwork.freedesktop.org/patch/432206/?series=89840&rev=1
Signed-off-by: Matthew Brost <matthew.br...@intel.com>
Chris Wilson (3):
drm/i915/gt: Move engine setup out of set_default_submission
drm/i915/gt: Move submission_method into intel_gt
drm/i915/gt: Move CS interrupt handler to the backend
Daniele Ceraolo Spurio (6):
drm/i915/guc: skip disabling CTBs before sanitizing the GuC
drm/i915/guc: use probe_error log for CT enablement failure
drm/i915/guc: enable only the user interrupt when using GuC submission
drm/i915/uc: turn on GuC/HuC auto mode by default
drm/i915/guc: Use guc_class instead of engine_class in fw interface
drm/i915/guc: Unblock GuC submission on Gen11+
John Harrison (13):
drm/i915/guc: Support per context scheduling policies
drm/i915/guc: Update firmware to v60.1.2
drm/i915: Engine relative MMIO
drm/i915/guc: Module load failure test for CT buffer creation
drm/i915: Track 'serial' counts for virtual engines
drm/i915/guc: Provide mmio list to be saved/restored on engine reset
drm/i915/guc: Don't complain about reset races
drm/i915/guc: Enable GuC engine reset
drm/i915/guc: Fix for error capture after full GPU reset with GuC
drm/i915/guc: Hook GuC scheduling policies up
drm/i915/guc: Connect reset modparam updates to GuC policy flags
drm/i915/guc: Include scheduling policies in the debugfs state dump
drm/i915/guc: Add golden context to GuC ADS
Matthew Brost (53):
drm/i915: Introduce i915_sched_engine object
drm/i915/guc: Improve error message for unsolicited CT response
drm/i915/guc: Add non blocking CTB send function
drm/i915/guc: Add stall timer to non blocking CTB send function
drm/i915/guc: Optimize CTB writes and reads
drm/i915/guc: Increase size of CTB buffers
drm/i915/guc: Add new GuC interface defines and structures
drm/i915/guc: Remove GuC stage descriptor, add lrc descriptor
drm/i915/guc: Add lrc descriptor context lookup array
drm/i915/guc: Implement GuC submission tasklet
drm/i915/guc: Add bypass tasklet submission path to GuC
drm/i915/guc: Implement GuC context operations for new inteface
drm/i915/guc: Insert fence on context when deregistering
drm/i915/guc: Defer context unpin until scheduling is disabled
drm/i915/guc: Disable engine barriers with GuC during unpin
drm/i915/guc: Extend deregistration fence to schedule disable
drm/i915: Disable preempt busywait when using GuC scheduling
drm/i915/guc: Ensure request ordering via completion fences
drm/i915/guc: Disable semaphores when using GuC scheduling
drm/i915/guc: Ensure G2H response has space in buffer
drm/i915/guc: Update intel_gt_wait_for_idle to work with GuC
drm/i915/guc: Update GuC debugfs to support new GuC
drm/i915/guc: Add several request trace points
drm/i915: Add intel_context tracing
drm/i915/guc: GuC virtual engines
drm/i915: Hold reference to intel_context over life of i915_request
drm/i915/guc: Disable bonding extension with GuC submission
drm/i915/guc: Direct all breadcrumbs for a class to single breadcrumbs
drm/i915/guc: Reset implementation for new GuC interface
drm/i915: Reset GPU immediately if submission is disabled
drm/i915/guc: Add disable interrupts to guc sanitize
drm/i915/guc: Suspend/resume implementation for new interface
drm/i915/guc: Handle context reset notification
drm/i915/guc: Handle engine reset failure notification
drm/i915/guc: Enable the timer expired interrupt for GuC
drm/i915/guc: Capture error state on context reset
drm/i915/guc: Don't call ring_is_idle in GuC submission
drm/i915/guc: Implement banned contexts for GuC submission
drm/i915/guc: Allow flexible number of context ids
drm/i915/guc: Connect the number of guc_ids to debugfs
drm/i915/guc: Don't return -EAGAIN to user when guc_ids exhausted
drm/i915/guc: Don't allow requests not ready to consume all guc_ids
drm/i915/guc: Introduce guc_submit_engine object
drm/i915/guc: Implement GuC priority management
drm/i915/guc: Support request cancellation
drm/i915/guc: Check return of __xa_store when registering a context
drm/i915/guc: Non-static lrc descriptor registration buffer
drm/i915/guc: Take GT PM ref when deregistering context
drm/i915: Add GT PM delayed worker
drm/i915/guc: Take engine PM when a context is pinned with GuC
submission
drm/i915/guc: Don't call switch_to_kernel_context with GuC submission
drm/i915/guc: Selftest for GuC flow control
drm/i915/guc: Update GuC documentation
Michal Wajdeczko (21):
drm/i915/guc: Keep strict GuC ABI definitions
drm/i915/guc: Stop using fence/status from CTB descriptor
drm/i915: Promote ptrdiff() to i915_utils.h
drm/i915/guc: Only rely on own CTB size
drm/i915/guc: Don't repeat CTB layout calculations
drm/i915/guc: Replace CTB array with explicit members
drm/i915/guc: Update sizes of CTB buffers
drm/i915/guc: Relax CTB response timeout
drm/i915/guc: Start protecting access to CTB descriptors
drm/i915/guc: Stop using mutex while sending CTB messages
drm/i915/guc: Don't receive all G2H messages in irq handler
drm/i915/guc: Always copy CT message to new allocation
drm/i915/guc: Introduce unified HXG messages
drm/i915/guc: Update MMIO based communication
drm/i915/guc: Update CTB response status
drm/i915/guc: Add flag for mark broken CTB
drm/i915/guc: New definition of the CTB descriptor
drm/i915/guc: New definition of the CTB registration action
drm/i915/guc: New CTB based communication
drm/i915/guc: Kill guc_clients.ct_pool
drm/i915/guc: Early initialization of GuC send registers
Rodrigo Vivi (1):
drm/i915/guc: Remove sample_forcewake h2g action
drivers/gpu/drm/i915/Makefile | 2 +
drivers/gpu/drm/i915/gem/i915_gem_context.c | 39 +-
drivers/gpu/drm/i915/gem/i915_gem_context.h | 1 +
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 3 +-
drivers/gpu/drm/i915/gem/i915_gem_wait.c | 4 +-
drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 6 +-
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 44 +-
drivers/gpu/drm/i915/gt/intel_breadcrumbs.h | 14 +-
.../gpu/drm/i915/gt/intel_breadcrumbs_types.h | 7 +
drivers/gpu/drm/i915/gt/intel_context.c | 50 +-
drivers/gpu/drm/i915/gt/intel_context.h | 45 +-
drivers/gpu/drm/i915/gt/intel_context_types.h | 76 +-
drivers/gpu/drm/i915/gt/intel_engine.h | 96 +-
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 320 +-
.../gpu/drm/i915/gt/intel_engine_heartbeat.c | 75 +-
.../gpu/drm/i915/gt/intel_engine_heartbeat.h | 4 +
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 14 +-
drivers/gpu/drm/i915/gt/intel_engine_pm.h | 5 +
drivers/gpu/drm/i915/gt/intel_engine_types.h | 71 +-
drivers/gpu/drm/i915/gt/intel_engine_user.c | 6 +-
.../drm/i915/gt/intel_execlists_submission.c | 693 +--
.../drm/i915/gt/intel_execlists_submission.h | 14 -
drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 5 +
drivers/gpu/drm/i915/gt/intel_gt.c | 23 +
drivers/gpu/drm/i915/gt/intel_gt.h | 2 +
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 100 +-
drivers/gpu/drm/i915/gt/intel_gt_irq.h | 23 +
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 14 +-
drivers/gpu/drm/i915/gt/intel_gt_pm.h | 13 +
.../drm/i915/gt/intel_gt_pm_delayed_work.c | 35 +
.../drm/i915/gt/intel_gt_pm_delayed_work.h | 24 +
drivers/gpu/drm/i915/gt/intel_gt_requests.c | 23 +-
drivers/gpu/drm/i915/gt/intel_gt_requests.h | 7 +-
drivers/gpu/drm/i915/gt/intel_gt_types.h | 10 +
drivers/gpu/drm/i915/gt/intel_lrc_reg.h | 1 -
drivers/gpu/drm/i915/gt/intel_reset.c | 58 +-
.../gpu/drm/i915/gt/intel_ring_submission.c | 73 +-
drivers/gpu/drm/i915/gt/intel_rps.c | 6 +-
drivers/gpu/drm/i915/gt/intel_workarounds.c | 46 +-
.../gpu/drm/i915/gt/intel_workarounds_types.h | 1 +
drivers/gpu/drm/i915/gt/mock_engine.c | 58 +-
drivers/gpu/drm/i915/gt/selftest_context.c | 10 +
drivers/gpu/drm/i915/gt/selftest_execlists.c | 58 +-
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 6 +-
drivers/gpu/drm/i915/gt/selftest_lrc.c | 6 +-
drivers/gpu/drm/i915/gt/selftest_reset.c | 2 +-
.../drm/i915/gt/selftest_ring_submission.c | 2 +-
.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 177 +
.../gt/uc/abi/guc_communication_ctb_abi.h | 192 +
.../gt/uc/abi/guc_communication_mmio_abi.h | 35 +
.../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h | 13 +
.../gpu/drm/i915/gt/uc/abi/guc_messages_abi.h | 247 +
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 194 +-
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 131 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 484 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.h | 3 +
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 1088 +++--
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 49 +-
.../gpu/drm/i915/gt/uc/intel_guc_debugfs.c | 56 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 377 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4037 +++++++++++++++--
.../gpu/drm/i915/gt/uc/intel_guc_submission.h | 20 +-
.../i915/gt/uc/intel_guc_submission_types.h | 55 +
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 116 +-
drivers/gpu/drm/i915/gt/uc/intel_uc.h | 11 +
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 25 +-
.../i915/gt/uc/selftest_guc_flow_control.c | 589 +++
drivers/gpu/drm/i915/i915_active.c | 3 +
drivers/gpu/drm/i915/i915_debugfs.c | 8 +-
drivers/gpu/drm/i915/i915_debugfs_params.c | 31 +
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem_evict.c | 1 +
drivers/gpu/drm/i915/i915_gpu_error.c | 28 +-
drivers/gpu/drm/i915/i915_irq.c | 10 +-
drivers/gpu/drm/i915/i915_params.h | 2 +-
drivers/gpu/drm/i915/i915_perf.c | 16 +-
drivers/gpu/drm/i915/i915_reg.h | 2 +
drivers/gpu/drm/i915/i915_request.c | 218 +-
drivers/gpu/drm/i915/i915_request.h | 37 +-
drivers/gpu/drm/i915/i915_scheduler.c | 188 +-
drivers/gpu/drm/i915/i915_scheduler.h | 74 +-
drivers/gpu/drm/i915/i915_scheduler_types.h | 74 +
drivers/gpu/drm/i915/i915_trace.h | 219 +-
drivers/gpu/drm/i915/i915_utils.h | 5 +
drivers/gpu/drm/i915/i915_vma.h | 5 -
drivers/gpu/drm/i915/intel_wakeref.c | 5 +
drivers/gpu/drm/i915/intel_wakeref.h | 1 +
.../drm/i915/selftests/i915_live_selftests.h | 1 +
.../gpu/drm/i915/selftests/igt_live_test.c | 2 +-
.../i915/selftests/intel_scheduler_helpers.c | 101 +
.../i915/selftests/intel_scheduler_helpers.h | 37 +
.../gpu/drm/i915/selftests/mock_gem_device.c | 3 +-
include/uapi/drm/i915_drm.h | 9 +
93 files changed, 8954 insertions(+), 2222 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_pm_delayed_work.c
create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_pm_delayed_work.h
create mode 100644 drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
create mode 100644 drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
create mode 100644 drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
create mode 100644 drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
create mode 100644 drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_submission_types.h
create mode 100644 drivers/gpu/drm/i915/gt/uc/selftest_guc_flow_control.c
create mode 100644 drivers/gpu/drm/i915/selftests/intel_scheduler_helpers.c
create mode 100644 drivers/gpu/drm/i915/selftests/intel_scheduler_helpers.h
