- 21 Jan, 2013 2 commits
-
-
Daniel Vetter authored
Damien Lespiau wondered how race the gpu reset/hang detection code is against concurrent gpu resets/hang detections or combinations thereof. Luckily the single work item is guranteed to never run concurrently, so reset handling is already single-threaded. Hence we only have to worry about concurrent hang detections, or a hang detection firing off while we're still processing an older gpu reset request. Due to the new mechanism of setting the reset in progress flag and the ordering guaranteed by the schedule_work function there's nothing to do but add a comment explaining why we're safe. The only thing I've noticed is that we still try to reset the gpu now, even when it is declared terminally wedged. Add a check for that to avoid continous warnings about failed resets, in case the hangcheck timer ever gets stuck. Reviewed-by:
Damien Lespiau <damien.lespiau@intel.com> Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Daniel Vetter authored
With the previous patch the state transition handling of the reset code itself is now (hopefully) race free and solid. But that still leaves out everyone else - with the various lock-free wait paths we have there's the possibility that the reset happens between the point where we read the seqno we should wait on and the actual wait. And if __wait_seqno then never sees the RESET_IN_PROGRESS state, we'll happily wait for a seqno which will in all likelyhood never signal. In practice this is not a big problem since the X server gets constantly interrupted, and can then submit more work (hopefully) to unblock everyone else: As soon as a new seqno write lands, all waiters will unblock. But running the i-g-t reset testcase ZZ_hangman can expose this race, especially on slower hw with fewer cpu cores. Now looking forward to ARB_robustness and friends that's not the best possible behaviour, hence this patch adds a reset_counter to be able to detect any reset, even if a given thread never observed the in-progress state. The important part is to correctly order things: - The write side needs to increment the counter after any seqno gets reset. Hence we need to do that at the end of the reset work, and again wake everyone up. We also need to place a barrier in between any possible seqno changes and the counter increment, since any unlock operations only guarantee that nothing leaks out, but not that at later load operation gets moved ahead. - On the read side we need to ensure that no reset can sneak in and invalidate the seqno. In all cases we can use the one-sided barrier that unlock operations guarantee (of the lock protecting the respective seqno/ring pair) to ensure correct ordering. Hence it is sufficient to place the atomic read before the mutex/spin_unlock and no additional barriers are required. The end-result of all this is that we need to wake up everyone twice in a reset operation: - First, before the reset starts, to get any lockholders of the locks, so that the reset can proceed. - Second, after the reset is completed, to allow waiters to properly and reliably detect the reset condition and bail out. I admit that this entire reset_counter thing smells a bit like overkill, but I think it's justified since it makes it really explicit what the bail-out condition is. And we need a reset counter anyway to implement ARB_robustness, and imo with finer-grained locking on the horizont this is the most resilient scheme I could think of. v2: Drop spurious change in the wait_for_error EXIT_COND - we only need to wait until we leave the reset-in-progress wedged state. v3: Don't play tricks with barriers in the throttle ioctl, the spin_unlock is barrier enough. I've also considered using a little helper to grab the current reset_counter, but then decided that hiding the atomic_read isn't a great idea, since having it explicitly show up in the code is a nice remainder to reviews to check the memory barriers. v4: Add a comment to explain why we need to fall through in __wait_seqno in the end variable assignments. v5: Review from Damien: - s/smb/smp/ in a comment - don't increment the reset counter after we've set it to WEDGED. Now we (again) properly wedge the gpu when the reset fails. Reviewed-by:
-
- 20 Jan, 2013 16 commits
-
-
Chris Wilson authored
Now that we seem to have brought order to the GTT barriers, the last one to review is the terminal barrier before we unbind the buffer from the GTT. This needs to only be performed if the buffer still resides in the GTT domain, and so we can skip some needless barriers otherwise. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by:
-
Chris Wilson authored
With a fence, we only need to insert a memory barrier around the actual fence alteration for CPU accesses through the GTT. Performing the barrier in flush-fence was inserting unnecessary and expensive barriers for never fenced objects. Note removing the barriers from flush-fence, which was effectively a barrier before every direct access through the GTT, revealed that we where missing a barrier before the first access through the GTT. Lack of that barrier was sufficient to cause GPU hangs. v2: Add a couple more comments to explain the new barriers Signed-off-by:
-
Daniel Vetter authored
We have two important transitions of the wedged state in the current code: - 0 -> 1: This means a hang has been detected, and signals to everyone that they please get of any locks, so that the reset work item can do its job. - 1 -> 0: The reset handler has completed. Now the last transition mixes up two states: "Reset completed and successful" and "Reset failed". To distinguish these two we do some tricks with the reset completion, but I simply could not convince myself that this doesn't race under odd circumstances. Hence split this up, and add a new terminal state indicating that the hw is gone for good. Also add explicit #defines for both states, update comments. v2: Split out the reset handling bugfix for the throttle ioctl. v3: s/tmp/wedged/ sugested by Chris Wilson. Also fixup up a rebase error which prevented this patch from actually compiling. v4: To unify the wedged state with the reset counter, keep the reset-in-progress state just as a flag. The terminally-wedged state is now denoted with a big number. v5: Add a comment to the reset_counter special values explaining that WEDGED & RESET_IN_PROGRESS needs to be true for the code to be correct. v6: Fixup logic errors introduced with the wedged+reset_counter unification. Since WEDGED implies reset-in-progress (in a way we're terminally stuck in the dead-but-reset-not-completed state), we need ensure that we check for this everywhere. The specific bug was in wait_for_error, which would simply have timed out. v7: Extract an inline i915_reset_in_progress helper to make the code more readable. Also annote the reset-in-progress case with an unlikely, to help the compiler optimize the fastpath. Do the same for the terminally wedged case with i915_terminally_wedged. Reviewed-by:
-
Daniel Vetter authored
While auditing the code I've noticed one place (the throttle ioctl) which does not yet wait for the reset handler to complete and doesn't properly decode the wedge state into -EAGAIN/-EIO. Fix this up by calling the right helpers. This might explain the oddball "my compositor just died in a successfull gpu reset" reports. Or maybe not, since current mesa doesn't use this ioctl to throttle command submission. The throttle ioctl doesn't take the struct_mutex, so to avoid busy-looping with -EAGAIN while a reset is in process, check for errors first and wait for the handler to complete if a reset is pending by calling i915_gem_wait_for_error. Reviewed-by:
-
Daniel Vetter authored
And to make Ben Widawsky happier, use the gpu_error instead of the entire device as the argument in some functions. Drop the outdated comment on ->wedged for now, a follow-up patch will change the semantics and add a proper comment again. Reviewed-by:
-
Daniel Vetter authored
This has been sprinkled all over the place in dev_priv. I think it'd be good to also move all the code into a separate file like i915_gem_error.c, but that's for another patch. Reviewed-by:
-
Daniel Vetter authored
Tha one is really big, since it contains tons of comments explaining how things work. Which is nice ;-) Reviewed-by:
-
Ben Widawsky authored
It is no longer used in the i915 code, so isolate it from the shared struct. This was originally part of: commit 0e275518f325418d559c05327775bff894b237f7 Author: Ben Widawsky <ben@bwidawsk.net> Date: Mon Jan 14 13:35:33 2013 -0800 agp/intel: decouple more of the agp-i915 sharing Reviewed-by:Rodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by:
Ben Widawsky <ben@bwidawsk.net> That commit had some other hunks which can't be used due to issues Daniel found in previous commits. Signed-off-by:
-
Ben Widawsky authored
The reasoning behind our code taking two paths depending upon whether or not we may have been configured for IOMMU isn't clear to me. It should always be safe to use the pci mapping functions as they are designed to abstract the decision we were handling in i915. Aside from simpler code, removing another member for the intel_gtt struct is a nice motivation. I ran this by Chris, and he wasn't concerned about the extra kzalloc, and memory references vs. page_to_phys calculation in the case without IOMMU. v2: Update commit message v3: Remove needs_dmar addition from Zhenyu upstream This reverts (and then other stuff) commit 20652097 Author: Zhenyu Wang <zhenyuw@linux.intel.com> Date: Thu Dec 13 23:47:47 2012 +0800 drm/i915: Fix missed needs_dmar setting Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> (v2) Cc: Zhenyu Wang <zhenyuw@linux.intel.com> Signed-off-by:
-
Ben Widawsky authored
We already had a mapping in both (minus the phys_addr in AGP). Reviewed-by:
-
Ben Widawsky authored
And, move it to where the rest of the logic is. There is some slight functionality changes. There was extra paranoid checks in AGP code making sure we never do idle maps on gen2 parts. That was not duplicated as the simple PCI id check should do the right thing. v2: use IS_GEN5 && IS_MOBILE check instead. For now, this is the same as IS_IRONLAKE_M but is more future proof. The workaround docs hint that more than one platform may be effected, but we've never seen such a platform in the wild. (Rodrigo, Daniel) Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> (v1) Cc: Dave Airlie <airlied@redhat.com> Signed-off-by:
-
Ville Syrjälä authored
The AVI infoframe is able to inform the display whether the source is sending full or limited range RGB data. As per CEA-861 [1] we must first check whether the display reports the quantization range as selectable, and if so we can set the approriate bits in the AVI inforframe. [1] CEA-861-E - 6.4 Format of Version 2 AVI InfoFrame v2: Give the Q bits better names, add spec chapter information Reviewed-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
-
Ville Syrjälä authored
drm_rgb_quant_range_selectable() will report whether the monitor claims to support for RGB quantization range selection. The information can be found in the CEA Video capability block. v2: s/quantzation/quantization/ in the comment Reviewed-by:
David Airlie <airlied@linux.ie> Signed-off-by:
-
Ville Syrjälä authored
Add a new "Automatic" mode to the "Broadcast RGB" range property. When selected the driver automagically selects between full range and limited range output. Based on CEA-861 [1] guidelines, limited range output is selected if the mode is a CEA mode, except 640x480. Otherwise full range output is used. Additionally DVI monitors should most likely default to full range always. As per DP1.2a [2] DisplayPort should always use full range for 18bpp, and otherwise will follow CEA-861 rules. NOTE: The default value for the property will now be "Automatic" so some people may be affected in case they're relying on the current full range default. [1] CEA-861-E - 5.1 Default Encoding Parameters [2] VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry v2: Use has_hdmi_sink to check if a HDMI monitor is present v3: Add information about relevant spec chapters Reviewed-by:
-
Ville Syrjälä authored
The RGB color range select bit on the DP/SDVO/HDMI registers disappeared when PCH was introduced, and instead a new PIPECONF bit was added that performs the same function. Add a new INTEL_MODE_LIMITED_COLOR_RANGE private mode flag, and set it in the encoder mode_fixup if limited color range is requested. Set the the PIPECONF bit 13 based on the flag. Experimentation showed that simply toggling the bit while the pipe is active doesn't work. We need to restart the pipe, which luckily already happens. The DP/SDVO/HDMI bit 8 is marked MBZ in the docs, so avoid setting it, although it doesn't seem to do any harm in practice. TODO: - the PIPECONF bit too seems to have disappeared from HSW. Need a volunteer to test if it's just a documentation issue or if it's really gone. If the bit is gone and no easy replacement is found, then I suppose we may need to use the pipe CSC unit to perform the range compression. v2: Use mode private_flags instead of intel_encoder virtual functions v3: Moved the intel_dp color_range handling after bpc check to help later patches Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46800Reviewed-by: -
Ben Widawsky authored
Mappable_end, ie. size is almost always what you want as opposed to the number of entries. Since we already have that information, we can scrap the number of entries and only calculate it when needed. If gtt_start is !0, this will have slightly different behavior. This difference can only occur in DRI1, and exists when we try to kick out the firmware fb. The new code seems like a bugfix to me. The other case where we've changed the behavior is during init we check the mappable region against our current known upper and lower limits (64MB, and 512MB). This now matches the comment, and makes things more convenient after removing gtt_mappable_entries. Also worth noting is the setting of mappable_end is taken out of setup because we do it earlier now in the DRI2 case and therefore need to add that tiny hunk to support the DRI1 IOCTL. v2: Move up mappable end to before legacy AGP init v3: Add the dev_priv inclusion here from previous rebase error in patch 5 Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> (v2) Signed-off-by:
-
- 17 Jan, 2013 19 commits
-
-
Ben Widawsky authored
We have enough info to not use the intel_gtt bridge stuff. v2: Move setup of mappable_base above the legacy init stuff because we still need that on older platforms. (Daniel) v3: Remove the dev_priv hunk which was rebased in by accident Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> (v2) Signed-off-by:
-
Ben Widawsky authored
The purpose of the gtt structure is to help isolate our gtt specific properties from the rest of the code (in doing so it help us finish the isolation from the AGP connection). The following members are pulled out (and renamed): gtt_start gtt_total gtt_mappable_end gtt_mappable gtt_base_addr gsm The gtt structure will serve as a nice place to put gen specific gtt routines in upcoming patches. As far as what else I feel belongs in this structure: it is meant to encapsulate the GTT's physical properties. This is why I've not added fields which track various drm_mm properties, or things like gtt_mtrr (which is itself a pretty transient field). Reviewed-by:
-
Ben Widawsky authored
With the assertion from the previous patch in place, it should be safe to get rid gtt_mappable_total. Keeps things saner to not have to track the same info in two places. In order to keep the diff as simple as possible and keep with the existing gtt_setup semantics we opt to keep gtt_mappable_end. It's not as consistent with the 'total' used in the previous patch, but that can be fixed later. Reviewed-by:
-
Ben Widawsky authored
Both DRI1 and DRI2 can never specify a mappable size which goes past the GTT size. Don't pretend otherwise. Reviewed-by:
-
Ben Widawsky authored
It's duplicated in the more useful gtt_total. Reviewed-by:
-
Ville Syrjälä authored
Fix up some copypaste errors in the PIPESTAT register for VLV. SPRITE0_FLIP_DONE_INT_EN_VLV is bit 22, not bit 26. SPRITE0_FLIPDONE_INT_STATUS_VLV is bit 14, not bit 15. Signed-off-by:
-
Chris Wilson authored
Using copywinwin10 as an example that is dependent upon emitting a lot of relocations (2 per operation), we see improvements of: c2d/gm45: 618000.0/sec to 623000.0/sec. i3-330m: 748000.0/sec to 789000.0/sec. (measured relative to a baseline with neither optimisations applied). Signed-off-by:
Imre Deak <imre.deak@intel.com> Signed-off-by:
-
Daniel Vetter authored
Userspace is able to hint to the kernel that its command stream and auxiliary state buffers already hold the correct presumed addresses and so the relocation process may be skipped if the kernel does not need to move any buffers in preparation for the execbuffer. Thus for the common case where the allotment of buffers is static between batches, we can avoid the overhead of individually checking the relocation entries. Note that this requires userspace to supply the domain tracking and requests for workarounds itself that would otherwise be computed based upon the relocation entries. Using copywinwin10 as an example that is dependent upon emitting a lot of relocations (2 per operation), we see improvements of: c2d/gm45: 618000.0/sec to 632000.0/sec. i3-330m: 748000.0/sec to 830000.0/sec. (measured relative to a baseline with neither optimisations applied). Signed-off-by:
-
Chris Wilson authored
Instead of passing around the eb-objects hashtable and a separate object list, we can include the object list into the eb-objects structure for convenience. Signed-off-by:
-
Chris Wilson authored
Signed-off-by:
-
Chris Wilson authored
The difference is that the kernel will then know that this memory will be reclaimable in the near future. Signed-off-by:
-
Chris Wilson authored
Move the existing checking inside bind_to_gtt() to the more appropriate layer in order to prevent recreation of the pages after they have been explicitly truncated. Signed-off-by:
-
Chris Wilson authored
As a means to investigate some bad system behaviour related to the purging of the active, inactive and unbound lists, it is useful to be able to manually control when those lists should be cleared. v2: use _safe list iterators as we kick objects from the list as we walk. Signed-off-by:
-
Imre Deak authored
Signed-off-by:
-
Imre Deak authored
The two functions are rather similar, so merge them. Signed-off-by:
-
Imre Deak authored
The two functions are rather similar, so merge them. Signed-off-by:
-
Egbert Eich authored
This variable is only used locally in the irq postinstall functions for ivybridge and ironlake. Signed-off-by:
Egbert Eich <eich@suse.de> Signed-off-by:
-
Daniel Vetter authored
Otherwise it seems like we can get stuck with concurrent waiters. Right now this /shouldn't/ be a problem, since all pending pageflip waiters are serialized by the one mode_config.mutex, so there's at most on waiter. But better paranoid than sorry, since this is tricky code. v2: WARN_ON(waitqueue_active) before waiting, as suggested by Chris Wilson. Acked-by:
-
Dave Airlie authored
Merge tag 'drm-intel-next-2012-12-21' of git://people.freedesktop.org/~danvet/drm-intel into drm-next Daniel writes: - seqno wrap fixes and debug infrastructure from Mika Kuoppala and Chris Wilson - some leftover kill-agp on gen6+ patches from Ben - hotplug improvements from Damien - clear fb when allocated from stolen, avoids dirt on the fbcon (Chris) - Stolen mem support from Chris Wilson, one of the many steps to get to real fastboot support. - Some DDI code cleanups from Paulo. - Some refactorings around lvds and dp code. - some random little bits&pieces * tag 'drm-intel-next-2012-12-21' of git://people.freedesktop.org/~danvet/drm-intel: (93 commits) drm/i915: Return the real error code from intel_set_mode() drm/i915: Make GSM void drm/i915: Move GSM mapping into dev_priv drm/i915: Move even more gtt code to i915_gem_gtt drm/i915: Make next_seqno debugs entry to use i915_gem_set_seqno drm/i915: Introduce i915_gem_set_seqno() drm/i915: Always clear semaphore mboxes on seqno wrap drm/i915: Initialize hardware semaphore state on ring init drm/i915: Introduce ring set_seqno drm/i915: Missed conversion to gtt_pte_t drm/i915: Bug on unsupported swizzled platforms drm/i915: BUG() if fences are used on unsupported platform drm/i915: fixup overlay stolen memory leak drm/i915: clean up PIPECONF bpc #defines drm/i915: add intel_dp_set_signal_levels drm/i915: remove leftover display.update_wm assignment drm/i915: check for the PCH when setting pch_transcoder drm/i915: Clear the stolen fb before enabling drm/i915: Access to snooped system memory through the GTT is incoherent drm/i915: Remove stale comment about intel_dp_detect() ... Conflicts: drivers/gpu/drm/i915/intel_display.c
-
- 10 Jan, 2013 1 commit
-
-
Linus Torvalds authored
-
- 09 Jan, 2013 2 commits
-
-
git://git.linaro.org/people/rmk/linux-armLinus Torvalds authored
Pull ARM fixes from Russell King. * 'fixes' of git://git.linaro.org/people/rmk/linux-arm: ARM: 7616/1: cache-l2x0: aurora: Use writel_relaxed instead of writel ARM: 7615/1: cache-l2x0: aurora: Invalidate during clean operation with WT enable ARM: 7614/1: mm: fix wrong branch from Cortex-A9 to PJ4b ARM: 7612/1: imx: Do not select some errata that depends on !ARCH_MULTIPLATFORM ARM: 7611/1: VIC: fix bug in VIC irqdomain code ARM: 7610/1: versatile: bump IRQ numbers ARM: 7609/1: disable errata work-arounds which access secure registers ARM: 7608/1: l2x0: Only set .set_debug on PL310 r3p0 and earlier
-
git://git.kernel.org/pub/scm/linux/kernel/git/bp/bpLinus Torvalds authored
Pull EDAC fixes from Borislav Petkov: "Two error path fixes causing a crash and a Kconfig fix for an issue which spilled all EDAC suboptions into the 'Device Drivers' menu." * tag 'edac_fixes_for_3.8' of git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp: EDAC: Cleanup device deregistering path EDAC: Fix EDAC Kconfig menu EDAC: Fix kernel panic on module unloading
-
