aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2016-01-28Fix pointer tests in error-handling pathsDave Gordon2-2/+2
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 2682708 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 <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Signed-off-by: Tvrtko Ursulin <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-28drm/i915: Fix premature LRC unpin in GuC modeTvrtko Ursulin2-5/+21
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: Tvrtko Ursulin <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Issue: VIZ-4277 Cc: Chris Wilson <[email protected]> Cc: Nick Hoath <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-28drm/i915: Extract context unpinning to its own functionTvrtko Ursulin1-18/+12
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 <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Cc: Chris Wilson <[email protected]>
2016-01-28drm/i915: Make LRC pinning own a reference to the contextTvrtko Ursulin1-0/+4
Will simplify the following fix and sounds logical. v2: Add some whitespace to separate logic better. (Chris Wilson) Signed-off-by: Tvrtko Ursulin <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Cc: Chris Wilson <[email protected]> Cc: Nick Hoath <[email protected]>
2016-01-28drm/i915: Make LRC (un)pinning work on context and engineTvrtko Ursulin3-26/+28
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 <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Cc: Chris Wilson <[email protected]> Cc: Nick Hoath <[email protected]>
2016-01-28drm/i915: Fix VCS ring selection after uapi decouplingTvrtko Ursulin2-4/+7
This got broken in: commit de1add360522c876c25ef2bbbbab1c94bdb509ab Author: Tvrtko Ursulin <[email protected]> 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: Tvrtko Ursulin <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Chris Wilson <[email protected]> Cc: Zhipeng Gong <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Testcase: igt/gem_exec_basic # bdw-gt3
2016-01-27drm/i915: Move stolen memory initialization earlier during loadingImre Deak2-10/+15
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: Chris Wilson <[email protected]> Signed-off-by: Imre Deak <[email protected]> Reviewed-by: David Weinehall <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-27drm/i915: Move MCHBAR setup earlier during initImre Deak1-26/+45
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: Chris Wilson <[email protected]> Signed-off-by: Imre Deak <[email protected]> Reviewed-by: David Weinehall <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-27drm/i915: Move allocation of various workqueues earlier during initImre Deak1-45/+56
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: Chris Wilson <[email protected]> Signed-off-by: Imre Deak <[email protected]> Reviewed-by: David Weinehall <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-27drm/i915: Sanitize i915_gem_load() init and clean-upImre Deak3-9/+15
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 <[email protected]> Reviewed-by: David Weinehall <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-27drm/i915: Sanitize GEM shrinker init and clean-upImre Deak4-8/+18
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 <[email protected]> Reviewed-by: David Weinehall <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-27drm/i915: Sanitize i915_get_bridge_dev() error pathImre Deak1-3/+2
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 <[email protected]> Reviewed-by: David Weinehall <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-27drm/i915: Sanitize DMC/CSR ucode cleanup codeImre Deak1-7/+7
commit ebae38d061df3deffa7c17b030ea14a5216ee55f Author: Animesh Manna <[email protected]> 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: Imre Deak <[email protected]> Reviewed-by: David Weinehall <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-27Revert "drm/i915: Fix context/engine cleanup order"Daniel Vetter3-15/+14
This reverts commit 1803c035efb88afb9d3e7feb279ac29a83216382. 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 <[email protected]> Cc: Dave Gordon <[email protected]> Cc: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: Mika Kuoppala <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Tested-by: Mika Kuoppala <[email protected]> Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915: Fix context/engine cleanup orderNick Hoath3-14/+15
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 <[email protected]> Signed-off-by: Dave Gordon <[email protected]> Reviewed-by: Chris Wilson <[email protected]> (v2) Cc: Mika Kuoppala <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Chris Wilson <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915: Improve handling of overlapping objectsChris Wilson1-131/+50
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: Chris Wilson <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: Michał Winiarski <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Reviewed-by: Michał Winiarski <[email protected]> Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915/gen9: Add WaOCLCoherentLineFlushArun Siluvery1-0/+4
This is mainly required for future enabling of pre-emptive command execution. v2: explain purpose of change (Chris) Reviewed-by: Nick Hoath <[email protected]> Cc: Dave Gordon <[email protected]> Signed-off-by: Arun Siluvery <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915/skl: Enable Per context Preemption granularity controlArun Siluvery2-0/+13
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: Nick Hoath <[email protected]> Cc: Dave Gordon <[email protected]> Signed-off-by: Arun Siluvery <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915/skl: Add GEN8_L3SQCREG4 to HW whitelistArun Siluvery1-0/+5
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: Nick Hoath <[email protected]> Signed-off-by: Arun Siluvery <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915/bxt: Add GEN8_L3SQCREG4 to HW whitelistArun Siluvery1-0/+5
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: Nick Hoath <[email protected]> Signed-off-by: Arun Siluvery <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915/bxt: Add GEN9_CS_DEBUG_MODE1 to HW whitelistArun Siluvery2-0/+10
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: Nick Hoath <[email protected]> Signed-off-by: Arun Siluvery <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915/gen9: Add HDC_CHICKEN1 to HW whitelistArun Siluvery2-0/+7
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: Nick Hoath <[email protected]> Signed-off-by: Arun Siluvery <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915/gen9: Add GEN8_CS_CHICKEN1 to HW whitelistArun Siluvery2-0/+8
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: Nick Hoath <[email protected]> Signed-off-by: Arun Siluvery <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915/gen9: Add framework to whitelist specific GPU registersArun Siluvery4-6/+38
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: Chris Wilson <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Cc: Mika Kuoppala <[email protected]> Cc: Chris Wilson <[email protected]> Signed-off-by: Arun Siluvery <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915: Remove select to deleted STOP_MACHINE from KconfigAndreas Ziegler1-1/+0
Commit 5bab6f60cb4d ("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 86fffe4a61dd ("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 <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Cc: [email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-25drm/i915/guc: Decouple GuC engine id from ring idAlex Dai6-34/+45
Previously GuC uses ring id as engine id because of same definition. But this is not true since this commit: commit de1add360522c876c25ef2bbbbab1c94bdb509ab Author: Tvrtko Ursulin <[email protected]> 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 <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-25drm/i915: More use of the cached LRC stateTvrtko Ursulin2-29/+4
Since: commit 82352e908acd36d7244c75a008c9f27a2ced44d5 Author: Tvrtko Ursulin <[email protected]> Date: Fri Jan 15 17:12:45 2016 +0000 drm/i915: Cache LRC state page in the context and: commit 0eb973d31d0aadb6bc801fd6d796afecbbfc3d5b Author: Tvrtko Ursulin <[email protected]> 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: Tvrtko Ursulin <[email protected]> Cc: Alex Dai <[email protected]> Cc: Dave Gordon <[email protected]> Cc: Chris Wilson <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-24drm/i915: Update DRIVER_DATE to 20160124Daniel Vetter1-1/+1
Signed-off-by: Daniel Vetter <[email protected]>
2016-01-21drm/i915: Seal busy-ioctl uABI and prevent leaking of internal idsChris Wilson5-8/+54
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: Chris Wilson <[email protected]> Testcase: igt/gem_busy Cc: Tvrtko Ursulin <[email protected]> Cc: Daniel Vetter <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Acked-by: Daniel Vetter <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-21drm/i915: Decouple execbuf uAPI from internal implementationTvrtko Ursulin4-74/+81
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: Tvrtko Ursulin <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Chris Wilson <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Acked-by: Daniel Vetter <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-21drm/i915: Use ordered seqno write interrupt generation on gen8+ execlistsChris Wilson2-28/+62
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: Chris Wilson <[email protected]> Cc: Mika Kuoppala <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-21drm/i915: Limit the auto arming of mmio debugs on vlv/chvMika Kuoppala1-0/+9
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 <[email protected]> Signed-off-by: Mika Kuoppala <[email protected]> Reviewed-by: Daniel Vetter <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-21drm/i915: Tune down "GT register while GT waking disabled" messageDaniel Vetter1-1/+1
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 <[email protected]> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93121 Signed-off-by: Daniel Vetter <[email protected]> Reviewed-by: Mika Kuoppala <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-21drm/i915: tidy up a few leftoversDave Gordon3-35/+24
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 <[email protected]> Reviewed-by: Nick Hoath <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-21drm/i915: abolish separate per-ring default_context pointersDave Gordon8-36/+31
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 42f1cae8c 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 <[email protected]> Reviewed-by: Nick Hoath <[email protected]> Cc: Chris Wilson <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-21drm/i915: simplify allocation of driver-internal requestsDave Gordon6-40/+74
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: Dave Gordon <[email protected]> Reviewed-by: Nick Hoath <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-20drm/i915: Fix NULL plane->fb oops on SKLVille Syrjälä1-1/+1
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 ce7f17285639 ("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: [email protected] Cc: Paulo Zanoni <[email protected]> Testcase: igt/kms_plane Reported-by: Paulo Zanoni <[email protected]> Signed-off-by: Ville Syrjälä <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Reviewed-by: Daniel Vetter <[email protected]>
2016-01-20drm/i915: Do not put big intel_crtc_state on the stackTvrtko Ursulin3-26/+46
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: Tvrtko Ursulin <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: John Harrison <[email protected]> Cc: Ville Syrjälä <[email protected]> Reviewed-by: Daniel Vetter <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-20Revert "drm/i915: Add two-stage ILK-style watermark programming (v10)"Matt Roper6-241/+56
This reverts commit 396e33ae204f52abebec9e578f44c749305500f4. 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 <[email protected]> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93640 Signed-off-by: Matt Roper <[email protected]> Reviewed-by: Daniel Vetter <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Daniel Vetter <[email protected]>
2016-01-20drm/i915: add DOC: headline to RC6 kernel-docJani Nikula1-0/+2
Without the DOC:, kernel-doc confuses the documentation block for something else. Reviewed-by: Daniel Vetter <[email protected]> Signed-off-by: Jani Nikula <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-20drm/i915: turn some bogus kernel-doc comments to normal commentsJani Nikula2-2/+2
Apparently accidental or misplaced /** kernel-doc comments, confusing the tool. Turn them to normal comments. Reviewed-by: Daniel Vetter <[email protected]> Signed-off-by: Jani Nikula <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-20drm/i915/sdvo: revert bogus kernel-doc comments to normal commentsJani Nikula1-38/+38
The comments were never proper kernel-doc, but with SDVO it's not worth the trouble to make them kernel-doc. Just turn them into normal comments. Reviewed-by: Daniel Vetter <[email protected]> Signed-off-by: Jani Nikula <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-19drm/i915/gen9: Correct max save/restore register count during gpu reset with GuCArun Siluvery1-1/+1
In GuC submission mode, driver has to provide a list of registers to be save/restored during gpu reset, make the max no. of registers value consistent with that of the value defined in FW. If they are not in sync then register save/restore during gpu reset won't work as expected. Cc: Alex Dai <[email protected]> Cc: Dave Gordon <[email protected]> Signed-off-by: Arun Siluvery <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Reviewed-by: Alex Dai <[email protected]> Signed-off-by: Daniel Vetter <[email protected]>
2016-01-19drm/i915: Demote user facing DMC firmware load failure messageChris Wilson1-2/+7
This is an expected error given the lack of the firmware so emit it at KERN_NOTICE and not KERN_ERROR. Also include the firmware URL in the user facing message so that the user can investigate and fix the issue on their own, and also explain the consequence in plain language. The complete failure message, including the first line from the firmware loader, becomes i915 0000:00:02.0: Direct firmware load for i915/skl_dmc_ver1.bin failed with error -2 i915 0000:00:02.0: Failed to load DMC firmware [https://01.org/linuxgraphics/intel-linux-graphics-firmwares], disabling runtime power management. Signed-off-by: Chris Wilson <[email protected]> Cc: Damien Lespiau <[email protected]> Cc: Imre Deak <[email protected]> Cc: Sunil Kamath <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Animesh Manna <[email protected]> Cc: Jani Nikula <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Reviewed-by: Damien Lespiau <[email protected]> Signed-off-by: Daniel Vetter <[email protected]>
2016-01-18drm/i915: use hlist_for_each_entryGeliang Tang1-5/+2
Use hlist_for_each_entry() instead of hlist_for_each() to simplify the code. Signed-off-by: Geliang Tang <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/b80568b2990ebcc145229a132f045e852ac51ad6.1453126187.git.geliangtang@163.com Signed-off-by: Daniel Vetter <[email protected]>
2016-01-18drm/i915: skl_update_scaler() wants a rotation bitmask instead of bit numberVille Syrjälä1-1/+1
Pass BIT(DRM_ROTATE_0) instead of DRM_ROTATE_0 to skl_update_scaler(). The former is a mask, the latter just the bit number. Fortunately the only thing skl_update_scaler() does with the rotation is check if it's 90/270 degrees or not, and so in this case it would still do the right thing. Cc: Chandra Konduru <[email protected]> Signed-off-by: Ville Syrjälä <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Fixes: 6156a45602f9 ("drm/i915: skylake primary plane scaling using shared scalers") Reviewed-by: Matt Roper <[email protected]>
2016-01-18drm/i915: Don't reject primary plane windowing with color keying enabled on SKL+Ville Syrjälä1-5/+6
On SKL+ plane scaling is mutually exclusive with color keying. The code check for this, but during some refactoring the code got changed to also reject primary plane windowing when color keying is used. There is no such restriction in the hardware, so restore the original logic. Cc: Maarten Lankhorst <[email protected]> Fixes: 061e4b8d650a ("drm/i915: clean up atomic plane check functions, v2.") Signed-off-by: Ville Syrjälä <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected] Cc: [email protected] Reviewed-by: Matt Roper <[email protected]> Reviewed-by: Maarten Lankhorst <[email protected]>
2016-01-18drm/i915/guc: Fix a memory leak where guc->execbuf_client is not freedAlex Dai1-1/+5
During driver unloading, the guc_client created for command submission needs to be released to avoid memory leak. The struct_mutex needs to be held before tearing down GuC. v1: Move i915_guc_submission_disable out of i915_guc_submission_fini and take struct_mutex lock before release GuC client. (Dave Gordon) v2: Add the locking for failure case in guc_fw_fetch. (Dave Gordon) Add i915_guc_submission_fini for failure case in intel_guc_ucode_load. Signed-off-by: Alex Dai <[email protected]> Reviewed-by: Dave Gordon <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-18drm/i915: Cache LRC state page in the contextTvrtko Ursulin2-18/+22
LRC lifetime is well defined so we can cache the page pointing to the object backing store in the context in order to avoid walking over the object SG page list from the interrupt context without the big lock held. v2: Also cache the mapping. (Chris Wilson) v3: Unmap on the error path. v4: No need to cache the page. (Chris Wilson) v5: No need to dirty the page on unpin. (Chris Wilson) v6: kmap() cannot fail and use kmap_to_page to simplify unpin. (Chris Wilson) Signed-off-by: Tvrtko Ursulin <[email protected]> Cc: Chris Wilson <[email protected]> Cc: Dave Gordon <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
2016-01-18drm/i915: Cache ringbuffer GTT VMATvrtko Ursulin3-2/+5
Purpose is to avoid calling i915_gem_obj_ggtt_offset from the interrupt context without the big lock held. v2: Renamed gtt_start to gtt_offset. (Daniel Vetter) v3: Cache the VMA instead of address. (Chris Wilson) Signed-off-by: Tvrtko Ursulin <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Chris Wilson <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]