1. 07 Oct, 2021 1 commit
    • Lucas De Marchi's avatar
      drm/i915: remove IS_ACTIVE · 1a839e01
      Lucas De Marchi authored
      When trying to bring IS_ACTIVE to linux/kconfig.h I thought it wouldn't
      provide much value just encapsulating it in a boolean context. So I also
      added the support for handling undefined macros as the IS_ENABLED()
      counterpart. However the feedback received from Masahiro Yamada was that
      it is too ugly, not providing much value. And just wrapping in a boolean
      context is too dumb - we could simply open code it.
      
      As detailed in commit babaab2f ("drm/i915: Encapsulate kconfig
      constant values inside boolean predicates"), the IS_ACTIVE macro was
      added to workaround a compilation warning. However after checking again
      our current uses of IS_ACTIVE it turned out there is only
      1 case in which it triggers a warning in clang (due
      -Wconstant-logical-operand) and 2 in smatch. All the others
      can simply use the shorter version, without wrapping it in any macro.
      
      So here I'm dialing all the way back to simply removing the macro. That
      single case hit by clang can be changed to make the constant come first,
      so it doesn't think it's mask:
      
      	-       if (context && CONFIG_DRM_I915_FENCE_TIMEOUT)
      	+       if (CONFIG_DRM_I915_FENCE_TIMEOUT && context)
      
      As talked with Dan Carpenter, that logic will be added in smatch as
      well, so it will also stop warning about it.
      Signed-off-by: default avatarLucas De Marchi <lucas.demarchi@intel.com>
      Reviewed-by: default avatarJani Nikula <jani.nikula@intel.com>
      Reviewed-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20211005171728.3147094-1-lucas.demarchi@intel.com
      1a839e01
  2. 06 Oct, 2021 1 commit
  3. 05 Oct, 2021 1 commit
  4. 04 Oct, 2021 18 commits
  5. 01 Oct, 2021 6 commits
  6. 30 Sep, 2021 1 commit
  7. 29 Sep, 2021 1 commit
  8. 27 Sep, 2021 1 commit
  9. 25 Sep, 2021 1 commit
    • Janusz Krzysztofik's avatar
      drm/i915: Flush buffer pools on driver remove · 74af1e2c
      Janusz Krzysztofik authored
      We currently do an explicit flush of the buffer pools within the call path
      of drm_driver.release(); this removes all buffers, regardless of their age,
      freeing the buffers' associated resources (objects, address space areas).
      However there is other code that runs within the drm_driver.release() call
      chain that expects objects and their associated address space areas have
      already been flushed.
      
      Since buffer pools auto-flush old buffers once per second in a worker
      thread, there's a small window where if we remove the driver while there
      are still objects in buffers with an age of less than one second, the
      assumptions of the other release code may be violated.
      
      By moving the flush to driver remove (which executes earlier via the
      pci_driver.remove() flow) we're ensuring that all buffers are flushed and
      their associated objects freed before some other code in
      pci_driver.remove() flushes those objects so they are released before
      _any_ code in drm_driver.release() that check completness of those
      flushes executes.
      
      v2: Reword commit description as suggested by Matt.
      Signed-off-by: default avatarJanusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Matt Roper <matthew.d.roper@intel.com>
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210924163825.634606-1-janusz.krzysztofik@linux.intel.com
      74af1e2c
  10. 24 Sep, 2021 9 commits
    • Tejas Upadhyay's avatar
      drm/i915: Remove warning from the rps worker · a837a068
      Tejas Upadhyay authored
      In commit 4e5c8a99 ("drm/i915: Drop i915_request.lock requirement
      for intel_rps_boost()"), we decoupled the rps worker from the pm so
      that we could avoid the synchronization penalty which makes the
      assertion liable to run too early. Which makes warning invalid hence
      removed.
      
      Fixes: 4e5c8a99 ("drm/i915: Drop i915_request.lock requirement for intel_rps_boost()")
      Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarTejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210914090412.1393498-1-tejaskumarx.surendrakumar.upadhyay@intel.com
      a837a068
    • Matthew Auld's avatar
      drm/i915/selftests: exercise shmem_writeback with THP · 6341eb6f
      Matthew Auld authored
      In commit:
      
      commit 1e6decf3
      Author: Hugh Dickins <hughd@google.com>
      Date:   Thu Sep 2 14:54:43 2021 -0700
      
          shmem: shmem_writepage() split unlikely i915 THP
      
      it looks THP + shmem_writeback was an unexpected combination, and ends up
      hitting some BUG_ON, but it also looks like that is now fixed.
      
      While the IGTs did eventually hit this(although not during pre-merge it
      seems), it's likely worthwhile adding some explicit coverage for this
      scenario in the shrink_thp selftest.
      
      References: https://gitlab.freedesktop.org/drm/intel/-/issues/4166Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
      Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210921142116.3807946-1-matthew.auld@intel.com
      6341eb6f
    • Matthew Auld's avatar
      drm/i915/request: fix early tracepoints · be988eae
      Matthew Auld authored
      Currently we blow up in trace_dma_fence_init, when calling into
      get_driver_name or get_timeline_name, since both the engine and context
      might be NULL(or contain some garbage address) in the case of newly
      allocated slab objects via the request ctor. Note that we also use
      SLAB_TYPESAFE_BY_RCU here, which allows requests to be immediately
      freed, but delay freeing the underlying page by an RCU grace period.
      With this scheme requests can be re-allocated, at the same time as they
      are also being read by some lockless RCU lookup mechanism.
      
      In the ctor case, which is only called for new slab objects(i.e allocate
      new page and call the ctor for each object) it's safe to reset the
      context/engine prior to calling into dma_fence_init, since we can be
      certain that no one is doing an RCU lookup which might depend on peeking
      at the engine/context, like in active_engine(), since the object can't
      yet be externally visible.
      
      In the recycled case(which might also be externally visible) the request
      refcount always transitions from 0->1 after we set the context/engine
      etc, which should ensure it's valid to dereference the engine for
      example, when doing an RCU list-walk, so long as we can also increment
      the refcount first. If the refcount is already zero, then the request is
      considered complete/released.  If it's non-zero, then the request might
      be in the process of being re-allocated, or potentially still in flight,
      however after successfully incrementing the refcount, it's possible to
      carefully inspect the request state, to determine if the request is
      still what we were looking for. Note that all externally visible
      requests returned to the cache must have zero refcount.
      
      One possible fix then is to move dma_fence_init out from the request
      ctor. Originally this was how it was done, but it was moved in:
      
      commit 855e39e6
      Author: Chris Wilson <chris@chris-wilson.co.uk>
      Date:   Mon Feb 3 09:41:48 2020 +0000
      
          drm/i915: Initialise basic fence before acquiring seqno
      
      where it looks like intel_timeline_get_seqno() relied on some of the
      rq->fence state, but that is no longer the case since:
      
      commit 12ca695d
      Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Date:   Tue Mar 23 16:49:50 2021 +0100
      
          drm/i915: Do not share hwsp across contexts any more, v8.
      
      intel_timeline_get_seqno() could also be cleaned up slightly by dropping
      the request argument.
      
      Moving dma_fence_init back out of the ctor, should ensure we have enough
      of the request initialised in case of trace_dma_fence_init.
      Functionally this should be the same, and is effectively what we were
      already open coding before, except now we also assign the fence->lock
      and fence->ops, but since these are invariant for recycled
      requests(which might be externally visible), and will therefore already
      hold the same value, it shouldn't matter.
      
      An alternative fix, since we don't yet have a fully initialised request
      when in the ctor, is just setting the context/engine as NULL, but this
      does require adding some extra handling in get_driver_name etc.
      
      v2(Daniel):
        - Try to make the commit message less confusing
      
      Fixes: 855e39e6 ("drm/i915: Initialise basic fence before acquiring seqno")
      Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Cc: Michael Mason <michael.w.mason@intel.com>
      Cc: Daniel Vetter <daniel@ffwll.ch>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210921134202.3803151-1-matthew.auld@intel.com
      be988eae
    • Thomas Hellström's avatar
      drm/i915: Reduce the number of objects subject to memcpy recover · a259cc14
      Thomas Hellström authored
      We really only need memcpy restore for objects that affect the
      operability of the migrate context. That is, primarily the page-table
      objects of the migrate VM.
      
      Add an object flag, I915_BO_ALLOC_PM_EARLY for objects that need early
      restores using memcpy and a way to assign LMEM page-table object flags
      to be used by the vms.
      
      Restore objects without this flag with the gpu blitter and only objects
      carrying the flag using TTM memcpy.
      
      Initially mark the migrate, gt, gtt and vgpu vms to use this flag, and
      defer for a later audit which vms actually need it. Most importantly, user-
      allocated vms with pinned page-table objects can be restored using the
      blitter.
      
      Performance-wise memcpy restore is probably as fast as gpu restore if not
      faster, but using gpu restore will help tackling future restrictions in
      mappable LMEM size.
      
      v4:
      - Don't mark the aliasing ppgtt page table flags for early resume, but
        rather the ggtt page table flags as intended. (Matthew Auld)
      - The check for user buffer objects during early resume is pointless, since
        they are never marked I915_BO_ALLOC_PM_EARLY. (Matthew Auld)
      v5:
      - Mark GuC LMEM objects with I915_BO_ALLOC_PM_EARLY to have them restored
        before we fire up the migrate context.
      
      Cc: Matthew Brost <matthew.brost@intel.com>
      Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210922062527.865433-8-thomas.hellstrom@linux.intel.com
      a259cc14
    • Thomas Hellström's avatar
      drm/i915: Don't back up pinned LMEM context images and rings during suspend · 0d8ee5ba
      Thomas Hellström authored
      Pinned context images are now reset during resume. Don't back them up,
      and assuming that rings can be assumed empty at suspend, don't back them
      up either.
      
      Introduce a new object flag, I915_BO_ALLOC_PM_VOLATILE meaning that an
      object is allowed to lose its content on suspend.
      
      v3:
      - Slight documentation clarification (Matthew Auld)
      Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210922062527.865433-7-thomas.hellstrom@linux.intel.com
      0d8ee5ba
    • Thomas Hellström's avatar
      drm/i915/gt: Register the migrate contexts with their engines · 3e42cc61
      Thomas Hellström authored
      Pinned contexts, like the migrate contexts need reset after resume
      since their context image may have been lost. Also the GuC needs to
      register pinned contexts.
      
      Add a list to struct intel_engine_cs where we add all pinned contexts on
      creation, and traverse that list at resume time to reset the pinned
      contexts.
      
      This fixes the kms_pipe_crc_basic@suspend-read-crc-pipe-a selftest for now,
      but proper LMEM backup / restore is needed for full suspend functionality.
      However, note that even with full LMEM backup / restore it may be
      desirable to keep the reset since backing up the migrate context images
      must happen using memcpy() after the migrate context has become inactive,
      and for performance- and other reasons we want to avoid memcpy() from
      LMEM.
      
      Also traverse the list at guc_init_lrc_mapping() calling
      guc_kernel_context_pin() for the pinned contexts, like is already done
      for the kernel context.
      
      v2:
      - Don't reset the contexts on each __engine_unpark() but rather at
        resume time (Chris Wilson).
      v3:
      - Reset contexts in the engine sanitize callback. (Chris Wilson)
      
      Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
      Cc: Matthew Auld <matthew.auld@intel.com>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Brost Matthew <matthew.brost@intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210922062527.865433-6-thomas.hellstrom@linux.intel.com
      3e42cc61
    • Thomas Hellström's avatar
      drm/i915 Implement LMEM backup and restore for suspend / resume · c56ce956
      Thomas Hellström authored
      Just evict unpinned objects to system. For pinned LMEM objects,
      make a backup system object and blit the contents to that.
      
      Backup is performed in three steps,
      1: Opportunistically evict evictable objects using the gpu blitter.
      2: After gt idle, evict evictable objects using the gpu blitter. This will
      be modified in an upcoming patch to backup pinned objects that are not used
      by the blitter itself.
      3: Backup remaining pinned objects using memcpy.
      
      Also move uC suspend to after 2) to make sure we have a functional GuC
      during 2) if using GuC submission.
      
      v2:
      - Major refactor to make sure gem_exec_suspend@hang-SX subtests work, and
        suspend / resume works with a slightly modified GuC submission enabling
        patch series.
      
      v3:
      - Fix a potential use-after-free (Matthew Auld)
      - Use i915_gem_object_create_shmem() instead of
        i915_gem_object_create_region (Matthew Auld)
      - Minor simplifications (Matthew Auld)
      - Fix up kerneldoc for i195_ttm_restore_region().
      - Final lmem_suspend() call moved to i915_gem_backup_suspend from
        i915_gem_suspend_late, since the latter gets called at driver unload
        and we don't unnecessarily want to run it at that time.
      
      v4:
      - Interface change of ttm- & lmem suspend / resume functions to use
        flags rather than bools. (Matthew Auld)
      - Completely drop the i915_gem_backup_suspend change (Matthew Auld)
      Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210922062527.865433-5-thomas.hellstrom@linux.intel.com
      c56ce956
    • Thomas Hellström's avatar
      drm/i915/gt: Increase suspend timeout · 81387fc4
      Thomas Hellström authored
      With GuC submission on DG1, the execution of the requests times out
      for the gem_exec_suspend igt test case after executing around 800-900
      of 1000 submitted requests.
      
      Given the time we allow elsewhere for fences to signal (in the order of
      seconds), increase the timeout before we mark the gt wedged and proceed.
      Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Acked-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210922062527.865433-4-thomas.hellstrom@linux.intel.com
      81387fc4
    • Thomas Hellström's avatar
      drm/i915/gem: Implement a function to process all gem objects of a region · d80ee88e
      Thomas Hellström authored
      An upcoming common pattern is to traverse the region object list and
      perform certain actions on all objects in a region. It's a little tricky
      to get the list locking right, in particular since a gem object may
      change region unless it's pinned or the object lock is held.
      
      Define a function that does this for us and that takes an argument that
      defines the action to be performed on each object.
      
      v3:
      - Improve structure documentation a bit (Matthew Auld)
      Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
      Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210922062527.865433-3-thomas.hellstrom@linux.intel.com
      d80ee88e