Skip to content
  1. Jul 09, 2021
    • Jason Ekstrand's avatar
      drm/i915/gt: Drop i915_address_space::file (v2) · 8579d37e
      Jason Ekstrand authored
      There's a big comment saying how useful it is but no one is using this
      for anything anymore.
      
      It was added in 2bfa996e ("drm/i915: Store owning file on the
      i915_address_space") and used for debugfs at the time as well as telling
      the difference between the global GTT and a PPGTT.  In f6e8aa38
      ("drm/i915: Report the number of closed vma held by each context in
      debugfs") we removed one use of it by switching to a context walk and
      comparing with the VM in the context.  Finally, VM stats for debugfs
      were entirely nuked in db80a129
      
       ("drm/i915/gem: Remove per-client
      stats from debugfs/i915_gem_objects")
      
      v2 (Daniel Vetter):
       - Delete a struct drm_i915_file_private pre-declaration
       - Add a comment to the commit message about history
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-24-jason@jlekstrand.net
      8579d37e
    • Jason Ekstrand's avatar
      drm/i915/gem: Return an error ptr from context_lookup · 046d1660
      Jason Ekstrand authored
      
      
      We're about to start doing lazy context creation which means contexts
      get created in i915_gem_context_lookup and we may start having more
      errors than -ENOENT.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-23-jason@jlekstrand.net
      046d1660
    • Jason Ekstrand's avatar
      drm/i915/gem: Use the proto-context to handle create parameters (v5) · d4433c76
      Jason Ekstrand authored
      
      
      This means that the proto-context needs to grow support for engine
      configuration information as well as setparam logic.  Fortunately, we'll
      be deleting a lot of setparam logic on the primary context shortly so it
      will hopefully balance out.
      
      There's an extra bit of fun here when it comes to setting SSEU and the
      way it interacts with PARAM_ENGINES.  Unfortunately, thanks to
      SET_CONTEXT_PARAM and not being allowed to pick the order in which we
      handle certain parameters, we have think about those interactions.
      
      v2 (Daniel Vetter):
       - Add a proto_context_free_user_engines helper
       - Comment on SSEU in the commit message
       - Use proto_context_set_persistence in set_proto_ctx_param
      
      v3 (Daniel Vetter):
       - Fix a doc comment
       - Do an explicit HAS_FULL_PPGTT check in set_proto_ctx_vm instead of
         relying on pc->vm != NULL.
       - Handle errors for CONTEXT_PARAM_PERSISTENCE
       - Don't allow more resetting user engines
       - Rework initialization of UCONTEXT_PERSISTENCE
      
      v4 (Jason Ekstrand):
       - Move hand-rolled initialization of UCONTEXT_PERSISTENCE to an
         earlier patch
      
      v5 (Jason Ekstrand):
       - Move proto_context_set_persistence to this patch
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-22-jason@jlekstrand.net
      d4433c76
    • Jason Ekstrand's avatar
      drm/i915/gem: Make an alignment check more sensible · def25b7b
      Jason Ekstrand authored
      
      
      What we really want to check is that size of the engines array, i.e.
      args->size - sizeof(*user) is divisible by the element size, i.e.
      sizeof(*user->engines) because that's what's required for computing the
      array length right below the check.  However, we're currently not doing
      this and instead doing a compile-time check that sizeof(*user) is
      divisible by sizeof(*user->engines) and avoiding the subtraction.  As
      far as I can tell, the only reason for the more confusing pair of checks
      is to avoid a single subtraction of a constant.
      
      The other thing the BUILD_BUG_ON might be trying to implicitly check is
      that offsetof(user->engines) == sizeof(*user) and we don't have any
      weird padding throwing us off.  However, that's not the check it's doing
      and it's not even a reliable way to do that check.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-21-jason@jlekstrand.net
      def25b7b
    • Jason Ekstrand's avatar
      drm/i915: Add an i915_gem_vm_lookup helper · bc2ceb7a
      Jason Ekstrand authored
      
      
      This is the VM equivalent of i915_gem_context_lookup.  It's only used
      once in this patch but future patches will need to duplicate this lookup
      code so it's better to have it in a helper.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-20-jason@jlekstrand.net
      bc2ceb7a
    • Jason Ekstrand's avatar
      drm/i915/gem: Optionally set SSEU in intel_context_set_gem · 263ae12c
      Jason Ekstrand authored
      
      
      For now this is a no-op because everyone passes in a null SSEU but it
      lets us get some of the error handling and selftest refactoring plumbed
      through.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-19-jason@jlekstrand.net
      263ae12c
    • Jason Ekstrand's avatar
      drm/i915/gem: Rework error handling in default_engines · 07a635a8
      Jason Ekstrand authored
      
      
      Since free_engines works for partially constructed engine sets, we can
      use the usual goto pattern.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-18-jason@jlekstrand.net
      07a635a8
    • Jason Ekstrand's avatar
      drm/i915/gem: Add an intermediate proto_context struct (v5) · a34857dc
      Jason Ekstrand authored
      
      
      The current context uAPI allows for two methods of setting context
      parameters: SET_CONTEXT_PARAM and CONTEXT_CREATE_EXT_SETPARAM.  The
      former is allowed to be called at any time while the later happens as
      part of GEM_CONTEXT_CREATE.  Currently, everything settable via one is
      settable via the other.  While some params are fairly simple and setting
      them on a live context is harmless such the context priority, others are
      far trickier such as the VM or the set of engines.  In order to swap out
      the VM, for instance, we have to delay until all current in-flight work
      is complete, swap in the new VM, and then continue.  This leads to a
      plethora of potential race conditions we'd really rather avoid.
      
      Unfortunately, both methods of setting the VM and the engine set are in
      active use today so we can't simply disallow setting the VM or engine
      set vial SET_CONTEXT_PARAM.  In order to work around this wart, this
      commit adds a proto-context struct which contains all the context create
      parameters.
      
      v2 (Daniel Vetter):
       - Better commit message
       - Use __set/clear_bit instead of set/clear_bit because there's no race
         and we don't need the atomics
      
      v3 (Daniel Vetter):
       - Use manual bitops and BIT() instead of __set_bit
      
      v4 (Daniel Vetter):
       - Add a changelog to the commit message
       - Better hyperlinking in docs
       - Create the default PPGTT in i915_gem_create_context
      
      v5 (Daniel Vetter):
       - Hand-roll the initialization of UCONTEXT_PERSISTENCE
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-17-jason@jlekstrand.net
      a34857dc
    • Jason Ekstrand's avatar
      drm/i915: Add gem/i915_gem_context.h to the docs · f8a9a5c2
      Jason Ekstrand authored
      
      
      In order to prevent kernel doc warnings, also fill out docs for any
      missing fields and fix those that forgot the "@".
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-16-jason@jlekstrand.net
      f8a9a5c2
    • Jason Ekstrand's avatar
      drm/i915/gem: Add a separate validate_priority helper · aaa5957c
      Jason Ekstrand authored
      
      
      With the proto-context stuff added later in this series, we end up
      having to duplicate set_priority.  This lets us avoid duplicating the
      validation logic.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-15-jason@jlekstrand.net
      aaa5957c
    • Jason Ekstrand's avatar
      drm/i915: Stop manually RCU banging in reset_stats_ioctl (v2) · a4839cb1
      Jason Ekstrand authored
      
      
      As far as I can tell, the only real reason for this is to avoid taking a
      reference to the i915_gem_context.  The cost of those two atomics
      probably pales in comparison to the cost of the ioctl itself so we're
      really not buying ourselves anything here.  We're about to make context
      lookup a tiny bit more complicated, so let's get rid of the one hand-
      rolled case.
      
      Some usermode drivers such as our Vulkan driver call GET_RESET_STATS on
      every execbuf so the perf here could theoretically be an issue.  If this
      ever does become a performance issue for any such userspace drivers,
      they can use set CONTEXT_PARAM_RECOVERABLE to false and look for -EIO
      coming from execbuf to check for hangs instead.
      
      v2 (Daniel Vetter):
       - Add a comment in the commit message about recoverable contexts
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-14-jason@jlekstrand.net
      a4839cb1
    • Jason Ekstrand's avatar
      drm/i915/gem: Disallow creating contexts with too many engines · ebb1ca74
      Jason Ekstrand authored
      
      
      There's no sense in allowing userspace to create more engines than it
      can possibly access via execbuf.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-13-jason@jlekstrand.net
      ebb1ca74
    • Jason Ekstrand's avatar
      drm/i915/request: Remove the hook from await_execution · 5ac545b8
      Jason Ekstrand authored
      
      
      This was only ever used for FENCE_SUBMIT automatic engine selection
      which was removed in the previous commit.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-12-jason@jlekstrand.net
      5ac545b8
    • Jason Ekstrand's avatar
      drm/i915/gem: Remove engine auto-magic with FENCE_SUBMIT (v2) · dd4f1bba
      Jason Ekstrand authored
      
      
      Even though FENCE_SUBMIT is only documented to wait until the request in
      the in-fence starts instead of waiting until it completes, it has a bit
      more magic than that.  If FENCE_SUBMIT is used to submit something to a
      balanced engine, we would wait to assign engines until the primary
      request was ready to start and then attempt to assign it to a different
      engine than the primary.  There is an IGT test (the bonded-slice subtest
      of gem_exec_balancer) which exercises this by submitting a primary batch
      to a specific VCS and then using FENCE_SUBMIT to submit a secondary
      which can run on any VCS and have i915 figure out which VCS to run it on
      such that they can run in parallel.
      
      However, this functionality has never been used in the real world.  The
      media driver (the only user of FENCE_SUBMIT) always picks exactly two
      physical engines to bond and never asks us to pick which to use.
      
      v2 (Daniel Vetter):
       - Mention the exact IGT test this breaks
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-11-jason@jlekstrand.net
      dd4f1bba
    • Jason Ekstrand's avatar
      drm/i915/gem: Disallow bonding of virtual engines (v3) · 521695c6
      Jason Ekstrand authored
      
      
      This adds a bunch of complexity which the media driver has never
      actually used.  The media driver does technically bond a balanced engine
      to another engine but the balanced engine only has one engine in the
      sibling set.  This doesn't actually result in a virtual engine.
      
      This functionality was originally added to handle cases where we may
      have more than two video engines and media might want to load-balance
      their bonded submits by, for instance, submitting to a balanced vcs0-1
      as the primary and then vcs2-3 as the secondary.  However, no such
      hardware has shipped thus far and, if we ever want to enable such
      use-cases in the future, we'll use the up-and-coming parallel submit API
      which targets GuC submission.
      
      This makes I915_CONTEXT_ENGINES_EXT_BOND a total no-op.  We leave the
      validation code in place in case we ever decide we want to do something
      interesting with the bonding information.
      
      v2 (Jason Ekstrand):
       - Don't delete quite as much code.
      
      v3 (Tvrtko Ursulin):
       - Add some history to the commit message
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-10-jason@jlekstrand.net
      521695c6
    • Jason Ekstrand's avatar
      drm/i915: Drop getparam support for I915_CONTEXT_PARAM_ENGINES · c7a71fc8
      Jason Ekstrand authored
      
      
      This has never been used by any userspace except IGT and provides no
      real functionality beyond parroting back parameters userspace passed in
      as part of context creation or via setparam.  If the context is in
      legacy mode (where you use I915_EXEC_RENDER and friends), it returns
      success with zero data so it's not useful for discovering what engines
      are in the context.  It's also not a replacement for the recently
      removed I915_CONTEXT_CLONE_ENGINES because it doesn't return any of the
      balancing or bonding information.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-9-jason@jlekstrand.net
      c7a71fc8
    • Jason Ekstrand's avatar
      drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4) · 00dae4d3
      Jason Ekstrand authored
      
      
      This API is entirely unnecessary and I'd love to get rid of it.  If
      userspace wants a single timeline across multiple contexts, they can
      either use implicit synchronization or a syncobj, both of which existed
      at the time this feature landed.  The justification given at the time
      was that it would help GL drivers which are inherently single-timeline.
      However, neither of our GL drivers actually wanted the feature.  i965
      was already in maintenance mode at the time and iris uses syncobj for
      everything.
      
      Unfortunately, as much as I'd love to get rid of it, it is used by the
      media driver so we can't do that.  We can, however, do the next-best
      thing which is to embed a syncobj in the context and do exactly what
      we'd expect from userspace internally.  This isn't an entirely identical
      implementation because it's no longer atomic if userspace races with
      itself by calling execbuffer2 twice simultaneously from different
      threads.  It won't crash in that case; it just doesn't guarantee any
      ordering between those two submits.  It also means that sync files
      exported from different engines on a SINGLE_TIMELINE context will have
      different fence contexts.  This is visible to userspace if it looks at
      the obj_name field of sync_fence_info.
      
      Moving SINGLE_TIMELINE to a syncobj emulation has a couple of technical
      advantages beyond mere annoyance.  One is that intel_timeline is no
      longer an api-visible object and can remain entirely an implementation
      detail.  This may be advantageous as we make scheduler changes going
      forward.  Second is that, together with deleting the CLONE_CONTEXT API,
      we should now have a 1:1 mapping between intel_context and
      intel_timeline which may help us reduce locking.
      
      v2 (Tvrtko Ursulin):
       - Update the comment on i915_gem_context::syncobj to mention that it's
         an emulation and the possible race if userspace calls execbuffer2
         twice on the same context concurrently.
      v2 (Jason Ekstrand):
       - Wrap the checks for eb.gem_context->syncobj in unlikely()
       - Drop the dma_fence reference
       - Improved commit message
      
      v3 (Jason Ekstrand):
       - Move the dma_fence_put() to before the error exit
      
      v4 (Tvrtko Ursulin):
       - Add a comment about fence contexts to the commit message
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-8-jason@jlekstrand.net
      00dae4d3
    • Jason Ekstrand's avatar
      drm/i915: Drop the CONTEXT_CLONE API (v2) · 4a766ae4
      Jason Ekstrand authored
      
      
      This API allows one context to grab bits out of another context upon
      creation.  It can be used as a short-cut for setparam(getparam()) for
      things like I915_CONTEXT_PARAM_VM.  However, it's never been used by any
      real userspace.  It's used by a few IGT tests and that's it.  Since it
      doesn't add any real value (most of the stuff you can CLONE you can copy
      in other ways), drop it.
      
      There is one thing that this API allows you to clone which you cannot
      clone via getparam/setparam: timelines.  However, timelines are an
      implementation detail of i915 and not really something that needs to be
      exposed to userspace.  Also, sharing timelines between contexts isn't
      obviously useful and supporting it has the potential to complicate i915
      internally.  It also doesn't add any functionality that the client can't
      get in other ways.  If a client really wants a shared timeline, they can
      use a syncobj and set it as an in and out fence on every submit.
      
      v2 (Jason Ekstrand):
       - More detailed commit message
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-7-jason@jlekstrand.net
      4a766ae4
    • Jason Ekstrand's avatar
      drm/i915/gem: Return void from context_apply_all · 8cc256a2
      Jason Ekstrand authored
      
      
      None of the callbacks we use with it return an error code anymore; they
      all return 0 unconditionally.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-6-jason@jlekstrand.net
      8cc256a2
    • Jason Ekstrand's avatar
      drm/i915/gem: Set the watchdog timeout directly in intel_context_set_gem (v2) · 677db6ad
      Jason Ekstrand authored
      
      
      Instead of handling it like a context param, unconditionally set it when
      intel_contexts are created.  For years we've had the idea of a watchdog
      uAPI floating about. The aim was for media, so that they could set very
      tight deadlines for their transcodes jobs, so that if you have a corrupt
      bitstream (especially for decoding) you don't hang your desktop too
      hard.  But it's been stuck in limbo since forever, and this simplifies
      things a bit in preparation for the proto-context work.  If we decide to
      actually make said uAPI a reality, we can do it through the proto-
      context easily enough.
      
      This does mean that we move from reading the request_timeout_ms param
      once per engine when engines are created instead of once at context
      creation.  If someone changes request_timeout_ms between creating a
      context and setting engines, it will mean that they get the new timeout.
      If someone races setting request_timeout_ms and context creation, they
      can theoretically end up with different timeouts.  However, since both
      of these are fairly harmless and require changing kernel params, we
      don't care.
      
      v2 (Tvrtko Ursulin):
       - Add a comment about races with request_timeout_ms
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-5-jason@jlekstrand.net
      677db6ad
    • Jason Ekstrand's avatar
      drm/i915: Drop I915_CONTEXT_PARAM_NO_ZEROMAP · 6ff6d61d
      Jason Ekstrand authored
      
      
      The idea behind this param is to support OpenCL drivers with relocations
      because OpenCL reserves 0x0 for NULL and, if we placed memory there, it
      would confuse CL kernels.  It was originally sent out as part of a patch
      series including libdrm [1] and Beignet [2] support.  However, the
      libdrm and Beignet patches never landed in their respective upstream
      projects so this API has never been used.  It's never been used in Mesa
      or any other driver, either.
      
      Dropping this API allows us to delete a small bit of code.
      
      [1]: https://lists.freedesktop.org/archives/intel-gfx/2015-May/067030.html
      [2]: https://lists.freedesktop.org/archives/intel-gfx/2015-May/067031.html
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-4-jason@jlekstrand.net
      6ff6d61d
    • Jason Ekstrand's avatar
      drm/i915: Stop storing the ring size in the ring pointer (v3) · 74e4b909
      Jason Ekstrand authored
      
      
      Previously, we were storing the ring size in the ring pointer before it
      was actually allocated.  We would then guard setting the ring size on
      checking for CONTEXT_ALLOC_BIT.  This is error-prone at best and really
      only saves us a few bytes on something that already burns at least 4K.
      Instead, this patch adds a new ring_size field and makes everything use
      that.
      
      v2 (Daniel Vetter):
       - Replace 512 * SZ_4K with SZ_2M
      
      v2 (Jason Ekstrand):
       - Rebase on top of page migration code
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-3-jason@jlekstrand.net
      74e4b909
    • Jason Ekstrand's avatar
      drm/i915: Drop I915_CONTEXT_PARAM_RINGSIZE · fe4751c3
      Jason Ekstrand authored
      This reverts commit 88be76cd
      
       ("drm/i915: Allow userspace to specify
      ringsize on construction").  This API was originally added for OpenCL
      but the compute-runtime PR has sat open for a year without action so we
      can still pull it out if we want.  I argue we should drop it for three
      reasons:
      
       1. If the compute-runtime PR has sat open for a year, this clearly
          isn't that important.
      
       2. It's a very leaky API.  Ring size is an implementation detail of the
          current execlist scheduler and really only makes sense there.  It
          can't apply to the older ring-buffer scheduler on pre-execlist
          hardware because that's shared across all contexts and it won't
          apply to the GuC scheduler that's in the pipeline.
      
       3. Having userspace set a ring size in bytes is a bad solution to the
          problem of having too small a ring.  There is no way that userspace
          has the information to know how to properly set the ring size so
          it's just going to detect the feature and always set it to the
          maximum of 512K.  This is what the compute-runtime PR does.  The
          scheduler in i915, on the other hand, does have the information to
          make an informed choice.  It could detect if the ring size is a
          problem and grow it itself.  Or, if that's too hard, we could just
          increase the default size from 16K to 32K or even 64K instead of
          relying on userspace to do it.
      
      Let's drop this API for now and, if someone decides they really care
      about solving this problem, they can do it properly.
      
      Signed-off-by: default avatarJason Ekstrand <jason@jlekstrand.net>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-2-jason@jlekstrand.net
      fe4751c3
    • John Harrison's avatar
      drm/i915/adlp: Add ADL-P GuC/HuC firmware files · 520dfc80
      John Harrison authored
      
      
      Add ADL-P to the list of supported GuC and HuC firmware versions. For
      HuC, it reuses the existing TGL firmware file. For GuC, there is a
      dedicated firmware release.
      
      Signed-off-by: default avatarJohn Harrison <John.C.Harrison@Intel.com>
      Reviewed-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210626004522.1699509-3-John.C.Harrison@Intel.com
      520dfc80
    • John Harrison's avatar
      drm/i915/huc: Update TGL and friends to HuC 7.9.3 · 4a832721
      John Harrison authored
      
      
      A new HuC is available for TGL and compatible platforms, so switch to
      using it.
      
      Signed-off-by: default avatarJohn Harrison <John.C.Harrison@Intel.com>
      Reviewed-by: default avatarDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210626004522.1699509-2-John.C.Harrison@Intel.com
      4a832721
  2. Jul 08, 2021
    • Tejas Upadhyay's avatar
      drm/i915/adl_s: Fix dma_mask_size to 39 bit · 88c6317b
      Tejas Upadhyay authored
      
      
      46 bit addressing enables you to use 4 bits  to support some
      MKTME features, and 3 more bits for Optane support that uses
      a subset of MTKME for persistent memory.
      
      But GTT addressing sticking to 39 bit addressing, thus setting
      dma_mask_size to 39 fixes below tests :
      igt@i915_selftest@live@mman
      igt@kms_big_fb@linear-32bpp-rotate-0
      igt@gem_create@create-clear
      igt@gem_mmap_offset@clear
      igt@gem_mmap_gtt@cpuset-big-copy
      
      In a way solves Gitlab#3142
      https://gitlab.freedesktop.org/drm/intel/-/issues/3142, which had
      following errors :
      DMAR: DRHD: handling fault status reg 2
      DMAR: [DMA Write] Request device [00:02.0] PASID ffffffff fault addr
      7effff9000 [fault reason 05] PTE Write access is not set
      
      0x7effff9000 is suspiciously exactly 39 bits, so it seems likely that
      the HW just ends up masking off those extra bits hence DMA errors.
      
      Changes since V2 :
      	- dim checkpatch error solved
      Changes since V1 :
      	- Added more details to commit message - Matthew Auld
      
      Signed-off-by: default avatarTejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
      Acked-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210708071222.955455-1-tejaskumarx.surendrakumar.upadhyay@intel.com
      88c6317b
    • Lucas De Marchi's avatar
      drm/i915/gt: finish INTEL_GEN and friends conversion · 7c517f83
      Lucas De Marchi authored
      Commit c816723b
      
       ("drm/i915/gt: replace IS_GEN and friends with
      GRAPHICS_VER") converted INTEL_GEN and friends to the new version check
      macros.  Meanwhile, some changes sneaked in to use INTEL_GEN. Remove the
      last users so we can remove the macros.
      
      Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
      Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210707181325.2130821-2-lucas.demarchi@intel.com
      7c517f83
  3. Jul 07, 2021
  4. Jul 06, 2021
    • Daniel Vetter's avatar
      drm/i915: Improve debug Kconfig texts a bit · 7e8376f1
      Daniel Vetter authored
      We're not consistently recommending these for developers only.
      
      I stumbled over this due to DRM_I915_LOW_LEVEL_TRACEPOINTS, which was
      added in
      
      commit 354d036f
      
      
      Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
      Date:   Tue Feb 21 11:01:42 2017 +0000
      
          drm/i915/tracepoints: Add request submit and execute tracepoints
      
      to "alleviate the performance impact concerns."
      
      Which is nonsense.
      
      Tvrtko and Joonas pointed out on irc that the real (but undocumented
      reason) was stable abi concerns for tracepoints, see
      
      https://lwn.net/Articles/705270/
      
      and the specific change that was blocked around tracepoints:
      
      https://lwn.net/Articles/442113/
      
      Anyway to make it a notch clearer why we have this Kconfig option
      consistly add the "Recommended for driver developers only." to it and
      all the other debug options we have.
      
      Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Cc: Matthew Brost <matthew.brost@intel.com>
      Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210702201708.2075793-1-daniel.vetter@ffwll.ch
      7e8376f1
  5. Jul 03, 2021
    • Thomas Zimmermann's avatar
      drm/i915: Drop all references to DRM IRQ midlayer · 91b96f00
      Thomas Zimmermann authored
      
      
      Remove all references to DRM's IRQ midlayer. i915 uses Linux' interrupt
      functions directly.
      
      v2:
      	* also remove an outdated comment
      	* move IRQ fix into separate patch
      	* update Fixes tag (Daniel)
      
      Signed-off-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
      Fixes: b318b824
      
       ("drm/i915: Nuke drm_driver irq vfuncs")
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Jani Nikula <jani.nikula@linux.intel.com>
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
      Cc: intel-gfx@lists.freedesktop.org
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210701173618.10718-3-tzimmermann@suse.de
      91b96f00
    • Thomas Zimmermann's avatar
      drm/i915: Use the correct IRQ during resume · 27e4b467
      Thomas Zimmermann authored
      
      
      The code in xcs_resume() probably didn't work as intended. It uses
      struct drm_device.irq, which is allocated to 0, but never initialized
      by i915 to the device's interrupt number.
      
      Change all calls to synchronize_hardirq() to intel_synchronize_irq(),
      which uses the correct interrupt. _hardirq() functions are not needed
      in this context.
      
      v5:
      	* go back to _hardirq() after PCI probe reported wrong
      	  context; add rsp comment
      v4:
      	* switch everything to intel_synchronize_irq() (Daniel)
      v3:
      	* also use intel_synchronize_hardirq() at another callsite
      v2:
      	* wrap irq code in intel_synchronize_hardirq() (Ville)
      
      Signed-off-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
      Fixes: 536f77b1
      
       ("drm/i915/gt: Call stop_ring() from ring resume, again")
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Lucas De Marchi <lucas.demarchi@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210701173618.10718-2-tzimmermann@suse.de
      27e4b467
  6. Jun 30, 2021
    • Matthew Auld's avatar
      drm/i915/gtt: ignore min_page_size for paging structures · 32334c9b
      Matthew Auld authored
      
      
      The min_page_size is only needed for pages inserted into the GTT, and
      for our paging structures we only need at most 4K bytes, so simply
      ignore the min_page_size restrictions here, otherwise we might see some
      severe overallocation on some devices.
      
      v2(Thomas): add some commentary
      
      Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
      Reviewed-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210625103824.558481-2-matthew.auld@intel.com
      32334c9b
    • Matthew Auld's avatar
      drm/i915: support forcing the page size with lmem · d22632c8
      Matthew Auld authored
      
      
      For some specialised objects we might need something larger than the
      regions min_page_size due to some hw restriction, and slightly more
      hairy is needing something smaller with the guarantee that such objects
      will never be inserted into any GTT, which is the case for the paging
      structures.
      
      This also fixes how we setup the BO page_alignment, if we later migrate
      the object somewhere else. For example if the placements are {SMEM,
      LMEM}, then we might get this wrong. Pushing the min_page_size behaviour
      into the manager should fix this.
      
      v2(Thomas): push the default page size behaviour into buddy_man, and let
      the user override it with the page-alignment, which looks cleaner
      
      v3: rebase on ttm sys changes
      
      Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
      Reviewed-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210625103824.558481-1-matthew.auld@intel.com
      d22632c8
    • Thomas Hellström's avatar
      drm/i915/display: Migrate objects to LMEM if possible for display · e11b7b6e
      Thomas Hellström authored
      
      
      Objects intended to be used as display framebuffers must reside in
      LMEM for discrete. If they happen to not do that, migrate them to
      LMEM before pinning.
      
      Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210629151203.209465-4-thomas.hellstrom@linux.intel.com
      e11b7b6e
    • Matthew Auld's avatar
      drm/i915/gem: Introduce a selftest for the gem object migrate functionality · bf74a18c
      Matthew Auld authored
      
      
      A selftest for the gem object migrate functionality. Slightly adapted
      from the original by Matthew to the new interface and new fill blit
      code.
      
      v4:
      - Initialize buffers and check contents after migration
        (Suggested by Matthew Auld)
      - Perform async migration (if implemented) in the igt_lmem_pages_migrate
        test
      - Test also migration to the current region.
      
      Co-developed-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> #v3
      Link: https://patchwork.freedesktop.org/patch/msgid/20210629151203.209465-3-thomas.hellstrom@linux.intel.com
      bf74a18c
    • Thomas Hellström's avatar
      drm/i915/gem: Implement object migration · b6e913e1
      Thomas Hellström authored
      
      
      Introduce an interface to migrate objects between regions.
      This is primarily intended to migrate objects to LMEM for display and
      to SYSTEM for dma-buf, but might be reused in one form or another for
      performance-based migration.
      
      v2:
      - Verify that the memory region given as an id really exists.
        (Reported by Matthew Auld)
      - Call i915_gem_object_{init,release}_memory_region() when switching region
        to handle also switching region lists. (Reported by Matthew Auld)
      v3:
      - Fix i915_gem_object_can_migrate() to return true if object is already in
        the correct region, even if the object ops doesn't have a migrate()
        callback.
      - Update typo in commit message.
      - Fix kerneldoc of i915_gem_object_wait_migration().
      v4:
      - Improve documentation (Suggested by Mattew Auld and Michael Ruhl)
      - Always assume TTM migration hits a TTM move and unsets the pages through
        move_notify. (Reported by Matthew Auld)
      - Add a dma_fence_might_wait() annotation to
        i915_gem_object_wait_migration() (Suggested by Daniel Vetter)
      v5:
      - Re-add might_sleep() instead of __dma_fence_might_wait(), Sent
        v4 with the wrong version, didn't compile and __dma_fence_might_wait()
        is not exported.
      - Added an R-B.
      
      Reported-by: default avatarkernel test robot <lkp@intel.com>
      Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Reviewed-by: default avatarMichael J. Ruhl <michael.j.ruhl@intel.com>
      Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210629151203.209465-2-thomas.hellstrom@linux.intel.com
      b6e913e1
    • Matthew Brost's avatar
      drm/doc/rfc: i915 new parallel submission uAPI plan · 0454a490
      Matthew Brost authored
      
      
      Add entry for i915 new parallel submission uAPI plan.
      
      v2:
       (Daniel Vetter):
        - Expand logical order explaination
        - Add dummy header
        - Only allow N BBs in execbuf IOCTL
        - Configure parallel submission per slot not per gem context
      v3:
       (Marcin Ślusarz):
        - Lot's of typos / bad english fixed
       (Tvrtko Ursulin):
        - Consistent pseudo code, clean up wording in descriptions
      v4:
       (Daniel Vetter)
        - Drop flags
        - Add kernel doc
        - Reword a few things / fix typos
       (Tvrtko)
        - Reword a few things / fix typos
      v5:
       (Checkpatch)
        - Fix typos
       (Docs)
        - Fix warning
      
      Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
      Cc: Tony Ye <tony.ye@intel.com>
      CC: Carl Zhang <carl.zhang@intel.com>
      Cc: Daniel Vetter <daniel.vetter@intel.com>
      Cc: Jason Ekstrand <jason@jlekstrand.net>
      Signed-off-by: default avatarMatthew Brost <matthew.brost@intel.com>
      Acked-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Acked-by: default avatarTony Ye <tony.ye@intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210629193511.124099-3-matthew.brost@intel.com
      0454a490
    • Matthew Brost's avatar
      drm/doc/rfc: i915 GuC submission / DRM scheduler · f587623b
      Matthew Brost authored
      
      
      Add entry for i915 GuC submission / DRM scheduler integration plan.
      Follow up patch with details of new parallel submission uAPI to come.
      
      v2:
       (Daniel Vetter)
        - Expand explaination of why bonding isn't supported for GuC
          submission
        - CC some of the DRM scheduler maintainers
        - Add priority inheritance / boosting use case
        - Add reasoning for removing in order assumptions
       (Daniel Stone)
        - Add links to priority spec
      v4:
       (Tvrtko)
        - Add TODOs section
       (Daniel Vetter)
        - Pull in 1 line from following patch
      v5:
       (Checkpatch)
        - Fix typos
      
      Cc: Christian König <christian.koenig@amd.com>
      Cc: Luben Tuikov <luben.tuikov@amd.com>
      Cc: Alex Deucher <alexander.deucher@amd.com>
      Cc: Steven Price <steven.price@arm.com>
      Cc: Jon Bloomfield <jon.bloomfield@intel.com>
      Cc: Dave Airlie <airlied@gmail.com>
      Cc: Daniel Vetter <daniel.vetter@intel.com>
      Cc: Jason Ekstrand <jason@jlekstrand.net>
      Cc: dri-devel@lists.freedesktop.org
      Signed-off-by: default avatarMatthew Brost <matthew.brost@intel.com>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Acked-by: default avatarDave Airlie <airlied@redhat.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210629193511.124099-2-matthew.brost@intel.com
      f587623b
  7. Jun 25, 2021