- 23 Aug, 2013 17 commits
-
-
Rafael Barbalho authored
As the relocate entry function was getting a bit too big I've moved the code that used to use either the cpu or the gtt to for the relocation into two separate functions. Signed-off-by:
Rafael Barbalho <rafael.barbalho@intel.com> Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Paulo Zanoni authored
Because hsw_pm_irq_handler does exactly what gen6_rps_irq_handler does and also processes the 2 additional VEBOX bits. So merge those functions and wrap the VEBOX bits on a HAS_VEBOX check. This check isn't really necessary since the bits are reserved on SNB/IVB/VLV, but it's a good documentation on who uses them. v2: - Change IS_HASWELL check to HAS_VEBOX Signed-off-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by:
Ben Widawsky <ben@bwidawsk.net> Signed-off-by:
-
Paulo Zanoni authored
It seems we've been doing this ever since we started processing the RPS events on a work queue, on commit "drm/i915: move gen6 rps handling to workqueue", 4912d041. The problem is: when we add work to the queue, instead of just masking the bits we queued and leaving all the others on their current state, we mask the bits we queued and unmask all the others. This basically means we'll be unmasking a bunch of interrupts we're not going to process. And if you look at gen6_pm_rps_work, we unmask back only GEN6_PM_RPS_EVENTS, which means the bits we unmasked when adding work to the queue will remain unmasked after we process the queue. Notice that even though we unmask those unrelated interrupts, we never enable them on IER, so they don't fire our interrupt handler, they just stay there on IIR waiting to be cleared when something else triggers the interrupt handler. So this patch does what seems to make more sense: mask only the bits we add to the queue, without unmasking anything else, and so we'll unmask them after we process the queue. As a side effect we also have to remove that WARN, because it is not only making sure we don't mask useful interrupts, it is also making sure we do unmask useless interrupts! That piece of code should not be responsible for knowing which bits should be unmasked, so just don't assert anything, and trust that snb_disable_pm_irq should be doing the right thing. With i915.enable_pc8=1 I was getting ocasional "GEN6_PMIIR is not 0" error messages due to the fact that we unmask those unrelated interrupts but don't enable them. Note: if bugs start bisecting to this patch, then it probably means someone was relying on the fact that we unmask everything by accident, then we should fix gen5_gt_irq_postinstall or whoever needs the accidentally unmasked interrupts. Or maybe I was just wrong and we need to revert this patch :) Note: This started to be a more real issue with the addition of the VEBOX support since now we do enable more than just the minimal set of RPS interrupts in the IER register. Which means after the first rps interrupt has happened we will never mask the VEBOX user interrupts again and so will blow through cpu time needlessly when running video workloads. Signed-off-by:
-
Paulo Zanoni authored
On SNB/IVB/VLV we only call gen6_rps_irq_handler if one of the IIR bits set is part of GEN6_PM_RPS_EVENTS, but at gen6_rps_irq_handler we add all the enabled IIR bits to the work queue, not only the ones that are part of GEN6_PM_RPS_EVENTS. But then gen6_pm_rps_work only processes GEN6_PM_RPS_EVENTS, so it's useless to add anything that's not GEN6_PM_RPS_EVENTS to the work queue. As a bonus, gen6_rps_irq_handler looks more similar to hsw_pm_irq_handler, so we may be able to merge them in the future. v2: - Add a WARN in case we queued something we're not going to process. Signed-off-by: -
Paulo Zanoni authored
If the error interrupts are already disabled, don't disable and reenable them. This is going to be needed when we're in PC8+, where all the interrupts are disabled so we won't risk re-enabling DE_ERR_INT_IVB. v2: Use dev_priv->irq_mask (Chris) Signed-off-by:
Rodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by:
-
Paulo Zanoni authored
Just like irq_mask and gt_irq_mask, use it to track the status of GEN6_PMIMR so we don't need to read it again every time we call snb_update_pm_irq. Signed-off-by:
-
Paulo Zanoni authored
I did some brief tests and the "new_val = pmimr" condition usually happens a few times after exiting games. Note: This is also prep work to track the GEN6_PMIMR register state in dev_priv->pm_imr. This happens in the next patch. Signed-off-by:
-
Paulo Zanoni authored
Just like we're doing with the other IMR changes. One of the functional changes is that not every caller was doing the POSTING_READ. Signed-off-by:
-
Paulo Zanoni authored
Just like the functions that touch DEIMR and SDEIMR, but for GTIMR. The new functions contain a POSTING_READ(GTIMR) which was not present at the 2 callers inside i915_irq.c. The implementation is based on ibx_display_interrupt_update. Signed-off-by:
-
Paulo Zanoni authored
We already have code to disable LCPLL and switch to FCLK, so we need this too. We still don't call the code to disable LCPLL, but we'll call it when we add support for Package C8+. Signed-off-by:
-
Jesse Barnes authored
On SNB and IVB, there's an MSR (also exposed through MCHBAR) we can use to read out the amount of energy used over time. Expose this in sysfs to make it easy to do power comparisons with different configurations. If the platform supports it, the file will show up under the drm/card0/power subdirectory of the PCI device in sysfs as gt_energy_uJ. The value in the file is a running total of energy (in microjoules) consumed by the graphics device. v2: move to sysfs (Ben, Daniel) expose a simple value (Chris) drop unrelated hunk (Ben) Signed-off-by:Jesse Barnes <jbarnes@virtuousgeek.org> v3: by Ben Tied it into existing rc6 sysfs entries and named that a more generic "power attrs." Fixed rebase conflicts. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Damien Lespiau authored
The code directly uses the registers and ring->mmio_base. Signed-off-by:
Damien Lespiau <damien.lespiau@intel.com> Signed-off-by:
-
Damien Lespiau authored
This define hasn't been used since: commit cfdf1fa2 Author: Kristian Høgsberg <krh@bitplanet.net> Date: Wed Dec 16 15:16:16 2009 -0500 drm/i915: Implement IS_* macros using static tables Signed-off-by:
-
Damien Lespiau authored
The code using this was removed in: commit 88f23b8f Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Sun Dec 5 15:08:31 2010 +0000 drm/i915: Avoid using PIPE_CONTROL on Ironlake Signed-off-by:
-
Damien Lespiau authored
This define hasn't been used since: commit 652c393a Author: Jesse Barnes <jbarnes@virtuousgeek.org> Date: Mon Aug 17 13:31:43 2009 -0700 drm/i915: add dynamic clock frequency control Signed-off-by:
-
Jesse Barnes authored
The existing code was trying different vswing and preemphasis settings in the wrong place, and wasn't trying them enough. So add a loop to walk through them, properly disabling FDI TX and RX in between if a failure is detected. v2: remove unneeded reg writes, add delays around bit lock checks (Jesse) v3: fix TX and RX disable per spec (Paulo) fix delays per spec (Paulo) make RX symbol lock check match TX bit lock check (Paulo) Signed-off-by: -
Ben Widawsky authored
The vma will [possibly] be destroyed during unbind in eviction. Immediately after this, we try to delete the list entry. Chris and Ville did the debug on this before I woke up, I just get to take credit for the fix :p For future reference the Oops that Mika reported: [ 403.472448] BUG: unable to handle kernel paging request at 6b6b6b6b [ 403.472473] IP: [<c12c1500>] __list_del_entry+0x20/0xe0 [ 403.472514] *pdpt = 000000002e89c001 *pde = 0000000000000000 [ 403.472556] Oops: 0000 [#1] SMP [ 403.472582] Modules linked in: mxm_wmi snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_seq_midi snd_rawmidi psmouse snd_seq_midi_event snd_seq serio_raw snd_timer snd_seq_device snd soundcore snd_page_alloc wmi bnep rfcomm bluetooth mac_hid parport_pc ppdev lp parport usbhid dm_crypt firewire_ohci firewire_core crc_itu_t i915 drm_kms_helper e1000e ptp drm i2c_algo_bit pps_core xhci_hcd video [ 403.472895] CPU: 2 PID: 1940 Comm: Xorg Not tainted 3.11.0-rc2+ #827 [ 403.472938] Hardware name: /DZ77BH-55K, BIOS BHZ7710H.86A.0070.2012.0416.2117 04/16/2012 [ 403.473002] task: ec866c00 ti: ee6a2000 task.ti: ee6a2000 [ 403.473039] EIP: 0060:[<c12c1500>] EFLAGS: 00013202 CPU: 2 [ 403.473078] EIP is at __list_del_entry+0x20/0xe0 [ 403.473109] EAX: f016d9bc EBX: f016d9bc ECX: 6b6b6b6b EDX: 6b6b6b6b [ 403.473151] ESI: 00000000 EDI: ee6a3c90 EBP: ee6a3c60 ESP: ee6a3c48 [ 403.473193] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 [ 403.473230] CR0: 80050033 CR2: 6b6b6b6b CR3: 2ec43000 CR4: 001407f0 [ 403.473271] Stack: [ 403.473285] f63b2ff0 f61f98c0 f61f8000 f016d9bc 00000000 f016d9bc ee6a3cac f8519a4a [ 403.473347] 00000000 00000000 10000000 f61f8000 0100a000 10000000 00000001 008ca000 [ 403.473410] f64ee840 f61f98c0 f016d9bc f016dcec ee6a3c98 ee6a3c98 f61f98c0 dcc58f00 [ 403.473472] Call Trace: [ 403.473509] [<f8519a4a>] i915_gem_evict_something+0x17a/0x2d0 [i915] [ 403.473567] [<f8516ed1>] i915_gem_object_pin+0x271/0x660 [i915] [ 403.473622] [<f851c740>] ? i915_ggtt_clear_range+0x20/0x20 [i915] [ 403.473676] [<f8517afa>] i915_gem_object_pin_to_display_plane+0xda/0x190 [i915] [ 403.473742] [<f852d9fa>] intel_pin_and_fence_fb_obj+0xba/0x140 [i915] [ 403.473800] [<f852db40>] intel_gen7_queue_flip+0x30/0x1c0 [i915] [ 403.473856] [<f85337b0>] intel_crtc_page_flip+0x1a0/0x320 [i915] [ 403.473911] [<f847b549>] ? drm_framebuffer_reference+0x39/0x80 [drm] [ 403.473965] [<f847f9fb>] drm_mode_page_flip_ioctl+0x28b/0x320 [drm] [ 403.474018] [<f846fec8>] drm_ioctl+0x4b8/0x560 [drm] [ 403.474064] [<f847f770>] ? drm_mode_gamma_get_ioctl+0xd0/0xd0 [drm] [ 403.474113] [<c1140f8a>] ? do_sync_read+0x6a/0xa0 [ 403.474154] [<f846fa10>] ? drm_copy_field+0x80/0x80 [drm] [ 403.474193] [<c115134c>] do_vfs_ioctl+0x7c/0x5b0 [ 403.474228] [<c1141d2f>] ? vfs_read+0xef/0x160 [ 403.474263] [<c108dcbb>] ? ktime_get_ts+0x4b/0x120 [ 403.474298] [<c1151917>] SyS_ioctl+0x97/0xa0 [ 403.474330] [<c1590bc1>] sysenter_do_call+0x12/0x22 [ 403.474364] Code: 55 f4 8b 45 f8 e9 75 ff ff ff 90 55 89 e5 53 83 ec 14 8b 08 8b 50 04 81 f9 00 01 10 00 74 24 81 fa 00 02 20 00 0f 84 8e 00 00 00 <8b> 1a 39 d8 75 62 8b 59 04 39 d8 75 35 89 51 04 89 0a 83 c4 14 [ 403.474566] EIP: [<c12c1500>] __list_del_entry+0x20/0xe0 SS:ESP 0068:ee6a3c48 [ 403.476513] CR2: 000000006b6b6b6b v2: Missed the drm_object_unreference use after free (Ville) Daniel Vetter <daniel@ffwll.ch> writes: Reported-by:
Mika Kuoppala <mika.kuoppala@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
- 22 Aug, 2013 23 commits
-
-
Ben Widawsky authored
In the new execbuf code we want to track buffers using the vmas even before they're all properly mapped. Which means that bind_to_vm needs to deal with buffers which have preallocated vmas which aren't yet bound. This patch implements this prep work and adjusts our WARN/BUG checks. Signed-off-by:
-
Ben Widawsky authored
The execbuf wants to do relocations usings vmas, so we need a vma->exec_list. The eviction code also uses the old obj execbuf list for it's own book-keeping, but would really prefer to deal in vmas only. So switch it over to the new list. Again this is just a prep patch for the big execbuf vma conversion. Signed-off-by:
-
Ben Widawsky authored
To convert the execbuf code over to use vmas natively we need to shuffle the exec_list a bit. This patch here just prepares things with the debugfs code, which also uses the old exec_list list_head, newly called obj_exec_link. Signed-off-by:
-
Josh Triplett authored
When building kernels for a preliminary hardware target, having to add a kernel command-line option can prove inconvenient. Add a Kconfig option that changes the default of this option to 1. Signed-off-by:
Josh Triplett <josh@joshtriplett.org> Reviewed-by:
-
Paulo Zanoni authored
Our driver initialization doesn't seem to be ready to load when the power well is disabled: we hit a few "Unclaimed register" messages. So do just like we already do for the suspend/resume path: enable the power well before unloading. At some point we'll want to be able to survive suspend/resume and load/unload with the power well disabled, but for now let's just fix the regression. Regression introduced by the following commit: commit bf51d5e2 Author: Paulo Zanoni <paulo.r.zanoni@intel.com> Date: Wed Jul 3 17:12:13 2013 -0300 drm/i915: switch disable_power_well default value to 1 Bug can be reproduced by running the "module_reload" script from intel-gpu-tools. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67813Signed-off-by:
-
Daniel Vetter authored
Makes it more obviously correct what tricks we play by reusing the drm prime release helper. Reviewed-by:
-
Daniel Vetter authored
This fixes a WARN in i915_gem_free_object when the obj->pages_pin_count isn't 0. v2: Add locking to unmap, noticed by Chris Wilson. Note that even though we call unmap with our own dev->struct_mutex held that won't result in an immediate deadlock since we never go through the dma_buf interfaces for our own, reimported buffers. But it's still easy to blow up and anger lockdep, but that's already the case with our ->map implementation. Fixing this for real will involve per dma-buf ww mutex locking by the callers. And lots of fun. So go with the duct-tape approach for now. Cc: Chris Wilson <chris@chris-wilson.co.uk> Reported-by:
Maarten Lankhorst <maarten.lankhorst@canonical.com> Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com> Tested-by: Armin K. <krejzi@email.com> (v1) Acked-by:
-
Vinit Azad authored
Un-masking all PM interrupts causes hardware to generate interrupts regardless of whether the interrupts are enabled on the DE side. Since turbo only need up/down threshold and rc6 timeout interrupt, mask all other interrupts bits to avoid unnecessary overhead/wake up. Note that our interrupt handler isn't being fired since we do set the IER bits properly (IIR bits aren't set). The overhead isn't because our driver is reacting to these interrupts, but because hardware keeps generating internal messages when PMINTRMSK doesn't mask out the up/down EI interrupts (which happen periodically). Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827 Signed-off-by:
Vinit Azad <vinit.azad@intel.com> [danvet: Add follow-up explanation of the precise effects from Vinit as a note to the commit message.] Signed-off-by:
-
Paulo Zanoni authored
Whenever I need to work with the HSW_PWER_WELL_* register bits I have to look at the documentation to find out which bit is to request the power well and which one shows its current state. Rename the bits so I won't need to look the docs every time. Signed-off-by:
-
Paulo Zanoni authored
If the power well is disabled VGA is guaranteed to be disabled. This fixes unclaimed register messages that happen on suspend/resume. v2: Check the actual hw power well state instead of our own tracking to make sure VGA is _really_ off (in case the BIOS/KVMr has just its own request bit set). Requested by Ville. Note: Ville suggested whether it wouldn't be better to just enable the power well over a slightly longer time in our resume code, since we already do that. I tend to agree, but there's also the modeset force code in the lid notifier which _also_ eventually calls redisable_vga. We shouldn't ever need this on somewhat modern hw (everything with opregion essentially) but the code to bail out isn't there. Hence stick with this simple approach here for now. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67517Signed-off-by:
-
Chris Wilson authored
By our earlier reckoning, move from a snooped/llc setting to an uncached setting, leaves the CPU cache in a consistent state irrespective of our domain tracking - so we can forgo the warning about the lack of invalidation. Similarly for any writes posted to the snooped CPU domain, we know will be safely clflushed to the uncached PTEs after forcing the domain change. This WARN started to pop up with commit d46f1c3f Author: Chris Wilson <chris@chris-wilson.co.uk> AuthorDate: Thu Aug 8 14:41:06 2013 +0100 drm/i915: Allow the GPU to cache stolen memory Ville brought up a scenario where the interaction of a set_caching ioctl call from userspace on a scanout buffer (i.e. obj->pin_display is set) resulted in the code getting confused and not properly flushing stale cpu cachelines. Luckily we already prevent this by rejecting caching changes when obj->pin_count is set. Signed-off-by:
cancan,feng <cancan.feng@intel.com> [danvet: Add buglink, bisect result and explain why Ville's scenario is already taken care of.] Signed-off-by:
-
Ben Widawsky authored
The conditional is usually a recoverable driver bug, and so WARNing, and preventing the drm_mm code from doing potential damage (BUG) is desirable. This issue was hit and fixed twice while developing the i915 multiple address space code. The first fix is the patch just before this, and is hit on an not frequently occuring error path. Another was fixed during patch iteration, so it's hard to see from the patch: commit c6cfb325 Author: Ben Widawsky <ben@bwidawsk.net> Date: Fri Jul 5 14:41:06 2013 -0700 drm/i915: Embed drm_mm_node in i915 gem obj From the intel-gfx mailing list, we discussed this: References: <20130705191235.GA3057@bwidawsk.net> Cc: Dave Airlie <airlied@redhat.com> CC: <dri-devel@lists.freedesktop.org> Acked-by:
Dave Airlie <airlied@redhat.com> Signed-off-by:
-
Daniel Vetter authored
Use () to make for neater alignment of the split lines, too. With this we ditch another jump through the obj_gtt_size/offset indirection maze. Cc: Ben Widawsky <benjamin.widawsky@intel.com> Signed-off-by: -
Ben Widawsky authored
Cleanup the map and fenceable setting during bind to make more sense, and not check i915_is_ggtt() 2 unnecessary times v2: Move the bools into the if block (Chris) - There are ways to tidy this function (fence calculations for instance) even further, but they are quite invasive, so I am punting on those unless specifically asked. v3: Add newline between variable declaration and logic (Chris) Recommended-by:
-
Ben Widawsky authored
VMAs can be created and not bound. One may think of it as lazy cleanup, and safely gloss over the conditions which manufacture it. In either case, when the object backing the i915 vma is destroyed, we must cleanup the vma without stumbling into a bunch of pitfalls that assume the vma is bound. NOTE: I was pretty certain the above condition could only happen when we introduced the use of VMAs being looked up at execbuf, and already existing. Paulo has hit this though, so I must be missing something. As I believe the patch is correct anyway, therefore I won't scratch my head too hard. v2: use goto destroy as a compromise (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by:
-
Daniel Vetter authored
Use the standard inversely ordered goto label stack for everything. Spotted while reviewing place where we might need to to call vma_destroy but failed to do so. Cc: Ben Widawsky <ben@bwidawsk.net> Signed-off-by: -
Ben Widawsky authored
Ideally we could use for_each_ring with the ring flags as I've done a couple times (http://lists.freedesktop.org/archives/intel-gfx/2013-June/029450.html). Until Daniel merges that patch though, we can just use this. Cc: Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by:
-
Ben Widawsky authored
We require n-1 mailboxes for proper semaphore synchronization. All semaphore synchronization code relies on proper values in these mailboxes. The fact that we failed to touch the vebox ring by itself was unlikely to be an issue since the HW should be initializing the values to 0. However the error framework for testing seqno wrap introduced by Mika, in addition to the hangcheck via seqno, and i915_error_first_batchbuffer() combined caused a nice explosion. The problem is caused by seqno wrap because the wrap condition is not properly setup. The wrap code attempts to set the sync mailboxes all to 0, and then set the current seqno to one less than 0. In all cases, the vebox mailbox wasn't properly being initialized. This caused a wrap to not occur. When hangcheck kicks in with the bogus seqno values, the rest just doesn't work. It makes me wonder if we shouldn't consider a dumber version of hangcheck... How we messed this up: VECS support was written before the aforementioned other features. Upon VECS being rebased, these facts were missed. Cc: Mika Kuoppala <mika.kuoppala@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65387 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67198Signed-off-by:
-
Stéphane Marchesin authored
It's basically the same deal as the RC6+ issues on ivy bridge except this time with RC6 on sandy bridge. Like last time the core of the issue is that the timings don't work 100% with our voltage regulator. So from time to time, the kernel will print a warning message about the GPU not getting out of RC6. In particular, I found this fairly easy to reproduce during suspend/resume. Changing the threshold to 125000 instead of 50000 seems to fix the issue. The previous patch used 150000 but as it turns out this doesn't work everywhere. After getting such a machine, I bisected the highest value which works, which is 125000, so here it is. I also measured the idle power usage before/after this patch and didn't see a difference on a sandy bridge laptop. On haswell and up, it makes a big difference, so we want to keep it at 50k there. It also seems like haswell doesn't have the RC6 issues that sandy bridge has so the 50k value is fine. Signed-off-by:
Stéphane Marchesin <marcheu@chromium.org> Acked-by:
-
Paulo Zanoni authored
The machines that fall in this category are the SDVs that have a PCI ID starting with 0x0C. These are very early pre-production machines and may not fully work. Other Haswell SDVs have PCI IDs that match the real Haswell machines and we expect them to work better. Even though they have problems, they still mostly work so I don't see a reason to refuse loading our driver. But I do see a reason to reject bug reports from these machines, so the message should help the bug triagers. As far as I know, we don't implement some workarounds that are specific to these machines and suspend/resume may not work on most of them, but besides this, they may work. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61508Signed-off-by:
-
Chris Wilson authored
After computing the stage changes for the set_config, record those in the debug log. Signed-off-by:
-
Paulo Zanoni authored
Caught by "make W=1 drivers/gpu/drm/i915/". Signed-off-by:
-
Chris Wilson authored
This is primarily for the benefit of the create2 ioctl so that the caller can avoid the later step of rebinding the bo with new PTE bits. After introducing WT (and possibly GFDT) cacheing for display targets, not everything in the display is earmarked as UC, and more importantly what is is controlled by the kernel. Note that set_cache_level/get_cache_level for DISPLAY is not necessarily idempotent; get_cache_level may return UC for architectures that have no special cache domain for the display engine. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
-
