1. 27 Aug, 2016 2 commits
  2. 26 Aug, 2016 4 commits
  3. 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
  4. 24 Aug, 2016 11 commits
  5. 23 Aug, 2016 17 commits
  6. 22 Aug, 2016 3 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