1. 26 Aug, 2016 2 commits
  2. 25 Aug, 2016 3 commits
    • Lyude's avatar
      drm/i915/skl: Update DDB values atomically with wms/plane attrs · 27082493
      Lyude authored
      Now that we can hook into update_crtcs and control the order in which we
      update CRTCs at each modeset, we can finish the final step of fixing
      Skylake's watermark handling by performing DDB updates at the same time
      as plane updates and watermark updates.
      
      The first major change in this patch is skl_update_crtcs(), which
      handles ensuring that we order each CRTC update in our atomic commits
      properly so that they honor the DDB flush order.
      
      The second major change in this patch is the order in which we flush the
      pipes. While the previous order may have worked, it can't be used in
      this approach since it no longer will do the right thing. For example,
      using the old ddb flush order:
      
      We have pipes A, B, and C enabled, and we're disabling C. Initial ddb
      allocation looks like this:
      
      |   A   |   B   |xxxxxxx|
      
      Since we're performing the ddb updates after performing any CRTC
      disablements in intel_atomic_commit_tail(), the space to the right of
      pipe B is unallocated.
      
      1. Flush pipes with new allocation contained into old space. None
         apply, so we skip this
      2. Flush pipes having their allocation reduced, but overlapping with a
         previous allocation. None apply, so we also skip this
      3. Flush pipes that got more space allocated. This applies to A and B,
         giving us the following update order: A, B
      
      This is wrong, since updating pipe A first will cause it to overlap with
      B and potentially burst into flames. Our new order (see the code
      comments for details) would update the pipes in the proper order: B, A.
      
      As well, we calculate the order for each DDB update during the check
      phase, and reference it later in the commit phase when we hit
      skl_update_crtcs().
      
      This long overdue patch fixes the rest of the underruns on Skylake.
      
      Changes since v1:
       - Add skl_ddb_entry_write() for cursor into skl_write_cursor_wm()
      Changes since v2:
       - Use the method for updating CRTCs that Ville suggested
       - In skl_update_wm(), only copy the watermarks for the crtc that was
         passed to us
      Changes since v3:
       - Small comment fix in skl_ddb_allocation_overlaps()
      Changes since v4:
       - Remove the second loop in intel_update_crtcs() and use Ville's
         suggestion for updating the ddb allocations in the right order
       - Get rid of the second loop and just use the ddb state as it updates
         to determine what order to update everything in (thanks for the
         suggestion Ville)
       - Simplify skl_ddb_allocation_overlaps()
       - Split actual overlap checking into it's own helper
      
      Fixes: 0e8fb7ba ("drm/i915/skl: Flush the WM configuration")
      Fixes: 8211bd5b ("drm/i915/skl: Program the DDB allocation")
      [omitting CC for stable, since this patch will need to be changed for
      such backports first]
      
      Testcase: kms_cursor_legacy
      Testcase: plane-all-modeset-transition
      Signed-off-by: default avatarLyude <cpaul@redhat.com>
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: Daniel Vetter <daniel.vetter@intel.com>
      Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
      Cc: Hans de Goede <hdegoede@redhat.com>
      Cc: Matt Roper <matthew.d.roper@intel.com>
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471961565-28540-2-git-send-email-cpaul@redhat.com
      27082493
    • Lyude's avatar
      drm/i915: Move CRTC updating in atomic_commit into it's own hook · 896e5bb0
      Lyude authored
      Since we have to write ddb allocations at the same time as we do other
      plane updates, we're going to need to be able to control the order in
      which we execute modesets on each pipe. The easiest way to do this is to
      just factor this section of intel_atomic_commit_tail()
      (intel_atomic_commit() for stable branches) into it's own function, and
      add an appropriate display function hook for it.
      
      Based off of Matt Rope's suggestions
      
      Changes since v1:
       - Drop pipe_config->base.active check in intel_update_crtcs() since we
         check that before calling the function
      Signed-off-by: default avatarLyude <cpaul@redhat.com>
      Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      [omitting CC for stable, since this patch will need to be changed for
      such backports first]
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: Daniel Vetter <daniel.vetter@intel.com>
      Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
      Cc: Hans de Goede <hdegoede@redhat.com>
      Signed-off-by: default avatarLyude <cpaul@redhat.com>
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471961565-28540-1-git-send-email-cpaul@redhat.com
      896e5bb0
    • Chris Wilson's avatar
      drm/i915: Restore lost "Initialized i915" welcome message · bc5ca47c
      Chris Wilson authored
      A side effect of removing the midlayer from driver loading was the loss
      of a useful message announcing to userspace that i915 had successfully
      started, e.g.:
      
      	[drm] Initialized i915 1.6.0 20160425 for 0000:00:02.0 on minor 0
      Reported-by: default avatarTimo Aaltonen <tjaalton@ubuntu.com>
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Fixes: 8f460e2c ("drm/i915: Demidlayer driver loading")
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: drm-intel-fixes@lists.freedesktop.org
      Link: http://patchwork.freedesktop.org/patch/msgid/20160825072314.17402-1-chris@chris-wilson.co.ukReviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      bc5ca47c
  3. 24 Aug, 2016 11 commits
  4. 23 Aug, 2016 17 commits
  5. 22 Aug, 2016 7 commits
    • Chris Wilson's avatar
      drm/i915: Take forcewake once for the entire GMBUS transaction · 4e6c2d58
      Chris Wilson authored
      As we do many register reads within a very short period of time, hold
      the GMBUS powerwell from start to finish.
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Cc: David Weinehall <david.weinehall@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160819164503.17845-1-chris@chris-wilson.co.ukReviewed-by: default avatarDavid Weinehall <david.weinehall@linux.intel.com>
      4e6c2d58
    • Chris Wilson's avatar
      drm/i915: Fix nesting of filelist_mutex vs struct_mutex in i915_ppgtt_info · 637ee29e
      Chris Wilson authored
      An unlikely ABBA deadlock in debugfs that no one has reported.
      
      [  284.922349] ======================================================
      [  284.922355] [ INFO: possible circular locking dependency detected ]
      [  284.922361] 4.8.0-rc2+ #430 Tainted: G        W
      [  284.922366] -------------------------------------------------------
      [  284.922371] cat/1197 is trying to acquire lock:
      [  284.922376]  (&dev->filelist_mutex){+.+...}, at: [<ffffffffa0055ba2>] i915_ppgtt_info+0x82/0x390 [i915]
      [  284.922423]
      [  284.922423] but task is already holding lock:
      [  284.922429]  (&dev->struct_mutex){+.+.+.}, at: [<ffffffffa0055b55>] i915_ppgtt_info+0x35/0x390 [i915]
      [  284.922465]
      [  284.922465] which lock already depends on the new lock.
      [  284.922465]
      [  284.922471]
      [  284.922471] the existing dependency chain (in reverse order) is:
      [  284.922477]
      -> #1 (&dev->struct_mutex){+.+.+.}:
      [  284.922493]        [<ffffffff81087710>] lock_acquire+0x60/0x80
      [  284.922505]        [<ffffffff8143e96f>] mutex_lock_nested+0x5f/0x360
      [  284.922520]        [<ffffffffa004f877>] print_context_stats+0x37/0xf0 [i915]
      [  284.922549]        [<ffffffffa00535f5>] i915_gem_object_info+0x265/0x490 [i915]
      [  284.922581]        [<ffffffff81144491>] seq_read+0xe1/0x3b0
      [  284.922592]        [<ffffffff811f77b3>] full_proxy_read+0x83/0xb0
      [  284.922604]        [<ffffffff8111ba03>] __vfs_read+0x23/0x110
      [  284.922616]        [<ffffffff8111c9b9>] vfs_read+0x89/0x110
      [  284.922626]        [<ffffffff8111dbf4>] SyS_read+0x44/0xa0
      [  284.922636]        [<ffffffff81442be9>] entry_SYSCALL_64_fastpath+0x1c/0xac
      [  284.922648]
      -> #0 (&dev->filelist_mutex){+.+...}:
      [  284.922667]        [<ffffffff810871fc>] __lock_acquire+0x10fc/0x1270
      [  284.922678]        [<ffffffff81087710>] lock_acquire+0x60/0x80
      [  284.922689]        [<ffffffff8143e96f>] mutex_lock_nested+0x5f/0x360
      [  284.922701]        [<ffffffffa0055ba2>] i915_ppgtt_info+0x82/0x390 [i915]
      [  284.922729]        [<ffffffff81144491>] seq_read+0xe1/0x3b0
      [  284.922739]        [<ffffffff811f77b3>] full_proxy_read+0x83/0xb0
      [  284.922750]        [<ffffffff8111ba03>] __vfs_read+0x23/0x110
      [  284.922761]        [<ffffffff8111c9b9>] vfs_read+0x89/0x110
      [  284.922771]        [<ffffffff8111dbf4>] SyS_read+0x44/0xa0
      [  284.922781]        [<ffffffff81442be9>] entry_SYSCALL_64_fastpath+0x1c/0xac
      [  284.922793]
      [  284.922793] other info that might help us debug this:
      [  284.922793]
      [  284.922809]  Possible unsafe locking scenario:
      [  284.922809]
      [  284.922818]        CPU0                    CPU1
      [  284.922825]        ----                    ----
      [  284.922831]   lock(&dev->struct_mutex);
      [  284.922842]                                lock(&dev->filelist_mutex);
      [  284.922854]                                lock(&dev->struct_mutex);
      [  284.922865]   lock(&dev->filelist_mutex);
      [  284.922875]
      [  284.922875]  *** DEADLOCK ***
      [  284.922875]
      [  284.922888] 3 locks held by cat/1197:
      [  284.922895]  #0:  (debugfs_srcu){......}, at: [<ffffffff811f7730>] full_proxy_read+0x0/0xb0
      [  284.922919]  #1:  (&p->lock){+.+.+.}, at: [<ffffffff811443e8>] seq_read+0x38/0x3b0
      [  284.922942]  #2:  (&dev->struct_mutex){+.+.+.}, at: [<ffffffffa0055b55>] i915_ppgtt_info+0x35/0x390 [i915]
      [  284.922983]
      
      Fixes: 1d2ac403 ("drm: Protect dev->filelist with its own mutex")
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160822132820.21725-1-chris@chris-wilson.co.uk
      637ee29e
    • Chris Wilson's avatar
      drm/i915: Ignore stuck requests when considering hangs · 34730fed
      Chris Wilson authored
      If the engine isn't being retired (worker starvation?) then it is
      possible for us to repeatedly observe that between consecutive
      hangchecks the seqno on the ring to be the same and there remain
      unretired requests. Ignore these completely and only regard the engine
      as busy for the purpose of hang detection (not stall detection) if there
      are outstanding breadcrumbs.
      
      In recent history we have looked at using both the request and seqno as
      indication of activity on the engine, but that was reduced to just
      inspecting seqno in commit cffa781e ("drm/i915: Simplify check for
      idleness in hangcheck"). However, in commit dcff85c8 ("drm/i915:
      Enable i915_gem_wait_for_idle() without holding struct_mutex"), I made
      the decision to use the new common lockless function, under the
      assumption that request retirement was more frequent than hangcheck and
      so we would not have a stuck busy check. The flaw there was in
      forgetting that we accumulate the hang score, and so successive checks
      seeing a stuck request, albeit with the GPU advancing elsewhere and so
      not necessary the same stuck request, would eventually trigger the hang.
      
      Fixes: dcff85c8 ("drm/i915: Enable i915_gem_wait_for_idle()...")
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Cc: Mika Kuoppala <mika.kuoppala@intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160820145408.32180-1-chris@chris-wilson.co.ukReviewed-by: default avatarMika Kuoppala <mika.kuoppala@intel.com>
      34730fed
    • Chris Wilson's avatar
      drm/i915: Allow DMA pagetables to use highmem · bb8f9cff
      Chris Wilson authored
      As we never need to directly access the pages we allocate for scratch and
      the pagetables, and always remap them into the GTT through the dma
      remapper, we do not need to limit the allocations to lowmem i.e. we can
      pass in the __GFP_HIGHMEM flag to the page allocation.
      
      For backwards compatibility, e.g. certain old GPUs not liking highmem
      for certain functions that may be accidentally mapped to the scratch
      page by userspace, keep the GMCH probe as only allocating from DMA32.
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160822074431.26872-3-chris@chris-wilson.co.ukReviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      bb8f9cff
    • Chris Wilson's avatar
      drm/i915: Embed the scratch page struct into each VM · 8bcdd0f7
      Chris Wilson authored
      As the scratch page is no longer shared between all VM, and each has
      their own, forgo the small allocation and simply embed the scratch page
      struct into the i915_address_space.
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160822074431.26872-2-chris@chris-wilson.co.ukReviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Acked-by: default avatarMika Kuoppala <mika.kuoppala@intel.com>
      8bcdd0f7
    • David Weinehall's avatar
      drm/i915: debugfs spring cleaning · 36cdd013
      David Weinehall authored
      Just like with sysfs, we do some major overhaul.
      
      Pass dev_priv instead of dev to all feature macros (IS_, HAS_,
      INTEL_, etc.). This has the side effect that a bunch of functions
      now get dev_priv passed instead of dev.
      
      All calls to INTEL_INFO()->gen have been replaced with
      INTEL_GEN().
      
      We want access to to_i915(node->minor->dev) in a lot of places,
      so add the node_to_i915() helper to accommodate for this.
      
      Finally, we have quite a few cases where we get a void * pointer,
      and need to cast it to drm_device *, only to run to_i915() on it.
      Add cast_to_i915() to do this.
      
      v2: Don't introduce extra dev (Chris)
      
      v3: Make pipe_crc_info have a pointer to drm_i915_private instead of
          drm_device. This saves a bit of space, since we never use
          drm_device anywhere in these functions.
      
          Also some minor fixup that I missed in the previous version.
      
      v4: Changed the code a bit so that dev_priv is passed directly
          to various functions, thus removing the need for the
          cast_to_i915() helper. Also did some additional cleanup.
      
      v5: Additional cleanup of newly introduced changes.
      
      v6: Rebase again because of conflict.
      Signed-off-by: default avatarDavid Weinehall <david.weinehall@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160822105931.pcbe2lpsgzckzboa@boomReviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      36cdd013
    • David Weinehall's avatar
      drm/i915: pdev cleanup · 52a05c30
      David Weinehall authored
      In an effort to simplify things for a future push of dev_priv instead
      of dev wherever possible, always take pdev via dev_priv where
      feasible, eliminating the direct access from dev. Right now this
      only eliminates a few cases of dev, but it also obviates that we pass
      dev into a lot of functions where dev_priv would be the more obvious
      choice.
      
      v2: Fixed one more place missing in the previous patch set
      Signed-off-by: default avatarDavid Weinehall <david.weinehall@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160822103245.24069-5-david.weinehall@linux.intel.comReviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      52a05c30