1. 14 Aug, 2015 38 commits
  2. 13 Aug, 2015 2 commits
    • Maarten Lankhorst's avatar
      drm/i915: Commit planes on each crtc separately. · d2944cf2
      Maarten Lankhorst authored
      This patch is based on the upstream commit 5ac1c4bc and amended
      for v4.2 to make sure it works as intended.
      
      Repeated calls to begin_crtc_commit can cause warnings like this:
      [  169.127746] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:616
      [  169.127835] in_atomic(): 0, irqs_disabled(): 1, pid: 1947, name: kms_flip
      [  169.127840] 3 locks held by kms_flip/1947:
      [  169.127843]  #0:  (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffff814774bc>] __drm_modeset_lock_all+0x9c/0x130
      [  169.127860]  #1:  (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffff814774cd>] __drm_modeset_lock_all+0xad/0x130
      [  169.127870]  #2:  (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffff81477178>] drm_modeset_lock+0x38/0x110
      [  169.127879] irq event stamp: 665690
      [  169.127882] hardirqs last  enabled at (665689): [<ffffffff817ffdb5>] _raw_spin_unlock_irqrestore+0x55/0x70
      [  169.127889] hardirqs last disabled at (665690): [<ffffffffc0197a23>] intel_pipe_update_start+0x113/0x5c0 [i915]
      [  169.127936] softirqs last  enabled at (665470): [<ffffffff8108a766>] __do_softirq+0x236/0x650
      [  169.127942] softirqs last disabled at (665465): [<ffffffff8108ae75>] irq_exit+0xc5/0xd0
      [  169.127951] CPU: 1 PID: 1947 Comm: kms_flip Not tainted 4.1.0-rc4-patser+ #4039
      [  169.127954] Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014
      [  169.127957]  ffff8800c49036f0 ffff8800cde5fa28 ffffffff817f6907 0000000080000001
      [  169.127964]  0000000000000000 ffff8800cde5fa58 ffffffff810aebed 0000000000000046
      [  169.127970]  ffffffff81c5d518 0000000000000268 0000000000000000 ffff8800cde5fa88
      [  169.127981] Call Trace:
      [  169.127992]  [<ffffffff817f6907>] dump_stack+0x4f/0x7b
      [  169.128001]  [<ffffffff810aebed>] ___might_sleep+0x16d/0x270
      [  169.128008]  [<ffffffff810aed38>] __might_sleep+0x48/0x90
      [  169.128017]  [<ffffffff817fc359>] mutex_lock_nested+0x29/0x410
      [  169.128073]  [<ffffffffc01635f0>] ? vgpu_write64+0x220/0x220 [i915]
      [  169.128138]  [<ffffffffc017fddf>] ? ironlake_update_primary_plane+0x2ff/0x410 [i915]
      [  169.128198]  [<ffffffffc0190e75>] intel_frontbuffer_flush+0x25/0x70 [i915]
      [  169.128253]  [<ffffffffc01831ac>] intel_finish_crtc_commit+0x4c/0x180 [i915]
      [  169.128279]  [<ffffffffc00784ac>] drm_atomic_helper_commit_planes+0x12c/0x240 [drm_kms_helper]
      [  169.128338]  [<ffffffffc0184264>] __intel_set_mode+0x684/0x830 [i915]
      [  169.128378]  [<ffffffffc018a84a>] intel_crtc_set_config+0x49a/0x620 [i915]
      [  169.128385]  [<ffffffff817fdd39>] ? mutex_unlock+0x9/0x10
      [  169.128391]  [<ffffffff81467b69>] drm_mode_set_config_internal+0x69/0x120
      [  169.128398]  [<ffffffff8119b547>] ? might_fault+0x57/0xb0
      [  169.128403]  [<ffffffff8146bf93>] drm_mode_setcrtc+0x253/0x620
      [  169.128409]  [<ffffffff8145c600>] drm_ioctl+0x1a0/0x6a0
      [  169.128415]  [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50
      [  169.128424]  [<ffffffff811e9ab8>] do_vfs_ioctl+0x2f8/0x530
      [  169.128429]  [<ffffffff810d0fcd>] ? trace_hardirqs_on+0xd/0x10
      [  169.128435]  [<ffffffff812e7676>] ? selinux_file_ioctl+0x56/0x100
      [  169.128439]  [<ffffffff811e9d71>] SyS_ioctl+0x81/0xa0
      [  169.128445]  [<ffffffff81800697>] system_call_fastpath+0x12/0x6f
      
      Solve it by using the newly introduced drm_atomic_helper_commit_planes_on_crtc.
      
      The problem here was that the drm_atomic_helper_commit_planes() helper
      we were using was basically designed to do
      
          begin_crtc_commit(crtc #1)
          begin_crtc_commit(crtc #2)
          ...
          commit all planes
          finish_crtc_commit(crtc #1)
          finish_crtc_commit(crtc #2)
      
      The problem here is that since our hardware relies on vblank evasion,
      our CRTC 'begin' function waits until we're out of the danger zone in
      which register writes might wind up straddling the vblank, then disables
      interrupts; our 'finish' function re-enables interrupts after the
      registers have been written.  The expectation is that the operations between
      'begin' and 'end' must be performed without sleeping (since interrupts
      are disabled) and should happen as quickly as possible.  By clumping all
      of the 'begin' calls together, we introducing a couple problems:
       * Subsequent 'begin' invocations might sleep (which is illegal)
       * The first 'begin' ensured that we were far enough from the vblank that
         we could write our registers safely and ensure they all fell within
         the same frame.  Adding extra delay waiting for subsequent CRTC's
         wasn't accounted for and could put us back into the 'danger zone' for
         CRTC #1.
      
      This commit solves the problem by using a new helper that allows an
      order of operations like:
      
         for each crtc {
              begin_crtc_commit(crtc)  // sleep (maybe), then disable interrupts
              commit planes for this specific CRTC
              end_crtc_commit(crtc)    // reenable interrupts
         }
      
      so that sleeps will only be performed while interrupts are enabled and
      we can be sure that registers for a CRTC will be written immediately
      once we know we're in the safe zone.
      
      The crtc->config->base.crtc update may seem unrelated, but the helper
      will use it to obtain the crtc for the state. Without the update it
      will dereference NULL and crash.
      
      Changes since v1:
      - Use Matt Roper's commit message.
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      References: https://bugs.freedesktop.org/show_bug.cgi?id=90398Reviewed-by: default avatarAnder Conselvan de Oliveira <conselvan2@gmail.com>
      Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
      d2944cf2
    • Maarten Lankhorst's avatar
      f0fdc55d