- 11 Aug, 2016 4 commits
-
-
Ville Syrjälä authored
Redo the fb rotation handling in order to: - eliminate the NV12 special casing - handle fb->offsets[] properly - make the rotation handling easier for the plane code To achieve these goals we reduce intel_rotation_info to only contain (for each plane) the rotated view width,height,stride in tile units, and the page offset into the object where the plane starts. Each plane is handled exactly the same way, no special casing for NV12 or other formats. We then store the computed rotation_info under intel_framebuffer so that we don't have to recompute it again. To handle fb->offsets[] we treat them as a linear offsets and convert them to x/y offsets from the start of the relevant GTT mapping (either normal or rotated). We store the x/y offsets under intel_framebuffer, and for some extra convenience we also store the rotated pitch (ie. tile aligned plane height). So for each plane we have the normal x/y offsets, rotated x/y offsets, and the rotated pitch. The normal pitch is available already in fb->pitches[]. While we're gathering up all that extra information, we can also easily compute the storage requirements for the framebuffer, so that we can check that the object is big enough to hold it. When it comes time to deal with the plane source coordinates, we first rotate the clipped src coordinates to match the relevant GTT view orientation, then add to them the fb x/y offsets. Next we compute the aligned surface page offset, and as a result we're left with some residual x/y offsets. Finally, if required by the hardware, we convert the remaining x/y offsets into a linear offset. For gen2/3 we simply skip computing the final page offset, and just convert the src+fb x/y offsets directly into a linear offset since that's what the hardware wants. After this all platforms, incluing SKL+, compute these things in exactly the same way (excluding alignemnt differences). v2: Use BIT(DRM_ROTATE_270) instead of ROTATE_270 when rotating plane src coordinates Drop some spurious changes that got left behind during development v3: Split out more changes to prep patches (Daniel) s/intel_fb->plane[].foo.bar/intel_fb->foo[].bar/ for brevity Rename intel_surf_gtt_offset to intel_fb_gtt_offset Kill the pointless 'plane' parameter from intel_fb_gtt_offset() v4: Fix alignment vs. alignment-1 when calling _intel_compute_tile_offset() from intel_fill_fb_info() Pass the pitch in tiles in stad of pixels to intel_adjust_tile_offset() from intel_fill_fb_info() Pass the full width/height of the rotated area to drm_rect_rotate() for clarity Use u32 for more offsets v5: Preserve the upper_32_bits()/lower_32_bits() handling for the fb ggtt offset (Sivakumar) v6: Rebase due to drm_plane_state src/dst rects Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> Signed-off-by:Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by:
Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470821001-25272-2-git-send-email-ville.syrjala@linux.intel.comAcked-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ville Syrjälä authored
Looks like the TSEG lives just above TOUD, stolen comes after TSEG. The spec seems somewhat self-contradictory in places, in the ESMRAMC register desctription it says: TSEG Size: 10=(TOUD + 512 KB) to TOUD 11 =(TOUD + 1 MB) to TOUD so that agrees with TSEG being at TOUD. But the example given elsehwere in the spec says: TOUD equals 62.5 MB = 03E7FFFFh TSEG selected as 512 KB in size, Graphics local memory selected as 1 MB in size General System RAM available in system = 62.5 MB General system RAM range00000000h to 03E7FFFFh TSEG address range03F80000h to 03FFFFFFh TSEG pre-allocated from03F80000h to 03FFFFFFh Graphics local memory pre-allocated from03E80000h to 03F7FFFFh so here we have TSEG above stolen. Real world evidence agrees with the TOUD->TSEG->stolen order however, so let's fix up the code to account for the TSEG size. Cc: Taketo Kabe <fdporg@vega.pgw.jp> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: x86@kernel.org Cc: stable@vger.kernel.org Fixes: 0ad98c74 ("drm/i915: Determine the stolen memory base address on gen2") Fixes: a4dff769 ("x86/gpu: Add Intel graphics stolen memory quirk for gen2 platforms") Reported-by:
Taketo Kabe <fdporg@vega.pgw.jp> Tested-by:
Chris Wilson <chris@chris-wilson.co.uk>
-
Tvrtko Ursulin authored
Currently to change the firmware one has to update the exported module firmware string and the major-minor versions used for verification after load. Consolidate that to a single place defining correct major and minor versions per platform. v2: Rebased for KBL. Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Dave Gordon <david.s.gordon@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Peter Antoine <peter.antoine@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Reviewed-by:
Dave Gordon <david.s.gordon@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470842206-35685-1-git-send-email-tvrtko.ursulin@linux.intel.com
-
Tvrtko Ursulin authored
Until now code was calling hweight32 to figure out the number from device_info->ring_mask at runtime. Instead we can cache it at engine init time and use directly. Signed-off-by:
-
- 10 Aug, 2016 17 commits
-
-
Chris Wilson authored
request->batch_obj is only set by execbuffer for the convenience of debugging hangs. By moving that operation to the callsite, we can simplify all other callers and future patches. We also move the complications of reference handling of the request->batch_obj next to where the active tracking is set up for the request. Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1470832906-13972-2-git-send-email-chris@chris-wilson.co.uk
-
Chris Wilson authored
We allocate a few objects into the GGTT that we never need to access via the mappable aperture (such as contexts, status pages). We can request that these are bound high in the VM to increase the amount of mappable aperture available. However, anything that may be frequently pinned (such as logical contexts) we want to use the fast search & insert. Signed-off-by:
-
Chris Wilson authored
During intel_gt_powersave_init() we take the RPS mutex to ensure that all locking requirements are met as we talk to the punit, but we also require the struct_mutex for allocating a slice of the global GTT for a power context on Valleyview. struct_mutex must be the outer lock here, as we nest rps.mutex inside later on. Reported-by:
-
Imre Deak authored
In the preceding patches we made sure that: - the LVDS encoder takes care of reiniting both the LVDS register and its PPS - the eDP encoder takes care of reiniting its PPS - the PPS register unlocking workaround is applied explicitly whenever the PPS context is lost Based on the above we can safely remove the opaque LVDS and PPS save / restore from generic code. Signed-off-by:
Imre Deak <imre.deak@intel.com> Reviewed-by:
-
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 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 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 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 5 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:
-
