1. 24 Aug, 2016 5 commits
    • Matthew Auld's avatar
      drm/i915: free intel_fb · d1a3a036
      Matthew Auld authored
      We need to free the allocated intel_fb in the error path, not
      intel_fb->base. Otherwise we risk calling kfree with a non-kmalloc'd
      address, which is bound to give us grief at some point.
      Signed-off-by: default avatarMatthew Auld <matthew.auld@intel.com>
      Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471964444-24460-1-git-send-email-matthew.auld@intel.com
      d1a3a036
    • Chris Wilson's avatar
      drm/i915/dvo: Remove dangling call to drm_encoder_cleanup() · 8f76aa0e
      Chris Wilson authored
      If we hit the error path, we have never called drm_encoder_init() and so
      have nothing to cleanup. Doing so hits a null dereference:
      
      [   10.066261] BUG: unable to handle kernel NULL pointer dereference at 00000104
      [   10.066273] IP: [<c16054b4>] mutex_lock+0xa/0x15
      [   10.066287] *pde = 00000000
      [   10.066295] Oops: 0002 [#1]
      [   10.066302] Modules linked in: i915(+) video i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm iTCO_wdt iTCO_vendor_support ppdev evdev snd_intel8x0 snd_ac97_codec ac97_bus psmouse snd_pcm snd_timer snd pcspkr uhci_hcd ehci_pci soundcore sr_mod ehci_hcd serio_raw i2c_i801 usbcore i2c_smbus cdrom lpc_ich mfd_core rng_core e100 mii floppy parport_pc parport acpi_cpufreq button processor usb_common eeprom lm85 hwmon_vid autofs4
      [   10.066378] CPU: 0 PID: 132 Comm: systemd-udevd Not tainted 4.8.0-rc3-00013-gef0e1ea8 #34
      [   10.066389] Hardware name: MicroLink                               /D865GLC                        , BIOS BF86510A.86A.0077.P25.0508040031 08/04/2005
      [   10.066401] task: f62db800 task.stack: f5970000
      [   10.066409] EIP: 0060:[<c16054b4>] EFLAGS: 00010286 CPU: 0
      [   10.066417] EIP is at mutex_lock+0xa/0x15
      [   10.066424] EAX: 00000104 EBX: 00000104 ECX: 00000000 EDX: 80000000
      [   10.066432] ESI: 00000000 EDI: 00000104 EBP: f5be8000 ESP: f5971b58
      [   10.066439]  DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
      [   10.066446] CR0: 80050033 CR2: 00000104 CR3: 35945000 CR4: 000006d0
      [   10.066453] Stack:
      [   10.066459]  f503d740 f824dddf 00000000 f61170c0 f61170c0 f82371ae f850f40e 00000001
      [   10.066476]  f61170c0 f5971bcc f5be8000 f9c2d401 00000001 f8236fcc 00000001 00000000
      [   10.066491]  f5144014 f5be8104 00000008 f9c5267c 00000007 f61170c0 f5144400 f9c4ff00
      [   10.066507] Call Trace:
      [   10.066526]  [<f824dddf>] ? drm_modeset_lock_all+0x27/0xb3 [drm]
      [   10.066545]  [<f82371ae>] ? drm_encoder_cleanup+0x1a/0x132 [drm]
      [   10.066559]  [<f850f40e>] ? drm_atomic_helper_connector_reset+0x3f/0x5c [drm_kms_helper]
      [   10.066644]  [<f9c2d401>] ? intel_dvo_init+0x569/0x788 [i915]
      [   10.066663]  [<f8236fcc>] ? drm_encoder_init+0x43/0x20b [drm]
      [   10.066734]  [<f9bf1fce>] ? intel_modeset_init+0x1436/0x17dd [i915]
      [   10.066791]  [<f9b37636>] ? i915_driver_load+0x85a/0x15d3 [i915]
      [   10.066846]  [<f9b3603d>] ? i915_driver_open+0x5/0x5 [i915]
      [   10.066857]  [<c14af4d0>] ? firmware_map_add_entry.part.2+0xc/0xc
      [   10.066868]  [<c1343daf>] ? pci_device_probe+0x8e/0x11c
      [   10.066878]  [<c140cec8>] ? driver_probe_device+0x1db/0x62e
      [   10.066888]  [<c120c010>] ? kernfs_new_node+0x29/0x9c
      [   10.066897]  [<c13438e0>] ? pci_match_device+0xd9/0x161
      [   10.066905]  [<c120c48b>] ? kernfs_create_dir_ns+0x42/0x88
      [   10.066914]  [<c140d401>] ? __driver_attach+0xe6/0x11b
      [   10.066924]  [<c1303b13>] ? kobject_add_internal+0x1bb/0x44f
      [   10.066933]  [<c140d31b>] ? driver_probe_device+0x62e/0x62e
      [   10.066941]  [<c140a2d2>] ? bus_for_each_dev+0x46/0x7f
      [   10.066950]  [<c140c502>] ? driver_attach+0x1a/0x34
      [   10.066958]  [<c140d31b>] ? driver_probe_device+0x62e/0x62e
      [   10.066966]  [<c140b758>] ? bus_add_driver+0x217/0x32a
      [   10.066975]  [<f8403000>] ? 0xf8403000
      [   10.066982]  [<c140de27>] ? driver_register+0x5f/0x108
      [   10.066991]  [<c1000493>] ? do_one_initcall+0x49/0x1f6
      [   10.067000]  [<c1082299>] ? pick_next_task_fair+0x14b/0x2a3
      [   10.067008]  [<c1603c8d>] ? __schedule+0x15c/0x4fe
      [   10.067016]  [<c1604104>] ? preempt_schedule_common+0x19/0x3c
      [   10.067027]  [<c11051de>] ? do_init_module+0x17/0x230
      [   10.067035]  [<c1604139>] ? _cond_resched+0x12/0x1a
      [   10.067044]  [<c116f9aa>] ? kmem_cache_alloc+0x8f/0x11f
      [   10.067052]  [<c11051de>] ? do_init_module+0x17/0x230
      [   10.067060]  [<c11703dd>] ? kfree+0x137/0x203
      [   10.067068]  [<c110523d>] ? do_init_module+0x76/0x230
      [   10.067078]  [<c10cadf3>] ? load_module+0x2a39/0x333f
      [   10.067087]  [<c10cb8b2>] ? SyS_finit_module+0x96/0xd5
      [   10.067096]  [<c1132231>] ? vm_mmap_pgoff+0x79/0xa0
      [   10.067105]  [<c1001e96>] ? do_fast_syscall_32+0xb5/0x1b0
      [   10.067114]  [<c16086a6>] ? sysenter_past_esp+0x47/0x75
      [   10.067121] Code: c8 f7 76 c1 e8 8e cc d2 ff e9 45 fe ff ff 66 90 66 90 66 90 66 90 90 ff 00 7f 05 e8 4e 0c 00 00 c3 53 89 c3 e8 75 ec ff ff 89 d8 <ff> 08 79 05 e8 fa 0a 00 00 5b c3 53 89 c3 85 c0 74 1b 8b 03 83
      [   10.067180] EIP: [<c16054b4>] mutex_lock+0xa/0x15 SS:ESP 0068:f5971b58
      [   10.067190] CR2: 0000000000000104
      [   10.067222] ---[ end trace 049f1f09da45a856 ]---
      Reported-by: default avatarMeelis Roos <mroos@linux.ee>
      Fixes: 580d8ed5 ("drm/i915: Give encoders useful names")
      Reviewed-by: default avatarDavid Weinehall <david.weinehall@linux.intel.com>
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: drm-intel-fixes@lists.freedesktop.org
      Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160823092558.14931-1-chris@chris-wilson.co.uk
      8f76aa0e
    • Maarten Lankhorst's avatar
      drm/i915: Cleanup crt disable sequence on hsw+ · b7076546
      Maarten Lankhorst authored
      Instead of iterating overthe connectors manually, run the last part of
      DDI disabling inside the crt post disable function.
      
      This was meant to be addressed before submitting the other commit,
      but I missed the review comments.
      
      Fixes: fd6bbda9 ("drm/i915: Pass crtc_state and connector_state to encoder functions")
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471961888-10771-2-git-send-email-maarten.lankhorst@linux.intel.comReviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      [mlankhorst: Fix extra whitespace between functions.]
      b7076546
    • Maarten Lankhorst's avatar
      drm/i915: Create a intel_encoder_find_connector helper function. · 496b0fc3
      Maarten Lankhorst authored
      This makes the code in intel_sanitize_encoder slightly more readable.
      This was meant to be addressed in fd6bbda9, but I missed that
      review comment.
      
      Fixes: fd6bbda9 ("drm/i915: Pass crtc_state and connector_state to encoder functions")
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471961888-10771-1-git-send-email-maarten.lankhorst@linux.intel.comReviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      [mlankhorst: Fix unused variable reported by kbuild.]
      496b0fc3
    • Daniel Vetter's avatar
      io-mapping: Fixup for different names of writecombine · 80c33624
      Daniel Vetter authored
      Somehow architectures can't agree on this. And for good measure make
      sure we have a fallback which should work everywhere (fingers
      crossed).
      
      v2: Make it compile properly, needs a defined() for the #elif.
      
      Fixes: ac96b556 ("io-mapping.h: s/PAGE_KERNEL_IO/PAGE_KERNEL/")
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Cc: linux-mm@kvack.org
      Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Reviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160823202233.4681-1-daniel.vetter@ffwll.ch
      80c33624
  2. 23 Aug, 2016 17 commits
  3. 22 Aug, 2016 18 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
    • David Weinehall's avatar
      drm/i915: i915_sysfs.c cleanup · 694c2828
      David Weinehall authored
      Various cleanup for i915_sysfs.c; we now use dev_priv whenever
      possible. The kdev_to_drm_minor() helper function has been
      replaced by one that converts from struct device *
      to struct drm_i915_private *.
      
      We already have a seemingly identical helper (kdev_to_i915())
      in i915_drv.h. But that one cannot be used here.
      Unlike the version in i915_drv.h, this helper
      reaches i915 through drm_minor.
      
      v2: Rename kdev_to_i915_dm() to kdev_minor_to_i915() (Chris)
      Signed-off-by: default avatarDavid Weinehall <david.weinehall@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160822103245.24069-4-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>
      694c2828
    • David Weinehall's avatar
      drm/i915: consistent struct device naming · c49d13ee
      David Weinehall authored
      We currently have a mix of struct device *device, struct device *kdev,
      and struct device *dev (the latter forcing us to refer to
      struct drm_device as something else than the normal dev).
      
      To simplify things, always use kdev when referring to struct device.
      
      v2: Replace the dev_to_drm_minor() macro with the inline function
          kdev_to_drm_minor().
      Signed-off-by: default avatarDavid Weinehall <david.weinehall@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160822103245.24069-3-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>
      c49d13ee
    • David Weinehall's avatar
    • Maarten Lankhorst's avatar
      drm/i915: Fix botched merge that downgrades CSR versions. · 536ab3ca
      Maarten Lankhorst authored
      Merge commit 5e580523 reverts the version bumping parts of
      commit 4aa7fb9c. Bump the versions again and request the specific
      firmware version.
      
      The currently recommended versions are: SKL 1.26, KBL 1.01 and BXT 1.07.
      
      Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
      Cc: Imre Deak <imre.deak@intel.com>
      Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97242
      Cc: drm-intel-fixes@lists.freedesktop.org
      Fixes: 5e580523 ("Backmerge tag 'v4.7' into drm-next")
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471266567-22443-1-git-send-email-maarten.lankhorst@linux.intel.comReviewed-by: default avatarImre Deak <imre.deak@intel.com>
      536ab3ca
    • Lyude's avatar
      drm/i915/skl: Ensure pipes with changed wms get added to the state · 05a76d3d
      Lyude authored
      If we're enabling a pipe, we'll need to modify the watermarks on all
      active planes. Since those planes won't be added to the state on
      their own, we need to add them ourselves.
      Signed-off-by: default avatarLyude <cpaul@redhat.com>
      Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Cc: stable@vger.kernel.org
      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 avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-6-git-send-email-cpaul@redhat.com
      05a76d3d
    • Matt Roper's avatar
      drm/i915/gen9: Only copy WM results for changed pipes to skl_hw · 2722efb9
      Matt Roper authored
      When we write watermark values to the hardware, those values are stored
      in dev_priv->wm.skl_hw.  However with recent watermark changes, the
      results structure we're copying from only contains valid watermark and
      DDB values for the pipes that are actually changing; the values for
      other pipes remain 0.  Thus a blind copy of the entire skl_wm_values
      structure will clobber the values for unchanged pipes...we need to be
      more selective and only copy over the values for the changing pipes.
      
      This mistake was hidden until recently due to another bug that caused us
      to erroneously re-calculate watermarks for all active pipes rather than
      changing pipes.  Only when that bug was fixed was the impact of this bug
      discovered (e.g., modesets failing with "Requested display configuration
      exceeds system watermark limitations" messages and leaving watermarks
      non-functional, even ones initiated by intel_fbdev_restore_mode).
      
      Changes since v1:
       - Add a function for copying a pipe's wm values
         (skl_copy_wm_for_pipe()) so we can reuse this later
      
      Fixes: 734fa01f ("drm/i915/gen9: Calculate watermarks during atomic 'check' (v2)")
      Fixes: 9b613022 ("drm/i915/gen9: Re-allocate DDB only for changed pipes")
      Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Signed-off-by: default avatarLyude <cpaul@redhat.com>
      Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
      Cc: stable@vger.kernel.org
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.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>
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-4-git-send-email-cpaul@redhat.com
      2722efb9
    • Lyude's avatar
      drm/i915/skl: Add support for the SAGV, fix underrun hangs · 656d1b89
      Lyude authored
      Since the watermark calculations for Skylake are still broken, we're apt
      to hitting underruns very easily under multi-monitor configurations.
      While it would be lovely if this was fixed, it's not. Another problem
      that's been coming from this however, is the mysterious issue of
      underruns causing full system hangs. An easy way to reproduce this with
      a skylake system:
      
      - Get a laptop with a skylake GPU, and hook up two external monitors to
        it
      - Move the cursor from the built-in LCD to one of the external displays
        as quickly as you can
      - You'll get a few pipe underruns, and eventually the entire system will
        just freeze.
      
      After doing a lot of investigation and reading through the bspec, I
      found the existence of the SAGV, which is responsible for adjusting the
      system agent voltage and clock frequencies depending on how much power
      we need. According to the bspec:
      
      "The display engine access to system memory is blocked during the
       adjustment time. SAGV defaults to enabled. Software must use the
       GT-driver pcode mailbox to disable SAGV when the display engine is not
       able to tolerate the blocking time."
      
      The rest of the bspec goes on to explain that software can simply leave
      the SAGV enabled, and disable it when we use interlaced pipes/have more
      then one pipe active.
      
      Sure enough, with this patchset the system hangs resulting from pipe
      underruns on Skylake have completely vanished on my T460s. Additionally,
      the bspec mentions turning off the SAGV	with more then one pipe enabled
      as a workaround for display underruns. While this patch doesn't entirely
      fix that, it looks like it does improve the situation a little bit so
      it's likely this is going to be required to make watermarks on Skylake
      fully functional.
      
      This will still need additional work in the future: we shouldn't be
      enabling the SAGV if any of the currently enabled planes can't enable WM
      levels that introduce latencies >= 30 µs.
      
      Changes since v11:
       - Add skl_can_enable_sagv()
       - Make sure we don't enable SAGV when not all planes can enable
         watermarks >= the SAGV engine block time. I was originally going to
         save this for later, but I recently managed to run into a machine
         that was having problems with a single pipe configuration + SAGV.
       - Make comparisons to I915_SKL_SAGV_NOT_CONTROLLED explicit
       - Change I915_SAGV_DYNAMIC_FREQ to I915_SAGV_ENABLE
       - Move printks outside of mutexes
       - Don't print error messages twice
      Changes since v10:
       - Apparently sandybridge_pcode_read actually writes values and reads
         them back, despite it's misleading function name. This means we've
         been doing this mostly wrong and have been writing garbage to the
         SAGV control. Because of this, we no longer attempt to read the SAGV
         status during initialization (since there are no helpers for this).
       - mlankhorst noticed that this patch was breaking on some very early
         pre-release Skylake machines, which apparently don't allow you to
         disable the SAGV. To prevent machines from failing tests due to SAGV
         errors, if the first time we try to control the SAGV results in the
         mailbox indicating an invalid command, we just disable future attempts
         to control the SAGV state by setting dev_priv->skl_sagv_status to
         I915_SKL_SAGV_NOT_CONTROLLED and make a note of it in dmesg.
       - Move mutex_unlock() a little higher in skl_enable_sagv(). This
         doesn't actually fix anything, but lets us release the lock a little
         sooner since we're finished with it.
      Changes since v9:
       - Only enable/disable sagv on Skylake
      Changes since v8:
       - Add intel_state->modeset guard to the conditional for
         skl_enable_sagv()
      Changes since v7:
       - Remove GEN9_SAGV_LOW_FREQ, replace with GEN9_SAGV_IS_ENABLED (that's
         all we use it for anyway)
       - Use GEN9_SAGV_IS_ENABLED instead of 0x1 for clarification
       - Fix a styling error that snuck past me
      Changes since v6:
       - Protect skl_enable_sagv() with intel_state->modeset conditional in
         intel_atomic_commit_tail()
      Changes since v5:
       - Don't use is_power_of_2. Makes things confusing
       - Don't use the old state to figure out whether or not to
         enable/disable the sagv, use the new one
       - Split the loop in skl_disable_sagv into it's own function
       - Move skl_sagv_enable/disable() calls into intel_atomic_commit_tail()
      Changes since v4:
       - Use is_power_of_2 against active_crtcs to check whether we have > 1
         pipe enabled
       - Fix skl_sagv_get_hw_state(): (temp & 0x1) indicates disabled, 0x0
         enabled
       - Call skl_sagv_enable/disable() from pre/post-plane updates
      Changes since v3:
       - Use time_before() to compare timeout to jiffies
      Changes since v2:
       - Really apply minor style nitpicks to patch this time
      Changes since v1:
       - Added comments about this probably being one of the requirements to
         fixing Skylake's watermark issues
       - Minor style nitpicks from Matt Roper
       - Disable these functions on Broxton, since it doesn't have an SAGV
      Signed-off-by: default avatarLyude <cpaul@redhat.com>
      Cc: Matt Roper <matthew.d.roper@intel.com>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-3-git-send-email-cpaul@redhat.com
      [mlankhorst: ENOSYS -> ENXIO, whitespace fixes]
      656d1b89
    • Lyude's avatar
      drm/i915/gen6+: Interpret mailbox error flags · 87660502
      Lyude authored
      In order to add proper support for the SAGV, we need to be able to know
      what the cause of a failure to change the SAGV through the pcode mailbox
      was. The reasoning for this is that some very early pre-release Skylake
      machines don't actually allow you to control the SAGV on them, and
      indicate an invalid mailbox command was sent.
      
      This also might come in handy in the future for debugging.
      
      Changes since v1:
       - Add functions for interpreting gen6 mailbox error codes along with
         gen7+ error codes, and actually interpret those codes properly
       - Renamed patch to reflect new behavior
      Signed-off-by: default avatarLyude <cpaul@redhat.com>
      Cc: Matt Roper <matthew.d.roper@intel.com>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-2-git-send-email-cpaul@redhat.com
      [mlankhorst: -ENOSYS -> -ENXIO for checkpatch]
      87660502
    • Paulo Zanoni's avatar
      drm/i915: Call intel_fbc_pre_update() after pinning the new pageflip · 1f061316
      Paulo Zanoni authored
      intel_fbc_pre_update() depends upon the new state being already pinned
      in place in the Global GTT (primarily for both fencing which wants both
      an offset and a fence register, if assigned). This requires the call to
      intel_fbc_pre_update() be after intel_pin_and_fence_fb() - but commit
      e8216e50 ("drm/i915/fbc: call intel_fbc_pre_update earlier during
      page flips") moved the code way too much up in its attempt to call it
      before the page flip.
      
      v2 (from Paulo):
       - Point the original bad commit.
       - Add a comment to maybe prevent further regressions.
      
      Fixes: e8216e50 ("drm/i915/fbc: call intel_fbc_pre_update earlier...")
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
      Cc: Daniel Vetter <daniel.vetter@intel.com>
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
      Cc: drm-intel-fixes@lists.freedesktop.org
      Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471462904-842-1-git-send-email-paulo.r.zanoni@intel.com
      Cc: stable@vger.kernel.org
      1f061316
    • Daniel Vetter's avatar
      drm/i915: Ensure consistent control flow __i915_gem_active_get_rcu · c75870d8
      Daniel Vetter authored
      This issue here is (I think) purely theoretical, since a compiler
      would need to be especially foolish to recompute the value of
      i915_gem_request_completed right after it was already used. Hence the
      additional barrier() is also not really a restriction.
      
      But I believe this to be at least permissible, and since our rcu
      trickery is a beast it's worth to annotate all the corner cases.
      Chris proposed to instead just wrap a READ_ONCE around
      request->fence.seqno in i915_gem_request_completed. But that has a
      measurable impact on code size, and everywhere we hold a full
      reference to the underlying request it's also not needed. And
      personally I'd like to have just enough barriers and locking needed
      for correctness, but not more - it makes it much easier in the future
      to understand what's going on.
      
      Since the busy ioctl has now fully embraced it's races there's no
      point annotating it there too. We really only need it in
      active_get_rcu, since that function _must_ deliver a correct snapshot
      of the active fences (and not chase something else).
      
      v2: Polish the comment a bit more (Chris).
      
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Link: http://patchwork.freedesktop.org/patch/msgid/1471856122-466-1-git-send-email-daniel.vetter@ffwll.ch
      c75870d8
    • Chris Wilson's avatar
      drm/i915: Stop marking the unaccessible scratch page as UC · 14daa63b
      Chris Wilson authored
      Since by design, if not entirely by practice, nothing is allowed to
      access the scratch page we use to background fill the VM, then we do not
      need to ensure that it is coherent between the CPU and GPU.
      set_pages_uc() does a stop_machine() after changing the PAT, and that
      significantly impacts upon context creation throughput.
      Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
      Link: http://patchwork.freedesktop.org/patch/msgid/20160822074431.26872-1-chris@chris-wilson.co.ukReviewed-by: default avatarJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      14daa63b