- 27 Jan, 2016 2 commits
-
-
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: Imre Deak <imre.deak@intel.com> Reviewed-by: David Weinehall <david.weinehall@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453209992-25995-2-git-send-email-imre.deak@intel.com
-
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: Mika Kuoppala <mika.kuoppala@linux.intel.com> Signed-off-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: Dave Gordon <david.s.gordon@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v2) Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1453405067-32890-3-git-send-email-david.s.gordon@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453397563-2848-1-git-send-email-chris@chris-wilson.co.ukReviewed-by: Michał Winiarski <michal.winiarski@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Arun Siluvery authored
This is mainly required for future enabling of pre-emptive command execution. v2: explain purpose of change (Chris) Reviewed-by: Nick Hoath <nicholas.hoath@intel.com> Cc: Dave Gordon <david.s.gordon@intel.com> Signed-off-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: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Nick Hoath <nicholas.hoath@intel.com> Cc: Dave Gordon <david.s.gordon@intel.com> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453412634-29238-8-git-send-email-arun.siluvery@linux.intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Nick Hoath <nicholas.hoath@intel.com> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453412634-29238-7-git-send-email-arun.siluvery@linux.intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Nick Hoath <nicholas.hoath@intel.com> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453412634-29238-6-git-send-email-arun.siluvery@linux.intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Nick Hoath <nicholas.hoath@intel.com> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453412634-29238-5-git-send-email-arun.siluvery@linux.intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Nick Hoath <nicholas.hoath@intel.com> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453412634-29238-4-git-send-email-arun.siluvery@linux.intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Nick Hoath <nicholas.hoath@intel.com> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453412634-29238-3-git-send-email-arun.siluvery@linux.intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Chris Wilson <chris@chris-wilson.co.uk> 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: Arun Siluvery <arun.siluvery@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453412634-29238-2-git-send-email-arun.siluvery@linux.intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Chris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1453722079-2604-1-git-send-email-andreas.ziegler@fau.de Cc: drm-intel-fixes@lists.freedesktop.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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 <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453579094-29860-1-git-send-email-yu.dai@intel.com
-
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: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Alex Dai <yu.dai@intel.com> Cc: Dave Gordon <david.s.gordon@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1453466567-33369-1-git-send-email-tvrtko.ursulin@linux.intel.com
-
- 24 Jan, 2016 1 commit
-
-
Daniel Vetter authored
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
- 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: Chris Wilson <chris@chris-wilson.co.uk> Testcase: igt/gem_busy Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1452876706-21620-1-git-send-email-chris@chris-wilson.co.uk
-
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: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1452870770-13981-1-git-send-email-tvrtko.ursulin@linux.intel.com
-
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: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453297415-17793-1-git-send-email-mika.kuoppala@intel.com
-
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: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1453285943-24614-1-git-send-email-mika.kuoppala@intel.com
-
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: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453233656-12955-1-git-send-email-daniel.vetter@ffwll.ch
-
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 <david.s.gordon@intel.com> Reviewed-by: Nick Hoath <nicholas.hoath@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453230175-19330-4-git-send-email-david.s.gordon@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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 <david.s.gordon@intel.com> Reviewed-by: Nick Hoath <nicholas.hoath@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1453230175-19330-3-git-send-email-david.s.gordon@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Dave Gordon <david.s.gordon@intel.com> Reviewed-by: Nick Hoath <nicholas.hoath@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453230175-19330-2-git-send-email-david.s.gordon@intel.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
- 20 Jan, 2016 6 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: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453220597-28973-1-git-send-email-ville.syrjala@linux.intel.comReviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
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: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: John Harrison <john.c.harrison@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1453217117-26125-1-git-send-email-tvrtko.ursulin@linux.intel.com
-
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: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Jani Nikula authored
Without the DOC:, kernel-doc confuses the documentation block for something else. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453101588-18008-3-git-send-email-jani.nikula@intel.com
-
Jani Nikula authored
Apparently accidental or misplaced /** kernel-doc comments, confusing the tool. Turn them to normal comments. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453101588-18008-2-git-send-email-jani.nikula@intel.com
-
Jani Nikula authored
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 <daniel.vetter@ffwll.ch> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453101588-18008-1-git-send-email-jani.nikula@intel.com
-
- 19 Jan, 2016 2 commits
-
-
Arun Siluvery authored
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 <yu.dai@intel.com> Cc: Dave Gordon <david.s.gordon@intel.com> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453132776-22229-1-git-send-email-arun.siluvery@linux.intel.comReviewed-by: Alex Dai <yu.dai@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Chris Wilson authored
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 <chris@chris-wilson.co.uk> Cc: Damien Lespiau <damien.lespiau@intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Sunil Kamath <sunil.kamath@intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Animesh Manna <animesh.manna@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1452706695-13518-1-git-send-email-chris@chris-wilson.co.ukReviewed-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
- 18 Jan, 2016 8 commits
-
-
Geliang Tang authored
Use hlist_for_each_entry() instead of hlist_for_each() to simplify the code. Signed-off-by: Geliang Tang <geliangtang@163.com> Link: http://patchwork.freedesktop.org/patch/msgid/b80568b2990ebcc145229a132f045e852ac51ad6.1453126187.git.geliangtang@163.comSigned-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-
Ville Syrjälä authored
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 <chandra.konduru@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1444917718-28495-1-git-send-email-ville.syrjala@linux.intel.com Fixes: 6156a456 ("drm/i915: skylake primary plane scaling using shared scalers") Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
-
Ville Syrjälä authored
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 <maarten.lankhorst@linux.intel.com> Fixes: 061e4b8d ("drm/i915: clean up atomic plane check functions, v2.") Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1452883613-28549-1-git-send-email-ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
Alex Dai authored
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 <yu.dai@intel.com> Reviewed-by: Dave Gordon <david.s.gordon@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1452711710-4505-1-git-send-email-yu.dai@intel.com
-
Tvrtko Ursulin authored
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 <tvrtko.ursulin@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Dave Gordon <david.s.gordon@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1452877965-32042-1-git-send-email-tvrtko.ursulin@linux.intel.com
-
Tvrtko Ursulin authored
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 <tvrtko.ursulin@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1452870629-13830-2-git-send-email-tvrtko.ursulin@linux.intel.com
-
Tvrtko Ursulin authored
LRC code was calling GEM API like i915_gem_obj_ggtt_offset from places where the struct_mutex cannot be grabbed (irq handlers). To avoid that this patch caches some interesting bits and values in the engine and context structures. Some usages are also removed where they are not needed like a few asserts which are either impossible or have been checked already during engine initialization. Side benefit is also that interrupt handlers and command submission stop evaluating invariant conditionals, like what Gen we are running on, on every interrupt and every command submitted. This patch deals with logical ring context id and descriptors while subsequent patches will deal with the remaining issues. v2: * Cache the VMA instead of the address. (Chris Wilson) * Incorporate Dave Gordon's good comments and function name. v3: * Extract ctx descriptor template to a function and group functions dealing with ctx descriptor & co together near top of the file. (Dave Gordon) Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Dave Gordon <david.s.gordon@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1452870629-13830-1-git-send-email-tvrtko.ursulin@linux.intel.com
-
Tvrtko Ursulin authored
No need to call ktime_get_raw_ns twice per unlimited wait and can also elimate a local variable. v2: Added comment about silencing the compiler warning. (Daniel Vetter) Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Dave Gordon <david.s.gordon@intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1452870672-13901-1-git-send-email-tvrtko.ursulin@linux.intel.com
-