- 28 Jan, 2016 7 commits
-
-
Ville Syrjälä authored
Also rename 'rotation_info' to 'rotated' to match the view type exactly, this should avoid confusion which union members is valid for each view type. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453316739-13296-2-git-send-email-ville.syrjala@linux.intel.comReviewed-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Dave Gordon authored
In the error-handling paths of i915_gem_do_execbuffer() and intel_crtc_page_flip(), the local pointer-to-request variables were expected to be either valid pointers or NULL. Since 26827088 drm/i915: simplify allocation of driver-internal requests they could also be ERR_PTR() values, so the tests need to be updated to accommodate this case. Signed-off-by:
Dave Gordon <david.s.gordon@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by:
-
Tvrtko Ursulin authored
In GuC mode LRC pinning lifetime depends exclusively on the request liftime. Since that is terminated by the seqno update that opens up a race condition between GPU finishing writing out the context image and the driver unpinning the LRC. To extend the LRC lifetime we will employ a similar approach to what legacy ringbuffer submission does. We will start tracking the last submitted context per engine and keep it pinned until it is replaced by another one. Note that the driver unload path is a bit fragile and could benefit greatly from efforts to unify the legacy and exec list submission code paths. At the moment i915_gem_context_fini has special casing for the two which are potentialy not needed, and also depends on i915_gem_cleanup_ringbuffer running before itself. v2: * Move pinning into engine->emit_request and actually fix the reference/unreference logic. (Chris Wilson) * ring->dev can be NULL on driver unload so use a different route towards it. v3: * Rebase. * Handle the reset path. (Chris Wilson) * Exclude default context from the pinning - it is impossible to get it right before default context special casing in general is eliminated. v4: * Rebased & moved context tracking to intel_logical_ring_advance_and_submit. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Issue: VIZ-4277 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Nick Hoath <nicholas.hoath@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453976997-25424-1-git-send-email-tvrtko.ursulin@linux.intel.com
-
Tvrtko Ursulin authored
Will enable cleaner implementation of a following fix and easier code unification in the future. Idea and code by Chris Wilson. v2: Do not return before last_contexts on engines are unpinned. Signed-off-by:
-
Tvrtko Ursulin authored
Will simplify the following fix and sounds logical. v2: Add some whitespace to separate logic better. (Chris Wilson) Signed-off-by:
-
Tvrtko Ursulin authored
Previously intel_lr_context_(un)pin were operating on requests which is in conflict with their names. If we make them take a context and an engine, it makes the names make more sense and it also makes future fixes possible. v2: Rebase for default_context/kernel_context change. Signed-off-by:
-
Tvrtko Ursulin authored
This got broken in: commit de1add36 Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Date: Fri Jan 15 15:12:50 2016 +0000 drm/i915: Decouple execbuf uAPI from internal implementation BSD ring flags need to be shifted before they can be considered indices into the ring array. Reported by Zhipeng Gong. v2: Simplify the code. (Chris Wilson) Signed-off-by:
-
- 27 Jan, 2016 8 commits
-
-
Imre Deak authored
The only device specific dependency of the stolen memory setup is the MMIO mapping and the stolen memory size. Both are already available in i915_gtt_init(), so move the stolen initialization to there. The clean-up code for i915_gtt_init() is in i915_global_gtt_cleanup(), so move the stolen memory clean-up code there too. This will be needed by an upcoming patch that needs the details of the memory we reserve, but the change is also part of our generic goal to move the initialization of resources with no or little dependencies on other device specific resources towards the beginning of the init sequence. Suggested-by:
Imre Deak <imre.deak@intel.com> Reviewed-by:
David Weinehall <david.weinehall@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453209992-25995-8-git-send-email-imre.deak@intel.com
-
Imre Deak authored
Move the MCHBAR setup right after the MMIO setup, since the two things are logically related and the MCHBAR setup code doesn't depend on any other device specific resource. We'll also need MCHBAR to be ready earlier in an upcoming patch, so this is also a preparation for that. Factor out the init/clean-up code to separate functions to make things clearer in the i915_driver_load()/unload() functions. Suggested-by:
-
Imre Deak authored
Workqueue initalization doesn't depend on any other device specific resource, so move it close to the beginning, so we don't need to consider them when thinking about dependencies for other resources. Also factor out things to separate init/cleanup functions to make i915_driver_load()/unload() clearer, atm it's somewhat difficult to follow there in what order resources are inited/cleaned-up. Suggested-by:
-
Imre Deak authored
Factor out common clean-up code for the GEM load time init function. Also rename i915_gem_load() to i915_gem_load_init() to have a better match with its new clean-up function. No functional change. Signed-off-by:
-
Imre Deak authored
Factor out the common GEM shrinker clean-up code and call the shrinker init function from the same function from where the corresponding shrinker clean-up function is called. Also add sanity checking to the shrinker and OOM registration calls. Signed-off-by:
-
Imre Deak authored
Clarify the name of the label on the error path, making it clear what's being cleaned up. The kmem_cache_destroy() calls are NOPs on the corresponding error path. Signed-off-by:
-
Imre Deak authored
commit ebae38d0 Author: Animesh Manna <animesh.manna@intel.com> Date: Wed Oct 28 23:58:55 2015 +0200 drm/i915/gen9: csr_init after runtime pm enable moved the DMC/CSR initialization later during driver loading, but didn't move the cleanup earlier correspondingly during unloading. Fix this up. Signed-off-by:
-
Daniel Vetter authored
This reverts commit 1803c035. It seems to blow up on module unload due to a use-after free hitting a BUG_ON with CONFIG_DEBUG_SG. Quoting from Tvrtko's mail: "I've decoded the instructions and it pointed to SG_MAGIC checking: 488b8098010000 mov 0x198(%rax),%rax ba21436587 mov $0x87654321,%edx 488b00 mov (%rax),%rax *** CRASH "Grep showed 0x87654321 is SG_MAGIC, so likely candidate for this code pattern is: static inline struct page *sg_page(struct scatterlist *sg) { BUG_ON(sg->sg_magic != SG_MAGIC); BUG_ON(sg_is_chain(sg)); return (struct page *)((sg)->page_link & ~0x3); } "Which would mean the offender is in intel_logical_ring_cleanup is most likely: ... if (ring->status_page.obj) { kunmap(sg_page(ring->status_page.obj->pages->sgl)); ring->status_page.obj = NULL; } ... "I think that the i915_gem_context_fini will do a final unref on dev_priv->kernel_context and then the ring buff has a copy which is left dangling because: lrc_setup_hardware_status_page(ring, dev_priv->kernel_context->engine[ring->id].state); and: ring->status_page.obj = default_ctx_obj; "Where default_ctx_obj == dev_priv->kernel_context->engine[ring->id].state So indeed looks like the unload ordering is the trigger. In fact it is almost the same fragility wrt/ kernel_context hidden dependency I expressed my worry about in an e-mail yesterday or so. It only shows if CONFIG_DEBUG_SG is set, otherwise it accesses freed memory and probably just survives." This causes serious trouble in our CI system since it took out all gen8+ machines. Not yet clear why this wasn't caught in pre-merge testing. Backtrace from CI, for posterity: [ 163.737836] general protection fault: 0000 [#1] PREEMPT SMP [ 163.737849] Modules linked in: ax88179_178a usbnet mii snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915(-) x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm mei_me mei i2c_hid e1000e ptp pps_core [last unloaded: snd_hda_intel] [ 163.737902] CPU: 0 PID: 5812 Comm: rmmod Tainted: G U W 4.5.0-rc1-gfxbench+ #1 [ 163.737911] Hardware name: System manufacturer System Product Name/Z170M-PLUS, BIOS 0505 11/16/2015 [ 163.737920] task: ffff8800bb99cf80 ti: ffff88022ff2c000 task.ti: ffff88022ff2c000 [ 163.737928] RIP: 0010:[<ffffffffa018f723>] [<ffffffffa018f723>] intel_logical_ring_cleanup+0x83/0x100 [i915] [ 163.737969] RSP: 0018:ffff88022ff2fd30 EFLAGS: 00010282 [ 163.737975] RAX: 6b6b6b6b6b6b6b6b RBX: ffff8800bb2f31b8 RCX: 0000000000000002 [ 163.737982] RDX: 0000000087654321 RSI: 000000000000000d RDI: ffff8800bb2f31f0 [ 163.737989] RBP: ffff88022ff2fd40 R08: 0000000000000000 R09: 0000000000000001 [ 163.737996] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800bb2f0000 [ 163.738003] R13: ffff8800bb2f8fc8 R14: ffff8800bb285668 R15: 000055af1ae55210 [ 163.738010] FS: 00007f187014b700(0000) GS:ffff88023bc00000(0000) knlGS:0000000000000000 [ 163.738021] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 163.738030] CR2: 0000558f84e4cbc8 CR3: 000000022cd55000 CR4: 00000000003406f0 [ 163.738039] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 163.738048] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 163.738057] Stack: [ 163.738062] ffff8800bb2f31b8 ffff8800bb2f0000 ffff88022ff2fd70 ffffffffa0180414 [ 163.738079] ffff8800bb2f0000 ffff8800bb285668 ffff8800bb2856c8 ffffffffa0242460 [ 163.738094] ffff88022ff2fd98 ffffffffa0202d30 ffff8800bb285668 ffff8800bb285668 [ 163.738109] Call Trace: [ 163.738140] [<ffffffffa0180414>] i915_gem_cleanup_engines+0x34/0x60 [i915] [ 163.738185] [<ffffffffa0202d30>] i915_driver_unload+0x150/0x270 [i915] [ 163.738198] [<ffffffff815100f4>] drm_dev_unregister+0x24/0xa0 [ 163.738208] [<ffffffff815106ce>] drm_put_dev+0x1e/0x60 [ 163.738225] [<ffffffffa01412a0>] i915_pci_remove+0x10/0x20 [i915] [ 163.738237] [<ffffffff8143d9b4>] pci_device_remove+0x34/0xb0 [ 163.738249] [<ffffffff81533d15>] __device_release_driver+0x95/0x140 [ 163.738259] [<ffffffff81533eb6>] driver_detach+0xb6/0xc0 [ 163.738268] [<ffffffff81532de3>] bus_remove_driver+0x53/0xd0 [ 163.738278] [<ffffffff815348d7>] driver_unregister+0x27/0x50 [ 163.738289] [<ffffffff8143ca15>] pci_unregister_driver+0x25/0x70 [ 163.738299] [<ffffffff81511de4>] drm_pci_exit+0x74/0x90 [ 163.738337] [<ffffffffa02034a9>] i915_exit+0x20/0x1a5 [i915] [ 163.738349] [<ffffffff8110400f>] SyS_delete_module+0x18f/0x1f0 [ 163.738361] [<ffffffff817b8a9b>] entry_SYSCALL_64_fastpath+0x16/0x73 [ 163.738370] Code: ff d0 48 89 df e8 de a1 fd ff 48 8d 7b 38 e8 25 ab fd ff 48 8b 83 90 00 00 00 48 85 c0 74 25 48 8b 80 98 01 00 00 ba 21 43 65 87 <48> 8b 00 48 39 10 75 3c f6 40 08 01 75 38 48 c7 83 90 00 00 00 [ 163.738459] RIP [<ffffffffa018f723>] intel_logical_ring_cleanup+0x83/0x100 [i915] [ 163.738498] RSP <ffff88022ff2fd30> [ 163.738507] ---[ end trace 68f69ce4740fa44f ]--- Cc: Nick Hoath <nicholas.hoath@intel.com> Cc: Dave Gordon <david.s.gordon@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Tested-by:
Daniel Vetter <daniel.vetter@intel.com>
-
- 25 Jan, 2016 13 commits
-
-
Nick Hoath authored
Swap the order of context & engine cleanup, so that contexts are cleaned up first, and *then* engines. This is a more sensible order anyway, but in particular has become necessary since the 'intel_ring_initialized() must be simple and inline' patch, which now uses ring->dev as an 'initialised' flag, so it can now be NULL after engine teardown. This in turn can cause a problem in the context code, which (used to) check the ring->dev->struct_mutex -- causing a fault if ring->dev was NULL. Also rename the cleanup function to reflect what it actually does (cleanup engines, not a ringbuffer), and fix an annoying whitespace issue. v2: Also make the fix in i915_load_modeset_init, not just in i915_driver_unload (Chris Wilson) v3: Had extra stuff in it. v4: Reverted extra stuff (so we're back to v2). Rebased and updated commentary above (Dave Gordon). Signed-off-by:Nick Hoath <nicholas.hoath@intel.com> Signed-off-by:
-
Chris Wilson authored
The generic interval tree we use to speed up range invalidation is an augmented rbtree that can report all overlapping intervals for a given range. Therefore we do not need to degrade to a linear list if we find overlapping objects. Oops. Signed-off-by:
Michał Winiarski <michal.winiarski@intel.com> Signed-off-by:
-
Arun Siluvery authored
This is mainly required for future enabling of pre-emptive command execution. v2: explain purpose of change (Chris) Reviewed-by:
Arun Siluvery <arun.siluvery@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453412634-29238-9-git-send-email-arun.siluvery@linux.intel.comSigned-off-by:
-
Arun Siluvery authored
Per context preemption granularity control is only available from SKL:E0+ Actual WA is to disable percontext preemption granularity control until D0 which is the default case so this is equivalent to the inverse of WaDisablePerCtxtPreemptionGranularityControl:skl v2: add some detail to commit msg (Chris) Reviewed-by:
-
Arun Siluvery authored
Required for WaDisableLSQCROPERFforOCL:skl This register is added to HW whitelist to support WA required for future enabling of pre-emptive command execution, WA implementation will be in userspace and it cannot program this register if it is not on HW whitelist. v2: explain purpose of changes (Chris) Reviewed-by:
-
Arun Siluvery authored
Required for WaDisableLSQCROPERFforOCL:bxt According to WA database these are only applicable for BXT:A0 but since A0 and A1 shares the same GT these are extended for A1 as well. This register is added to HW whitelist to support WA required for future enabling of pre-emptive command execution, WA implementation will be in userspace and it cannot program this register if it is not on HW whitelist. v2: explain purpose of changes (Chris) Reviewed-by:
-
Arun Siluvery authored
Required for, WaDisableObjectLevelPreemptionForTrifanOrPolygon:bxt WaDisableObjectLevelPreemptionForInstancedDraw:bxt WaDisableObjectLevelPreemtionForInstanceId:bxt According to WA database these are only applicable for BXT:A0 but since A0 and A1 shares the same GT these are extended for A1 as well. These are also required for SKL until B0 but not adding them because they are pre-production steppings. This register is added to HW whitelist to support WA required for future enabling of pre-emptive command execution, WA implementation will be in userspace and it cannot program this register if it is not on HW whitelist. v2: use lower case in register defines (Nick) v3: explain purpose of changes (Chris) Reviewed-by:
-
Arun Siluvery authored
Required for WaAllowUMDToModifyHDCChicken1:skl,bxt This register is added to HW whitelist to support WA required for future enabling of pre-emptive command execution, WA implementation will be in userspace and it cannot program this register if it is not on HW whitelist. v2: explain purpose of changes (Chris) Reviewed-by:
-
Arun Siluvery authored
Required for WaEnablePreemptionGranularityControlByUMD:skl,bxt This register is added to HW whitelist to support WA required for future enabling of pre-emptive command execution, WA implementation will be in userspace and it cannot program this register if it is not on HW whitelist. v2: explain purpose of WA (Chris) Reviewed-by:
-
Arun Siluvery authored
Some of the HW registers are privileged and cannot be written to from non-privileged batch buffers coming from userspace unless they are added to the HW whitelist. This whitelist is maintained by HW and it is different from SW whitelist. Userspace need write access to them to implement preemption related WA. The reason for using this approach is, the register bits that control preemption granularity at the HW level are not context save/restored; so even if we set these bits always in kernel they are going to change once the context is switched out. We can consider making them non-privileged by default but these registers also contain other chicken bits which should not be allowed to be modified. In the later revisions controlling bits are save/restored at context level but in the existing revisions these are exported via other debug registers and should be on the whitelist. This patch adds changes to provide HW with a list of registers to be whitelisted. HW checks this list during execution and provides access accordingly. HW imposes a limit on the number of registers on whitelist and it is per-engine. At this point we are only enabling whitelist for RCS and we don't foresee any requirement for other engines. The registers to be whitelisted are added using generic workaround list mechanism, even these are only enablers for userspace workarounds. But by sharing this mechanism we get some test assets without additional cost (Mika). v2: rebase v3: parameterize RING_FORCE_TO_NONPRIV() as _MMIO() should be limited to i915_reg.h (Ville), drop inline for wa_ring_whitelist_reg (Mika). v4: improvements suggested by Chris Wilson. Clarify that this is HW whitelist and different from the one maintained in driver. This list is engine specific but it gets initialized along with other WA which is RCS specific thing, so make it clear that we are not doing any cross engine setup during initialization. Make HW whitelist count of each engine available in debugfs. Reviewed-by:
Mika Kuoppala <mika.kuoppala@intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Andreas Ziegler authored
Commit 5bab6f60 ("drm/i915: Serialise updates to GGTT with access through GGTT on Braswell") depended upon a working stop_machine() and so forced the selection of STOP_MACHINE. However, commit 86fffe4a ("kernel: remove stop_machine() Kconfig dependency") removed the option STOP_MACHINE from init/Kconfig and ensured that stop_machine() universally works. Due to the order in which the patches were applied, removing the select from DRM_I915 got lost during merging. Remove the now obsolete select statement. Signed-off-by:
Andreas Ziegler <andreas.ziegler@fau.de> Reviewed-by:
-
Alex Dai authored
Previously GuC uses ring id as engine id because of same definition. But this is not true since this commit: commit de1add36 Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Date: Fri Jan 15 15:12:50 2016 +0000 drm/i915: Decouple execbuf uAPI from internal implementation Added GuC engine id into GuC interface to decouple it from ring id used by driver. v2: Keep ring name print out in debugfs; using for_each_ring() where possible to keep driver consistent. (Chris W.) Signed-off-by:
Alex Dai <yu.dai@intel.com> Reviewed-by:
-
Tvrtko Ursulin authored
Since: commit 82352e90 Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Date: Fri Jan 15 17:12:45 2016 +0000 drm/i915: Cache LRC state page in the context and: commit 0eb973d3 Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Date: Fri Jan 15 15:10:28 2016 +0000 drm/i915: Cache ringbuffer GTT VMA We can also remove the ring buffer start updates on every context update since the address will not change for the duration of the LRC pin. For GuC we can remove the update altogether because it only cares about the ring buffer start. Signed-off-by:
-
- 24 Jan, 2016 1 commit
-
-
Daniel Vetter authored
Signed-off-by:
-
- 21 Jan, 2016 8 commits
-
-
Chris Wilson authored
Tvrtko was looking through the execbuffer-ioctl and noticed that the uABI was tightly coupled to our internal engine identifiers. Close inspection also revealed that we leak those internal engine identifiers through the busy-ioctl, and those internal identifiers already do not match the user identifiers. Fortuitiously, there is only one user of the set of busy rings from the busy-ioctl, and they only wish to choose between the RENDER and the BLT engines. Let's fix the userspace ABI while we still can. v2: Update the uAPI documentation to explain the identifiers. Signed-off-by:
-
Tvrtko Ursulin authored
At the moment execbuf ring selection is fully coupled to internal ring ids which is not a good thing on its own. This dependency is also spread between two source files and not spelled out at either side which makes it hidden and fragile. This patch decouples this dependency by introducing an explicit translation table of execbuf uAPI to ring id close to the only call site (i915_gem_do_execbuffer). This way we are free to change driver internal implementation details without breaking userspace. All state relating to the uAPI is now contained in, or next to, i915_gem_do_execbuffer. As a side benefit, this patch decreases the compiled size of i915_gem_do_execbuffer. v2: Extract ring selection into eb_select_ring. (Chris Wilson) Signed-off-by:
-
Chris Wilson authored
Broadwell and later currently use the same unordered command sequence to update the seqno in the HWS status page and then assert the user interrupt. We should apply the w/a from legacy (where we do an mmio read to delay the seqno read after the interrupt), but this is not enough to enforce coherent seqno visibilty on Skylake. Rather than search for the proper post-interrupt seqno barrier, use a strongly ordered command sequence to write the seqno, then assert the user interrupt from the ring. v2: Move around the wa tail dwords to avoid adding duplicate code. v3: Add references, comments on workarounds and bit5 check. References: https://bugs.freedesktop.org/show_bug.cgi?id=93693 Testcase: igt/gem_ring_sync_loop #skl Signed-off-by:
-
Mika Kuoppala authored
The capability to detect unclaimed register access was recently introduced for vlv/chv platforms. Apparently there are plenty of unclaimed access on these platforms, resulting in new dmesg warns. But as we are trying to form a beachhead for CI/Bat, all new warns are adding to the noise and thus not desirable at this point in time. Make it so that if in these platforms the automatic arming was responsible for mmio_debug enabling, ignore the warns. If user/dev wants to fix these, he can still do so by i915.mmio_debug=1234. Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by:
-
Daniel Vetter authored
We've had this since forever, and's randomly reporting issues and as such causing piles&piles of CI noise. Mika is working on proper debug infrastructure for this, and on fixing this properly. Meanwhile make CI more useful for everyone else. Cc: Mika Kuoppala <mika.kuoppala@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93121Signed-off-by:
-
Dave Gordon authored
There are a few bits of code which the transformations implemented by the previous patch reveal to be suboptimal, once the notion of a per- ring default context has gone away. So this tidies up the leftovers. It could have been squashed into the previous patch, but that would have made that patch less clearly a simple transformation. In particular, any change which alters the code block structure or indentation has been deferred into this separate patch, because such things tend to make diffs more difficult to read. v4: Rebased Signed-off-by:
-
Dave Gordon authored
Now that we've eliminated a lot of uses of ring->default_context, we can eliminate the pointer itself. All the engines share the same default intel_context, so we can just keep a single reference to it in the dev_priv structure rather than one in each of the engine[] elements. This make refcounting more sensible too, as we now have a refcount of one for the one pointer, rather than a refcount of one but multiple pointers. From an idea by Chris Wilson. v2: transform an extra instance of ring->default_context introduced by 42f1cae8 drm/i915: Restore inhibiting the load of the default context That patch's commentary includes: v2: Mark the global default context as uninitialized on GPU reset so that the context-local workarounds are reloaded upon re-enabling The code implementing that now also benefits from the replacement of the multiple (per-ring) pointers to the default context with a single pointer to the unique kernel context. v4: Rebased, remove underused local (Nick Hoath) Signed-off-by: -
Dave Gordon authored
There are a number of places where the driver needs a request, but isn't working on behalf of any specific user or in a specific context. At present, we associate them with the per-engine default context. A future patch will abolish those per-engine context pointers; but we can already eliminate a lot of the references to them, just by making the allocator allow NULL as a shorthand for "an appropriate context for this ring", which will mean that the callers don't need to know anything about how the "appropriate context" is found (e.g. per-ring vs per-device, etc). So this patch renames the existing i915_gem_request_alloc(), and makes it local (static inline), and replaces it with a wrapper that provides a default if the context is NULL, and also has a nicer calling convention (doesn't require a pointer to an output parameter). Then we change all callers to use the new convention: OLD: err = i915_gem_request_alloc(ring, user_ctx, &req); if (err) ... NEW: req = i915_gem_request_alloc(ring, user_ctx); if (IS_ERR(req)) ... OLD: err = i915_gem_request_alloc(ring, ring->default_context, &req); if (err) ... NEW: req = i915_gem_request_alloc(ring, NULL); if (IS_ERR(req)) ... v4: Rebased Signed-off-by:
-
- 20 Jan, 2016 3 commits
-
-
Ville Syrjälä authored
In this atomic age, we can't trust the plane->fb pointer anymore. It might get update too late. Instead we are supposed to use the plane_state->fb pointer instead. Let's do that in intel_plane_obj_offset() and avoid problems from dereferencing the potentially stale plane->fb pointer. Paulo found this with 'kms_frontbuffer_tracking --show-hidden --run-subtest nop-1p-rte' but it can be reproduced with just plain old kms_setplane. I was too lazy to bisect this, so not sure exactly when it broke. The most obvious candidate commit ce7f1728 ("drm/i915: Fix i915_ggtt_view_equal to handle rotation correctly") was actually still fine, so it must have broken some time after that. Here's the resulting fireworks: BUG: unable to handle kernel NULL pointer dereference at (null) IP: [<ffffffffa02d2d9a>] intel_fill_fb_ggtt_view+0x1b/0x15a [i915] PGD 8a5f6067 PUD 8a5f5067 PMD 0 Oops: 0000 [#1] PREEMPT SMP Modules linked in: i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm intel_gtt agpgart netconsole mousedev hid_generic psmouse usbhid atkbd libps2 coretemp hwmon efi_pstore intel_rapl iosf_mbi x86_pkg_temp_thermal efivars pcspkr e1000e sdhci_pci ptp pps_core sdhci i2c_i801 mmc_core i2c_hid hid i8042 serio evdev sch_fq_codel ip_tables x_tables ipv6 autofs4 CPU: 1 PID: 260 Comm: kms_plane Not tainted 4.4.0-skl+ #171 Hardware name: Intel Corporation Skylake Client platform/Skylake Y LPDDR3 RVP3, BIOS SKLSE2R1.R00.B104.B00.1511030553 11/03/2015 task: ffff88008bde2d80 ti: ffff88008a6ec000 task.ti: ffff88008a6ec000 RIP: 0010:[<ffffffffa02d2d9a>] [<ffffffffa02d2d9a>] intel_fill_fb_ggtt_view+0x1b/0x15a [i915] RSP: 0018:ffff88008a6efa10 EFLAGS: 00010086 RAX: 0000000000000001 RBX: ffff8801674f4240 RCX: 0000000000000014 RDX: ffff88008a7440c0 RSI: 0000000000000000 RDI: ffff88008a6efa40 RBP: ffff88008a6efa30 R08: ffff88008bde3598 R09: 0000000000000001 R10: ffff88008b782000 R11: 0000000000000000 R12: 0000000000000000 R13: ffff88008a7440c0 R14: 0000000000000000 R15: ffff88008a7449c0 FS: 00007fa0c07a28c0(0000) GS:ffff88016ec40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 000000008a6ff000 CR4: 00000000003406e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Stack: ffff8801674f4240 0000000000000000 ffff88008a7440c0 0000000000000000 ffff88008a6efaa0 ffffffffa02daf25 ffffffff814ec80e 0000000000070298 ffff8800850d0000 ffff88008a6efaa0 ffffffffa02c49c2 0000000000000002 Call Trace: [<ffffffffa02daf25>] intel_plane_obj_offset+0x2d/0xa9 [i915] [<ffffffff814ec80e>] ? _raw_spin_unlock_irqrestore+0x4b/0x60 [<ffffffffa02c49c2>] ? gen9_write32+0x2e8/0x3b8 [i915] [<ffffffffa02eecfc>] skl_update_plane+0x203/0x4c5 [i915] [<ffffffffa02ca1ab>] intel_plane_atomic_update+0x53/0x6a [i915] [<ffffffffa02494a4>] drm_atomic_helper_commit_planes_on_crtc+0x142/0x1d5 [drm_kms_helper] [<ffffffffa02de44b>] intel_atomic_commit+0x1262/0x1350 [i915] [<ffffffffa024a0ee>] ? __drm_atomic_helper_crtc_duplicate_state+0x2f/0x41 [drm_kms_helper] [<ffffffffa01ef089>] ? drm_atomic_check_only+0x3e3/0x552 [drm] [<ffffffffa01ef245>] drm_atomic_commit+0x4d/0x52 [drm] [<ffffffffa024996b>] drm_atomic_helper_update_plane+0xcb/0x118 [drm_kms_helper] [<ffffffffa01e42e8>] __setplane_internal+0x1c8/0x224 [drm] [<ffffffffa01e477f>] drm_mode_setplane+0x14e/0x172 [drm] [<ffffffffa01d8117>] drm_ioctl+0x265/0x3ad [drm] [<ffffffffa01e4631>] ? drm_mode_cursor_common+0x158/0x158 [drm] [<ffffffff810d00ab>] ? current_kernel_time64+0x5e/0x98 [<ffffffff810a76ea>] ? trace_hardirqs_on_caller+0x17a/0x196 [<ffffffff8119880f>] do_vfs_ioctl+0x42b/0x4ea [<ffffffff811a2b72>] ? __fget_light+0x4d/0x71 [<ffffffff81198911>] SyS_ioctl+0x43/0x61 [<ffffffff814ed057>] entry_SYSCALL_64_fastpath+0x12/0x6f Cc: drm-intel-fixes@lists.freedesktop.org Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Testcase: igt/kms_plane Reported-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by:
-
Tvrtko Ursulin authored
Having this on stack triggers the -Wframe-larger-than=1024 and is not nice to put such big things on the kernel stack anyway. This required a little bit of refactoring to handle the new failure path from vlv_force_pll_on. v2: Corrected some whitespace. Signed-off-by:
-
Matt Roper authored
This reverts commit 396e33ae. This commit was triggering some FIFO underrun warnings on ILK-IVB platforms (but surprisingly not on HSW/BDW that share more or less the same codepaths). These underruns were caught by the continuous integration (CI) system and could be reproduced consistently when running the basic acceptance tests (BAT) on the affected platforms. Note that this revert will cause a visible regression for some end-users; the "flicker when mouse moves between monitors in X" issue that was reported before this patch was merged will now return. However regressions that are visible to CI have higher priority since they prevent proper testing of future patches on those platforms. Hopefully we'll be able to figure out the cause of the underruns quickly and remerge an improved version of this patch to fix the regression. Cc: Daniel Vetter <daniel@ffwll.ch> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93640Signed-off-by:
Matt Roper <matthew.d.roper@intel.com> Reviewed-by:
Daniel Vetter <daniel@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1453232584-8543-1-git-send-email-matthew.d.roper@intel.comSigned-off-by:
-
