- 10 Aug, 2016 13 commits
-
-
Imre Deak authored
Atm, we apply this workaround somewhat inconsistently at the following points: driver loading, LVDS init, eDP PPS init, system resume. As this workaround also affects registers other than PPS (timing, PLL) a more consistent way is to apply it early after the PPS HW context is known to be lost: driver loading, system resume and on VLV/CHV/BXT when turning on power domains. This is needed by the next patch that removes saving/restoring of the PP_CONTROL register. This also removes the incorrect programming of the workaround on HSW+ PCH platforms which don't have the register locking mechanism. v2: (Ville) - Don't apply the workaround on BXT. - Simplify platform checks using HAS_DDI(). v3: - Move the call of intel_pps_unlock_regs_wa() to the more logical vlv_display_power_well_init() (also fixing CHV) (Ville). Signed-off-by:
Imre Deak <imre.deak@intel.com> Reviewed-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470827254-21954-5-git-send-email-imre.deak@intel.com
-
Imre Deak authored
Similarly to the previous patch, initialize the PPS from the DP encoder's resume hook. Note that as opposed to LVDS we can't do this during encoder enabling, since we need the PPS for DP detection as well. The PPS init code is now the same for init and resume, so factor out a new intel_dp_pps_init() helper for this. v2: - Factor out intel_dp_pps_init() (Ville). Signed-off-by:
-
Imre Deak authored
Atm the LVDS encoder depends on the PPS HW context being saved/restored from generic suspend/resume code. Since the PPS is specific to the LVDS and eDP encoders a cleaner way is to reinitialize it during encoder enabling, so do this here for LVDS. Follow-up patches will init the PPS for the eDP encoder similarly and remove the suspend/resume time save / restore. v2: - Apply BSpec +1 offset and use DIV_ROUND_UP() when programming the power cycle delay. (Ville) v3: (Ville) - Fix +1 vs. round-up order. - s/reset_on_powerdown/powerdown_on_reset/ Signed-off-by:
-
Imre Deak authored
These two flags mean the same thing, so remove the duplication. Signed-off-by:
-
Imre Deak authored
The PPS registers are pretty much the same everywhere, the differences being: - Register fields appearing, disappearing from one platform to the next: panel-reset-on-powerdown, backlight-on, panel-port, register-unlock - Different register base addresses - Different number of PPS instances: 2 on VLV/CHV/BXT, 1 everywhere else. We can merge the separate set of PPS definitions by extending the PPS instance argument to all platforms and using instance 0 on platforms with a single instance. This means we'll need to calculate the register addresses dynamically based on the given platform and PPS instance. v2: - Simplify if ladder in intel_pps_get_registers(). (Ville) Signed-off-by:
-
Dave Gordon authored
As we're tweaking the GuC-related code in debugfs, we can drop the no-longer-used 'q_fail' and repack the structure. Signed-off-by:
Dave Gordon <david.s.gordon@intel.com> Reviewed-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by:
-
Dave Gordon authored
Now that host structures are indexed by host engine-id rather than guc_id, we can usefully convert some for_each_engine() loops to use for_each_engine_id() and avoid multiple dereferences of engine->id. Also a few related tweaks to cache structure members locally wherever they're used more than once or twice, hopefully eliminating memory references. Signed-off-by:
-
Dave Gordon authored
The Context Descriptor passed by the kernel to the GuC contains a field specifying which engine(s) the context will use. Historically, this was always set to "all of them", but if we had a separate client for each engine, we could be more precise, and set only the bit for the engine that the client was associated with. So this patch enables this usage, in preparation for having multiple clients, though at this point there is still only a single client used for all supported engines. Signed-off-by:
-
Dave Gordon authored
We have essentially the same code in each of two different loops, so we can refactor it into a little helper function. This also reduces the amount of work done during startup, as we now only reprogram h/w found to be in a state other than that expected, and so avoid the overhead of setting doorbell registers to the state they're already in. Signed-off-by:
-
Dave Gordon authored
guc_init_doorbell_hw() borrows the (currently single) GuC client to use in reinitialising ALL the doorbell registers (as the hardware doesn't reset them when the GuC is reset). As a prerequisite for accommodating multiple clients, it should only reset doorbells that are supposed to be disabled, avoiding those that are marked as in use by any client. Signed-off-by:
-
Chris Wilson authored
The bottom-half we use for processing the breadcrumb interrupt is a task, which is an RCU protected struct. When accessing this struct, we need to be holding the RCU read lock to prevent it disappearing beneath us. We can use the RCU annotation to mark our irq_seqno_bh pointer as being under RCU guard and then use the RCU accessors to both provide correct ordering of access through the pointer. Most notably, this fixes the access from hard irq context to use the RCU read lock, which both Daniel and Tvrtko complained about. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Reviewed-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1470761272-1245-3-git-send-email-chris@chris-wilson.co.uk
-
Chris Wilson authored
In commit 2529d570 ("drm/i915: Drop racy markup of missed-irqs from idle-worker") the racy detection of missed interrupts was removed when we went idle. This however opened up the issue that the stuck waiters were not being reported, causing a test case failure. If we move the stuck waiter detection out of hangcheck and into the breadcrumb mechanims (i.e. the waiter) itself, we can avoid this issue entirely. This leaves hangcheck looking for a stuck GPU (inspecting for request advancement and HEAD motion), and breadcrumbs looking for a stuck waiter - hopefully make both easier to understand by their segregation. v2: Reduce the error message as we now run independently of hangcheck, and the hanging batch used by igt also counts as a stuck waiter causing extra warnings in dmesg. v3: Move the breadcrumb's hangcheck kickstart to the first missed wait. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97104 Fixes: 2529d570 (waiter"drm/i915: Drop racy markup of missed-irqs...") Testcase: igt/drv_missed_irq Signed-off-by:
Mika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470761272-1245-2-git-send-email-chris@chris-wilson.co.uk
-
Chris Wilson authored
One of the few guarantees we want the busy ioctl to provide is that the reported busy writer is included in the set of busy read engines. This should be provided by the ordering of setting and retiring the active trackers, but we can do better by explicitly setting the busy read engine flag for the last writer. v2: More comments inside __busy_write_id() to explain why both fields are set. Fixes: 3fdc13c7 ("drm/i915: Remove (struct_mutex) locking for busy-ioctl") Signed-off-by:
-
- 09 Aug, 2016 2 commits
-
-
Dave Gordon authored
Recent versions of gcc say this: include/drm/i915_drm.h:96:34: warning: result of ‘65535 << 20’ requires 37 bits to represent, but ‘int’ only has 32 bits [-Wshift-overflow=] Reported-by:
David Binderman <linuxdev.baldrick@gmail.com> Signed-off-by:
-
Chris Wilson authored
I mistyped and added an extra _request_ to __i915_gem_active_get_rcu() Also, the same happened to another comment for i915_gem_active_get_rcu() Reported-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Signed-off-by:
-
- 05 Aug, 2016 6 commits
-
-
Ville Syrjälä authored
We don't have GPU reset support for gen2, which means the display hardware is unaffected when a GPU hang is handled. However as the ring has in fact stopped, any flips still in the ring will never complete, and thus the display base address updates will never happen. So we really need to fix that up manually just like we do on g4x+. In fact, let's just use intel_has_gpu_reset() instead of IS_GEN2() since that'll also handle cases where someone would disable the GPU reset support on gen3/4 for whatever reason. Signed-off-by:
-
Ville Syrjälä authored
Factor out the "does the GPU reset clobber the display?" check into a small helper. Signed-off-by:
-
Maarten Lankhorst authored
Add force_reset_modeset_test as a parameter to force the modeset path during gpu reset. This allows a IGT test to set the knob and trigger a hang to force the gpu reset, even on platforms that wouldn't otherwise require it. Changes since v1: - Split out fix to separate commit. Changes since v2: - This commit is purely about force_reset_modeset_test now. Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Testcase: drv_hangman.reset-with-forced-modeset Tested-by:
-
Maarten Lankhorst authored
This function would call drm_modeset_lock_all, while the suspend/resume functions already have their own locking. Fix this by factoring out __intel_display_resume, and calling the atomic helpers for duplicating atomic state and disabling all crtc's during suspend. Changes since v1: - Deal with -EDEADLK right after lock_all and clean up calls to hw readout. - Always take all modeset locks so updates during gpu reset are blocked. Changes since v2: - Fix deadlock in intel_update_primary_planes. - Move WARN_ON(EDEADLK) to __intel_display_resume. - pctx -> ctx - only call __intel_display_resume on success in intel_display_resume. Changes since v3: - Rebase on top of dev_priv -> dev change. - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all. Changes since v4 [by vsyrjala]: - Deal with skip_intermediate_wm - Update comment w.r.t. mode_config.mutex vs. ->detect() - Rebase due to INTEL_GEN() etc. Signed-off-by:
-
Ville Syrjälä authored
Marking PCH transcoder FIFO underrun reporting as disabled for transcoder B/C on LPT-H will block us from enabling the south error interrupt. So let's only mark transcoder A underrun reporting as disabled initially. This is a little tricky to hit since you need a machine with LPT-H, and the BIOS must enable either pipe B or C at boot. Then i915 would mark the "transcoder B/C" underrun reporting as disabled and never enable it again, meaning south interrupts would never get enabled either. The only other interrupt in there is actually the poison interrupt which, if we could ever trigger it, would just result in a little error in dmesg. Here's the resulting change in SDEIMR on my HSW when I boot it with multiple displays attached: - (0x000c4004): 0xf115ffff + (0x000c4004): 0xf114ffff My previous attempt [1] tried to fix this a little differently, but Daniel requested I do this instead. [1] https://lists.freedesktop.org/archives/intel-gfx/2015-November/081420.html Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by:
-
Ville Syrjälä authored
intel_enable_pipe() looks rather confusing when one side doesn't have the curly braces, and the other one does. And what's even worse, there's another if-else inside the braceless side. Let's put braces around it to make it clear which branch goes where. Signed-off-by:
-
- 09 Aug, 2016 5 commits
-
-
Chris Wilson authored
When using RCU lookup for the request, commit 0eafec6d ("drm/i915: Enable lockless lookup of request tracking via RCU"), we acknowledge that we may race with another thread that could have reallocated the request. In order for the first thread not to blow up, the second thread must not clear the request completed before overwriting it. In the RCU lookup, we allow for the engine/seqno to be replaced but we do not allow for it to be zeroed. The choice we make is to either add extra checking to the RCU lookup, or embrace the inherent races (as intended). It is more complicated as we need to manually clear everything we depend upon being zero initialised, but we benefit from not emiting the memset() to clear the entire frequently allocated structure (that memset turns up in throughput profiles). And at the same time, the lookup remains flexible for future adjustments. v2: Old style LRC requires another variable to be initialize. (The danger inherent in not zeroing everything.) v3: request->batch also needs to be cleared v4: signaling.tsk is no long used unset, but pid still exists Fixes: 0eafec6d ("drm/i915: Enable lockless lookup of request...") Signed-off-by:
-
Chris Wilson authored
In the debate as to whether the second read of active->request is ordered after the dependent reads of the first read of active->request, just give in and throw a smp_rmb() in there so that ordering of loads is assured. v2: Explain the manual smp_rmb() Signed-off-by:
-
Chris Wilson authored
When we force the cleanup after a GPU hang, we want to retire all requests, or else we may leak them if truly wedged (and the GPU never advances again). Converting to the active request helpers had the issue of doing the check against busyness before reporting the request, so if we claim the GPU had hung but this engine hadn't we could potential skip the request cleanup - triggering the self-check BUG. Fixes: dcff85c8 ("drm/i915: Enable i915_gem_wait_for_idle() ...") Signed-off-by:
-
Chris Wilson authored
As we do the lockdep protected RCU lookup in a couple of places, refactor that code to a common helper i915_gem_active_raw(). Signed-off-by:
-
Chris Wilson authored
This was originally introduced to be used by the busy-ioctl, but in the end busy ioctl performed a different dance. Since there are no users, and no likely users, remove an unwanted chunk of the API. Suggested-by:
-
- 08 Aug, 2016 1 commit
-
-
Daniel Vetter authored
Signed-off-by:
-
- 05 Aug, 2016 13 commits
-
-
Matthew Auld authored
In i915_ggtt_cleanup_hw we need to remember to free aliasing_ppgtt. This fixes the following kmemleak message: unreferenced object 0xffff880213cca000 (size 8192): comm "modprobe", pid 1298, jiffies 4294745402 (age 703.930s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [<ffffffff817c808e>] kmemleak_alloc+0x4e/0xb0 [<ffffffff8121f9c2>] kmem_cache_alloc_trace+0x142/0x1d0 [<ffffffffa06d11ef>] i915_gem_init_ggtt+0x10f/0x210 [i915] [<ffffffffa06d71bb>] i915_gem_init+0x5b/0xd0 [i915] [<ffffffffa069749a>] i915_driver_load+0x97a/0x1460 [i915] [<ffffffffa06a26ef>] i915_pci_probe+0x4f/0x70 [i915] [<ffffffff81423015>] local_pci_probe+0x45/0xa0 [<ffffffff81424463>] pci_device_probe+0x103/0x150 [<ffffffff81515e6c>] driver_probe_device+0x22c/0x440 [<ffffffff81516151>] __driver_attach+0xd1/0xf0 [<ffffffff8151379c>] bus_for_each_dev+0x6c/0xc0 [<ffffffff8151555e>] driver_attach+0x1e/0x20 [<ffffffff81514fa3>] bus_add_driver+0x1c3/0x280 [<ffffffff81516aa0>] driver_register+0x60/0xe0 [<ffffffff8142297c>] __pci_register_driver+0x4c/0x50 [<ffffffffa013605b>] 0xffffffffa013605b Signed-off-by:Matthew Auld <matthew.auld@intel.com> Reviewed-by:
-
Daniel Vetter authored
~jiffie and a few usecs is 3 orders of magnitude different. A bit much. This was changed in commit ca5b721e Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Fri Dec 11 11:32:58 2015 +0000 drm/i915: Limit the busy wait on requests to 5us not 10ms! But probably missed the comment since the change was non-local to the comment. v2: Polish comment more (Chris). Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Acked-by:
Daniel Vetter <daniel.vetter@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470413484-23775-1-git-send-email-daniel.vetter@ffwll.ch
-
Rodrigo Vivi authored
No functional change. Instead of defining a new empty function let's use what is available on drm. It gets cleaner, and easy to read, and understand. Signed-off-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by:
-
Rodrigo Vivi authored
Copy to user return the number of bytes it couldn't write and zero on success. So any number different than 0 should be considered a fault, not only when it doesn't write the full size. v2: fixed the inverted logic. (Ville) Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by:
-
Ville Syrjälä authored
This reverts commit f64425a8. active_streams will get totally out of whack with SST unless we sync up with the hw state at readout, obviously! We don't yet do that, so now the WARNs fire all the time. Let's revert :( Signed-off-by:
-
Matthew Auld authored
As pointed out by Chris Harris, we are using the wrong WA name, it should in fact be WaToEnableHwFixForPushConstHWBug, also it should be applied from C0 onwards for both BXT and KBL. Fixes: 7b9005cd ("drm/i915: Add WaInsertDummyPushConstP for bxt and kbl") Cc: Chris Harris <chris.harris@intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Reported-by:
Chris Harris <chris.harris@intel.com> Signed-off-by:
Arun Siluvery <arun.siluvery@linux.intel.com> Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470127013-29653-1-git-send-email-matthew.auld@intel.com
-
Chris Wilson authored
With all callers now not playing tricks with dropping the struct_mutex between waiting and retiring, we can assert that the request is ready to be retired. Signed-off-by:
-
Chris Wilson authored
In the previous commit, we moved the obj->tiling_mode out of a bitfield and into its own integer so that we could safely use READ_ONCE(). Let us now repair some of that damage by sharing the tiling_mode with its companion, the fence stride. v2: New magic Signed-off-by:
-
Chris Wilson authored
Through the GTT interface to the fence registers, we can only handle linear, X and Y tiling. The more esoteric tiling patterns are ignored. Document that the tiling ABI only supports upto Y tiling, and reject any attempts to set a tiling mode other than NONE, X or Y. Signed-off-by:
-
Chris Wilson authored
Since we are not concerned with userspace racing itself with set-tiling (the order is indeterminant even if we take a lock), then we can safely read back the single obj->tiling_mode and do the static lookup of swizzle mode without having to take a lock. get-tiling is reasonably frequent due to the back-channel passing around of tiling parameters in DRI2/DRI3. v2: Make tiling_mode a full unsigned int so that we can trivially use it with READ_ONCE(). Separating it out into manual control over the flags field was too noisy for a simple patch. Note that we could use the lower bits of obj->stride for the tiling mode. Signed-off-by:
-
Chris Wilson authored
We don't need to incur the overhead of checking whether the object is pinned prior to changing its madvise. If the object is pinned, the madvise will not take effect until it is unpinned and so we cannot free the pages being pointed at by hardware. Marking a pinned object with allocated pages as DONTNEED will not trigger any undue warnings. The check is therefore superfluous, and by removing it we can remove a linear walk over all the vma the object has. Still despite it being an overzealous check, that error code is part of the current ABI and so we must proceed with caution. Signed-off-by:
-
Chris Wilson authored
We only need to take the struct_mutex if the object is pinned to the display engine and so requires checking for clflush. (The race with userspace pinning the object to a framebuffer is irrelevant.) v2: Use access once for compiler hints (or not as it is a bitfield) v3: READ_ONCE, obj->pin_display is not a bitfield anymore v4: Don't be creative with goto. Signed-off-by:
-
Chris Wilson authored
By applying the same logic as for wait-ioctl, we can query whether a request has completed without holding struct_mutex. The biggest impact system-wide is removing the flush_active and the contention that causes. Testcase: igt/gem_busy Signed-off-by:
-
